<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> 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> <pre class="moz-signature" cols="72"> </pre> </body> </html>