AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Fix the warning info in mode1 reset
@ 2024-01-04  5:39 Ma Jun
  2024-01-04 15:00 ` Alex Deucher
  0 siblings, 1 reply; 6+ messages in thread
From: Ma Jun @ 2024-01-04  5:39 UTC (permalink / raw)
  To: amd-gfx, christian.koenig, Alexander.Deucher; +Cc: Ma Jun

Fix the warning info below during mode1 reset.
[  +0.000004] Call Trace:
[  +0.000004]  <TASK>
[  +0.000006]  ? show_regs+0x6e/0x80
[  +0.000011]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000005]  ? __warn+0x91/0x150
[  +0.000009]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000006]  ? report_bug+0x19d/0x1b0
[  +0.000013]  ? handle_bug+0x46/0x80
[  +0.000012]  ? exc_invalid_op+0x1d/0x80
[  +0.000011]  ? asm_exc_invalid_op+0x1f/0x30
[  +0.000014]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000007]  ? __flush_work.isra.0+0x208/0x390
[  +0.000007]  ? _prb_read_valid+0x216/0x290
[  +0.000008]  __cancel_work_timer+0x11d/0x1a0
[  +0.000007]  ? try_to_grab_pending+0xe8/0x190
[  +0.000012]  cancel_work_sync+0x14/0x20
[  +0.000008]  amddrm_sched_stop+0x3c/0x1d0 [amd_sched]
[  +0.000032]  amdgpu_device_gpu_recover+0x29a/0xe90 [amdgpu]

This warning info was printed after applying the patch
"drm/sched: Convert drm scheduler to use a work queue rather than kthread".
The root cause is that amdgpu driver tries to use the uninitialized
work_struct in the struct drm_gpu_scheduler

Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 16 +++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 ++-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 4b1d5f42249f..7bac3019aa0a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5575,6 +5575,16 @@ static inline void amdgpu_device_stop_pending_resets(struct amdgpu_device *adev)
 
 }
 
+static bool amdgpu_is_ring_sched_ready(struct amdgpu_ring *ring)
+{
+	if (!ring)
+		return false;
+
+	if (ring->no_scheduler || !drm_sched_wqueue_ready(&ring->sched))
+		return false;
+
+	return true;
+}
 /**
  * amdgpu_device_gpu_recover - reset the asic and recover scheduler
  *
@@ -5700,7 +5710,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 		for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 			struct amdgpu_ring *ring = tmp_adev->rings[i];
 
-			if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+			if (!amdgpu_is_ring_sched_ready(ring))
 				continue;
 
 			drm_sched_stop(&ring->sched, job ? &job->base : NULL);
@@ -5776,7 +5786,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 		for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 			struct amdgpu_ring *ring = tmp_adev->rings[i];
 
-			if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+			if (!amdgpu_is_ring_sched_ready(ring))
 				continue;
 
 			drm_sched_start(&ring->sched, true);
@@ -6265,7 +6275,7 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
 	for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 		struct amdgpu_ring *ring = adev->rings[i];
 
-		if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+		if (!amdgpu_is_ring_sched_ready(ring))
 			continue;
 
 		drm_sched_start(&ring->sched, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 41266bc99345..3650e4d06e53 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -636,7 +636,8 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
 		DRM_DEV_DEBUG(adev->dev, "ring test on %s succeeded\n",
 			      ring->name);
 
-	ring->sched.ready = !r;
+	if (!ring->no_scheduler)
+		ring->sched.ready = !r;
 	return r;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amdgpu: Fix the warning info in mode1 reset
  2024-01-04  5:39 [PATCH] drm/amdgpu: Fix the warning info in mode1 reset Ma Jun
@ 2024-01-04 15:00 ` Alex Deucher
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Deucher @ 2024-01-04 15:00 UTC (permalink / raw)
  To: Ma Jun; +Cc: Alexander.Deucher, christian.koenig, amd-gfx

