| Message ID | 20250327084215.26662-4-tvrtko.ursulin@igalia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few drm_syncobj optimisations | expand |
Hi Tvrtko, On 27/03/25 05:42, Tvrtko Ursulin wrote: > Drm_syncobj_array_find() helper is used from many userspace ioctl entry > points with the task of looking up userspace handles to internal objects. > > We can easily avoid one temporary allocation by making it read the handles > as it is looking them up. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> Reviewed-by: Maíra Canal <mcanal@igalia.com> Best Regards, - Maíra > --- > v2: > * Fix handle type. > * Undo pointless unwind change. > --- > drivers/gpu/drm/drm_syncobj.c | 36 +++++++++++++++-------------------- > 1 file changed, 15 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index 28081cf74052..cd36c888f112 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -1213,39 +1213,35 @@ signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec) > EXPORT_SYMBOL(drm_timeout_abs_to_jiffies); > > static int drm_syncobj_array_find(struct drm_file *file_private, > - void __user *user_handles, > - uint32_t count_handles, > + u32 __user *handles, > + uint32_t count, > struct drm_syncobj ***syncobjs_out) > { > - uint32_t i, *handles; > struct drm_syncobj **syncobjs; > + uint32_t i; > int ret; > > - handles = kmalloc_array(count_handles, sizeof(*handles), GFP_KERNEL); > - if (handles == NULL) > + if (!access_ok(handles, count * sizeof(*handles))) > + return -EFAULT; > + > + syncobjs = kmalloc_array(count, sizeof(*syncobjs), GFP_KERNEL); > + if (!syncobjs) > return -ENOMEM; > > - if (copy_from_user(handles, user_handles, > - sizeof(uint32_t) * count_handles)) { > - ret = -EFAULT; > - goto err_free_handles; > - } > + for (i = 0; i < count; i++) { > + u32 handle; > > - syncobjs = kmalloc_array(count_handles, sizeof(*syncobjs), GFP_KERNEL); > - if (syncobjs == NULL) { > - ret = -ENOMEM; > - goto err_free_handles; > - } > - > - for (i = 0; i < count_handles; i++) { > - syncobjs[i] = drm_syncobj_find(file_private, handles[i]); > + if (__get_user(handle, handles++)) { > + ret = -EFAULT; > + goto err_put_syncobjs; > + } > + syncobjs[i] = drm_syncobj_find(file_private, handle); > if (!syncobjs[i]) { > ret = -ENOENT; > goto err_put_syncobjs; > } > } > > - kfree(handles); > *syncobjs_out = syncobjs; > return 0; > > @@ -1253,8 +1249,6 @@ static int drm_syncobj_array_find(struct drm_file *file_private, > while (i-- > 0) > drm_syncobj_put(syncobjs[i]); > kfree(syncobjs); > -err_free_handles: > - kfree(handles); > > return ret; > }
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 28081cf74052..cd36c888f112 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -1213,39 +1213,35 @@ signed long drm_timeout_abs_to_jiffies(int64_t timeout_nsec) EXPORT_SYMBOL(drm_timeout_abs_to_jiffies); static int drm_syncobj_array_find(struct drm_file *file_private, - void __user *user_handles, - uint32_t count_handles, + u32 __user *handles, + uint32_t count, struct drm_syncobj ***syncobjs_out) { - uint32_t i, *handles; struct drm_syncobj **syncobjs; + uint32_t i; int ret; - handles = kmalloc_array(count_handles, sizeof(*handles), GFP_KERNEL); - if (handles == NULL) + if (!access_ok(handles, count * sizeof(*handles))) + return -EFAULT; + + syncobjs = kmalloc_array(count, sizeof(*syncobjs), GFP_KERNEL); + if (!syncobjs) return -ENOMEM; - if (copy_from_user(handles, user_handles, - sizeof(uint32_t) * count_handles)) { - ret = -EFAULT; - goto err_free_handles; - } + for (i = 0; i < count; i++) { + u32 handle; - syncobjs = kmalloc_array(count_handles, sizeof(*syncobjs), GFP_KERNEL); - if (syncobjs == NULL) { - ret = -ENOMEM; - goto err_free_handles; - } - - for (i = 0; i < count_handles; i++) { - syncobjs[i] = drm_syncobj_find(file_private, handles[i]); + if (__get_user(handle, handles++)) { + ret = -EFAULT; + goto err_put_syncobjs; + } + syncobjs[i] = drm_syncobj_find(file_private, handle); if (!syncobjs[i]) { ret = -ENOENT; goto err_put_syncobjs; } } - kfree(handles); *syncobjs_out = syncobjs; return 0; @@ -1253,8 +1249,6 @@ static int drm_syncobj_array_find(struct drm_file *file_private, while (i-- > 0) drm_syncobj_put(syncobjs[i]); kfree(syncobjs); -err_free_handles: - kfree(handles); return ret; }
Drm_syncobj_array_find() helper is used from many userspace ioctl entry points with the task of looking up userspace handles to internal objects. We can easily avoid one temporary allocation by making it read the handles as it is looking them up. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> --- v2: * Fix handle type. * Undo pointless unwind change. --- drivers/gpu/drm/drm_syncobj.c | 36 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 21 deletions(-)