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

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

[Yogesh Mohan Marimuthu]

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.

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 */
-- 
2.43.0

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

[Christian König]

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 */

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

[Khatri, Sunil]

[-- Attachment #1: Type: text/plain, Size: 3220 bytes --]


On 07-09-2026 02:23 pm, Christian König wrote:
> 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.
drm_exec_prepare_array()  returns immediately if no of objects is 0. If 
its not 0 in that case order of calls from prepare_array is 
drm_exec_prepare_obj -> drm_exec_lock_obj -> drm_exec_lock_contended
So contention is never cleaned.

I can patch this up in drm_exec_prepare_array in case no of objects is 0 
call the drm_exec_lock_contended like below.
if(!num_objects)
returndrm_exec_lock_contended(exec);

Regards
Sunil Khatri
>
> 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 */

[-- Attachment #2: Type: text/html, Size: 4907 bytes --]

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

[Christian König]

On 9/7/26 16:42, Khatri, Sunil wrote:
> 
> On 07-09-2026 02:23 pm, Christian König wrote:
>> 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.
> drm_exec_prepare_array()  returns immediately if no of objects is 0. If its not 0 in that case order of calls from prepare_array is drm_exec_prepare_obj -> drm_exec_lock_obj -> drm_exec_lock_contended
> So contention is never cleaned. 
> 
> I can patch this up in drm_exec_prepare_array in case no of objects is 0 call the drm_exec_lock_contended like below.
> if(!num_objects)
>                 returndrm_exec_lock_contended(exec);

That sounds reasonable, yes.

Thanks,
Christian.

> 
> Regards
> Sunil Khatri
>> 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 */

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

[Khatri, Sunil]

On 07-09-2026 08:41 pm, Christian König wrote:
> On 9/7/26 16:42, Khatri, Sunil wrote:
>> On 07-09-2026 02:23 pm, Christian König wrote:
>>> 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.
>> drm_exec_prepare_array()  returns immediately if no of objects is 0. If its not 0 in that case order of calls from prepare_array is drm_exec_prepare_obj -> drm_exec_lock_obj -> drm_exec_lock_contended
>> So contention is never cleaned.
>>
>> I can patch this up in drm_exec_prepare_array in case no of objects is 0 call the drm_exec_lock_contended like below.
>> if(!num_objects)
>>                  returndrm_exec_lock_contended(exec);
> That sounds reasonable, yes.

Thanks Christian,

sent the patch for review on dri-devel@lists.freedesktop.org

[PATCH] drm/drm_exec: remove contention for num_objects is 0

have a look please.

Regards
Sunil khatri


>
> Thanks,
> Christian.
>
>> Regards
>> Sunil Khatri
>>> 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 */