Re: [RFC 01/10] drm: Track clients by tgid and not tid

[Christian König <christian.koenig@xxxxxxx>]

Am 15.03.23 um 10:52 schrieb Tvrtko Ursulin:
> Hi,
>
> On 14/03/2023 15:33, Christian König wrote:
> > Am 14.03.23 um 15:18 schrieb Tvrtko Ursulin:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > >
> > > Thread group id (aka pid from userspace point of view) is a more
> > > interesting thing to show as an owner of a DRM fd, so track and show 
> > > that
> > > instead of the thread id.
> > >
> > > In the next patch we will make the owner updated post file descriptor
> > > handover, which will also be tgid based to avoid ping-pong when 
> > > multiple
> > > threads access the fd.
> > >
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > Cc: Zack Rusin <zackr@xxxxxxxxxx>
> > > Cc: linux-graphics-maintainer@xxxxxxxxxx
> > > Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> > > Cc: "Christian König" <christian.koenig@xxxxxxx>
> > > Reviewed-by: Zack Rusin <zackr@xxxxxxxxxx>
> >
> > Reviewed-by: Christian König <christian.koenig@xxxxxxx>
> >
> > Should we push the already reviewed cleanups like this one to 
> > drm-misc-next? That makes sense even without the rest of the 
> > functionality and reduce the amount of patches re-send.
>
> I don't have the commit rights so if you could do that I certainly 
> would not mind, thanks!

I've just pushed this patch here to drm-misc-next. As soon as Daniel or 
Dave gives his ok as well I'm going to also push the second one.

Regards,
Christian.

> Regards,
>
> Tvrtko
>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
> > >   drivers/gpu/drm/drm_debugfs.c           | 4 ++--
> > >   drivers/gpu/drm/drm_file.c              | 2 +-
> > >   drivers/gpu/drm/vmwgfx/vmwgfx_gem.c     | 2 +-
> > >   4 files changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > > index d8e683688daa..863cb668e000 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > > @@ -969,7 +969,7 @@ static int amdgpu_debugfs_gem_info_show(struct 
> > > seq_file *m, void *unused)
> > >            * Therefore, we need to protect this ->comm access using 
> > > RCU.
> > >            */
> > >           rcu_read_lock();
> > > -        task = pid_task(file->pid, PIDTYPE_PID);
> > > +        task = pid_task(file->pid, PIDTYPE_TGID);
> > >           seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
> > >                  task ? task->comm : "<unknown>");
> > >           rcu_read_unlock();
> > > diff --git a/drivers/gpu/drm/drm_debugfs.c 
> > > b/drivers/gpu/drm/drm_debugfs.c
> > > index 4f643a490dc3..4855230ba2c6 100644
> > > --- a/drivers/gpu/drm/drm_debugfs.c
> > > +++ b/drivers/gpu/drm/drm_debugfs.c
> > > @@ -80,7 +80,7 @@ static int drm_clients_info(struct seq_file *m, 
> > > void *data)
> > >       seq_printf(m,
> > >              "%20s %5s %3s master a %5s %10s\n",
> > >              "command",
> > > -           "pid",
> > > +           "tgid",
> > >              "dev",
> > >              "uid",
> > >              "magic");
> > > @@ -94,7 +94,7 @@ static int drm_clients_info(struct seq_file *m, 
> > > void *data)
> > >           bool is_current_master = drm_is_current_master(priv);
> > >           rcu_read_lock(); /* locks pid_task()->comm */
> > > -        task = pid_task(priv->pid, PIDTYPE_PID);
> > > +        task = pid_task(priv->pid, PIDTYPE_TGID);
> > >           uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
> > >           seq_printf(m, "%20s %5d %3d   %c    %c %5d %10u\n",
> > >                  task ? task->comm : "<unknown>",
> > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > > index a51ff8cee049..c1018c470047 100644
> > > --- a/drivers/gpu/drm/drm_file.c
> > > +++ b/drivers/gpu/drm/drm_file.c
> > > @@ -156,7 +156,7 @@ struct drm_file *drm_file_alloc(struct drm_minor 
> > > *minor)
> > >       if (!file)
> > >           return ERR_PTR(-ENOMEM);
> > > -    file->pid = get_pid(task_pid(current));
> > > +    file->pid = get_pid(task_tgid(current));
> > >       file->minor = minor;
> > >       /* for compatibility root is always authenticated */
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c 
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> > > index d6baf73a6458..c0da89e16e6f 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
> > > @@ -241,7 +241,7 @@ static int vmw_debugfs_gem_info_show(struct 
> > > seq_file *m, void *unused)
> > >            * Therefore, we need to protect this ->comm access using 
> > > RCU.
> > >            */
> > >           rcu_read_lock();
> > > -        task = pid_task(file->pid, PIDTYPE_PID);
> > > +        task = pid_task(file->pid, PIDTYPE_TGID);
> > >           seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
> > >                  task ? task->comm : "<unknown>");
> > >           rcu_read_unlock();