<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> </head> <body> <p style="font-family:Arial;font-size:10pt;color:#0078D7;margin:15pt;" align="Left"> [AMD Official Use Only - Internal Distribution Only]<br> </p> <br> <div> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> well, submit job with HW disabled shluld be no harm.<br> </div> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> <br> </div> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> </div> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> The only concern is that we might use up IBs if we park scheduler during recovery. I have saw recovery stuck in sa new functuon. </div> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> <br> </div> <div id="ms-outlook-mobile-signature" dir="auto" style="text-align: left;"> <div dir="auto" style="text-align: left;">ring test alloc IBs to test if recovery succeed or not. But if there is no enough IBs it will wait fences to signal. However we have parked the scheduler thread, the job will never run and no fences will be signaled.</div> <div dir="auto" style="text-align: left;"><br> </div> <div dir="auto" style="text-align: left;">see, deadlock indeed. Now we are allowing job submission here. it is more likely that IBs might be used up.</div> <div dir="auto" style="text-align: left;"><br> </div> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Koenig, Christian <Christian.Koenig@amd.com><br> <b>Sent:</b> Wednesday, March 25, 2020 7:13:13 PM<br> <b>To:</b> Das, Nirmoy <Nirmoy.Das@amd.com><br> <b>Cc:</b> Pan, Xinhui <Xinhui.Pan@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com><br> <b>Subject:</b> Re: [PATCH] drm/amdgpu: Check entity rq</font> <div> </div> </div> <style>  </style> <div> <div> <div dir="auto"> <div dir="auto"> <div dir="auto">Hi guys, <div dir="auto"><br> </div> <div dir="auto">thanks for pointing this out Nirmoy.</div> <div dir="auto"><br> </div> <div dir="auto">Yeah, could be that I forgot to commit the patch. Currently I don't know at which end of the chaos I should start to clean up.</div> <div dir="auto"><br> </div> <div dir="auto">Christian.</div> </div> <div><br> <div class="x_x_elided-text">Am 25.03.2020 12:09 schrieb "Das, Nirmoy" <Nirmoy.Das@amd.com>:<br type="attribution"> <blockquote style="margin:0 0 0 0.8ex; border-left:1px #ccc solid; padding-left:1ex"> <div><font size="2"><span style="font-size:11pt"> <div>Hi Xinhui,<br> <br> <br> Can you please check if you can reproduce the crash with <br> <a href="https://lists.freedesktop.org/archives/amd-gfx/2020-February/046414.html">https://lists.freedesktop.org/archives/amd-gfx/2020-February/046414.html</a><br> <br> Christian fix it earlier, I think he forgot to push it.<br> <br> <br> Regards,<br> <br> Nirmoy<br> <br> On 3/25/20 12:07 PM, xinhui pan wrote:<br> > gpu recover will call sdma suspend/resume. In this period, ring will be<br> > disabled. So the vm_pte_scheds(sdma.instance[X].ring.sched)->ready will<br> > be false.<br> ><br> > If we submit any jobs in this ring-disabled period. We fail to pick up<br> > a rq for vm entity and entity->rq will set to NULL.<br> > amdgpu_vm_sdma_commit did not check the entity->rq, so fix it. Otherwise<br> > hit panic.<br> ><br> > Cc: Christian König <christian.koenig@amd.com><br> > Cc: Alex Deucher <alexander.deucher@amd.com><br> > Cc: Felix Kuehling <Felix.Kuehling@amd.com><br> > Signed-off-by: xinhui pan <xinhui.pan@amd.com><br> > ---<br> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 2 ++<br> > 1 file changed, 2 insertions(+)<br> ><br> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > index cf96c335b258..d30d103e48a2 100644<br> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > @@ -95,6 +95,8 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,<br> > int r;<br> > <br> > entity = p->direct ? &p->vm->direct : &p->vm->delayed;<br> > + if (!entity->rq)<br> > + return -ENOENT;<br> > ring = container_of(entity->rq->sched, struct amdgpu_ring, sched);<br> > <br> > WARN_ON(ib->length_dw == 0);<br> </div> </span></font></div> </blockquote> </div> <br> </div> </div> <div><br> <div class="x_x_elided-text">Am 25.03.2020 12:09 schrieb "Das, Nirmoy" <Nirmoy.Das@amd.com>:<br type="attribution"> <blockquote style="margin:0 0 0 0.8ex; border-left:1px #ccc solid; padding-left:1ex"> <div><font size="2"><span style="font-size:11pt"> <div>Hi Xinhui,<br> <br> <br> Can you please check if you can reproduce the crash with <br> <a href="https://lists.freedesktop.org/archives/amd-gfx/2020-February/046414.html">https://lists.freedesktop.org/archives/amd-gfx/2020-February/046414.html</a><br> <br> Christian fix it earlier, I think he forgot to push it.<br> <br> <br> Regards,<br> <br> Nirmoy<br> <br> On 3/25/20 12:07 PM, xinhui pan wrote:<br> > gpu recover will call sdma suspend/resume. In this period, ring will be<br> > disabled. So the vm_pte_scheds(sdma.instance[X].ring.sched)->ready will<br> > be false.<br> ><br> > If we submit any jobs in this ring-disabled period. We fail to pick up<br> > a rq for vm entity and entity->rq will set to NULL.<br> > amdgpu_vm_sdma_commit did not check the entity->rq, so fix it. Otherwise<br> > hit panic.<br> ><br> > Cc: Christian König <christian.koenig@amd.com><br> > Cc: Alex Deucher <alexander.deucher@amd.com><br> > Cc: Felix Kuehling <Felix.Kuehling@amd.com><br> > Signed-off-by: xinhui pan <xinhui.pan@amd.com><br> > ---<br> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 2 ++<br> > 1 file changed, 2 insertions(+)<br> ><br> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > index cf96c335b258..d30d103e48a2 100644<br> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > @@ -95,6 +95,8 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,<br> > int r;<br> > <br> > entity = p->direct ? &p->vm->direct : &p->vm->delayed;<br> > + if (!entity->rq)<br> > + return -ENOENT;<br> > ring = container_of(entity->rq->sched, struct amdgpu_ring, sched);<br> > <br> > WARN_ON(ib->length_dw == 0);<br> </div> </span></font></div> </blockquote> </div> <br> </div> </div> <div class="x_x_gmail_extra"><br> <div class="x_x_gmail_quote">Am 25.03.2020 12:09 schrieb "Das, Nirmoy" <Nirmoy.Das@amd.com>:<br type="attribution"> </div> </div> </div> <font size="2"><span style="font-size:11pt"> <div class="x_PlainText">Hi Xinhui,<br> <br> <br> Can you please check if you can reproduce the crash with <br> <a href="https://lists.freedesktop.org/archives/amd-gfx/2020-February/046414.html">https://lists.freedesktop.org/archives/amd-gfx/2020-February/046414.html</a><br> <br> Christian fix it earlier, I think he forgot to push it.<br> <br> <br> Regards,<br> <br> Nirmoy<br> <br> On 3/25/20 12:07 PM, xinhui pan wrote:<br> > gpu recover will call sdma suspend/resume. In this period, ring will be<br> > disabled. So the vm_pte_scheds(sdma.instance[X].ring.sched)->ready will<br> > be false.<br> ><br> > If we submit any jobs in this ring-disabled period. We fail to pick up<br> > a rq for vm entity and entity->rq will set to NULL.<br> > amdgpu_vm_sdma_commit did not check the entity->rq, so fix it. Otherwise<br> > hit panic.<br> ><br> > Cc: Christian König <christian.koenig@amd.com><br> > Cc: Alex Deucher <alexander.deucher@amd.com><br> > Cc: Felix Kuehling <Felix.Kuehling@amd.com><br> > Signed-off-by: xinhui pan <xinhui.pan@amd.com><br> > ---<br> > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 2 ++<br> > 1 file changed, 2 insertions(+)<br> ><br> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > index cf96c335b258..d30d103e48a2 100644<br> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c<br> > @@ -95,6 +95,8 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,<br> > int r;<br> > <br> > entity = p->direct ? &p->vm->direct : &p->vm->delayed;<br> > + if (!entity->rq)<br> > + return -ENOENT;<br> > ring = container_of(entity->rq->sched, struct amdgpu_ring, sched);<br> > <br> > WARN_ON(ib->length_dw == 0);<br> </div> </span></font></div> </div> </body> </html>