On Thu, Jan 4, 2024 at 12:55 AM Ma Jun <Jun.Ma2@amd.com> wrote:
>
> Fix the warning info below during mode1 reset.
> [  +0.000004] Call Trace:
> [  +0.000004]  <TASK>
> [  +0.000006]  ? show_regs+0x6e/0x80
> [  +0.000011]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000005]  ? __warn+0x91/0x150
> [  +0.000009]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000006]  ? report_bug+0x19d/0x1b0
> [  +0.000013]  ? handle_bug+0x46/0x80
> [  +0.000012]  ? exc_invalid_op+0x1d/0x80
> [  +0.000011]  ? asm_exc_invalid_op+0x1f/0x30
> [  +0.000014]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000007]  ? __flush_work.isra.0+0x208/0x390
> [  +0.000007]  ? _prb_read_valid+0x216/0x290
> [  +0.000008]  __cancel_work_timer+0x11d/0x1a0
> [  +0.000007]  ? try_to_grab_pending+0xe8/0x190
> [  +0.000012]  cancel_work_sync+0x14/0x20
> [  +0.000008]  amddrm_sched_stop+0x3c/0x1d0 [amd_sched]
> [  +0.000032]  amdgpu_device_gpu_recover+0x29a/0xe90 [amdgpu]
>
> This warning info was printed after applying the patch
> "drm/sched: Convert drm scheduler to use a work queue rather than kthread".
> The root cause is that amdgpu driver tries to use the uninitialized
> work_struct in the struct drm_gpu_scheduler
>
> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 16 +++++++++++++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 ++-
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 4b1d5f42249f..7bac3019aa0a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5575,6 +5575,16 @@ static inline void amdgpu_device_stop_pending_resets(struct amdgpu_device *adev)
>
>  }
>
> +static bool amdgpu_is_ring_sched_ready(struct amdgpu_ring *ring)

I would either rename this amdgpu_device_is_ring_sched_ready() for
consistency, or move it to amdgpu_ring.c and rename it to
amdgpu_ring_sched_ready().

Alex

> +{
> +       if (!ring)
> +               return false;
> +
> +       if (ring->no_scheduler || !drm_sched_wqueue_ready(&ring->sched))
> +               return false;
> +
> +       return true;
> +}
>  /**
>   * amdgpu_device_gpu_recover - reset the asic and recover scheduler
>   *
> @@ -5700,7 +5710,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>                 for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                         struct amdgpu_ring *ring = tmp_adev->rings[i];
>
> -                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +                       if (!amdgpu_is_ring_sched_ready(ring))
>                                 continue;
>
>                         drm_sched_stop(&ring->sched, job ? &job->base : NULL);
> @@ -5776,7 +5786,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>                 for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                         struct amdgpu_ring *ring = tmp_adev->rings[i];
>
> -                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +                       if (!amdgpu_is_ring_sched_ready(ring))
>                                 continue;
>
>                         drm_sched_start(&ring->sched, true);
> @@ -6265,7 +6275,7 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
>         for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                 struct amdgpu_ring *ring = adev->rings[i];
>
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_is_ring_sched_ready(ring))
>                         continue;
>
>                 drm_sched_start(&ring->sched, true);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 41266bc99345..3650e4d06e53 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -636,7 +636,8 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
>                 DRM_DEV_DEBUG(adev->dev, "ring test on %s succeeded\n",
>                               ring->name);
>
> -       ring->sched.ready = !r;
> +       if (!ring->no_scheduler)
> +               ring->sched.ready = !r;
>         return r;
>  }
>
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] drm/amdgpu: Fix the warning info in mode1 reset
@ 2024-01-25 16:15 Alex Deucher
  2024-01-29 15:33 ` Deucher, Alexander
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Deucher @ 2024-01-25 16:15 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Ma Jun, Vitaly Prosyak

From: Ma Jun <Jun.Ma2@amd.com>

Fix the warning info below during mode1 reset.
[  +0.000004] Call Trace:
[  +0.000004]  <TASK>
[  +0.000006]  ? show_regs+0x6e/0x80
[  +0.000011]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000005]  ? __warn+0x91/0x150
[  +0.000009]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000006]  ? report_bug+0x19d/0x1b0
[  +0.000013]  ? handle_bug+0x46/0x80
[  +0.000012]  ? exc_invalid_op+0x1d/0x80
[  +0.000011]  ? asm_exc_invalid_op+0x1f/0x30
[  +0.000014]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000007]  ? __flush_work.isra.0+0x208/0x390
[  +0.000007]  ? _prb_read_valid+0x216/0x290
[  +0.000008]  __cancel_work_timer+0x11d/0x1a0
[  +0.000007]  ? try_to_grab_pending+0xe8/0x190
[  +0.000012]  cancel_work_sync+0x14/0x20
[  +0.000008]  amddrm_sched_stop+0x3c/0x1d0 [amd_sched]
[  +0.000032]  amdgpu_device_gpu_recover+0x29a/0xe90 [amdgpu]

This warning info was printed after applying the patch
"drm/sched: Convert drm scheduler to use a work queue rather than kthread".
The root cause is that amdgpu driver tries to use the uninitialized
work_struct in the struct drm_gpu_scheduler

v2:
 - Rename the function to amdgpu_ring_sched_ready and move it to
amdgpu_ring.c (Alex)
v3:
- Fix a few more checks based on Vitaly's patch (Alex)

Fixes: 11b3b9f461c5 ("drm/sched: Check scheduler ready before calling timeout handling")
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c        |  8 ++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c         | 12 ++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c           | 14 +++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h           |  2 +-
 5 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
index 899e31e3a5e8..3a3f3ce09f00 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
@@ -290,7 +290,7 @@ static int suspend_resume_compute_scheduler(struct amdgpu_device *adev, bool sus
 	for (i = 0; i < adev->gfx.num_compute_rings; i++) {
 		struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];
 
-		if (!(ring && drm_sched_wqueue_ready(&ring->sched)))
+		if (!amdgpu_ring_sched_ready(ring))
 			continue;
 
 		/* stop secheduler and drain ring. */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index e485dd3357c6..1afbb2e932c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1678,7 +1678,7 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
 	for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
 		struct amdgpu_ring *ring = adev->rings[i];
 
