On 7/27/26 15:35, Prike Liang wrote: > Suspend the KMS/DRM clients before locking the reset domain. > drm_client_dev_suspend() acquires clientlist_mutex, which must be > taken outside reset_domain->sem. Acquiring it while holding the > reset domain lock inverts the ordering established elsewhere > (clientlist_mutex -> ... -> reset_domain->sem) and can deadlock.
That is a good catch, but clearly not the correct resolution. drm_client_dev_suspend() should *never* be called from the reset path in the first place. This is only for suspend/resume and can trivially deadlock here through quite a number of other locks. Regards, Christian. > > Signed-off-by: Prike Liang <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 472e96ae884e..c670d5cfe3cc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -5371,8 +5371,6 @@ static void amdgpu_device_halt_activities(struct > amdgpu_device *adev, > */ > amdgpu_unregister_gpu_instance(tmp_adev); > > - drm_client_dev_suspend(adev_to_drm(tmp_adev)); > - > /* disable ras on ALL IPs */ > if (!need_emergency_restart && !amdgpu_reset_in_dpc(adev)) > amdgpu_ras_suspend(tmp_adev); > @@ -5535,6 +5533,22 @@ static void amdgpu_device_gpu_resume(struct > amdgpu_device *adev, > } > > > +static void amdgpu_device_recovery_suspend_clients(struct list_head > *device_list) > +{ > + struct amdgpu_device *tmp_adev = NULL; > + > + list_for_each_entry(tmp_adev, device_list, reset_list) > + drm_client_dev_suspend(adev_to_drm(tmp_adev)); > +} > + > +static void amdgpu_device_recovery_resume_clients(struct list_head > *device_list) > +{ > + struct amdgpu_device *tmp_adev = NULL; > + > + list_for_each_entry(tmp_adev, device_list, reset_list) > + drm_client_dev_resume(adev_to_drm(tmp_adev)); > +} > + > /** > * amdgpu_device_gpu_recover - reset the asic and recover scheduler > * > @@ -5613,6 +5627,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device > *adev, > /* Cannot be called after locking reset domain */ > amdgpu_ras_pre_reset(adev, &device_list); > > + amdgpu_device_recovery_suspend_clients(&device_list); > /* We need to lock reset domain only once both for XGMI and single > device */ > amdgpu_device_recovery_get_reset_lock(adev, &device_list); > > @@ -5649,6 +5664,7 @@ 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); > + amdgpu_device_recovery_resume_clients(&device_list); > amdgpu_ras_post_reset(adev, &device_list); > end_reset: > if (hive) {
