RE: [PATCH 5/5] drm/amdgpu: move drm_client_dev_resume outside reset_domain->sem

AMD General

Regards,
      Prike

> -----Original Message-----
> From: amd-gfx <[email protected]> On Behalf Of
> [email protected]
> Sent: Tuesday, August 25, 2026 7:30 AM
> To: [email protected]
> Cc: Koenig, Christian <[email protected]>; Deucher, Alexander
> <[email protected]>; Prosyak, Vitaly <[email protected]>;
> Khatri, Sunil <[email protected]>
> Subject: [PATCH 5/5] drm/amdgpu: move drm_client_dev_resume outside
> reset_domain->sem
>
> From: Vitaly Prosyak <[email protected]>
>
> drm_client_dev_resume() acquires clientlist_mutex. Calling it inside
> amdgpu_device_reinit_after_reset() while reset_domain->sem is held violates the
> same ordering fixed on the suspend side:
>
>   clientlist_mutex -> userq_mutex -> reset_domain->sem
>
>   WARNING: possible circular locking dependency detected
>   kworker/u128:0 is trying to acquire lock:
>     (&dev->clientlist_mutex), at: drm_client_dev_resume+0x2d/0x120
>   but task is already holding lock:
>     (&reset_domain->sem), at: amdgpu_device_lock_reset_domain+0x1c/0x30
> [amdgpu]
>   Chain exists of:
>     &dev->clientlist_mutex --> &userq_mgr->userq_mutex --> &reset_domain->sem
>
> Move drm_client_dev_resume() to after
> amdgpu_device_recovery_put_reset_lock() in amdgpu_device_gpu_recover().
>
> Cc: Christian Koenig <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: Sunil Khatri <[email protected]>
> Signed-off-by: Vitaly Prosyak <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c248d589f755..30e8b1963204 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5210,8 +5210,6 @@ int amdgpu_device_reinit_after_reset(struct
> amdgpu_reset_context *reset_context)
>                               if (r)
>                                       goto out;
>
> -                             drm_client_dev_resume(adev_to_drm(tmp_adev));
> -
>                               /*
>                                * The GPU enters bad state once faulty pages
>                                * by ECC has reached the threshold, and ras @@ -
> 5826,6 +5824,15 @@ int amdgpu_device_gpu_recover(struct amdgpu_device
> *adev,
>       amdgpu_device_gpu_resume(adev, &device_list, need_emergency_restart);
>  reset_unlock:
>       amdgpu_device_recovery_put_reset_lock(adev, &device_list);
> +     /* Resume DRM clients after releasing reset lock: clientlist_mutex
> +      * must be taken before reset_domain->sem in lock ordering.
> +      */
> +     if (!need_emergency_restart && !r) {
> +             struct amdgpu_device *tmp_adev;
> +
> +             list_for_each_entry(tmp_adev, &device_list, reset_list)
> +                     drm_client_dev_resume(adev_to_drm(tmp_adev));
> +     }

I am also working on the same userq lockdep issue, and as to this issue we may need to remove the DRM client suspend/resume thoroughly.

To avoid duplicated efforts on those series topic, how about align our patch sets and consolidate them into a single upstream thread?

Thanks,
Prike

>       amdgpu_ras_post_reset(adev, &device_list);
>  end_reset:
>       if (hive) {
> --
> 2.54.0