Re: [RFC v4 11/11] Revert 'drm/amdgpu: annotate a false positive recursive locking'

[Christian König]

Am 09.02.22 um 01:23 schrieb Andrey Grodzovsky:

Since we have a single instance of reset semaphore which we
lock only once even for XGMI hive we don't need the nested
locking hint anymore.

Signed-off-by: Andrey Grodzovsky 


Oh, yes please :)

Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 14 --
  1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index aaecf0797484..75d0dd289023 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4825,16 +4825,10 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
return r;
  }
  
-static void amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain *reset_domain,

-   struct amdgpu_hive_info *hive)
+static void amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain 
*reset_domain)
  {
atomic_set(_domain->in_gpu_reset, 1);
-
-   if (hive) {
-   down_write_nest_lock(_domain->sem, >hive_lock);
-   } else {
-   down_write(_domain->sem);
-   }
+   down_write(_domain->sem);
  }
  
  static void amdgpu_device_set_mp1_state(struct amdgpu_device *adev)

@@ -5072,7 +5066,7 @@ int amdgpu_device_gpu_recover_imp(struct amdgpu_device 
*adev,
/* We need to lock reset domain only once both for XGMI and single 
device */
tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
reset_list);
-   amdgpu_device_lock_reset_domain(tmp_adev->reset_domain, hive);
+   amdgpu_device_lock_reset_domain(tmp_adev->reset_domain);
  
  	/* block all schedulers and reset given job's ring */

list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
@@ -5496,7 +5490,7 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev 
*pdev, pci_channel_sta
 * Locking adev->reset_domain->sem will prevent any external 
access
 * to GPU during PCI error recovery
 */
-   amdgpu_device_lock_reset_domain(adev->reset_domain, NULL);
+   amdgpu_device_lock_reset_domain(adev->reset_domain);
amdgpu_device_set_mp1_state(adev);
  
  		/*

[RFC v4 11/11] Revert 'drm/amdgpu: annotate a false positive recursive locking'

[Andrey Grodzovsky]

Since we have a single instance of reset semaphore which we
lock only once even for XGMI hive we don't need the nested
locking hint anymore.

Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index aaecf0797484..75d0dd289023 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4825,16 +4825,10 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,
return r;
 }
 
-static void amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain 
*reset_domain,
-   struct amdgpu_hive_info *hive)
+static void amdgpu_device_lock_reset_domain(struct amdgpu_reset_domain 
*reset_domain)
 {
atomic_set(_domain->in_gpu_reset, 1);
-
-   if (hive) {
-   down_write_nest_lock(_domain->sem, >hive_lock);
-   } else {
-   down_write(_domain->sem);
-   }
+   down_write(_domain->sem);
 }
 
 static void amdgpu_device_set_mp1_state(struct amdgpu_device *adev)
@@ -5072,7 +5066,7 @@ int amdgpu_device_gpu_recover_imp(struct amdgpu_device 
*adev,
/* We need to lock reset domain only once both for XGMI and single 
device */
tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
reset_list);
-   amdgpu_device_lock_reset_domain(tmp_adev->reset_domain, hive);
+   amdgpu_device_lock_reset_domain(tmp_adev->reset_domain);
 
/* block all schedulers and reset given job's ring */
list_for_each_entry(tmp_adev, device_list_handle, reset_list) {
@@ -5496,7 +5490,7 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev 
*pdev, pci_channel_sta
 * Locking adev->reset_domain->sem will prevent any external 
access
 * to GPU during PCI error recovery
 */
-   amdgpu_device_lock_reset_domain(adev->reset_domain, NULL);
+   amdgpu_device_lock_reset_domain(adev->reset_domain);
amdgpu_device_set_mp1_state(adev);
 
/*
-- 
2.25.1