Re: [PATCH] drm/amdgpu/userq: fix userq_signal_ioctl stuck in drm_exec_until_all_locked()

["Christian König" <christian.koenig@amd.com>]

On 9/7/26 10:47, Yogesh Mohan Marimuthu wrote:
> If in userq_signal_ioctl only bo_write_handles is passed and
> num_bo_read_handles is zero then if there is contention the code is stuck
> in drm_exec_until_all_locked()
> 
> This happens because read bo's are handled first and then write bo's in
> drm_exec_until_all_locked loop. When there is contention in one of the
> write bo, exec->contended bo is set and the loop is retried, but
> read bo is zero, still drm_exec_retry_on_contention() macro for read bo
> is executed without drm_exec_lock_contended() getting executed.
> drm_exec_retry_on_contention will keep going to beginning of the loop
> causing infinite loop.

Well that is a really good find but clear NAK to the solution.

This if this causes an infinite loop there is a bug somewhere in the drm_exec object.

My educated guess is that drm_exec_prepare_array() needs to call drm_exec_lock_contended() even when num_objects is zero.

Regards,
Christian.

> 
> Fix this by only locking and reserving fence for bo only if there are
> bo passed userq_signal_ioctl.
> 
> Observed this issue when testing with MR
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/40808
> 
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohanmarimuthu@amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 24 +++++++++++--------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index c270635c9..135e77837 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -572,17 +572,21 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>  		      (num_read_bo_handles + num_write_bo_handles));
>  
>  	drm_exec_until_all_locked(&exec) {
> -		r = drm_exec_prepare_array(&exec, gobj_read,
> -					   num_read_bo_handles, 1);
> -		drm_exec_retry_on_contention(&exec);
> -		if (r)
> -			goto exec_fini;
> +		if (num_read_bo_handles) {
> +			r = drm_exec_prepare_array(&exec, gobj_read,
> +						   num_read_bo_handles, 1);
> +			drm_exec_retry_on_contention(&exec);
> +			if (r)
> +				goto exec_fini;
> +		}
>  
> -		r = drm_exec_prepare_array(&exec, gobj_write,
> -					   num_write_bo_handles, 1);
> -		drm_exec_retry_on_contention(&exec);
> -		if (r)
> -			goto exec_fini;
> +		if (num_write_bo_handles) {
> +			r = drm_exec_prepare_array(&exec, gobj_write,
> +						   num_write_bo_handles, 1);
> +			drm_exec_retry_on_contention(&exec);
> +			if (r)
> +				goto exec_fini;
> +		}
>  	}
>  
>  	/* And publish the new fence in the BOs and syncobj */