| Message ID | 20240911145836.734080-1-pierre-eric.pelloux-prayer@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm: add DRM_SET_NAME ioctl | expand |
On 11/09/2024 15:58, Pierre-Eric Pelloux-Prayer wrote: > Giving the opportunity to userspace to associate a free-form > name with a drm_file struct is helpful for tracking and debugging. > > This is similar to the existing DMA_BUF_SET_NAME ioctl. > > Access to name is protected by a mutex, and the 'clients' debugfs > file has been updated to print it. > > Userspace MR to use this ioctl: > https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1428 Idea seems useful to me. Various classes of comments/questions below: > Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com> > --- > drivers/gpu/drm/drm_debugfs.c | 12 ++++++++---- > drivers/gpu/drm/drm_file.c | 5 +++++ > drivers/gpu/drm/drm_ioctl.c | 28 ++++++++++++++++++++++++++++ > include/drm/drm_file.h | 9 +++++++++ > include/uapi/drm/drm.h | 14 ++++++++++++++ > 5 files changed, 64 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c > index 6b239a24f1df..b7492225ae88 100644 > --- a/drivers/gpu/drm/drm_debugfs.c > +++ b/drivers/gpu/drm/drm_debugfs.c > @@ -78,12 +78,13 @@ static int drm_clients_info(struct seq_file *m, void *data) > kuid_t uid; > > seq_printf(m, > - "%20s %5s %3s master a %5s %10s\n", > + "%20s %5s %3s master a %5s %10s %20s\n", > "command", > "tgid", > "dev", > "uid", > - "magic"); > + "magic", > + "name"); > > /* dev->filelist is sorted youngest first, but we want to present > * oldest first (i.e. kernel, servers, clients), so walk backwardss. > @@ -94,19 +95,22 @@ static int drm_clients_info(struct seq_file *m, void *data) > struct task_struct *task; > struct pid *pid; > > + mutex_lock(&priv->name_lock); > 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", > + seq_printf(m, "%20s %5d %3d %c %c %5d %10u %20s\n", > task ? task->comm : "<unknown>", > pid_vnr(pid), > priv->minor->index, > is_current_master ? 'y' : 'n', > priv->authenticated ? 'y' : 'n', > from_kuid_munged(seq_user_ns(m), uid), > - priv->magic); > + priv->magic, > + priv->name ? priv->name : ""); > rcu_read_unlock(); > + mutex_unlock(&priv->name_lock); FWIW it is possible you could get away without the need for a lock on the read side if you make the pointer RCU managed and stick a synchronize_rcu before kfree in the ioctl update path. Not because this lock would be a contentended one per se, but mostly to avoid complications such as amdgpu_debugfs_gem_info_show() where 3/3 has it broken - cannot take the mutex in rcu locked section. Just something to consider in case it would end up simpler code. > } > mutex_unlock(&dev->filelist_mutex); > return 0; > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index 01fde94fe2a9..558151c3912e 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -158,6 +158,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) > > spin_lock_init(&file->master_lookup_lock); > mutex_init(&file->event_read_lock); > + mutex_init(&file->name_lock); > > if (drm_core_check_feature(dev, DRIVER_GEM)) > drm_gem_open(dev, file); > @@ -259,6 +260,10 @@ void drm_file_free(struct drm_file *file) > WARN_ON(!list_empty(&file->event_list)); > > put_pid(rcu_access_pointer(file->pid)); > + > + mutex_destroy(&file->name_lock); > + kvfree(file->name); I think kfree is correct here. > + > kfree(file); > } > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 51f39912866f..ba2f2120e99b 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -540,6 +540,32 @@ int drm_version(struct drm_device *dev, void *data, > return err; > } > > +static int drm_set_name(struct drm_device *dev, void *data, > + struct drm_file *file_priv) > +{ > + struct drm_set_name *name = data; > + void *user_ptr; > + char *new_name; > + > + if (name->name_len >= NAME_MAX) > + return -EINVAL; Any special reason to use the filesystem NAME_MAX? > + > + user_ptr = u64_to_user_ptr(name->name); > + > + new_name = memdup_user_nul(user_ptr, name->name_len); > + > + if (IS_ERR(new_name)) > + return PTR_ERR(new_name); > + > + mutex_lock(&file_priv->name_lock); > + if (file_priv->name) > + kvfree(file_priv->name); > + file_priv->name = new_name; > + mutex_unlock(&file_priv->name_lock); One question is whether allowing name changes is interesting or it should be one shot? Second issue is to avoid any Little Bobby Tables situations and somewhat validate the input. At least when kernel dumps in in debugfs and fdinfo, we probably don't want to allow userspace to be too creative. Like output newlines or some other special characters. Regards, Tvrtko > + > + return 0; > +} > + > static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) > { > /* ROOT_ONLY is only for CAP_SYS_ADMIN */ > @@ -610,6 +636,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW), > DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW), > > + DRM_IOCTL_DEF(DRM_IOCTL_SET_NAME, drm_set_name, DRM_RENDER_ALLOW), > + > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER), > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > index 8c0030c77308..df26eee8f79c 100644 > --- a/include/drm/drm_file.h > +++ b/include/drm/drm_file.h > @@ -388,6 +388,15 @@ struct drm_file { > * Per-file buffer caches used by the PRIME buffer sharing code. > */ > struct drm_prime_file_private prime; > + > + /** > + * @name: > + * > + * Userspace-provided name; useful for accounting and debugging. > + */ > + const char *name; > + /** @name_lock: Protects @name. */ > + struct mutex name_lock; > }; > > /** > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index 16122819edfe..fc62bb21f79e 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -1024,6 +1024,12 @@ struct drm_crtc_queue_sequence { > __u64 user_data; /* user data passed to event */ > }; > > +struct drm_set_name { > + __u64 name_len; > + __u64 name; > +}; > + > + > #if defined(__cplusplus) > } > #endif > @@ -1288,6 +1294,14 @@ extern "C" { > */ > #define DRM_IOCTL_MODE_CLOSEFB DRM_IOWR(0xD0, struct drm_mode_closefb) > > +/** > + * DRM_IOCTL_SET_NAME - Attach a name to a drm_file > + * > + * This ioctl is similar to DMA_BUF_SET_NAME - it allows for easier tracking > + * and debugging. > + */ > +#define DRM_IOCTL_SET_NAME DRM_IOWR(0xD1, struct drm_set_name) > + > /* > * Device specific ioctls should only be in their respective headers > * The device specific ioctl range is from 0x40 to 0x9f.
Hi Tvrtko, Le 12/09/2024 à 10:13, Tvrtko Ursulin a écrit : > > On 11/09/2024 15:58, Pierre-Eric Pelloux-Prayer wrote: >> Giving the opportunity to userspace to associate a free-form >> name with a drm_file struct is helpful for tracking and debugging. >> >> This is similar to the existing DMA_BUF_SET_NAME ioctl. >> >> Access to name is protected by a mutex, and the 'clients' debugfs >> file has been updated to print it. >> >> Userspace MR to use this ioctl: >> https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1428 > > Idea seems useful to me. Various classes of comments/questions below: > >> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com> >> --- >> drivers/gpu/drm/drm_debugfs.c | 12 ++++++++---- >> drivers/gpu/drm/drm_file.c | 5 +++++ >> drivers/gpu/drm/drm_ioctl.c | 28 ++++++++++++++++++++++++++++ >> include/drm/drm_file.h | 9 +++++++++ >> include/uapi/drm/drm.h | 14 ++++++++++++++ >> 5 files changed, 64 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c >> index 6b239a24f1df..b7492225ae88 100644 >> --- a/drivers/gpu/drm/drm_debugfs.c >> +++ b/drivers/gpu/drm/drm_debugfs.c >> @@ -78,12 +78,13 @@ static int drm_clients_info(struct seq_file *m, void *data) >> kuid_t uid; >> seq_printf(m, >> - "%20s %5s %3s master a %5s %10s\n", >> + "%20s %5s %3s master a %5s %10s %20s\n", >> "command", >> "tgid", >> "dev", >> "uid", >> - "magic"); >> + "magic", >> + "name"); >> /* dev->filelist is sorted youngest first, but we want to present >> * oldest first (i.e. kernel, servers, clients), so walk backwardss. >> @@ -94,19 +95,22 @@ static int drm_clients_info(struct seq_file *m, void *data) >> struct task_struct *task; >> struct pid *pid; >> + mutex_lock(&priv->name_lock); >> 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", >> + seq_printf(m, "%20s %5d %3d %c %c %5d %10u %20s\n", >> task ? task->comm : "<unknown>", >> pid_vnr(pid), >> priv->minor->index, >> is_current_master ? 'y' : 'n', >> priv->authenticated ? 'y' : 'n', >> from_kuid_munged(seq_user_ns(m), uid), >> - priv->magic); >> + priv->magic, >> + priv->name ? priv->name : ""); >> rcu_read_unlock(); >> + mutex_unlock(&priv->name_lock); > > FWIW it is possible you could get away without the need for a lock on the read side if you make the > pointer RCU managed and stick a synchronize_rcu before kfree in the ioctl update path. > > Not because this lock would be a contentended one per se, but mostly to avoid complications such as > amdgpu_debugfs_gem_info_show() where 3/3 has it broken - cannot take the mutex in rcu locked > section. Just something to consider in case it would end up simpler code. I don't mind using RCU or a mutex. Christian suggested a mutex, so I used that, but I'm happy to switch if the RCU approach is preferred. > >> } >> mutex_unlock(&dev->filelist_mutex); >> return 0; >> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c >> index 01fde94fe2a9..558151c3912e 100644 >> --- a/drivers/gpu/drm/drm_file.c >> +++ b/drivers/gpu/drm/drm_file.c >> @@ -158,6 +158,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) >> spin_lock_init(&file->master_lookup_lock); >> mutex_init(&file->event_read_lock); >> + mutex_init(&file->name_lock); >> if (drm_core_check_feature(dev, DRIVER_GEM)) >> drm_gem_open(dev, file); >> @@ -259,6 +260,10 @@ void drm_file_free(struct drm_file *file) >> WARN_ON(!list_empty(&file->event_list)); >> put_pid(rcu_access_pointer(file->pid)); >> + >> + mutex_destroy(&file->name_lock); >> + kvfree(file->name); > > I think kfree is correct here. > OK, I'll update in v2. >> + >> kfree(file); >> } >> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c >> index 51f39912866f..ba2f2120e99b 100644 >> --- a/drivers/gpu/drm/drm_ioctl.c >> +++ b/drivers/gpu/drm/drm_ioctl.c >> @@ -540,6 +540,32 @@ int drm_version(struct drm_device *dev, void *data, >> return err; >> } >> +static int drm_set_name(struct drm_device *dev, void *data, >> + struct drm_file *file_priv) >> +{ >> + struct drm_set_name *name = data; >> + void *user_ptr; >> + char *new_name; >> + >> + if (name->name_len >= NAME_MAX) >> + return -EINVAL; > > Any special reason to use the filesystem NAME_MAX? Not really - feel free to suggest something else. > >> + >> + user_ptr = u64_to_user_ptr(name->name); >> + >> + new_name = memdup_user_nul(user_ptr, name->name_len); >> + >> + if (IS_ERR(new_name)) >> + return PTR_ERR(new_name); >> + >> + mutex_lock(&file_priv->name_lock); >> + if (file_priv->name) >> + kvfree(file_priv->name); >> + file_priv->name = new_name; >> + mutex_unlock(&file_priv->name_lock); > > One question is whether allowing name changes is interesting or it should be one shot? dma_buf_set_name allows to override the name, so I re-used the same logic. > > Second issue is to avoid any Little Bobby Tables situations and somewhat validate the input. At > least when kernel dumps in in debugfs and fdinfo, we probably don't want to allow userspace to be > too creative. Like output newlines or some other special characters. You mean checking "isgraph(c)" for each char? Or even stricter "isalnum(c) || c == '_' || c == '-'"? Thanks, Pierre-Eric > > Regards, > > Tvrtko > >> + >> + return 0; >> +} >> + >> static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) >> { >> /* ROOT_ONLY is only for CAP_SYS_ADMIN */ >> @@ -610,6 +636,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { >> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW), >> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW), >> + DRM_IOCTL_DEF(DRM_IOCTL_SET_NAME, drm_set_name, DRM_RENDER_ALLOW), >> + >> DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0), >> DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0), >> DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER), >> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h >> index 8c0030c77308..df26eee8f79c 100644 >> --- a/include/drm/drm_file.h >> +++ b/include/drm/drm_file.h >> @@ -388,6 +388,15 @@ struct drm_file { >> * Per-file buffer caches used by the PRIME buffer sharing code. >> */ >> struct drm_prime_file_private prime; >> + >> + /** >> + * @name: >> + * >> + * Userspace-provided name; useful for accounting and debugging. >> + */ >> + const char *name; >> + /** @name_lock: Protects @name. */ >> + struct mutex name_lock; >> }; >> /** >> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h >> index 16122819edfe..fc62bb21f79e 100644 >> --- a/include/uapi/drm/drm.h >> +++ b/include/uapi/drm/drm.h >> @@ -1024,6 +1024,12 @@ struct drm_crtc_queue_sequence { >> __u64 user_data; /* user data passed to event */ >> }; >> +struct drm_set_name { >> + __u64 name_len; >> + __u64 name; >> +}; >> + >> + >> #if defined(__cplusplus) >> } >> #endif >> @@ -1288,6 +1294,14 @@ extern "C" { >> */ >> #define DRM_IOCTL_MODE_CLOSEFB DRM_IOWR(0xD0, struct drm_mode_closefb) >> +/** >> + * DRM_IOCTL_SET_NAME - Attach a name to a drm_file >> + * >> + * This ioctl is similar to DMA_BUF_SET_NAME - it allows for easier tracking >> + * and debugging. >> + */ >> +#define DRM_IOCTL_SET_NAME DRM_IOWR(0xD1, struct drm_set_name) >> + >> /* >> * Device specific ioctls should only be in their respective headers >> * The device specific ioctl range is from 0x40 to 0x9f.
Am 13.09.24 um 14:17 schrieb Pierre-Eric Pelloux-Prayer: > Hi Tvrtko, > > Le 12/09/2024 à 10:13, Tvrtko Ursulin a écrit : >> >> On 11/09/2024 15:58, Pierre-Eric Pelloux-Prayer wrote: >>> Giving the opportunity to userspace to associate a free-form >>> name with a drm_file struct is helpful for tracking and debugging. >>> >>> This is similar to the existing DMA_BUF_SET_NAME ioctl. >>> >>> Access to name is protected by a mutex, and the 'clients' debugfs >>> file has been updated to print it. >>> >>> Userspace MR to use this ioctl: >>> https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1428 >>> >> >> Idea seems useful to me. Various classes of comments/questions below: >> >>> Signed-off-by: Pierre-Eric Pelloux-Prayer >>> <pierre-eric.pelloux-prayer@amd.com> >>> --- >>> drivers/gpu/drm/drm_debugfs.c | 12 ++++++++---- >>> drivers/gpu/drm/drm_file.c | 5 +++++ >>> drivers/gpu/drm/drm_ioctl.c | 28 ++++++++++++++++++++++++++++ >>> include/drm/drm_file.h | 9 +++++++++ >>> include/uapi/drm/drm.h | 14 ++++++++++++++ >>> 5 files changed, 64 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_debugfs.c >>> b/drivers/gpu/drm/drm_debugfs.c >>> index 6b239a24f1df..b7492225ae88 100644 >>> --- a/drivers/gpu/drm/drm_debugfs.c >>> +++ b/drivers/gpu/drm/drm_debugfs.c >>> @@ -78,12 +78,13 @@ static int drm_clients_info(struct seq_file *m, >>> void *data) >>> kuid_t uid; >>> seq_printf(m, >>> - "%20s %5s %3s master a %5s %10s\n", >>> + "%20s %5s %3s master a %5s %10s %20s\n", >>> "command", >>> "tgid", >>> "dev", >>> "uid", >>> - "magic"); >>> + "magic", >>> + "name"); >>> /* dev->filelist is sorted youngest first, but we want to present >>> * oldest first (i.e. kernel, servers, clients), so walk >>> backwardss. >>> @@ -94,19 +95,22 @@ static int drm_clients_info(struct seq_file *m, >>> void *data) >>> struct task_struct *task; >>> struct pid *pid; >>> + mutex_lock(&priv->name_lock); >>> 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", >>> + seq_printf(m, "%20s %5d %3d %c %c %5d %10u %20s\n", >>> task ? task->comm : "<unknown>", >>> pid_vnr(pid), >>> priv->minor->index, >>> is_current_master ? 'y' : 'n', >>> priv->authenticated ? 'y' : 'n', >>> from_kuid_munged(seq_user_ns(m), uid), >>> - priv->magic); >>> + priv->magic, >>> + priv->name ? priv->name : ""); >>> rcu_read_unlock(); >>> + mutex_unlock(&priv->name_lock); >> >> FWIW it is possible you could get away without the need for a lock on >> the read side if you make the pointer RCU managed and stick a >> synchronize_rcu before kfree in the ioctl update path. >> >> Not because this lock would be a contentended one per se, but mostly >> to avoid complications such as amdgpu_debugfs_gem_info_show() where >> 3/3 has it broken - cannot take the mutex in rcu locked section. Just >> something to consider in case it would end up simpler code. > > I don't mind using RCU or a mutex. Christian suggested a mutex, so I > used that, but I'm happy to switch if the RCU approach is preferred. RCU on a pure string is easier said than done. So I strongly suggest to stick with a mutex for now. It's not that contention is performance critical here. Regards, Christian. > > >> >>> } >>> mutex_unlock(&dev->filelist_mutex); >>> return 0; >>> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c >>> index 01fde94fe2a9..558151c3912e 100644 >>> --- a/drivers/gpu/drm/drm_file.c >>> +++ b/drivers/gpu/drm/drm_file.c >>> @@ -158,6 +158,7 @@ struct drm_file *drm_file_alloc(struct drm_minor >>> *minor) >>> spin_lock_init(&file->master_lookup_lock); >>> mutex_init(&file->event_read_lock); >>> + mutex_init(&file->name_lock); >>> if (drm_core_check_feature(dev, DRIVER_GEM)) >>> drm_gem_open(dev, file); >>> @@ -259,6 +260,10 @@ void drm_file_free(struct drm_file *file) >>> WARN_ON(!list_empty(&file->event_list)); >>> put_pid(rcu_access_pointer(file->pid)); >>> + >>> + mutex_destroy(&file->name_lock); >>> + kvfree(file->name); >> >> I think kfree is correct here. >> > > OK, I'll update in v2. > >>> + >>> kfree(file); >>> } >>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c >>> index 51f39912866f..ba2f2120e99b 100644 >>> --- a/drivers/gpu/drm/drm_ioctl.c >>> +++ b/drivers/gpu/drm/drm_ioctl.c >>> @@ -540,6 +540,32 @@ int drm_version(struct drm_device *dev, void >>> *data, >>> return err; >>> } >>> +static int drm_set_name(struct drm_device *dev, void *data, >>> + struct drm_file *file_priv) >>> +{ >>> + struct drm_set_name *name = data; >>> + void *user_ptr; >>> + char *new_name; >>> + >>> + if (name->name_len >= NAME_MAX) >>> + return -EINVAL; >> >> Any special reason to use the filesystem NAME_MAX? > > Not really - feel free to suggest something else. > >> >>> + >>> + user_ptr = u64_to_user_ptr(name->name); >>> + >>> + new_name = memdup_user_nul(user_ptr, name->name_len); >>> + >>> + if (IS_ERR(new_name)) >>> + return PTR_ERR(new_name); >>> + >>> + mutex_lock(&file_priv->name_lock); >>> + if (file_priv->name) >>> + kvfree(file_priv->name); >>> + file_priv->name = new_name; >>> + mutex_unlock(&file_priv->name_lock); >> >> One question is whether allowing name changes is interesting or it >> should be one shot? > > dma_buf_set_name allows to override the name, so I re-used the same > logic. > >> >> Second issue is to avoid any Little Bobby Tables situations and >> somewhat validate the input. At least when kernel dumps in in debugfs >> and fdinfo, we probably don't want to allow userspace to be too >> creative. Like output newlines or some other special characters. > > You mean checking "isgraph(c)" for each char? Or even stricter > "isalnum(c) || c == '_' || c == '-'"? > > Thanks, > Pierre-Eric > > >> >> Regards, >> >> Tvrtko >> >>> + >>> + return 0; >>> +} >>> + >>> static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) >>> { >>> /* ROOT_ONLY is only for CAP_SYS_ADMIN */ >>> @@ -610,6 +636,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { >>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, >>> drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW), >>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, >>> drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW), >>> + DRM_IOCTL_DEF(DRM_IOCTL_SET_NAME, drm_set_name, DRM_RENDER_ALLOW), >>> + >>> DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, >>> drm_mode_getplane_res, 0), >>> DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0), >>> DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, >>> DRM_MASTER), >>> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h >>> index 8c0030c77308..df26eee8f79c 100644 >>> --- a/include/drm/drm_file.h >>> +++ b/include/drm/drm_file.h >>> @@ -388,6 +388,15 @@ struct drm_file { >>> * Per-file buffer caches used by the PRIME buffer sharing code. >>> */ >>> struct drm_prime_file_private prime; >>> + >>> + /** >>> + * @name: >>> + * >>> + * Userspace-provided name; useful for accounting and debugging. >>> + */ >>> + const char *name; >>> + /** @name_lock: Protects @name. */ >>> + struct mutex name_lock; >>> }; >>> /** >>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h >>> index 16122819edfe..fc62bb21f79e 100644 >>> --- a/include/uapi/drm/drm.h >>> +++ b/include/uapi/drm/drm.h >>> @@ -1024,6 +1024,12 @@ struct drm_crtc_queue_sequence { >>> __u64 user_data; /* user data passed to event */ >>> }; >>> +struct drm_set_name { >>> + __u64 name_len; >>> + __u64 name; >>> +}; >>> + >>> + >>> #if defined(__cplusplus) >>> } >>> #endif >>> @@ -1288,6 +1294,14 @@ extern "C" { >>> */ >>> #define DRM_IOCTL_MODE_CLOSEFB DRM_IOWR(0xD0, struct >>> drm_mode_closefb) >>> +/** >>> + * DRM_IOCTL_SET_NAME - Attach a name to a drm_file >>> + * >>> + * This ioctl is similar to DMA_BUF_SET_NAME - it allows for easier >>> tracking >>> + * and debugging. >>> + */ >>> +#define DRM_IOCTL_SET_NAME DRM_IOWR(0xD1, struct drm_set_name) >>> + >>> /* >>> * Device specific ioctls should only be in their respective headers >>> * The device specific ioctl range is from 0x40 to 0x9f.
On 13/09/2024 13:17, Pierre-Eric Pelloux-Prayer wrote: > Hi Tvrtko, > > Le 12/09/2024 à 10:13, Tvrtko Ursulin a écrit : >> >> On 11/09/2024 15:58, Pierre-Eric Pelloux-Prayer wrote: >>> Giving the opportunity to userspace to associate a free-form >>> name with a drm_file struct is helpful for tracking and debugging. >>> >>> This is similar to the existing DMA_BUF_SET_NAME ioctl. >>> >>> Access to name is protected by a mutex, and the 'clients' debugfs >>> file has been updated to print it. >>> >>> Userspace MR to use this ioctl: >>> >>> https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1428 >> >> Idea seems useful to me. Various classes of comments/questions below: >> >>> Signed-off-by: Pierre-Eric Pelloux-Prayer >>> <pierre-eric.pelloux-prayer@amd.com> >>> --- >>> drivers/gpu/drm/drm_debugfs.c | 12 ++++++++---- >>> drivers/gpu/drm/drm_file.c | 5 +++++ >>> drivers/gpu/drm/drm_ioctl.c | 28 ++++++++++++++++++++++++++++ >>> include/drm/drm_file.h | 9 +++++++++ >>> include/uapi/drm/drm.h | 14 ++++++++++++++ >>> 5 files changed, 64 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_debugfs.c >>> b/drivers/gpu/drm/drm_debugfs.c >>> index 6b239a24f1df..b7492225ae88 100644 >>> --- a/drivers/gpu/drm/drm_debugfs.c >>> +++ b/drivers/gpu/drm/drm_debugfs.c >>> @@ -78,12 +78,13 @@ static int drm_clients_info(struct seq_file *m, >>> void *data) >>> kuid_t uid; >>> seq_printf(m, >>> - "%20s %5s %3s master a %5s %10s\n", >>> + "%20s %5s %3s master a %5s %10s %20s\n", >>> "command", >>> "tgid", >>> "dev", >>> "uid", >>> - "magic"); >>> + "magic", >>> + "name"); >>> /* dev->filelist is sorted youngest first, but we want to present >>> * oldest first (i.e. kernel, servers, clients), so walk >>> backwardss. >>> @@ -94,19 +95,22 @@ static int drm_clients_info(struct seq_file *m, >>> void *data) >>> struct task_struct *task; >>> struct pid *pid; >>> + mutex_lock(&priv->name_lock); >>> 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", >>> + seq_printf(m, "%20s %5d %3d %c %c %5d %10u %20s\n", >>> task ? task->comm : "<unknown>", >>> pid_vnr(pid), >>> priv->minor->index, >>> is_current_master ? 'y' : 'n', >>> priv->authenticated ? 'y' : 'n', >>> from_kuid_munged(seq_user_ns(m), uid), >>> - priv->magic); >>> + priv->magic, >>> + priv->name ? priv->name : ""); >>> rcu_read_unlock(); >>> + mutex_unlock(&priv->name_lock); >> >> FWIW it is possible you could get away without the need for a lock on >> the read side if you make the pointer RCU managed and stick a >> synchronize_rcu before kfree in the ioctl update path. >> >> Not because this lock would be a contentended one per se, but mostly >> to avoid complications such as amdgpu_debugfs_gem_info_show() where >> 3/3 has it broken - cannot take the mutex in rcu locked section. Just >> something to consider in case it would end up simpler code. > > I don't mind using RCU or a mutex. Christian suggested a mutex, so I > used that, but I'm happy to switch if the RCU approach is preferred. Mutex is fine as I said. Just mentioning RCU since it feels trivial and avoids the complications in amdgpu_debugfs_gem_info_show(). >>> mutex_unlock(&dev->filelist_mutex); >>> return 0; >>> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c >>> index 01fde94fe2a9..558151c3912e 100644 >>> --- a/drivers/gpu/drm/drm_file.c >>> +++ b/drivers/gpu/drm/drm_file.c >>> @@ -158,6 +158,7 @@ struct drm_file *drm_file_alloc(struct drm_minor >>> *minor) >>> spin_lock_init(&file->master_lookup_lock); >>> mutex_init(&file->event_read_lock); >>> + mutex_init(&file->name_lock); >>> if (drm_core_check_feature(dev, DRIVER_GEM)) >>> drm_gem_open(dev, file); >>> @@ -259,6 +260,10 @@ void drm_file_free(struct drm_file *file) >>> WARN_ON(!list_empty(&file->event_list)); >>> put_pid(rcu_access_pointer(file->pid)); >>> + >>> + mutex_destroy(&file->name_lock); >>> + kvfree(file->name); >> >> I think kfree is correct here. >> > > OK, I'll update in v2. > >>> + >>> kfree(file); >>> } >>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c >>> index 51f39912866f..ba2f2120e99b 100644 >>> --- a/drivers/gpu/drm/drm_ioctl.c >>> +++ b/drivers/gpu/drm/drm_ioctl.c >>> @@ -540,6 +540,32 @@ int drm_version(struct drm_device *dev, void *data, >>> return err; >>> } >>> +static int drm_set_name(struct drm_device *dev, void *data, >>> + struct drm_file *file_priv) >>> +{ >>> + struct drm_set_name *name = data; >>> + void *user_ptr; >>> + char *new_name; >>> + >>> + if (name->name_len >= NAME_MAX) >>> + return -EINVAL; >> >> Any special reason to use the filesystem NAME_MAX? > > Not really - feel free to suggest something else. I was just curios because dma-buf uses own much shorter limit. You could always follow the same approach so someone else does not have to wonder what is the connection with NAME_MAX. :) I would also think 255 is way too generous but meh. >>> + >>> + user_ptr = u64_to_user_ptr(name->name); >>> + >>> + new_name = memdup_user_nul(user_ptr, name->name_len); >>> + >>> + if (IS_ERR(new_name)) >>> + return PTR_ERR(new_name); >>> + >>> + mutex_lock(&file_priv->name_lock); >>> + if (file_priv->name) >>> + kvfree(file_priv->name); >>> + file_priv->name = new_name; >>> + mutex_unlock(&file_priv->name_lock); >> >> One question is whether allowing name changes is interesting or it >> should be one shot? > > dma_buf_set_name allows to override the name, so I re-used the same logic. No complaints per se, again just curiousity. But it could be worth to think whether renames are required and if not simplify the code by not allowing them. May remove the need for any locking on the read side. >> Second issue is to avoid any Little Bobby Tables situations and >> somewhat validate the input. At least when kernel dumps in in debugfs >> and fdinfo, we probably don't want to allow userspace to be too >> creative. Like output newlines or some other special characters. > > You mean checking "isgraph(c)" for each char? Or even stricter > "isalnum(c) || c == '_' || c == '-'"? Hard to say apart that newlines definitely feel like a no go. As said, we don't want someone to do: drm_set_name(f, "little\ndrm-client-name: bobby\ndrm-client-name: tables") :)) Or even worse, finding creative ways to hide yourself from gputop and such be exploiting some userspace bugs. Hmm.. isgraph sounds potentially good enough but hopefully someone else provides an opinion here. Part of me thinks we should be stricter but I can't really justify it on the spot. But it is the first userspace provided free form string in drm-usage-stats.rst so it deserves some care to define the rules. Regards, Tvrtko >> Tvrtko >> >>> + >>> + return 0; >>> +} >>> + >>> static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) >>> { >>> /* ROOT_ONLY is only for CAP_SYS_ADMIN */ >>> @@ -610,6 +636,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { >>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, >>> drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW), >>> DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, >>> drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW), >>> + DRM_IOCTL_DEF(DRM_IOCTL_SET_NAME, drm_set_name, DRM_RENDER_ALLOW), >>> + >>> DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, >>> drm_mode_getplane_res, 0), >>> DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0), >>> DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, >>> DRM_MASTER), >>> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h >>> index 8c0030c77308..df26eee8f79c 100644 >>> --- a/include/drm/drm_file.h >>> +++ b/include/drm/drm_file.h >>> @@ -388,6 +388,15 @@ struct drm_file { >>> * Per-file buffer caches used by the PRIME buffer sharing code. >>> */ >>> struct drm_prime_file_private prime; >>> + >>> + /** >>> + * @name: >>> + * >>> + * Userspace-provided name; useful for accounting and debugging. >>> + */ >>> + const char *name; >>> + /** @name_lock: Protects @name. */ >>> + struct mutex name_lock; >>> }; >>> /** >>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h >>> index 16122819edfe..fc62bb21f79e 100644 >>> --- a/include/uapi/drm/drm.h >>> +++ b/include/uapi/drm/drm.h >>> @@ -1024,6 +1024,12 @@ struct drm_crtc_queue_sequence { >>> __u64 user_data; /* user data passed to event */ >>> }; >>> +struct drm_set_name { >>> + __u64 name_len; >>> + __u64 name; >>> +}; >>> + >>> + >>> #if defined(__cplusplus) >>> } >>> #endif >>> @@ -1288,6 +1294,14 @@ extern "C" { >>> */ >>> #define DRM_IOCTL_MODE_CLOSEFB DRM_IOWR(0xD0, struct >>> drm_mode_closefb) >>> +/** >>> + * DRM_IOCTL_SET_NAME - Attach a name to a drm_file >>> + * >>> + * This ioctl is similar to DMA_BUF_SET_NAME - it allows for easier >>> tracking >>> + * and debugging. >>> + */ >>> +#define DRM_IOCTL_SET_NAME DRM_IOWR(0xD1, struct drm_set_name) >>> + >>> /* >>> * Device specific ioctls should only be in their respective headers >>> * The device specific ioctl range is from 0x40 to 0x9f.
Hi Pierre-Eric, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-exynos/exynos-drm-next] [also build test WARNING on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-misc/drm-misc-next drm-tip/drm-tip linus/master v6.11-rc7 next-20240913] [cannot apply to drm/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Pierre-Eric-Pelloux-Prayer/drm-use-drm_file-name-in-fdinfo/20240911-230058 base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next patch link: https://lore.kernel.org/r/20240911145836.734080-1-pierre-eric.pelloux-prayer%40amd.com patch subject: [PATCH 1/3] drm: add DRM_SET_NAME ioctl config: x86_64-randconfig-121-20240913 (https://download.01.org/0day-ci/archive/20240914/202409140642.ZDKf0cja-lkp@intel.com/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240914/202409140642.ZDKf0cja-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409140642.ZDKf0cja-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/drm_ioctl.c:553:18: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *user_ptr @@ got void [noderef] __user * @@ drivers/gpu/drm/drm_ioctl.c:553:18: sparse: expected void *user_ptr drivers/gpu/drm/drm_ioctl.c:553:18: sparse: got void [noderef] __user * >> drivers/gpu/drm/drm_ioctl.c:555:36: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] __user * @@ got void *user_ptr @@ drivers/gpu/drm/drm_ioctl.c:555:36: sparse: expected void const [noderef] __user * drivers/gpu/drm/drm_ioctl.c:555:36: sparse: got void *user_ptr vim +553 drivers/gpu/drm/drm_ioctl.c 542 543 static int drm_set_name(struct drm_device *dev, void *data, 544 struct drm_file *file_priv) 545 { 546 struct drm_set_name *name = data; 547 void *user_ptr; 548 char *new_name; 549 550 if (name->name_len >= NAME_MAX) 551 return -EINVAL; 552 > 553 user_ptr = u64_to_user_ptr(name->name); 554 > 555 new_name = memdup_user_nul(user_ptr, name->name_len); 556 557 if (IS_ERR(new_name)) 558 return PTR_ERR(new_name); 559 560 mutex_lock(&file_priv->name_lock); 561 if (file_priv->name) 562 kvfree(file_priv->name); 563 file_priv->name = new_name; 564 mutex_unlock(&file_priv->name_lock); 565 566 return 0; 567 } 568
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 6b239a24f1df..b7492225ae88 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -78,12 +78,13 @@ static int drm_clients_info(struct seq_file *m, void *data) kuid_t uid; seq_printf(m, - "%20s %5s %3s master a %5s %10s\n", + "%20s %5s %3s master a %5s %10s %20s\n", "command", "tgid", "dev", "uid", - "magic"); + "magic", + "name"); /* dev->filelist is sorted youngest first, but we want to present * oldest first (i.e. kernel, servers, clients), so walk backwardss. @@ -94,19 +95,22 @@ static int drm_clients_info(struct seq_file *m, void *data) struct task_struct *task; struct pid *pid; + mutex_lock(&priv->name_lock); 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", + seq_printf(m, "%20s %5d %3d %c %c %5d %10u %20s\n", task ? task->comm : "<unknown>", pid_vnr(pid), priv->minor->index, is_current_master ? 'y' : 'n', priv->authenticated ? 'y' : 'n', from_kuid_munged(seq_user_ns(m), uid), - priv->magic); + priv->magic, + priv->name ? priv->name : ""); rcu_read_unlock(); + mutex_unlock(&priv->name_lock); } mutex_unlock(&dev->filelist_mutex); return 0; diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 01fde94fe2a9..558151c3912e 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -158,6 +158,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) spin_lock_init(&file->master_lookup_lock); mutex_init(&file->event_read_lock); + mutex_init(&file->name_lock); if (drm_core_check_feature(dev, DRIVER_GEM)) drm_gem_open(dev, file); @@ -259,6 +260,10 @@ void drm_file_free(struct drm_file *file) WARN_ON(!list_empty(&file->event_list)); put_pid(rcu_access_pointer(file->pid)); + + mutex_destroy(&file->name_lock); + kvfree(file->name); + kfree(file); } diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 51f39912866f..ba2f2120e99b 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -540,6 +540,32 @@ int drm_version(struct drm_device *dev, void *data, return err; } +static int drm_set_name(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_set_name *name = data; + void *user_ptr; + char *new_name; + + if (name->name_len >= NAME_MAX) + return -EINVAL; + + user_ptr = u64_to_user_ptr(name->name); + + new_name = memdup_user_nul(user_ptr, name->name_len); + + if (IS_ERR(new_name)) + return PTR_ERR(new_name); + + mutex_lock(&file_priv->name_lock); + if (file_priv->name) + kvfree(file_priv->name); + file_priv->name = new_name; + mutex_unlock(&file_priv->name_lock); + + return 0; +} + static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv) { /* ROOT_ONLY is only for CAP_SYS_ADMIN */ @@ -610,6 +636,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_SET_NAME, drm_set_name, DRM_RENDER_ALLOW), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, 0), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, 0), DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER), diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 8c0030c77308..df26eee8f79c 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -388,6 +388,15 @@ struct drm_file { * Per-file buffer caches used by the PRIME buffer sharing code. */ struct drm_prime_file_private prime; + + /** + * @name: + * + * Userspace-provided name; useful for accounting and debugging. + */ + const char *name; + /** @name_lock: Protects @name. */ + struct mutex name_lock; }; /** diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 16122819edfe..fc62bb21f79e 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -1024,6 +1024,12 @@ struct drm_crtc_queue_sequence { __u64 user_data; /* user data passed to event */ }; +struct drm_set_name { + __u64 name_len; + __u64 name; +}; + + #if defined(__cplusplus) } #endif @@ -1288,6 +1294,14 @@ extern "C" { */ #define DRM_IOCTL_MODE_CLOSEFB DRM_IOWR(0xD0, struct drm_mode_closefb) +/** + * DRM_IOCTL_SET_NAME - Attach a name to a drm_file + * + * This ioctl is similar to DMA_BUF_SET_NAME - it allows for easier tracking + * and debugging. + */ +#define DRM_IOCTL_SET_NAME DRM_IOWR(0xD1, struct drm_set_name) + /* * Device specific ioctls should only be in their respective headers * The device specific ioctl range is from 0x40 to 0x9f.
Giving the opportunity to userspace to associate a free-form name with a drm_file struct is helpful for tracking and debugging. This is similar to the existing DMA_BUF_SET_NAME ioctl. Access to name is protected by a mutex, and the 'clients' debugfs file has been updated to print it. Userspace MR to use this ioctl: https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1428 Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com> --- drivers/gpu/drm/drm_debugfs.c | 12 ++++++++---- drivers/gpu/drm/drm_file.c | 5 +++++ drivers/gpu/drm/drm_ioctl.c | 28 ++++++++++++++++++++++++++++ include/drm/drm_file.h | 9 +++++++++ include/uapi/drm/drm.h | 14 ++++++++++++++ 5 files changed, 64 insertions(+), 4 deletions(-)