<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <div class="WordSection1"> [AMD Official Use Only]<o:p></o:p> <o:p> </o:p> In the previous discussion, you guys stated that we should drop the “kthread_should_park” in cleanup_job.<o:p></o:p> <o:p> </o:p> @@ -676,15 +676,6 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)<o:p></o:p> {<o:p></o:p> struct drm_sched_job *job, *next;<o:p></o:p> <o:p> </o:p> - /*<o:p></o:p> - * Don't destroy jobs while the timeout worker is running OR thread<o:p></o:p> - * is being parked and hence assumed to not touch pending_list<o:p></o:p> - */<o:p></o:p> - if ((sched->timeout != MAX_SCHEDULE_TIMEOUT &&<o:p></o:p> - !cancel_delayed_work(&sched->work_tdr)) ||<o:p></o:p> - kthread_should_park())<o:p></o:p> - return NULL;<o:p></o:p> <o:p> </o:p> But I suddenly have a question here: if return the timedout job no matter kthread_should_park() or not, then we are backing to the original problem again: that the timedout_job is suddenly signaling and cleanup_job still returns it to sched_main and job is freed while it is still handling by vendor’s timeout callback<o:p></o:p> <o:p> </o:p> If we return NULL when kthread_should_park() in cleanup_job, we can prevent above scenario from happening: once a job is processed by job_timedout we can stop its scheduler, and after that even this job suddenly signaled the cleanup_job won’t return it so sched_main won’t free it in parallel … <o:p></o:p> <o:p> </o:p> What do you think ?<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 0in 0in 0in"> From: Liu, Monk Sent: Wednesday, September 1, 2021 9:23 AM To: Koenig, Christian <Christian.Koenig@amd.com>; Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com>; Daniel Vetter <daniel@ffwll.ch>; Chen, JingWen <JingWen.Chen2@amd.com> Cc: DRI Development <dri-devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org Subject: [diagnostic TDR mode patches] unify our solution opinions/suggestions in one thread<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Official Use Only]<o:p></o:p> <o:p> </o:p> Hi Daniel/Christian/Andrey<o:p></o:p> <o:p> </o:p> It looks the voice from you three are spread over those email floods to me, the feature we are working on (diagnostic TDR scheme) is pending there for more than 6 month (we started it from feb 2021).<o:p></o:p> <o:p> </o:p> Honestly speaking the email ways that we are using now is not friendly and quite painful to me ….<o:p></o:p> Can we try to put all our opinions, suggestions, or even objects here together, let’s go through them one by one, it’s too hard for us to reply each email on different questions .<o:p></o:p> <o:p> </o:p> For [PATCH 1/2] drm/sched: fix the bug of time out calculation(v4)<o:p></o:p> <o:p> </o:p> This is a fixing patch on the timeout timer in scheduler, can we complete this one first ? it should already resolved all the questions and suggestions.<o:p></o:p> <o:p> </o:p> For [PATCH 2/2] drm/sched: serialize job_timeout and scheduler<o:p></o:p> <o:p> </o:p> I think I already explained the questions raised by Daniel in other thread , regarding why I use __kthread_should_park()<o:p></o:p> For other aspects, can we put all our opinion synthesized here ? <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> </div> </body> </html>