[PATCH 2/7] drm/syncobj: Do not allocate an array to store zeros when waiting

[Maíra Canal]

Hi Tvrtko,

On 18/03/25 12:54, Tvrtko Ursulin wrote:
> When waiting on syncobjs the current code allocates a temporary array only
> to fill it up with all zeros.
> 
> We can avoid that by relying on the allocated entry array already being
> zero allocated.
> 
> For the timeline mode we can fetch the timeline point values as we
> populate the entries array so also do not need this additional temporary
> allocation.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>

Reviewed-by: Maíra Canal <mcanal at igalia.com>

Best Regards,
- Maíra

> ---
>   drivers/gpu/drm/drm_syncobj.c | 39 ++++++++++++++---------------------
>   1 file changed, 15 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index d0d60c331df8..fd5ba6c89666 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1028,7 +1028,7 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj,
>   }
>   
>   static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
> -						  void __user *user_points,
> +						  u64 __user *user_points,
>   						  uint32_t count,
>   						  uint32_t flags,
>   						  signed long timeout,
> @@ -1036,9 +1036,8 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   						  ktime_t *deadline)
>   {
>   	struct syncobj_wait_entry *entries;
> -	struct dma_fence *fence;
> -	uint64_t *points;
>   	uint32_t signaled_count, i;
> +	struct dma_fence *fence;
>   
>   	if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>   		     DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) {
> @@ -1046,24 +1045,14 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   		lockdep_assert_none_held_once();
>   	}
>   
> -	points = kmalloc_array(count, sizeof(*points), GFP_KERNEL);
> -	if (points == NULL)
> -		return -ENOMEM;
> -
> -	if (!user_points) {
> -		memset(points, 0, count * sizeof(uint64_t));
> -
> -	} else if (copy_from_user(points, user_points,
> -				  sizeof(uint64_t) * count)) {
> -		timeout = -EFAULT;
> -		goto err_free_points;
> -	}
> +	if (user_points &&
> +	    !access_ok(user_points, count * sizeof(*user_points)))
> +		return -EFAULT;
>   
>   	entries = kcalloc(count, sizeof(*entries), GFP_KERNEL);
> -	if (!entries) {
> -		timeout = -ENOMEM;
> -		goto err_free_points;
> -	}
> +	if (!entries)
> +		return -ENOMEM;
> +
>   	/* Walk the list of sync objects and initialize entries.  We do
>   	 * this up-front so that we can properly return -EINVAL if there is
>   	 * a syncobj with a missing fence and then never have the chance of
> @@ -1074,9 +1063,14 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   		struct dma_fence *fence;
>   
>   		entries[i].task = current;
> -		entries[i].point = points[i];
> +		if (user_points &&
> +		    __get_user(entries[i].point, user_points++)) {
> +			timeout = -EFAULT;
> +			goto cleanup_entries;
> +		}
>   		fence = drm_syncobj_fence_get(syncobjs[i]);
> -		if (!fence || dma_fence_chain_find_seqno(&fence, points[i])) {
> +		if (!fence ||
> +		    dma_fence_chain_find_seqno(&fence, entries[i].point)) {
>   			dma_fence_put(fence);
>   			if (flags & (DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT |
>   				     DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE)) {
> @@ -1182,9 +1176,6 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>   	}
>   	kfree(entries);
>   
> -err_free_points:
> -	kfree(points);
> -
>   	return timeout;
>   }
>