Re: [PATCH] drm/amdgpu: Make the submission path memory reclaim safe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 03.12.24 um 15:24 schrieb Oleksandr Natalenko:
On středa 13. listopadu 2024 14:48:38, středoevropský standardní čas Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>

As commit 746ae46c1113 ("drm/sched: Mark scheduler work queues with WQ_MEM_RECLAIM")
points out, ever since
a6149f039369 ("drm/sched: Convert drm scheduler to use a work queue rather than kthread"),
any workqueue flushing done from the job submission path must only
involve memory reclaim safe workqueues to be safe against reclaim
deadlocks.

This is also pointed out by workqueue sanity checks:

  [ ] workqueue: WQ_MEM_RECLAIM sdma0:drm_sched_run_job_work [gpu_sched] is flushing !WQ_MEM_RECLAIM events:amdgpu_device_delay_enable_gfx_off [amdgpu]
...
  [ ] Workqueue: sdma0 drm_sched_run_job_work [gpu_sched]
...
  [ ] Call Trace:
  [ ]  <TASK>
...
  [ ]  ? check_flush_dependency+0xf5/0x110
...
  [ ]  cancel_delayed_work_sync+0x6e/0x80
  [ ]  amdgpu_gfx_off_ctrl+0xab/0x140 [amdgpu]
  [ ]  amdgpu_ring_alloc+0x40/0x50 [amdgpu]
  [ ]  amdgpu_ib_schedule+0xf4/0x810 [amdgpu]
  [ ]  ? drm_sched_run_job_work+0x22c/0x430 [gpu_sched]
  [ ]  amdgpu_job_run+0xaa/0x1f0 [amdgpu]
  [ ]  drm_sched_run_job_work+0x257/0x430 [gpu_sched]
  [ ]  process_one_work+0x217/0x720
...
  [ ]  </TASK>

Fix this by creating a memory reclaim safe driver workqueue and make the
submission path use it.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>
References: 746ae46c1113 ("drm/sched: Mark scheduler work queues with WQ_MEM_RECLAIM")
Fixes: a6149f039369 ("drm/sched: Convert drm scheduler to use a work queue rather than kthread")
Cc: stable@xxxxxxxxxxxxxxx
Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
Cc: Danilo Krummrich <dakr@xxxxxxxxxx>
Cc: Philipp Stanner <pstanner@xxxxxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: Christian König <christian.koenig@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  2 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 25 +++++++++++++++++++++++++
  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c |  5 +++--
  3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7645e498faa4..a6aad687537e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -268,6 +268,8 @@ extern int amdgpu_agp;
extern int amdgpu_wbrf; +extern struct workqueue_struct *amdgpu_reclaim_wq;
+
  #define AMDGPU_VM_MAX_NUM_CTX			4096
  #define AMDGPU_SG_THRESHOLD			(256*1024*1024)
  #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS	        3000
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 38686203bea6..f5b7172e8042 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -255,6 +255,8 @@ struct amdgpu_watchdog_timer amdgpu_watchdog_timer = {
  	.period = 0x0, /* default to 0x0 (timeout disable) */
  };
+struct workqueue_struct *amdgpu_reclaim_wq;
+
  /**
   * DOC: vramlimit (int)
   * Restrict the total amount of VRAM in MiB for testing.  The default is 0 (Use full VRAM).
@@ -2971,6 +2973,21 @@ static struct pci_driver amdgpu_kms_pci_driver = {
  	.dev_groups = amdgpu_sysfs_groups,
  };
+static int amdgpu_wq_init(void)
+{
+	amdgpu_reclaim_wq =
+		alloc_workqueue("amdgpu-reclaim", WQ_MEM_RECLAIM, 0);
+	if (!amdgpu_reclaim_wq)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void amdgpu_wq_fini(void)
+{
+	destroy_workqueue(amdgpu_reclaim_wq);
+}
+
  static int __init amdgpu_init(void)
  {
  	int r;
@@ -2978,6 +2995,10 @@ static int __init amdgpu_init(void)
  	if (drm_firmware_drivers_only())
  		return -EINVAL;
+ r = amdgpu_wq_init();
+	if (r)
+		goto error_wq;
+
  	r = amdgpu_sync_init();
  	if (r)
  		goto error_sync;
@@ -3006,6 +3027,9 @@ static int __init amdgpu_init(void)
  	amdgpu_sync_fini();
error_sync:
+	amdgpu_wq_fini();
+
+error_wq:
  	return r;
  }
@@ -3017,6 +3041,7 @@ static void __exit amdgpu_exit(void)
  	amdgpu_acpi_release();
  	amdgpu_sync_fini();
  	amdgpu_fence_slab_fini();
+	amdgpu_wq_fini();
  	mmu_notifier_synchronize();
  	amdgpu_xcp_drv_release();
  }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 2f3f09dfb1fd..f8fd71d9382f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -790,8 +790,9 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev, bool enable)
  						AMD_IP_BLOCK_TYPE_GFX, true))
  					adev->gfx.gfx_off_state = true;
  			} else {
-				schedule_delayed_work(&adev->gfx.gfx_off_delay_work,
-					      delay);
+				queue_delayed_work(amdgpu_reclaim_wq,
+						   &adev->gfx.gfx_off_delay_work,
+						   delay);
  			}
  		}
  	} else {
I can confirm this fixed the warning for me.

Tested-by: Oleksandr Natalenko <oleksandr@xxxxxxxxxxxxxx>

What's the fate of this submission?

It turned out that the warning is actually a false negative.

E.g. the warning shouldn't be printed in the first place.

Tvrtko pinged the relevant maintainer, but for far I haven't seen a reply.

Regards,
Christian.


Thank you.





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux