Hello Nicolas,
On Tue, 18 Aug 2026 21:35:24 +0200
Nicolas Frattaroli <[email protected]> wrote:
> Analogous to what was added in Commit d41c79838c47 ("drm/panfrost:
> Display list of device JM contexts over debugfs") for panfrost, add
> similar debugfs information for panthor.
>
> The group priority does not change over the lifetime of the group, so no
> effort to synchronise with the scheduler lock is being made.
>
> Signed-off-by: Nicolas Frattaroli <[email protected]>
> ---
> Some additional notes: I noticed panthor apparently uses
> xa_for_each{_marked} by guarding it with xa_lock/xa_unlock. That appears
> to be unnecessary, judging by the documentation of it and that nobody
> else does this.
> ---
> drivers/gpu/drm/panthor/panthor_drv.c | 1 +
> drivers/gpu/drm/panthor/panthor_sched.c | 89
> +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/panthor/panthor_sched.h | 5 ++
> 3 files changed, 95 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c
> b/drivers/gpu/drm/panthor/panthor_drv.c
> index 46a3080b0b20..8cdba0c1a14b 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1769,6 +1769,7 @@ static void panthor_debugfs_init(struct drm_minor
> *minor)
> {
> panthor_mmu_debugfs_init(minor);
> panthor_gem_debugfs_init(minor);
> + panthor_sched_debugfs_init(minor);
> }
> #endif
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5832dccfc093..44b61e946e2d 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0 or MIT
> /* Copyright 2023 Collabora ltd. */
>
> +#include <drm/drm_debugfs.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_exec.h>
> #include <drm/drm_file.h>
> @@ -4198,3 +4199,91 @@ int panthor_sched_init(struct panthor_device *ptdev)
> ptdev->scheduler = sched;
> return 0;
> }
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static const char *
> +panthor_sched_prio_str(enum panthor_csg_priority prio)
> +{
> + switch (prio) {
> + case PANTHOR_CSG_PRIORITY_LOW:
> + return "LOW";
> + case PANTHOR_CSG_PRIORITY_MEDIUM:
> + return "MEDIUM";
> + case PANTHOR_CSG_PRIORITY_HIGH:
> + return "HIGH";
> + case PANTHOR_CSG_PRIORITY_RT:
> + return "REAL-TIME";
> + default:
> + return "UNKNOWN";
> + }
> +}
> +
> +static int show_file_groups(struct panthor_file *pfile, struct seq_file *m)
> +{
> + struct panthor_group *group;
> + unsigned long i;
> +
> + if (IS_ERR_OR_NULL(pfile->groups))
> + return -ENOENT;
> +
> + xa_for_each_marked(&pfile->groups->xa, i, group, GROUP_REGISTERED) {
> + seq_printf(m, " Group %lu: priority %s\n", i,
> + panthor_sched_prio_str(group->priority));
> + }
Could this race with the GROUP_DESTROY ioctl and open the door for
potential UAFs on the group object? Seems
panthor_fdinfo_gather_group_samples() has this xa_for_each_marked()
inside an xa_lock()-ed section to cover for this.
> +
> + return 0;
> +}
> +
> +static int show_each_file(struct seq_file *m, void *arg)
> +{
> + struct drm_info_node *node = (struct drm_info_node *)m->private;
> + struct drm_device *ddev = node->minor->dev;
> + int (*show)(struct panthor_file *, struct seq_file *) =
> + node->info_ent->data;
> + struct drm_file *file;
> + int ret = 0;
> +
> + scoped_cond_guard(mutex_intr, return -EINTR, &ddev->filelist_mutex) {
> + list_for_each_entry(file, &ddev->filelist, lhead) {
> + struct task_struct *task;
> + struct panthor_file *pfile = file->driver_priv;
> + struct pid *pid;
> +
> + /*
> + * Although we have a valid reference on file->pid,
> that does
> + * not guarantee that the task_struct who called
> get_pid() is
> + * still alive (e.g. get_pid(current) => fork() =>
> exit()).
> + * Therefore, we need to protect this ->comm access
> using RCU.
> + */
> + rcu_read_lock();
> + pid = rcu_dereference(file->pid);
> + task = pid_task(pid, PIDTYPE_TGID);
> + seq_printf(m, "client_id %8llu pid %8d command %s:\n",
> + file->client_id, pid_nr(pid),
> + task ? task->comm : "<unknown>");
> + rcu_read_unlock();
Don't we have this piece of information stored in
panthor_group::task_info already? Unless you really want to reflect
clients that have no groups, or have groups from a given client clearly
outlined in the debugfs output, I'd flatten things out and have these
client-related info printed along the group info in show_file_groups()
(pass a drm_file instead of a panthor_file, so you can get the
client_id from there).
Regards,
Boris
> +
> + ret = show(pfile, m);
> + if (ret < 0)
> + break;
> +
> + seq_puts(m, "\n");
> + }
> + }
> +
> + return ret;