-		if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+		if (!amdgpu_ring_sched_ready(ring))
 			continue;
 		drm_sched_wqueue_stop(&ring->sched);
 	}
@@ -1694,7 +1694,7 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
 	for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
 		struct amdgpu_ring *ring = adev->rings[i];
 
-		if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+		if (!amdgpu_ring_sched_ready(ring))
 			continue;
 		drm_sched_wqueue_start(&ring->sched);
 	}
@@ -1916,8 +1916,8 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64 val)
 
 	ring = adev->rings[val];
 
-	if (!ring || !ring->funcs->preempt_ib ||
-	    !drm_sched_wqueue_ready(&ring->sched))
+	if (!amdgpu_ring_sched_ready(ring) ||
+	    !ring->funcs->preempt_ib)
 		return -EINVAL;
 
 	/* the last preemption failed */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1a04ccba9542..7ff17df7a5ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5042,7 +5042,7 @@ bool amdgpu_device_has_job_running(struct amdgpu_device *adev)
 	for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 		struct amdgpu_ring *ring = adev->rings[i];
 
-		if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+		if (!amdgpu_ring_sched_ready(ring))
 			continue;
 
 		spin_lock(&ring->sched.job_list_lock);
@@ -5181,7 +5181,7 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev,
 	for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 		struct amdgpu_ring *ring = adev->rings[i];
 
