| Message ID | 20230314141904.1210824-3-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DRM scheduling cgroup controller | expand |
Am 14.03.23 um 15:18 schrieb Tvrtko Ursulin: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > With the typical model where the display server opends the file descriptor > and then hands it over to the client we were showing stale data in > debugfs. > > Fix it by updating the drm_file->pid on ioctl access from a different > process. > > The field is also made RCU protected to allow for lockless readers. Update > side is protected with dev->filelist_mutex. > > Before: > > $ cat /sys/kernel/debug/dri/0/clients > command pid dev master a uid magic > Xorg 2344 0 y y 0 0 > Xorg 2344 0 n y 0 2 > Xorg 2344 0 n y 0 3 > Xorg 2344 0 n y 0 4 > > After: > > $ cat /sys/kernel/debug/dri/0/clients > command tgid dev master a uid magic > Xorg 830 0 y y 0 0 > xfce4-session 880 0 n y 0 1 > xfwm4 943 0 n y 0 2 > neverball 1095 0 n y 0 3 > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: "Christian König" <christian.koenig@amd.com> > Cc: Daniel Vetter <daniel@ffwll.ch> Looks completely correct to me, but I can't claim that I understand all those nasty details around drm_master handling. So only Acked-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 ++-- > drivers/gpu/drm/drm_auth.c | 3 +- > drivers/gpu/drm/drm_debugfs.c | 10 ++++--- > drivers/gpu/drm/drm_file.c | 40 +++++++++++++++++++++++-- > drivers/gpu/drm/drm_ioctl.c | 3 ++ > drivers/gpu/drm/nouveau/nouveau_drm.c | 5 +++- > drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 6 ++-- > include/drm/drm_file.h | 13 ++++++-- > 8 files changed, 71 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > index 863cb668e000..67ce634992f6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > @@ -960,6 +960,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused) > list_for_each_entry(file, &dev->filelist, lhead) { > struct task_struct *task; > struct drm_gem_object *gobj; > + struct pid *pid; > int id; > > /* > @@ -969,8 +970,9 @@ 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_TGID); > - seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), > + pid = rcu_dereference(file->pid); > + task = pid_task(pid, PIDTYPE_TGID); > + seq_printf(m, "pid %8d command %s:\n", pid_nr(pid), > task ? task->comm : "<unknown>"); > rcu_read_unlock(); > > diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c > index cf92a9ae8034..2ed2585ded37 100644 > --- a/drivers/gpu/drm/drm_auth.c > +++ b/drivers/gpu/drm/drm_auth.c > @@ -235,7 +235,8 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) > static int > drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv) > { > - if (file_priv->pid == task_pid(current) && file_priv->was_master) > + if (file_priv->was_master && > + rcu_access_pointer(file_priv->pid) == task_pid(current)) > return 0; > > if (!capable(CAP_SYS_ADMIN)) > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index 4855230ba2c6..b46f5ceb24c6 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -90,15 +90,17 @@ static int drm_clients_info(struct seq_file *m, void *data) > */ > mutex_lock(&dev->filelist_mutex); > list_for_each_entry_reverse(priv, &dev->filelist, lhead) { > - struct task_struct *task; > bool is_current_master = drm_is_current_master(priv); > + struct task_struct *task; > + struct pid *pid; > > - rcu_read_lock(); /* locks pid_task()->comm */ > - task = pid_task(priv->pid, PIDTYPE_TGID); > + rcu_read_lock(); /* Locks priv->pid and pid_task()->comm! */ > + pid = rcu_dereference(priv->pid); > + task = pid_task(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>", > - pid_vnr(priv->pid), > + pid_vnr(pid), > priv->minor->index, > is_current_master ? 'y' : 'n', > priv->authenticated ? 'y' : 'n', > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index c1018c470047..f2f8175ece15 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_tgid(current)); > + rcu_assign_pointer(file->pid, get_pid(task_tgid(current))); > file->minor = minor; > > /* for compatibility root is always authenticated */ > @@ -196,7 +196,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) > drm_syncobj_release(file); > if (drm_core_check_feature(dev, DRIVER_GEM)) > drm_gem_release(dev, file); > - put_pid(file->pid); > + put_pid(rcu_access_pointer(file->pid)); > kfree(file); > > return ERR_PTR(ret); > @@ -287,7 +287,7 @@ void drm_file_free(struct drm_file *file) > > WARN_ON(!list_empty(&file->event_list)); > > - put_pid(file->pid); > + put_pid(rcu_access_pointer(file->pid)); > kfree(file); > } > > @@ -501,6 +501,40 @@ int drm_release(struct inode *inode, struct file *filp) > } > EXPORT_SYMBOL(drm_release); > > +void drm_file_update_pid(struct drm_file *filp) > +{ > + struct drm_device *dev; > + struct pid *pid, *old; > + > + /* > + * Master nodes need to keep the original ownership in order for > + * drm_master_check_perm to keep working correctly. (See comment in > + * drm_auth.c.) > + */ > + if (filp->was_master) > + return; > + > + pid = task_tgid(current); > + > + /* > + * Quick unlocked check since the model is a single handover followed by > + * exclusive repeated use. > + */ > + if (pid == rcu_access_pointer(filp->pid)) > + return; > + > + dev = filp->minor->dev; > + mutex_lock(&dev->filelist_mutex); > + old = rcu_replace_pointer(filp->pid, pid, 1); > + mutex_unlock(&dev->filelist_mutex); > + > + if (pid != old) { > + get_pid(pid); > + synchronize_rcu(); > + put_pid(old); > + } > +} > + > /** > * drm_release_noglobal - release method for DRM file > * @inode: device inode > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 7c9d66ee917d..305b18d9d7b6 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -775,6 +775,9 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, > struct drm_device *dev = file_priv->minor->dev; > int retcode; > > + /* Update drm_file owner if fd was passed along. */ > + drm_file_update_pid(file_priv); > + > if (drm_dev_is_unplugged(dev)) > return -ENODEV; > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c > index cc7c5b4a05fd..57aeaf7af613 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -1095,7 +1095,10 @@ nouveau_drm_open(struct drm_device *dev, struct drm_file *fpriv) > } > > get_task_comm(tmpname, current); > - snprintf(name, sizeof(name), "%s[%d]", tmpname, pid_nr(fpriv->pid)); > + rcu_read_lock(); > + snprintf(name, sizeof(name), "%s[%d]", > + tmpname, pid_nr(rcu_dereference(fpriv->pid))); > + rcu_read_unlock(); > > if (!(cli = kzalloc(sizeof(*cli), GFP_KERNEL))) { > ret = -ENOMEM; > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > index c0da89e16e6f..a07e5b7e2f2f 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c > @@ -232,6 +232,7 @@ static int vmw_debugfs_gem_info_show(struct seq_file *m, void *unused) > list_for_each_entry(file, &dev->filelist, lhead) { > struct task_struct *task; > struct drm_gem_object *gobj; > + struct pid *pid; > int id; > > /* > @@ -241,8 +242,9 @@ 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_TGID); > - seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), > + pid = rcu_dereference(file->pid); > + task = pid_task(pid, PIDTYPE_TGID); > + seq_printf(m, "pid %8d command %s:\n", pid_nr(pid), > task ? task->comm : "<unknown>"); > rcu_read_unlock(); > > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > index 0d1f853092ab..27d545131d4a 100644 > --- a/include/drm/drm_file.h > +++ b/include/drm/drm_file.h > @@ -255,8 +255,15 @@ struct drm_file { > /** @master_lookup_lock: Serializes @master. */ > spinlock_t master_lookup_lock; > > - /** @pid: Process that opened this file. */ > - struct pid *pid; > + /** > + * @pid: Process that is using this file. > + * > + * Must only be dereferenced under a rcu_read_lock or equivalent. > + * > + * Updates are guarded with dev->filelist_mutex and reference must be > + * dropped after a RCU grace period to accommodate lockless readers. > + */ > + struct pid __rcu *pid; > > /** @magic: Authentication magic, see @authenticated. */ > drm_magic_t magic; > @@ -415,6 +422,8 @@ static inline bool drm_is_accel_client(const struct drm_file *file_priv) > return file_priv->minor->type == DRM_MINOR_ACCEL; > } > > +void drm_file_update_pid(struct drm_file *); > + > int drm_open(struct inode *inode, struct file *filp); > int drm_open_helper(struct file *filp, struct drm_minor *minor); > ssize_t drm_read(struct file *filp, char __user *buffer,
Greetings everyone, Above all - hell yeah. Thank you Tvrtko, this has been annoying the hell out of me for ages. On Tue, 14 Mar 2023 at 14:19, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > With the typical model where the display server opends the file descriptor > and then hands it over to the client we were showing stale data in > debugfs. s/opends/opens/ But as a whole the sentence is fairly misleading. Story time: The traditional model, the server was the orchestrator managing the primary device node. From the fd, to the master status and authentication. But looking at the fd alone, this has varied across the years. IIRC in the DRI1 days, Xorg (libdrm really) would have a list of open fd(s) and reuse those whenever needed, DRI2 the client was responsible for open() themselves and with DRI3 the fd was passed to the client. Around the inception of DRI3 and systemd-logind, the latter became another possible orchestrator. Whereby Xorg and Wayland compositors could ask it for the fd. For various reasons (hysterical and genuine ones) Xorg has a fallback path going the open(), whereas Wayland compositors are moving to solely relying on logind... some never had fallback even. Over the past few years, more projects have emerged which provide functionality similar (be that on API level, Dbus, or otherwise) to systemd-logind. Apart from that, the commit is spot on. I like the use of rcu and the was_master handling is correct. With some message polish this commit is: Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> I also had a brief look at 01/10, although I cannot find many references for the pid <> tguid mappings. Be that on the kernel side or userspace - do you have any links that I can educate myself? Thanks Emil
On 21/04/2023 13:13, Emil Velikov wrote: > Greetings everyone, > > Above all - hell yeah. Thank you Tvrtko, this has been annoying the > hell out of me for ages. Yay! > On Tue, 14 Mar 2023 at 14:19, Tvrtko Ursulin > <tvrtko.ursulin@linux.intel.com> wrote: >> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> With the typical model where the display server opends the file descriptor >> and then hands it over to the client we were showing stale data in >> debugfs. > > s/opends/opens/ Thanks! > But as a whole the sentence is fairly misleading. Story time: > > The traditional model, the server was the orchestrator managing the > primary device node. From the fd, to the master status and > authentication. But looking at the fd alone, this has varied across > the years. > > IIRC in the DRI1 days, Xorg (libdrm really) would have a list of open > fd(s) and reuse those whenever needed, DRI2 the client was responsible > for open() themselves and with DRI3 the fd was passed to the client. > > Around the inception of DRI3 and systemd-logind, the latter became > another possible orchestrator. Whereby Xorg and Wayland compositors > could ask it for the fd. For various reasons (hysterical and genuine > ones) Xorg has a fallback path going the open(), whereas Wayland > compositors are moving to solely relying on logind... some never had > fallback even. > > Over the past few years, more projects have emerged which provide > functionality similar (be that on API level, Dbus, or otherwise) to > systemd-logind. > > > Apart from that, the commit is spot on. I like the use of rcu and the > was_master handling is correct. With some message polish this commit > is: > Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Are you okay if I just paste your very fine explanation verbatim, with credits? > I also had a brief look at 01/10, although I cannot find many > references for the pid <> tguid mappings. Be that on the kernel side > or userspace - do you have any links that I can educate myself? TGID or thread group leader. For single threaded userspace TGID equals to PID, while for multi-threaded first thread TGID equals PID/TID, while additional threads PID/TID does not equal TGID. Clear, as mud? :) My POSIX book is misplaced somewhere having not consulted it years... :) Regards, Tvrtko
Hi Tvrtko Sorry for the delay, real life and other obligations got in the way. On Thu, 8 Jun 2023 at 15:26, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > On 21/04/2023 13:13, Emil Velikov wrote: > Are you okay if I just paste your very fine explanation verbatim, with > credits? > Yes, feel free to use as much of if as you see reasonable. > > I also had a brief look at 01/10, although I cannot find many > > references for the pid <> tguid mappings. Be that on the kernel side > > or userspace - do you have any links that I can educate myself? > > TGID or thread group leader. For single threaded userspace TGID equals > to PID, while for multi-threaded first thread TGID equals PID/TID, while > additional threads PID/TID does not equal TGID. Clear, as mud? :) My > POSIX book is misplaced somewhere having not consulted it years... :) > Ack. /me looks into actually buying one, perhaps Thanks Emil
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index 863cb668e000..67ce634992f6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -960,6 +960,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused) list_for_each_entry(file, &dev->filelist, lhead) { struct task_struct *task; struct drm_gem_object *gobj; + struct pid *pid; int id; /* @@ -969,8 +970,9 @@ 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_TGID); - seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), + pid = rcu_dereference(file->pid); + task = pid_task(pid, PIDTYPE_TGID); + seq_printf(m, "pid %8d command %s:\n", pid_nr(pid), task ? task->comm : "<unknown>"); rcu_read_unlock(); diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index cf92a9ae8034..2ed2585ded37 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -235,7 +235,8 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) static int drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv) { - if (file_priv->pid == task_pid(current) && file_priv->was_master) + if (file_priv->was_master && + rcu_access_pointer(file_priv->pid) == task_pid(current)) return 0; if (!capable(CAP_SYS_ADMIN)) diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 4855230ba2c6..b46f5ceb24c6 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -90,15 +90,17 @@ static int drm_clients_info(struct seq_file *m, void *data) */ mutex_lock(&dev->filelist_mutex); list_for_each_entry_reverse(priv, &dev->filelist, lhead) { - struct task_struct *task; bool is_current_master = drm_is_current_master(priv); + struct task_struct *task; + struct pid *pid; - rcu_read_lock(); /* locks pid_task()->comm */ - task = pid_task(priv->pid, PIDTYPE_TGID); + rcu_read_lock(); /* Locks priv->pid and pid_task()->comm! */ + pid = rcu_dereference(priv->pid); + task = pid_task(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>", - pid_vnr(priv->pid), + pid_vnr(pid), priv->minor->index, is_current_master ? 'y' : 'n', priv->authenticated ? 'y' : 'n', diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index c1018c470047..f2f8175ece15 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_tgid(current)); + rcu_assign_pointer(file->pid, get_pid(task_tgid(current))); file->minor = minor; /* for compatibility root is always authenticated */ @@ -196,7 +196,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) drm_syncobj_release(file); if (drm_core_check_feature(dev, DRIVER_GEM)) drm_gem_release(dev, file); - put_pid(file->pid); + put_pid(rcu_access_pointer(file->pid)); kfree(file); return ERR_PTR(ret); @@ -287,7 +287,7 @@ void drm_file_free(struct drm_file *file) WARN_ON(!list_empty(&file->event_list)); - put_pid(file->pid); + put_pid(rcu_access_pointer(file->pid)); kfree(file); } @@ -501,6 +501,40 @@ int drm_release(struct inode *inode, struct file *filp) } EXPORT_SYMBOL(drm_release); +void drm_file_update_pid(struct drm_file *filp) +{ + struct drm_device *dev; + struct pid *pid, *old; + + /* + * Master nodes need to keep the original ownership in order for + * drm_master_check_perm to keep working correctly. (See comment in + * drm_auth.c.) + */ + if (filp->was_master) + return; + + pid = task_tgid(current); + + /* + * Quick unlocked check since the model is a single handover followed by + * exclusive repeated use. + */ + if (pid == rcu_access_pointer(filp->pid)) + return; + + dev = filp->minor->dev; + mutex_lock(&dev->filelist_mutex); + old = rcu_replace_pointer(filp->pid, pid, 1); + mutex_unlock(&dev->filelist_mutex); + + if (pid != old) { + get_pid(pid); + synchronize_rcu(); + put_pid(old); + } +} + /** * drm_release_noglobal - release method for DRM file * @inode: device inode diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 7c9d66ee917d..305b18d9d7b6 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -775,6 +775,9 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t *func, void *kdata, struct drm_device *dev = file_priv->minor->dev; int retcode; + /* Update drm_file owner if fd was passed along. */ + drm_file_update_pid(file_priv); + if (drm_dev_is_unplugged(dev)) return -ENODEV; diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index cc7c5b4a05fd..57aeaf7af613 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -1095,7 +1095,10 @@ nouveau_drm_open(struct drm_device *dev, struct drm_file *fpriv) } get_task_comm(tmpname, current); - snprintf(name, sizeof(name), "%s[%d]", tmpname, pid_nr(fpriv->pid)); + rcu_read_lock(); + snprintf(name, sizeof(name), "%s[%d]", + tmpname, pid_nr(rcu_dereference(fpriv->pid))); + rcu_read_unlock(); if (!(cli = kzalloc(sizeof(*cli), GFP_KERNEL))) { ret = -ENOMEM; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index c0da89e16e6f..a07e5b7e2f2f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -232,6 +232,7 @@ static int vmw_debugfs_gem_info_show(struct seq_file *m, void *unused) list_for_each_entry(file, &dev->filelist, lhead) { struct task_struct *task; struct drm_gem_object *gobj; + struct pid *pid; int id; /* @@ -241,8 +242,9 @@ 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_TGID); - seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), + pid = rcu_dereference(file->pid); + task = pid_task(pid, PIDTYPE_TGID); + seq_printf(m, "pid %8d command %s:\n", pid_nr(pid), task ? task->comm : "<unknown>"); rcu_read_unlock(); diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 0d1f853092ab..27d545131d4a 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -255,8 +255,15 @@ struct drm_file { /** @master_lookup_lock: Serializes @master. */ spinlock_t master_lookup_lock; - /** @pid: Process that opened this file. */ - struct pid *pid; + /** + * @pid: Process that is using this file. + * + * Must only be dereferenced under a rcu_read_lock or equivalent. + * + * Updates are guarded with dev->filelist_mutex and reference must be + * dropped after a RCU grace period to accommodate lockless readers. + */ + struct pid __rcu *pid; /** @magic: Authentication magic, see @authenticated. */ drm_magic_t magic; @@ -415,6 +422,8 @@ static inline bool drm_is_accel_client(const struct drm_file *file_priv) return file_priv->minor->type == DRM_MINOR_ACCEL; } +void drm_file_update_pid(struct drm_file *); + int drm_open(struct inode *inode, struct file *filp); int drm_open_helper(struct file *filp, struct drm_minor *minor); ssize_t drm_read(struct file *filp, char __user *buffer,