<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> Am 27.02.21 um 04:50 schrieb Liu, Monk: <blockquote type="cite" cite="mid:DM5PR12MB1708E521038AD046F45943AE849C9@DM5PR12MB1708.namprd12.prod.outlook.com"> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> [AMD Official Use Only - Internal Distribution Only] <div> <div class="WordSection1"> the code I pasted is to illustrate why the innocent job is already taken out in the mirror list thus my suggested proposal won’t work unless we don’t delete the job in sched_job_timeout() routine, and the problem you stated is with my understanding also kind of related with my suggested solution – the job removing from list should be handled by driver instead of scheduler . </div> </div> </blockquote> Yes, exactly that's my thinking as well. <blockquote type="cite" cite="mid:DM5PR12MB1708E521038AD046F45943AE849C9@DM5PR12MB1708.namprd12.prod.outlook.com"> <div> <div class="WordSection1"> <o:p></o:p> <o:p> </o:p>let make scheduler’s duty clear and simple : the sched_job_timeout() only get notification when a sched_job timedout but it doesn’t judge if the leading job in mirror list should be blamed , all those checking should be left to driver to take action. </div> </div> </blockquote> Need to get a detailed look, but it sounds correct as well. <blockquote type="cite" cite="mid:DM5PR12MB1708E521038AD046F45943AE849C9@DM5PR12MB1708.namprd12.prod.outlook.com"> <div> <div class="WordSection1"> <o:p></o:p> <o:p> </o:p> >> If we do this we should probably make it configurable as a module parameter.<o:p></o:p> That’s ok, maybe we can reuse the existed parm “gpu_recovery”, extend it with:<o:p></o:p> l 0 – no recovery initiated after job timeout<o:p></o:p> l 1 – legacy TDR behave <o:p></o:p> l 2 – enhanced TDR behave (the one suggested here) </div> </div> </blockquote> Yes, something like that should work. Key point is we had a couple of people who already suggested to optimize the reset routine so that it doesn't take so long. So far I pushed back on this because the reset routine isn't something I would optimize for speed. But when it starts to take something like 10 seconds instead of halve a second because you had an extra long running compute job we will certainly see complains. Regards, Christian. <blockquote type="cite" cite="mid:DM5PR12MB1708E521038AD046F45943AE849C9@DM5PR12MB1708.namprd12.prod.outlook.com"> <div> <div class="WordSection1"> <o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm"> 发件人: Koenig, Christian <a class="moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a> 发送时间: 2021年2月26日 20:05 收件人: Liu, Monk <a class="moz-txt-link-rfc2396E" href="mailto:Monk.Liu@amd.com"><Monk.Liu@amd.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> 抄送: Zhang, Andy <a class="moz-txt-link-rfc2396E" href="mailto:Andy.Zhang@amd.com"><Andy.Zhang@amd.com></a>; Chen, Horace <a class="moz-txt-link-rfc2396E" href="mailto:Horace.Chen@amd.com"><Horace.Chen@amd.com></a>; Zhang, Jack (Jian) <a class="moz-txt-link-rfc2396E" href="mailto:Jack.Zhang1@amd.com"><Jack.Zhang1@amd.com></a> 主题: Re: [RFC] a new approach to detect which ring is the real black sheep upon TDR reported<o:p></o:p> </div> </div> <o:p> </o:p> Yeah that is exactly the stuff which doesn't works at all. We got feedback for multiple people that this whole approach of tying the job to the tdr was not a good idea at all. What we should do instead is to have a pointer in the scheduler fence to which job it belongs. Freeing up the job when the scheduler fence is signaled is then job of the driver and not the scheduler any more. The scheduler then gives the scheduler fence to the driver when a timeout is detected and the driver can do the rest of the handling all by itself. But this problem is orthogonal to the suggested solution here. <o:p></o:p> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> do you have a better solution or idea we review it as another candidate RFC ?<o:p></o:p> </blockquote> I don't see much other option either. We could do something like only allowing one application at a time to use the gfx/compute block, but that would be even worse. If we do this we should probably make it configurable as a module parameter. Regards, Christian.<o:p></o:p> <div> Am 26.02.21 um 12:57 schrieb Liu, Monk:<o:p></o:p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> [AMD Official Use Only - Internal Distribution Only]<o:p></o:p> <o:p></o:p> static void drm_sched_job_timedout(struct work_struct *work)<o:p></o:p> 279 {<o:p></o:p> 280 struct drm_gpu_scheduler *sched;<o:p></o:p> 281 struct drm_sched_job *job;<o:p></o:p> 282<o:p></o:p> 283 sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);<o:p></o:p> 284<o:p></o:p> 285 /* Protects against concurrent deletion in drm_sched_get_cleanup_job */<o:p></o:p> 286 spin_lock(&sched->job_list_lock);<o:p></o:p> 287 job = list_first_entry_or_null(&sched->ring_mirror_list,<o:p></o:p> 288 struct drm_sched_job, node);<o:p></o:p> 289<o:p></o:p> 290 if (job) {<o:p></o:p> 291 /*<o:p></o:p> 292 * Remove the bad job so it cannot be freed by concurrent<o:p></o:p> 293 * drm_sched_cleanup_jobs. It will be reinserted back after sched->thread<o:p></o:p> 294 * is parked at which point it's safe.<o:p></o:p> 295 */<o:p></o:p> 296 list_del_init(&job->node);<o:p></o:p> 297 spin_unlock(&sched->job_list_lock);<o:p></o:p> 298<o:p></o:p> 299 job->sched->ops->timedout_job(job);<o:p></o:p> <o:p></o:p> <div> Thanks <o:p></o:p> <o:p></o:p> ------------------------------------------<o:p></o:p> Monk Liu | Cloud-GPU Core team<o:p></o:p> ------------------------------------------<o:p></o:p> </div> <o:p></o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm"> From: Liu, Monk Sent: Friday, February 26, 2021 7:54 PM To: Koenig, Christian <a href="mailto:Christian.Koenig@amd.com" moz-do-not-send="true"><Christian.Koenig@amd.com></a>; <a href="mailto:amd-gfx@lists.freedesktop.org" moz-do-not-send="true">amd-gfx@lists.freedesktop.org</a> Cc: Zhang, Andy <a href="mailto:Andy.Zhang@amd.com" moz-do-not-send="true"><Andy.Zhang@amd.com></a>; Chen, Horace <a href="mailto:Horace.Chen@amd.com" moz-do-not-send="true"><Horace.Chen@amd.com></a>; Zhang, Jack (Jian) <a href="mailto:Jack.Zhang1@amd.com" moz-do-not-send="true"><Jack.Zhang1@amd.com></a> Subject: RE: [RFC] a new approach to detect which ring is the real black sheep upon TDR reported<o:p></o:p> </div> </div> <o:p></o:p> [AMD Official Use Only - Internal Distribution Only]<o:p></o:p> <o:p></o:p> See in line<o:p></o:p> <o:p></o:p> <div> Thanks <o:p></o:p> <o:p></o:p> ------------------------------------------<o:p></o:p> Monk Liu | Cloud-GPU Core team<o:p></o:p> ------------------------------------------<o:p></o:p> </div> <o:p></o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm"> From: Koenig, Christian <<a href="mailto:Christian.Koenig@amd.com" moz-do-not-send="true">Christian.Koenig@amd.com</a>> Sent: Friday, February 26, 2021 3:58 PM To: Liu, Monk <<a href="mailto:Monk.Liu@amd.com" moz-do-not-send="true">Monk.Liu@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org" moz-do-not-send="true"> amd-gfx@lists.freedesktop.org</a> Cc: Zhang, Andy <<a href="mailto:Andy.Zhang@amd.com" moz-do-not-send="true">Andy.Zhang@amd.com</a>>; Chen, Horace <<a href="mailto:Horace.Chen@amd.com" moz-do-not-send="true">Horace.Chen@amd.com</a>>; Zhang, Jack (Jian) <<a href="mailto:Jack.Zhang1@amd.com" moz-do-not-send="true">Jack.Zhang1@amd.com</a>> Subject: Re: [RFC] a new approach to detect which ring is the real black sheep upon TDR reported<o:p></o:p> </div> </div> <o:p></o:p> Hi Monk, in general an interesting idea, but I see two major problems with that: 1. It would make the reset take much longer. 2. Things get often stuck because of timing issues, so a guilty job might pass perfectly when run a second time.<o:p></o:p> [ML] but the innocent ring already reported a TDR, and the drm sched logic already deleted this “sched_job” in its mirror list, thus you don’t have chance to re-submit it again after reset, that’s the major problem here. Apart from that the whole ring mirror list turned out to be a really bad idea. E.g. we still struggle with object life time because the concept doesn't fit into the object model of the GPU scheduler under Linux. We should probably work on this separately and straighten up the job destruction once more and keep the recovery information in the fence instead.<o:p></o:p> [ML] we claim to our customer that no innocent process will be dropped or cancelled, and our current logic works for the most time, but only when there are different process running on gfx/computes rings then we would run into the tricky situation I stated here, and the proposal is the only way I can figure out so far, do you have a better solution or idea we review it as another candidate RFC ? Be note that we raised this proposal is because we do hit our trouble and we do need to resolve it …. So even a not perfect solution is still better than just cancel the innocent job (and their context/process)<o:p></o:p> Thanks ! <o:p> </o:p> Regards, Christian.<o:p></o:p> <div> Am 26.02.21 um 06:58 schrieb Liu, Monk:<o:p></o:p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> [AMD Public Use]<o:p></o:p> <o:p></o:p> Hi all<o:p></o:p> <o:p></o:p> NAVI2X project hit a really hard to solve issue now, and it is turned out to be a general headache of our TDR mechanism , check below scenario:<o:p></o:p> <o:p></o:p> <ol style="margin-top:0cm" type="1" start="1"> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1">There is a job1 running on compute1 ring at timestamp <o:p></o:p></li> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1">There is a job2 running on gfx ring at timestamp<o:p></o:p></li> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1">Job1 is the guilty one, and job1/job2 were scheduled to their rings at almost the same timestamp <o:p></o:p></li> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1">After 2 seconds we receive two TDR reporting from both GFX ring and compute ring<o:p></o:p></li> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1">Current scheme is that in drm scheduler all the head jobs of those two rings are considered “bad job” and taken away from the mirror list <o:p></o:p></li> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l0 level1 lfo1">The result is both the real guilty job (job1) and the innocent job (job2) were all deleted from mirror list, and their corresponding contexts were also treated as guilty (so the innocent process remains running is not secured)<o:p></o:p></li> </ol> <o:p></o:p> But by our wish the ideal case is TDR mechanism can detect which ring is the guilty ring and the innocent ring can resubmits all its pending jobs:<o:p></o:p> <ol style="margin-top:0cm" type="1" start="1"> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l3 level1 lfo2">Job1 to be deleted from compute1 ring’s mirror list<o:p></o:p></li> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l3 level1 lfo2">Job2 is kept and resubmitted later and its belonging process/context are even not aware of this TDR at all <o:p></o:p></li> </ol> <o:p></o:p> <o:p></o:p> Here I have a proposal tend to achieve above goal and it rough procedure is :<o:p></o:p> <ol style="margin-top:0cm" type="1" start="1"> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l1 level1 lfo3">Once any ring reports a TDR, the head job is *not* treated as “bad job”, and it is *not* deleted from the mirror list in drm sched functions<o:p></o:p></li> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l1 level1 lfo3">In vendor’s function (our amdgpu driver here):<o:p></o:p></li> </ol> <ol style="margin-top:0cm" type="1" start="2"> <ul style="margin-top:0cm" type="disc"> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l1 level2 lfo3">reset GPU<o:p></o:p></li> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l1 level2 lfo3">repeat below actions on each RINGS * one by one *:<o:p></o:p></li> </ul> </ol> 1. take the head job and submit it on this ring<o:p></o:p> 2. see if it completes, if not then this job is the real “bad job”<o:p></o:p> 3.  take it away from mirror list if this head job is “bad job”<o:p></o:p> <ol style="margin-top:0cm" type="1" start="2"> <ul style="margin-top:0cm" type="disc"> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l1 level2 lfo3">After above iteration on all RINGS, we already clears all the bad job(s)<o:p></o:p></li> </ul> </ol> <ol style="margin-top:0cm" type="1" start="3"> <li class="MsoListParagraph" style="margin-left:0cm;mso-list:l1 level1 lfo3">Resubmit all jobs from each mirror list to their corresponding rings (this is the existed logic)<o:p></o:p></li> </ol> <o:p></o:p> The idea of this is to use “serial” way to re-run and re-check each head job of each RING, in order to take out the real black sheep and its guilty context.<o:p></o:p> <o:p></o:p> P.S.: we can use this approaches only on GFX/KCQ ring reports TDR , since those rings are intermutually affected to each other. For SDMA ring timeout it definitely proves the head job on SDMA ring is really guilty.<o:p></o:p> <o:p></o:p> Thanks <o:p></o:p> <o:p></o:p> ------------------------------------------<o:p></o:p> Monk Liu | Cloud-GPU Core team<o:p></o:p> ------------------------------------------<o:p></o:p> <o:p></o:p> </blockquote> <o:p></o:p> </blockquote> <o:p> </o:p> </div> </div> <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">amd-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </pre> </blockquote> </body> </html>