| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250408172610.428f689d@collabora.com>
Date: Tue, 8 Apr 2025 17:26:10 +0200
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Adrián Larumbe <adrian.larumbe@...labora.com>
Cc: Steven Price <steven.price@....com>, Liviu Dudau <liviu.dudau@....com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
<airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Sumit Semwal
<sumit.semwal@...aro.org>, Christian König
<christian.koenig@....com>, kernel@...labora.com,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v4 4/4] drm/panthor: show device-wide list of DRM GEM
objects over DebugFS
On Wed, 2 Apr 2025 12:54:29 +0100
Adrián Larumbe <adrian.larumbe@...labora.com> wrote:
> +static void panthor_gem_debugfs_bo_print(struct panthor_gem_object *bo,
> + struct seq_file *m,
> + struct gem_size_totals *totals)
> +{
> + unsigned int refcount = kref_read(&bo->base.base.refcount);
> + char creator_info[32] = {};
> + bool has_flags = false;
> + size_t resident_size;
> +
> + /* Skip BOs being destroyed. */
> + if (!refcount)
> + return;
> +
> + resident_size = bo->base.pages != NULL ? bo->base.base.size : 0;
> +
> + snprintf(creator_info, sizeof(creator_info),
> + "%s/%d", bo->debugfs.creator.process_name, bo->debugfs.creator.tgid);
> + seq_printf(m, "%-32s%-16d%-16d%-16zd%-16zd%-16lx",
> + creator_info,
> + bo->base.base.name,
> + refcount,
> + bo->base.base.size,
> + resident_size,
> + drm_vma_node_start(&bo->base.base.vma_node));
> +
> + seq_puts(m, "(");
> + has_flags = panfrost_gem_print_flag("imported", bo->base.base.import_attach != NULL,
> + has_flags, m);
> + has_flags = panfrost_gem_print_flag("exported", bo->base.base.dma_buf != NULL,
> + has_flags, m);
> + if (bo->base.madv < 0)
> + has_flags = panfrost_gem_print_flag("purged", true, has_flags, m);
> + else if (bo->base.madv > 0)
> + has_flags = panfrost_gem_print_flag("purgeable", true, has_flags, m);
I would probably go:
has_flags = panfrost_gem_print_flag("purged", bo->base.madv < 0, has_flags, m);
has_flags = panfrost_gem_print_flag("purgeable", bo->base.madv > 0, has_flags, m);
to keep it one line per flag.
BTW, most of those flags are encoding the GEM state, so maybe the column
should be named state, and the helper panfrost_gem_print_state_flag().
> + if (!has_flags)
> + seq_puts(m, "none");
> + seq_puts(m, ")");
> +
> + seq_printf(m, "%-6s0x%-2x", "", bo->debugfs.bo_mask);
It's probably better if we print the debugfs flags like the GEM flags
(one string per flag, with a ',' separator). We can even make it a
helper function taking a list of flags and their associated strings so
we can use it for both panthor_gem_object::flags and
panthor_gem_object::debugfs::flags.
static void
panthor_gem_debugfs_print_flags(const char *names,
u32 name_count,
u32 flags)
{
bool first = true;
seq_puts(m, "(");
if (!flags)
seq_puts(m, "none");
while (flags) {
u32 bit = fls(flags) - 1;
if (!first)
seq_puts(m, ",");
if (bit >= name_count || !names[bit])
seq_printf(m, "unknown-bit%d", bit);
else
seq_puts(m, name);
first = false;
flags &= ~BIT(bit);
}
seq_puts(m, ")");
}
> +
> + mutex_lock(&bo->label.lock);
> + seq_printf(m, "%-6s%-60s", "", bo->label.str ? : NULL);
> + mutex_unlock(&bo->label.lock);
> + seq_puts(m, "\n");
> +
> + totals->size += bo->base.base.size;
> + totals->resident += resident_size;
> + if (bo->base.madv > 0)
> + totals->reclaimable += resident_size;
> +}
> +
> +void panthor_gem_debugfs_print_bos(struct panthor_device *ptdev,
> + struct seq_file *m)
> +{
> + struct gem_size_totals totals = {0};
> + struct panthor_gem_object *bo;
> +
> + seq_puts(m, "created-by global-name refcount size resident-size file-offset flags kflags label\n");
> + seq_puts(m, "------------------------------------------------------------------------------------------------------------------------------------------------\n");
> +
> + scoped_guard(mutex, &ptdev->gems.lock) {
> + list_for_each_entry(bo, &ptdev->gems.node, debugfs.node)
> + panthor_gem_debugfs_bo_print(bo, m, &totals);
> + }
> +
> + seq_puts(m, "==========================================================================================================================================================\n");
> + seq_printf(m, "Total size: %zd, Total resident: %zd, Total reclaimable: %zd\n",
> + totals.size, totals.resident, totals.reclaimable);
> +}
> +#endif
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
> index 49daa5088a0d..22ecc0d39d5e 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.h
> +++ b/drivers/gpu/drm/panthor/panthor_gem.h
> @@ -15,6 +15,32 @@ struct panthor_vm;
>
> #define PANTHOR_BO_LABEL_MAXLEN PAGE_SIZE
>
> +#define PANTHOR_BO_KERNEL BIT(0)
s/PANTHOR_BO_KERNEL/PANTHOR_DEBUGFS_BO_FLAG_KERNEL/
> +#define PANTHOR_BO_FW_MAPPED BIT(1)
s/PANTHOR_BO_FW_MAPPED/PANTHOR_DEBUGFS_BO_FLAG_FW_MAPPED/
And it'd be better if those flags were documented.
I would also add a
#define PANTHOR_DEBUGFS_BO_FLAG_INITIALIZED BIT(0)
and move the other flags one bit left.
> +
> +/**
> + * struct panthor_gem_debugfs - GEM object's DebugFS list information
> + */
> +struct panthor_gem_debugfs {
> + /**
> + * @node: Node used to insert the object in the device-wide list of
> + * GEM objects, to display information about it through a DebugFS file.
> + */
> + struct list_head node;
> +
> + /** @creator: Information about the UM process which created the GEM. */
> + struct {
> + /** @creator.process_name: Group leader name in owning thread's process */
> + char process_name[TASK_COMM_LEN];
> +
> + /** @creator.tgid: PID of the thread's group leader within its process */
> + pid_t tgid;
> + } creator;
> +
> + /** @bo_mask: Bitmask encoding BO type as {USER, KERNEL} x {GPU, FW} */
> + u32 bo_mask;
s/bo_mask/flags/ and mention that it's a combination of
PANTHOR_DEBUGFS_BO_FLAG_xxx in the doc.
> +};
> +
> /**
> * struct panthor_gem_object - Driver specific GEM object.
> */
> @@ -62,6 +88,10 @@ struct panthor_gem_object {
> /** @lock.str: Protects access to the @label.str field. */
> struct mutex lock;
> } label;
> +
> +#ifdef CONFIG_DEBUG_FS
> + struct panthor_gem_debugfs debugfs;
> +#endif
> };
>
> /**
> @@ -157,4 +187,9 @@ panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
>
> void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo);
>
> +#ifdef CONFIG_DEBUG_FS
> +void panthor_gem_debugfs_print_bos(struct panthor_device *pfdev,
> + struct seq_file *m);
> +#endif
> +
> #endif /* __PANTHOR_GEM_H__ */
Powered by blists - more mailing lists