-		if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+		if (!amdgpu_ring_sched_ready(ring))
 			continue;
 
 		/* Clear job fence from fence drv to avoid force_completion
@@ -5648,7 +5648,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 		for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 			struct amdgpu_ring *ring = tmp_adev->rings[i];
 
-			if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+			if (!amdgpu_ring_sched_ready(ring))
 				continue;
 
 			drm_sched_stop(&ring->sched, job ? &job->base : NULL);
@@ -5717,7 +5717,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 		for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 			struct amdgpu_ring *ring = tmp_adev->rings[i];
 
-			if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+			if (!amdgpu_ring_sched_ready(ring))
 				continue;
 
 			drm_sched_start(&ring->sched, true);
@@ -6072,7 +6072,7 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev *pdev, pci_channel_sta
 		for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 			struct amdgpu_ring *ring = adev->rings[i];
 
-			if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+			if (!amdgpu_ring_sched_ready(ring))
 				continue;
 
 			drm_sched_stop(&ring->sched, NULL);
@@ -6214,7 +6214,7 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
 	for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 		struct amdgpu_ring *ring = adev->rings[i];
 
-		if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+		if (!amdgpu_ring_sched_ready(ring))
 			continue;
 
 		drm_sched_start(&ring->sched, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 45424ebf9681..9ae386e9d41d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -634,7 +634,8 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
 		DRM_DEV_DEBUG(adev->dev, "ring test on %s succeeded\n",
 			      ring->name);
 
-	ring->sched.ready = !r;
+	if (!ring->no_scheduler)
+		ring->sched.ready = !r;
 	return r;
 }
 
@@ -717,3 +718,14 @@ void amdgpu_ring_ib_on_emit_de(struct amdgpu_ring *ring)
 	if (ring->is_sw_ring)
 		amdgpu_sw_ring_ib_mark_offset(ring, AMDGPU_MUX_OFFSET_TYPE_DE);
 }
+
+bool amdgpu_ring_sched_ready(struct amdgpu_ring *ring)
+{
+	if (!ring)
+		return false;
+
+	if (ring->no_scheduler || !drm_sched_wqueue_ready(&ring->sched))
+		return false;
+
+	return true;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index bbb53720a018..fe1a61eb6e4c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -450,5 +450,5 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 int amdgpu_ib_pool_init(struct amdgpu_device *adev);
 void amdgpu_ib_pool_fini(struct amdgpu_device *adev);
 int amdgpu_ib_ring_tests(struct amdgpu_device *adev);
-
+bool amdgpu_ring_sched_ready(struct amdgpu_ring *ring);
 #endif
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amdgpu: Fix the warning info in mode1 reset
  2024-01-25 16:15 Alex Deucher
@ 2024-01-29 15:33 ` Deucher, Alexander
  2024-01-29 19:28   ` vitaly prosyak
  2024-01-30  2:40   ` Ma, Jun
  0 siblings, 2 replies; 6+ messages in thread
From: Deucher, Alexander @ 2024-01-29 15:33 UTC (permalink / raw)
  To: amd-gfx@lists.freedesktop.org; +Cc: Ma, Jun, Prosyak, Vitaly

[-- Attachment #1: Type: text/plain, Size: 9157 bytes --]

[AMD Official Use Only - General]

Ping?

________________________________
From: Deucher, Alexander <Alexander.Deucher@amd.com>
Sent: Thursday, January 25, 2024 11:15 AM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Ma, Jun <Jun.Ma2@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Prosyak, Vitaly <Vitaly.Prosyak@amd.com>
Subject: [PATCH] drm/amdgpu: Fix the warning info in mode1 reset

From: Ma Jun <Jun.Ma2@amd.com>

Fix the warning info below during mode1 reset.
[  +0.000004] Call Trace:
[  +0.000004]  <TASK>
[  +0.000006]  ? show_regs+0x6e/0x80
[  +0.000011]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000005]  ? __warn+0x91/0x150
[  +0.000009]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000006]  ? report_bug+0x19d/0x1b0
[  +0.000013]  ? handle_bug+0x46/0x80
[  +0.000012]  ? exc_invalid_op+0x1d/0x80
[  +0.000011]  ? asm_exc_invalid_op+0x1f/0x30
[  +0.000014]  ? __flush_work.isra.0+0x2e8/0x390
[  +0.000007]  ? __flush_work.isra.0+0x208/0x390
[  +0.000007]  ? _prb_read_valid+0x216/0x290
[  +0.000008]  __cancel_work_timer+0x11d/0x1a0
[  +0.000007]  ? try_to_grab_pending+0xe8/0x190
[  +0.000012]  cancel_work_sync+0x14/0x20
[  +0.000008]  amddrm_sched_stop+0x3c/0x1d0 [amd_sched]
[  +0.000032]  amdgpu_device_gpu_recover+0x29a/0xe90 [amdgpu]

This warning info was printed after applying the patch
"drm/sched: Convert drm scheduler to use a work queue rather than kthread".
The root cause is that amdgpu driver tries to use the uninitialized
work_struct in the struct drm_gpu_scheduler

v2:
 - Rename the function to amdgpu_ring_sched_ready and move it to
amdgpu_ring.c (Alex)
v3:
- Fix a few more checks based on Vitaly's patch (Alex)

Fixes: 11b3b9f461c5 ("drm/sched: Check scheduler ready before calling timeout handling")
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c        |  8 ++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c         | 12 ++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c           | 14 +++++++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h           |  2 +-
 5 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
index 899e31e3a5e8..3a3f3ce09f00 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
@@ -290,7 +290,7 @@ static int suspend_resume_compute_scheduler(struct amdgpu_device *adev, bool sus
         for (i = 0; i < adev->gfx.num_compute_rings; i++) {
                 struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];

-               if (!(ring && drm_sched_wqueue_ready(&ring->sched)))
+               if (!amdgpu_ring_sched_ready(ring))
                         continue;

                 /* stop secheduler and drain ring. */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index e485dd3357c6..1afbb2e932c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1678,7 +1678,7 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
         for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
                 struct amdgpu_ring *ring = adev->rings[i];

-               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+               if (!amdgpu_ring_sched_ready(ring))
                         continue;
                 drm_sched_wqueue_stop(&ring->sched);
         }
@@ -1694,7 +1694,7 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
         for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
                 struct amdgpu_ring *ring = adev->rings[i];

-               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+               if (!amdgpu_ring_sched_ready(ring))
                         continue;
                 drm_sched_wqueue_start(&ring->sched);
         }
@@ -1916,8 +1916,8 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64 val)

         ring = adev->rings[val];

