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 <[email protected]> >>> --- >>> .../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 */
