On 3/10/26 09:47, Liang, Prike wrote: > [Public] > > Regards, > Prike > >> -----Original Message----- >> From: Koenig, Christian <[email protected]> >> Sent: Tuesday, March 10, 2026 4:22 PM >> To: Liang, Prike <[email protected]>; [email protected] >> Cc: Deucher, Alexander <[email protected]> >> Subject: Re: [PATCH] drm/amdgpu: remove the WAIT_FOR_SUBMIT flag under >> lock context >> >> On 3/10/26 08:02, Prike Liang wrote: >>> In the userq fence gather and emit IOCTL path we acquire BO locks (via >>> drm_exec/dma_resv_lock)before calling drm_syncobj_find_fence(). >>> This causes drm_syncobj_find_fence() to complain because it is entered >>> with locks held while the WAIT_FOR_SUBMIT flag is set in the calling >>> context. >>> >>> However, the userq userspace path does not rely on >>> DRM_IOCTL_SYNCOBJ_WAIT to wait on fences that are dependencies of userq >> submissions. >> >> That's not correct. >>> All waiting is >>> handled separately, so the WAIT_FOR_SUBMIT flag is effectively unused >>> for this IOCTL. >>> >>> Therefore, we simply clear the WAIT_FOR_SUBMIT flag for this path. >>> This avoids the lockdep / drm_syncobj_find_fence() warning about being >>> called under a locked context, and has no functional impact on userq >>> behavior since DRM_IOCTL_SYNCOBJ_WAIT is not part of the userq >> synchronization model. >> >> That doesn't event remotely work. >> >> See the patches I've send out a month ago or so for the correct fix. > > OK, after checking the Mesa code, DRM_IOCTL_SYNCOBJ_WAIT is still invoked > before creating the userq. > I had already considered resolving the syncobj-dependent fences before taking > the context lock; now given that > this flag is still used for syncobj wait fences, I’ll go with that approach > instead and drop this patch.
We actually just need to take the locks in the correct order, it's a bit code re-structuring but not much of an issue. Regards, Christian. > >> Regards, >> Christian. >> >> >>> >>> Signed-off-by: Prike Liang <[email protected]> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c >>> index 76f32fd768fb..7a309b0130d8 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c >>> @@ -708,7 +708,7 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void >> *data, >>> for (i = 0; i < num_points; i++) { >>> r = drm_syncobj_find_fence(filp, >>> timeline_handles[i], >>> timeline_points[i], >>> - >> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT, >>> + 0, >>> &fence); >>> if (r) >>> goto exec_fini; >>> @@ -726,7 +726,7 @@ int amdgpu_userq_wait_ioctl(struct drm_device >>> *dev, void *data, >>> >>> r = drm_syncobj_find_fence(filp, syncobj_handles[i], >>> 0, >>> - >> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT, >>> + 0, >>> &fence); >>> if (r) >>> goto exec_fini; >>> @@ -818,7 +818,7 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void >> *data, >>> for (i = 0; i < num_points; i++) { >>> r = drm_syncobj_find_fence(filp, >>> timeline_handles[i], >>> timeline_points[i], >>> - >> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT, >>> + 0, >>> &fence); >>> if (r) >>> goto free_fences; >>> @@ -844,7 +844,7 @@ int amdgpu_userq_wait_ioctl(struct drm_device >>> *dev, void *data, >>> >>> r = drm_syncobj_find_fence(filp, syncobj_handles[i], >>> 0, >>> - >> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT, >>> + 0, >>> &fence); >>> if (r) >>> goto free_fences; >