-       if (!ring || !ring->funcs->preempt_ib ||
-           !drm_sched_wqueue_ready(&ring->sched))
+       if (!amdgpu_ring_sched_ready(ring) ||
+           !ring->funcs->preempt_ib)
                 return -EINVAL;

         /* the last preemption failed */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1a04ccba9542..7ff17df7a5ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5042,7 +5042,7 @@ bool amdgpu_device_has_job_running(struct amdgpu_device *adev)
         for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
                 struct amdgpu_ring *ring = adev->rings[i];

-               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+               if (!amdgpu_ring_sched_ready(ring))
                         continue;

                 spin_lock(&ring->sched.job_list_lock);
@@ -5181,7 +5181,7 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev,
         for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
                 struct amdgpu_ring *ring = adev->rings[i];

-               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+               if (!amdgpu_ring_sched_ready(ring))
                         continue;

                 /* Clear job fence from fence drv to avoid force_completion
@@ -5648,7 +5648,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
                 for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
                         struct amdgpu_ring *ring = tmp_adev->rings[i];

-                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+                       if (!amdgpu_ring_sched_ready(ring))
                                 continue;

                         drm_sched_stop(&ring->sched, job ? &job->base : NULL);
@@ -5717,7 +5717,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
                 for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
                         struct amdgpu_ring *ring = tmp_adev->rings[i];

-                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+                       if (!amdgpu_ring_sched_ready(ring))
                                 continue;

                         drm_sched_start(&ring->sched, true);
@@ -6072,7 +6072,7 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev *pdev, pci_channel_sta
                 for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
                         struct amdgpu_ring *ring = adev->rings[i];

-                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+                       if (!amdgpu_ring_sched_ready(ring))
                                 continue;

                         drm_sched_stop(&ring->sched, NULL);
@@ -6214,7 +6214,7 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
         for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
                 struct amdgpu_ring *ring = adev->rings[i];

-               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
+               if (!amdgpu_ring_sched_ready(ring))
                         continue;

                 drm_sched_start(&ring->sched, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 45424ebf9681..9ae386e9d41d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -634,7 +634,8 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
                 DRM_DEV_DEBUG(adev->dev, "ring test on %s succeeded\n",
                               ring->name);

-       ring->sched.ready = !r;
+       if (!ring->no_scheduler)
+               ring->sched.ready = !r;
         return r;
 }

@@ -717,3 +718,14 @@ void amdgpu_ring_ib_on_emit_de(struct amdgpu_ring *ring)
         if (ring->is_sw_ring)
                 amdgpu_sw_ring_ib_mark_offset(ring, AMDGPU_MUX_OFFSET_TYPE_DE);
 }
+
+bool amdgpu_ring_sched_ready(struct amdgpu_ring *ring)
+{
+       if (!ring)
+               return false;
+
+       if (ring->no_scheduler || !drm_sched_wqueue_ready(&ring->sched))
+               return false;
+
+       return true;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index bbb53720a018..fe1a61eb6e4c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -450,5 +450,5 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 int amdgpu_ib_pool_init(struct amdgpu_device *adev);
 void amdgpu_ib_pool_fini(struct amdgpu_device *adev);
 int amdgpu_ib_ring_tests(struct amdgpu_device *adev);
-
+bool amdgpu_ring_sched_ready(struct amdgpu_ring *ring);
 #endif
--
2.42.0


[-- Attachment #2: Type: text/html, Size: 17967 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amdgpu: Fix the warning info in mode1 reset
  2024-01-29 15:33 ` Deucher, Alexander
@ 2024-01-29 19:28   ` vitaly prosyak
  2024-01-30  2:40   ` Ma, Jun
  1 sibling, 0 replies; 6+ messages in thread
From: vitaly prosyak @ 2024-01-29 19:28 UTC (permalink / raw)
  To: Deucher, Alexander, amd-gfx@lists.freedesktop.org
  Cc: Ma, Jun, Prosyak, Vitaly

[-- Attachment #1: Type: text/plain, Size: 12047 bytes --]

The change looks good to me.
Validated now on kernel 5.7 ( branch origin/adeucher/amd-staging-drm-next)


Thanks,  Vitaly


On 2024-01-29 10:33, Deucher, Alexander wrote:
>
> [AMD Official Use Only - General]
>
>
> Ping?
>
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> *From:* Deucher, Alexander <Alexander.Deucher@amd.com>
> *Sent:* Thursday, January 25, 2024 11:15 AM
> *To:* amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> *Cc:* Ma, Jun <Jun.Ma2@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Prosyak, Vitaly <Vitaly.Prosyak@amd.com>
> *Subject:* [PATCH] drm/amdgpu: Fix the warning info in mode1 reset
>  
> From: Ma Jun <Jun.Ma2@amd.com>
>
> Fix the warning info below during mode1 reset.
> [  +0.000004] Call Trace:
> [  +0.000004]  <TASK>
> [  +0.000006]  ? show_regs+0x6e/0x80
> [  +0.000011]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000005]  ? __warn+0x91/0x150
> [  +0.000009]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000006]  ? report_bug+0x19d/0x1b0
> [  +0.000013]  ? handle_bug+0x46/0x80
> [  +0.000012]  ? exc_invalid_op+0x1d/0x80
> [  +0.000011]  ? asm_exc_invalid_op+0x1f/0x30
> [  +0.000014]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000007]  ? __flush_work.isra.0+0x208/0x390
> [  +0.000007]  ? _prb_read_valid+0x216/0x290
> [  +0.000008]  __cancel_work_timer+0x11d/0x1a0
> [  +0.000007]  ? try_to_grab_pending+0xe8/0x190
> [  +0.000012]  cancel_work_sync+0x14/0x20
> [  +0.000008]  amddrm_sched_stop+0x3c/0x1d0 [amd_sched]
> [  +0.000032]  amdgpu_device_gpu_recover+0x29a/0xe90 [amdgpu]
>
> This warning info was printed after applying the patch
> "drm/sched: Convert drm scheduler to use a work queue rather than kthread".
> The root cause is that amdgpu driver tries to use the uninitialized
> work_struct in the struct drm_gpu_scheduler
>
> v2:
>  - Rename the function to amdgpu_ring_sched_ready and move it to
> amdgpu_ring.c (Alex)
> v3:
> - Fix a few more checks based on Vitaly's patch (Alex)
>
> Fixes: 11b3b9f461c5 ("drm/sched: Check scheduler ready before calling timeout handling")
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c    |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c        |  8 ++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c         | 12 ++++++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c           | 14 +++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h           |  2 +-
>  5 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> index 899e31e3a5e8..3a3f3ce09f00 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> @@ -290,7 +290,7 @@ static int suspend_resume_compute_scheduler(struct amdgpu_device *adev, bool sus
>          for (i = 0; i < adev->gfx.num_compute_rings; i++) {
>                  struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];
>  
> -               if (!(ring && drm_sched_wqueue_ready(&ring->sched)))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>  
>                  /* stop secheduler and drain ring. */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index e485dd3357c6..1afbb2e932c6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1678,7 +1678,7 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
>          for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>                  drm_sched_wqueue_stop(&ring->sched);
>          }
> @@ -1694,7 +1694,7 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
>          for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>                  drm_sched_wqueue_start(&ring->sched);
>          }
> @@ -1916,8 +1916,8 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64 val)
>  
>          ring = adev->rings[val];
>  
> -       if (!ring || !ring->funcs->preempt_ib ||
> -           !drm_sched_wqueue_ready(&ring->sched))
> +       if (!amdgpu_ring_sched_ready(ring) ||
> +           !ring->funcs->preempt_ib)
>                  return -EINVAL;
>  
>          /* the last preemption failed */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1a04ccba9542..7ff17df7a5ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5042,7 +5042,7 @@ bool amdgpu_device_has_job_running(struct amdgpu_device *adev)
>          for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>  
>                  spin_lock(&ring->sched.job_list_lock);
> @@ -5181,7 +5181,7 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev,
>          for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>  
>                  /* Clear job fence from fence drv to avoid force_completion
> @@ -5648,7 +5648,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>                  for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                          struct amdgpu_ring *ring = tmp_adev->rings[i];
>  
> -                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +                       if (!amdgpu_ring_sched_ready(ring))
>                                  continue;
>  
>                          drm_sched_stop(&ring->sched, job ? &job->base : NULL);
> @@ -5717,7 +5717,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>                  for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                          struct amdgpu_ring *ring = tmp_adev->rings[i];
>  
> -                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +                       if (!amdgpu_ring_sched_ready(ring))
>                                  continue;
>  
>                          drm_sched_start(&ring->sched, true);
> @@ -6072,7 +6072,7 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev *pdev, pci_channel_sta
>                  for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                          struct amdgpu_ring *ring = adev->rings[i];
>  
> -                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +                       if (!amdgpu_ring_sched_ready(ring))
>                                  continue;
>  
>                          drm_sched_stop(&ring->sched, NULL);
> @@ -6214,7 +6214,7 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
>          for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>  
>                  drm_sched_start(&ring->sched, true);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 45424ebf9681..9ae386e9d41d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -634,7 +634,8 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
>                  DRM_DEV_DEBUG(adev->dev, "ring test on %s succeeded\n",
>                                ring->name);
>  
> -       ring->sched.ready = !r;
> +       if (!ring->no_scheduler)
> +               ring->sched.ready = !r;
>          return r;
>  }
>  
> @@ -717,3 +718,14 @@ void amdgpu_ring_ib_on_emit_de(struct amdgpu_ring *ring)
>          if (ring->is_sw_ring)
>                  amdgpu_sw_ring_ib_mark_offset(ring, AMDGPU_MUX_OFFSET_TYPE_DE);
>  }
> +
> +bool amdgpu_ring_sched_ready(struct amdgpu_ring *ring)
> +{
> +       if (!ring)
> +               return false;
> +
> +       if (ring->no_scheduler || !drm_sched_wqueue_ready(&ring->sched))
> +               return false;
> +
> +       return true;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index bbb53720a018..fe1a61eb6e4c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -450,5 +450,5 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>  int amdgpu_ib_pool_init(struct amdgpu_device *adev);
>  void amdgpu_ib_pool_fini(struct amdgpu_device *adev);
>  int amdgpu_ib_ring_tests(struct amdgpu_device *adev);
> -
> +bool amdgpu_ring_sched_ready(struct amdgpu_ring *ring);
>  #endif
> -- 
> 2.42.0
>

[-- Attachment #2: Type: text/html, Size: 23588 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amdgpu: Fix the warning info in mode1 reset
  2024-01-29 15:33 ` Deucher, Alexander
  2024-01-29 19:28   ` vitaly prosyak
@ 2024-01-30  2:40   ` Ma, Jun
  1 sibling, 0 replies; 6+ messages in thread
From: Ma, Jun @ 2024-01-30  2:40 UTC (permalink / raw)
  To: Deucher, Alexander, amd-gfx@lists.freedesktop.org
  Cc: Ma, Jun, Prosyak, Vitaly

Reviewed-by: Ma Jun <Jun.Ma2@amd.com>

Regards,
Ma Jun

On 1/29/2024 11:33 PM, Deucher, Alexander wrote:
> [AMD Official Use Only - General]
> 
> 
> Ping?
> 
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> *From:* Deucher, Alexander <Alexander.Deucher@amd.com>
> *Sent:* Thursday, January 25, 2024 11:15 AM
> *To:* amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> *Cc:* Ma, Jun <Jun.Ma2@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Prosyak, Vitaly <Vitaly.Prosyak@amd.com>
> *Subject:* [PATCH] drm/amdgpu: Fix the warning info in mode1 reset
>  
> From: Ma Jun <Jun.Ma2@amd.com>
> 
> Fix the warning info below during mode1 reset.
> [  +0.000004] Call Trace:
> [  +0.000004]  <TASK>
> [  +0.000006]  ? show_regs+0x6e/0x80
> [  +0.000011]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000005]  ? __warn+0x91/0x150
> [  +0.000009]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000006]  ? report_bug+0x19d/0x1b0
> [  +0.000013]  ? handle_bug+0x46/0x80
> [  +0.000012]  ? exc_invalid_op+0x1d/0x80
> [  +0.000011]  ? asm_exc_invalid_op+0x1f/0x30
> [  +0.000014]  ? __flush_work.isra.0+0x2e8/0x390
> [  +0.000007]  ? __flush_work.isra.0+0x208/0x390
> [  +0.000007]  ? _prb_read_valid+0x216/0x290
> [  +0.000008]  __cancel_work_timer+0x11d/0x1a0
> [  +0.000007]  ? try_to_grab_pending+0xe8/0x190
> [  +0.000012]  cancel_work_sync+0x14/0x20
> [  +0.000008]  amddrm_sched_stop+0x3c/0x1d0 [amd_sched]
> [  +0.000032]  amdgpu_device_gpu_recover+0x29a/0xe90 [amdgpu]
> 
> This warning info was printed after applying the patch
> "drm/sched: Convert drm scheduler to use a work queue rather than kthread".
> The root cause is that amdgpu driver tries to use the uninitialized
> work_struct in the struct drm_gpu_scheduler
> 
> v2:
>  - Rename the function to amdgpu_ring_sched_ready and move it to
> amdgpu_ring.c (Alex)
> v3:
> - Fix a few more checks based on Vitaly's patch (Alex)
> 
> Fixes: 11b3b9f461c5 ("drm/sched: Check scheduler ready before calling timeout handling")
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c    |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c        |  8 ++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c         | 12 ++++++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c           | 14 +++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h           |  2 +-
>  5 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> index 899e31e3a5e8..3a3f3ce09f00 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> @@ -290,7 +290,7 @@ static int suspend_resume_compute_scheduler(struct amdgpu_device *adev, bool sus
>          for (i = 0; i < adev->gfx.num_compute_rings; i++) {
>                  struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];
>  
> -               if (!(ring && drm_sched_wqueue_ready(&ring->sched)))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>  
>                  /* stop secheduler and drain ring. */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index e485dd3357c6..1afbb2e932c6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1678,7 +1678,7 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
>          for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>                  drm_sched_wqueue_stop(&ring->sched);
>          }
> @@ -1694,7 +1694,7 @@ static int amdgpu_debugfs_test_ib_show(struct seq_file *m, void *unused)
>          for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>                  drm_sched_wqueue_start(&ring->sched);
>          }
> @@ -1916,8 +1916,8 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64 val)
>  
>          ring = adev->rings[val];
>  
> -       if (!ring || !ring->funcs->preempt_ib ||
> -           !drm_sched_wqueue_ready(&ring->sched))
> +       if (!amdgpu_ring_sched_ready(ring) ||
> +           !ring->funcs->preempt_ib)
>                  return -EINVAL;
>  
>          /* the last preemption failed */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1a04ccba9542..7ff17df7a5ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5042,7 +5042,7 @@ bool amdgpu_device_has_job_running(struct amdgpu_device *adev)
>          for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>  
>                  spin_lock(&ring->sched.job_list_lock);
> @@ -5181,7 +5181,7 @@ int amdgpu_device_pre_asic_reset(struct amdgpu_device *adev,
>          for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>  
>                  /* Clear job fence from fence drv to avoid force_completion
> @@ -5648,7 +5648,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>                  for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                          struct amdgpu_ring *ring = tmp_adev->rings[i];
>  
> -                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +                       if (!amdgpu_ring_sched_ready(ring))
>                                  continue;
>  
>                          drm_sched_stop(&ring->sched, job ? &job->base : NULL);
> @@ -5717,7 +5717,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>                  for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                          struct amdgpu_ring *ring = tmp_adev->rings[i];
>  
> -                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +                       if (!amdgpu_ring_sched_ready(ring))
>                                  continue;
>  
>                          drm_sched_start(&ring->sched, true);
> @@ -6072,7 +6072,7 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev *pdev, pci_channel_sta
>                  for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                          struct amdgpu_ring *ring = adev->rings[i];
>  
> -                       if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +                       if (!amdgpu_ring_sched_ready(ring))
>                                  continue;
>  
>                          drm_sched_stop(&ring->sched, NULL);
> @@ -6214,7 +6214,7 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
>          for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>                  struct amdgpu_ring *ring = adev->rings[i];
>  
> -               if (!ring || !drm_sched_wqueue_ready(&ring->sched))
> +               if (!amdgpu_ring_sched_ready(ring))
>                          continue;
>  
>                  drm_sched_start(&ring->sched, true);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 45424ebf9681..9ae386e9d41d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -634,7 +634,8 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
>                  DRM_DEV_DEBUG(adev->dev, "ring test on %s succeeded\n",
>                                ring->name);
>  
> -       ring->sched.ready = !r;
> +       if (!ring->no_scheduler)
> +               ring->sched.ready = !r;
>          return r;
>  }
>  
> @@ -717,3 +718,14 @@ void amdgpu_ring_ib_on_emit_de(struct amdgpu_ring *ring)
>          if (ring->is_sw_ring)
>                  amdgpu_sw_ring_ib_mark_offset(ring, AMDGPU_MUX_OFFSET_TYPE_DE);
>  }
> +
> +bool amdgpu_ring_sched_ready(struct amdgpu_ring *ring)
> +{
> +       if (!ring)
> +               return false;
> +
> +       if (ring->no_scheduler || !drm_sched_wqueue_ready(&ring->sched))
> +               return false;
> +
> +       return true;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index bbb53720a018..fe1a61eb6e4c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -450,5 +450,5 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>  int amdgpu_ib_pool_init(struct amdgpu_device *adev);
>  void amdgpu_ib_pool_fini(struct amdgpu_device *adev);
>  int amdgpu_ib_ring_tests(struct amdgpu_device *adev);
> -
> +bool amdgpu_ring_sched_ready(struct amdgpu_ring *ring);
>  #endif
> -- 
> 2.42.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-01-30  2:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-04  5:39 [PATCH] drm/amdgpu: Fix the warning info in mode1 reset Ma Jun
2024-01-04 15:00 ` Alex Deucher
  -- strict thread matches above, loose matches on Subject: below --
2024-01-25 16:15 Alex Deucher
2024-01-29 15:33 ` Deucher, Alexander
2024-01-29 19:28   ` vitaly prosyak
2024-01-30  2:40   ` Ma, Jun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox