<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix"> <blockquote type="cite"> amdgpu_do_asic_reset starting to resume blocks ... amdgpu 0000:03:00.0: couldn't schedule ib on ring <sdma0> [drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22) ... amdgpu_device_ip_resume_phase2 resumed gfx_v9_0 amdgpu_device_ip_resume_phase2 resumed sdma_v4_0 amdgpu_device_ip_resume_phase2 resumed powerplay</blockquote> This is what's the root of the problem. The scheduler should never be resumed before we are done with bringing back the hardware into an usable state. Regards, Christian. Am 25.10.19 um 10:50 schrieb S, Shirish: </div> <blockquote type="cite" cite="mid:b5e99dc3-5658-7e48-63f7-bf9533f582f8@amd.com"> Here is the call trace: <blockquote> Call Trace: dump_stack+0x4d/0x63 amdgpu_ib_schedule+0x86/0x4b7 ? __mod_timer+0x21e/0x244 amdgpu_job_run+0x108/0x178 drm_sched_main+0x253/0x2fa ? remove_wait_queue+0x51/0x51 ? drm_sched_cleanup_jobs.part.12+0xda/0xda kthread+0x14f/0x157 ? kthread_park+0x86/0x86 ret_from_fork+0x22/0x40 amdgpu 0000:03:00.0: couldn't schedule ib on ring <sdma0> [drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22) </blockquote> printed via below change: <blockquote> @@ -151,6 +152,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs, } if (!ring->sched.ready) { + dump_stack(); dev_err(adev->dev, "couldn't schedule ib on ring <%s>\n", ring->name); return -EINVAL; </blockquote> <div class="moz-cite-prefix">On 10/24/2019 10:00 PM, Christian König wrote: </div> <blockquote type="cite" cite="mid:d573688c-0997-1928-0c56-b60a29ff7fde@gmail.com"> <div class="moz-cite-prefix">Am 24.10.19 um 17:06 schrieb Grodzovsky, Andrey: </div> <blockquote type="cite" cite="mid:f3be329d-d350-c821-00b7-d94858335796@amd.com"> <div class="moz-cite-prefix">On 10/24/19 7:01 AM, Christian König wrote: </div> <blockquote type="cite" cite="mid:23ea615d-5ef4-d0b3-a0ec-6fae67b102f2@gmail.com"> Am 24.10.19 um 12:58 schrieb S, Shirish: <blockquote type="cite">[Why] Upon GPU reset, kernel cleans up already submitted jobs via drm_sched_cleanup_jobs. This schedules ib's via drm_sched_main()->run_job, leading to race condition of rings being ready or not, since during reset rings may be suspended. </blockquote> NAK, exactly that's what should not happen. The scheduler should be suspend while a GPU reset is in progress. So you are running into a completely different race here. </blockquote> </blockquote> </blockquote> Below is the series of events when the issue occurs. (Note that as you & Andrey mentioned the scheduler has been suspended but the job is scheduled nonetheless.) <blockquote> amdgpu 0000:03:00.0: GPU reset begin! ... amdgpu_device_gpu_recover stopping ring sdma0 via drm_sched_stop ... amdgpu 0000:03:00.0: GPU reset succeeded, trying to resume amdgpu_do_asic_reset starting to resume blocks ... amdgpu 0000:03:00.0: couldn't schedule ib on ring <sdma0> [drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22) ... amdgpu_device_ip_resume_phase2 resumed gfx_v9_0 amdgpu_device_ip_resume_phase2 resumed sdma_v4_0 amdgpu_device_ip_resume_phase2 resumed powerplay ... </blockquote> FWIW, since the job is always NULL when "drm_sched_stop(&ring->sched, job ? &job->base : NULL);" when called during reset, all drm_sched_stop() does is cancel delayed work and park the sched->thread. There is no job list to be iterated to de-activate or remove or update fences. Based on all this analysis, adding a mutex is more failsafe and less intrusive in the current code flow and lastly seems to be logical as well, hence I devised this approach <blockquote type="cite" cite="mid:d573688c-0997-1928-0c56-b60a29ff7fde@gmail.com"> <blockquote type="cite" cite="mid:f3be329d-d350-c821-00b7-d94858335796@amd.com"> <blockquote type="cite" cite="mid:23ea615d-5ef4-d0b3-a0ec-6fae67b102f2@gmail.com"> Please sync up with Andrey how this was able to happen. Regards, Christian. </blockquote> Shirish - Christian makes a good point - note that in amdgpu_device_gpu_recover drm_sched_stop which stop all the scheduler threads is called way before we suspend the HW in amdgpu_device_pre_asic_reset->amdgpu_device_ip_suspend where SDMA suspension is happening and where the HW ring marked as not ready - please provide call stack for when you hit <span style="color: rgb(51, 51, 51); font-family: Arial, sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"> [drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22) to identify the code path which tried to submit the SDMA IB </blockquote> Well the most likely cause of this is that the hardware failed to resume after the reset. </blockquote> Infact hardware resume has not yet started, when the job is scheduled, which is the race am trying to address with this patch. Regards, Shirish S <blockquote type="cite" cite="mid:d573688c-0997-1928-0c56-b60a29ff7fde@gmail.com"> Christian. <blockquote type="cite" cite="mid:f3be329d-d350-c821-00b7-d94858335796@amd.com"> <span style="color: rgb(51, 51, 51); font-family: Arial, sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"> <span style="color: rgb(51, 51, 51); font-family: Arial, sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">Andrey <span style="color: rgb(51, 51, 51); font-family: Arial, sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(245, 245, 245); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"> <blockquote type="cite" cite="mid:23ea615d-5ef4-d0b3-a0ec-6fae67b102f2@gmail.com"> <blockquote type="cite"> [How] make GPU reset's amdgpu_device_ip_resume_phase2() & amdgpu_ib_schedule() in amdgpu_job_run() mutually exclusive. Signed-off-by: Shirish S <a class="moz-txt-link-rfc2396E" href="mailto:shirish.s@amd.com" moz-do-not-send="true"> <shirish.s@amd.com></a> --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 ++ 3 files changed, 6 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index f4d9041..7b07a47b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -973,6 +973,7 @@ struct amdgpu_device { bool in_gpu_reset; enum pp_mp1_state mp1_state; struct mutex lock_reset; + struct mutex lock_ib_sched; struct amdgpu_doorbell_index doorbell_index; int asic_reset_res; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 676cad1..63cad74 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2759,6 +2759,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, mutex_init(&adev->virt.vf_errors.lock); hash_init(adev->mn_hash); mutex_init(&adev->lock_reset); + mutex_init(&adev->lock_ib_sched); mutex_init(&adev->virt.dpm_mutex); mutex_init(&adev->psp.mutex); @@ -3795,7 +3796,9 @@ static int amdgpu_do_asic_reset(struct amdgpu_hive_info *hive, if (r) return r; + mutex_lock(&tmp_adev->lock_ib_sched); r = amdgpu_device_ip_resume_phase2(tmp_adev); + mutex_unlock(&tmp_adev->lock_ib_sched); if (r) goto out; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index e1bad99..cd6082d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -233,8 +233,10 @@ static struct dma_fence *amdgpu_job_run(struct drm_sched_job *sched_job) if (finished->error < 0) { DRM_INFO("Skip scheduling IBs!\n"); } else { + mutex_lock(&ring->adev->lock_ib_sched); r = amdgpu_ib_schedule(ring, job->num_ibs, job->ibs, job, &fence); + mutex_unlock(&ring->adev->lock_ib_sched); if (r) DRM_ERROR("Error scheduling IBs (%d)\n", r); } </blockquote> </blockquote> <fieldset class="mimeAttachmentHeader"></fieldset> <pre class="moz-quote-pre" wrap="">_______________________________________________ amd-gfx mailing list <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org" moz-do-not-send="true">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" moz-do-not-send="true">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a></pre> </blockquote> </blockquote> </blockquote> </body> </html>