Re: [PATCH] drm/amdgpu: Avoid another list of reset devices

["Zhang, Hawking" <Hawking.Zhang@xxxxxxx>]

[AMD Official Use Only - General]

Reviewed-by: Hawking Zhang <Hawking.Zhang@xxxxxxx>

Regards,

Hawking

Get Outlook for iOS

---

From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Wednesday, August 3, 2022 7:36:20 PM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Subject: [PATCH] drm/amdgpu: Avoid another list of reset devices

 

A list of devices to be reset are already created in
amdgpu_device_gpu_recover function. Creating another list with the
same nodes is incorrect and not supported in list_head. Instead, pass
the device list as part of reset context.

Fixes: 9e08564727fc (drm/amdgpu: Refactor mode2 reset logic for v13.0.2)
Signed-off-by: Lijo Lazar <lijo.lazar@xxxxxxx>
---
 drivers/gpu/drm/amd/amdgpu/aldebaran.c     | 45 +++++++---------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h  |  1 +
 3 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/aldebaran.c b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
index c6cc493a5486..2b97b8a96fb4 100644
--- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
+++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
@@ -148,30 +148,22 @@ aldebaran_mode2_perform_reset(struct amdgpu_reset_control *reset_ctl,
                               struct amdgpu_reset_context *reset_context)
 {
         struct amdgpu_device *adev = (struct amdgpu_device *)reset_ctl->handle;
+       struct list_head *reset_device_list = reset_context->reset_device_list;
         struct amdgpu_device *tmp_adev = NULL;
-       struct list_head reset_device_list;
         int r = 0;
 
         dev_dbg(adev->dev, "aldebaran perform hw reset\n");
+
+       if (reset_device_list == NULL)
+               return -EINVAL;
+
         if (adev->ip_versions[MP1_HWIP][0] == IP_VERSION(13, 0, 2) &&
             reset_context->hive == NULL) {
                 /* Wrong context, return error */
                 return -EINVAL;
         }
 
-       INIT_LIST_HEAD(&reset_device_list);
-       if (reset_context->hive) {
-               list_for_each_entry (tmp_adev,
-                                    &reset_context->hive->device_list,
-                                    gmc.xgmi.head)
-                       list_add_tail(&tmp_adev->reset_list,
-                                     &reset_device_list);
-       } else {
-               list_add_tail(&reset_context->reset_req_dev->reset_list,
-                             &reset_device_list);
-       }
-
-       list_for_each_entry (tmp_adev, &reset_device_list, reset_list) {
+       list_for_each_entry(tmp_adev, reset_device_list, reset_list) {
                 mutex_lock(&tmp_adev->reset_cntl->reset_lock);
                 tmp_adev->reset_cntl->active_reset = AMD_RESET_METHOD_MODE2;
         }
@@ -179,7 +171,7 @@ aldebaran_mode2_perform_reset(struct amdgpu_reset_control *reset_ctl,
          * Mode2 reset doesn't need any sync between nodes in XGMI hive, instead launch
          * them together so that they can be completed asynchronously on multiple nodes
          */
-       list_for_each_entry (tmp_adev, &reset_device_list, reset_list) {
+       list_for_each_entry(tmp_adev, reset_device_list, reset_list) {
                 /* For XGMI run all resets in parallel to speed up the process */
                 if (tmp_adev->gmc.xgmi.num_physical_nodes > 1) {
                         if (!queue_work(system_unbound_wq,
@@ -197,7 +189,7 @@ aldebaran_mode2_perform_reset(struct amdgpu_reset_control *reset_ctl,
 
         /* For XGMI wait for all resets to complete before proceed */
         if (!r) {
-               list_for_each_entry (tmp_adev, &reset_device_list, reset_list) {
+               list_for_each_entry(tmp_adev, reset_device_list, reset_list) {
                         if (tmp_adev->gmc.xgmi.num_physical_nodes > 1) {
                                 flush_work(&tmp_adev->reset_cntl->reset_work);
                                 r = tmp_adev->asic_reset_res;
@@ -207,7 +199,7 @@ aldebaran_mode2_perform_reset(struct amdgpu_reset_control *reset_ctl,
                 }
         }
 
-       list_for_each_entry (tmp_adev, &reset_device_list, reset_list) {
+       list_for_each_entry(tmp_adev, reset_device_list, reset_list) {
                 mutex_unlock(&tmp_adev->reset_cntl->reset_lock);
                 tmp_adev->reset_cntl->active_reset = AMD_RESET_METHOD_NONE;
         }
@@ -339,10 +331,13 @@ static int
 aldebaran_mode2_restore_hwcontext(struct amdgpu_reset_control *reset_ctl,
                                   struct amdgpu_reset_context *reset_context)
 {
+       struct list_head *reset_device_list = reset_context->reset_device_list;
         struct amdgpu_device *tmp_adev = NULL;
-       struct list_head reset_device_list;
         int r;
 
+       if (reset_device_list == NULL)
+               return -EINVAL;
+
         if (reset_context->reset_req_dev->ip_versions[MP1_HWIP][0] ==
                     IP_VERSION(13, 0, 2) &&
             reset_context->hive == NULL) {
@@ -350,19 +345,7 @@ aldebaran_mode2_restore_hwcontext(struct amdgpu_reset_control *reset_ctl,
                 return -EINVAL;
         }
 
-       INIT_LIST_HEAD(&reset_device_list);
-       if (reset_context->hive) {
-               list_for_each_entry (tmp_adev,
-                                    &reset_context->hive->device_list,
-                                    gmc.xgmi.head)
-                       list_add_tail(&tmp_adev->reset_list,
-                                     &reset_device_list);
-       } else {
-               list_add_tail(&reset_context->reset_req_dev->reset_list,
-                             &reset_device_list);
-       }
-
-       list_for_each_entry (tmp_adev, &reset_device_list, reset_list) {
+       list_for_each_entry(tmp_adev, reset_device_list, reset_list) {
                 dev_info(tmp_adev->dev,
                          "GPU reset succeeded, trying to resume\n");
                 r = aldebaran_mode2_restore_ip(tmp_adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c4a6fe3070b6..e8a0b19b7398 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4742,6 +4742,8 @@ int amdgpu_do_asic_reset(struct list_head *device_list_handle,
         tmp_adev = list_first_entry(device_list_handle, struct amdgpu_device,
                                     reset_list);
         amdgpu_reset_reg_dumps(tmp_adev);
+
+       reset_context->reset_device_list = device_list_handle;
         r = amdgpu_reset_perform_reset(tmp_adev, reset_context);
         /* If reset handler not implemented, continue; otherwise return */
         if (r == -ENOSYS)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
index 9e55a5d7a825..ffda1560c648 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_reset.h
@@ -37,6 +37,7 @@ struct amdgpu_reset_context {
         struct amdgpu_device *reset_req_dev;
         struct amdgpu_job *job;
         struct amdgpu_hive_info *hive;
+       struct list_head *reset_device_list;
         unsigned long flags;
 };
 
--
2.25.1