From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Christian_K=c3=b6nig?= Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr Date: Wed, 13 Nov 2019 08:36:52 +0100 Message-ID: <33ffe2f1-32b6-a238-3752-cee67cd9e141@gmail.com> References: <1573122349-22080-1-git-send-email-Emily.Deng@amd.com> <70c2c1cc-40b8-30da-7aee-f59fbc4d0d42@amd.com> <91f4a0c4-23e3-a399-5cb1-fb01da922784@amd.com> <2f035f22-4057-dd9e-27ef-0f5612113e29@amd.com> <9269d447-ed32-81f7-ab43-cb16139096e2@amd.com> Reply-To: christian.koenig-5C7GfCeVMHo@public.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0761989273==" Return-path: In-Reply-To: <9269d447-ed32-81f7-ab43-cb16139096e2-5C7GfCeVMHo@public.gmane.org> Content-Language: en-US List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: Andrey Grodzovsky , =?UTF-8?Q?Christian_K=c3=b6nig?= , "Deng, Emily" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" This is a multi-part message in MIME format. --===============0761989273== Content-Type: multipart/alternative; boundary="------------76F2BAA5ED3003DCF53B1335" Content-Language: en-US This is a multi-part message in MIME format. --------------76F2BAA5ED3003DCF53B1335 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit The question is where do we rearm the timer for this problem to occur? Regards, Christian. Am 12.11.19 um 20:21 schrieb Andrey Grodzovsky: > > I was able to reproduce the crash by using the attached > simulate_crash.patch - waiting on guilty job to signal in reset work > and artificially rearming the timeout timer just before the check for > !cancel_delayed_work(&sched->work_tdr)  in drm_sched_cleanup_jobs - > crash log attached in crash.log. This I think confirms my theory i > described earlier in this thread. > > basic_fix.patch handles this by testing whether another timer already > armed ob this scheduler or is there a timeout work in execution right > now (see documentation for work_busy) - obviously  this is not a full > solution as this will not protect from races if for example there is > immediate work scheduling such as in drm_sched_fault -  so we probably > need to account for this by making drm_sched_cleanup_jobs (at least in > the part where it iterates ring mirror list and frees jobs) and GPU > reset really mutually exclusive and not like now. > > Andrey > > > On 11/11/19 4:11 PM, Christian König wrote: >> Hi Emily, >> >> you need to print which scheduler instance is freeing the jobs and >> which one is triggering the reset. The TID and PID is completely >> meaningless here since we are called from different worker threads >> and the TID/PID can change on each call. >> >> Apart from that I will look into this a bit deeper when I have time. >> >> Regards, >> Christian. >> >> Am 12.11.19 um 07:02 schrieb Deng, Emily: >>> Hi Christian, >>>     I add the follow print in function drm_sched_cleanup_jobs. From >>> the log it shows that only use cancel_delayed_work could not avoid >>> to free job when the sched is in reset. But don’t know exactly where >>> it is wrong about the driver. Do you have any suggestion about this? >>> + printk("Emily:drm_sched_cleanup_jobs:begin,tid:%lu, pid:%lu\n", >>> current->tgid, current->pid); >>>         /* >>>          * Don't destroy jobs while the timeout worker is running  >>> OR thread >>>          * is being parked and hence assumed to not touch >>> ring_mirror_list >>>          */ >>>          if ((sched->timeout != MAX_SCHEDULE_TIMEOUT && >>> !cancel_delayed_work(&sched->work_tdr))) >>>                 return; >>> +       printk("Emily:drm_sched_cleanup_jobs,tid:%lu, pid:%lu\n", >>> current->tgid, current->pid); >>> Best wishes >>> Emily Deng >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695091] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695104] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695105] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695107] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695107] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.222954] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 >>> timeout, signaled seq=78585, emitted seq=78587 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.224275] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process >>> information: process  pid 0 thread  pid 0, >>> s_job:00000000fe75ab36,tid=15603, pid=15603 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225413] amdgpu 0000:00:08.0: GPU reset begin! >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225417] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225425] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225425] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225428] Emily:amdgpu_job_free_cb,Process information: >>> process  pid 0 thread  pid 0, s_job:00000000fe75ab36, tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225429] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225430] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225473] Emily:drm_sched_cleanup_jobs:begin,tid:2253, pid:2253 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225486] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225489] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225494] Emily:amdgpu_job_free_cb,Process information: >>> process  pid 0 thread  pid 0, s_job:00000000f086ec84, tid:2262, pid:2262 >>> >-----Original Message----- >>> >From: Grodzovsky, Andrey >>> >Sent: Tuesday, November 12, 2019 11:28 AM >>> >To: Koenig, Christian ; Deng, Emily >>> >; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr >>> > >>> >Thinking more about this claim - we assume here that if cancel_delayed_work >>> >returned true it guarantees that timeout work is not running but, it merely >>> >means there was a pending timeout work which was removed from the >>> >workqueue before it's timer elapsed and so it didn't have a chance to be >>> >dequeued and executed, it doesn't cover already executing work. So there is a >>> >possibility where while timeout work started executing another timeout work >>> >already got enqueued (maybe through earlier cleanup jobs or through >>> >drm_sched_fault) and if at this point another drm_sched_cleanup_jobs runs >>> >cancel_delayed_work(&sched->work_tdr) will return true even while there is a >>> >timeout job in progress. >>> >Unfortunately we cannot change cancel_delayed_work to >>> >cancel_delayed_work_sync to flush the timeout work as timeout work itself >>> >waits for schedule thread  to be parked again when calling park_thread. >>> > >>> >Andrey >>> > >>> >________________________________________ >>> >From: amd-gfx on behalf of >>> >Koenig, Christian >>> >Sent: 08 November 2019 05:35:18 >>> >To: Deng, Emily; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr >>> > >>> >Hi Emily, >>> > >>> >exactly that can't happen. See here: >>> > >>> >>         /* Don't destroy jobs while the timeout worker is running */ >>> >>         if (sched->timeout != MAX_SCHEDULE_TIMEOUT && >>> >> !cancel_delayed_work(&sched->work_tdr)) >>> >>                 return NULL; >>> > >>> >We never free jobs while the timeout working is running to prevent exactly >>> >that issue. >>> > >>> >Regards, >>> >Christian. >>> > >>> >Am 08.11.19 um 11:32 schrieb Deng, Emily: >>> >> Hi Christian, >>> >>       The drm_sched_job_timedout-> amdgpu_job_timedout call >>> >amdgpu_device_gpu_recover. I mean the main scheduler free the jobs while >>> >in amdgpu_device_gpu_recover, and before calling drm_sched_stop. >>> >> >>> >> Best wishes >>> >> Emily Deng >>> >> >>> >> >>> >> >>> >>> -----Original Message----- >>> >>> From: Koenig, Christian >>> >>> Sent: Friday, November 8, 2019 6:26 PM >>> >>> To: Deng, Emily ; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr >>> >>> >>> >>> Hi Emily, >>> >>> >>> >>> well who is calling amdgpu_device_gpu_recover() in this case? >>> >>> >>> >>> When it's not the scheduler we shouldn't have a guilty job in the first place. >>> >>> >>> >>> Regards, >>> >>> Christian. >>> >>> >>> >>> Am 08.11.19 um 11:22 schrieb Deng, Emily: >>> >>>> Hi Chrisitan, >>> >>>>        No, I am with the new branch and also has the patch. Even it >>> >>>> are freed by >>> >>> main scheduler, how we could avoid main scheduler to free jobs while >>> >>> enter to function amdgpu_device_gpu_recover? >>> >>>> Best wishes >>> >>>> Emily Deng >>> >>>> >>> >>>> >>> >>>> >>> >>>>> -----Original Message----- >>> >>>>> From: Koenig, Christian >>> >>>>> Sent: Friday, November 8, 2019 6:15 PM >>> >>>>> To: Deng, Emily ; amd- >>> >gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr >>> >>>>> >>> >>>>> Hi Emily, >>> >>>>> >>> >>>>> in this case you are on an old code branch. >>> >>>>> >>> >>>>> Jobs are freed now by the main scheduler thread and only if no >>> >>>>> timeout handler is running. >>> >>>>> >>> >>>>> See this patch here: >>> >>>>>> commit 5918045c4ed492fb5813f980dcf89a90fefd0a4e >>> >>>>>> Author: Christian König >>> >>>>>> Date:   Thu Apr 18 11:00:21 2019 -0400 >>> >>>>>> >>> >>>>>>       drm/scheduler: rework job destruction >>> >>>>> Regards, >>> >>>>> Christian. >>> >>>>> >>> >>>>> Am 08.11.19 um 11:11 schrieb Deng, Emily: >>> >>>>>> Hi Christian, >>> >>>>>>         Please refer to follow log, when it enter to >>> >>>>>> amdgpu_device_gpu_recover >>> >>>>> function, the bad job 000000005086879e is freeing in function >>> >>>>> amdgpu_job_free_cb  at the same time, because of the hardware fence >>> >>> signal. >>> >>>>> But amdgpu_device_gpu_recover goes faster, at this case, the >>> >>>>> s_fence is already freed, but job is not freed in time. Then this issue >>> >occurs. >>> >>>>>> [  449.792189] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring >>> >>> sdma0 >>> >>>>>> timeout, signaled seq=2481, emitted seq=2483 [  449.793202] >>> >>>>>> [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: >>> >>>>> process  pid 0 thread  pid 0, s_job:000000005086879e [  449.794163] >>> >>>>> amdgpu >>> >>>>> 0000:00:08.0: GPU reset begin! >>> >>>>>> [  449.794175] Emily:amdgpu_job_free_cb,Process information: >>> >>>>>> process pid 0 thread  pid 0, s_job:000000005086879e [  449.794221] >>> >>>>>> Emily:amdgpu_job_free_cb,Process information: process pid 0 >>> >>>>>> thread pid 0, s_job:0000000066eb74ab [  449.794222] >>> >>>>>> Emily:amdgpu_job_free_cb,Process information: process pid 0 >>> >>>>>> thread pid 0, s_job:00000000d4438ad9 [  449.794255] >>> >>>>>> Emily:amdgpu_job_free_cb,Process information: process pid 0 >>> >>>>>> thread pid 0, s_job:00000000b6d69c65 [  449.794257] >>> >>>>>> Emily:amdgpu_job_free_cb,Process information: process pid 0 >>> >>>>>> thread pid 0, >>> >>>>> s_job:00000000ea85e922 [ 449.794287] >>> >>>>> Emily:amdgpu_job_free_cb,Process >>> >>>>> information: process  pid 0 thread  pid 0, s_job:00000000ed3a5ac6 [ >>> >>>>> 449.794366] BUG: unable to handle kernel NULL pointer dereference >>> >>>>> at >>> >>>>> 00000000000000c0 [  449.800818] PGD 0 P4D 0 [  449.801040] Oops: >>> >>>>> 0000 [#1] SMP PTI >>> >>>>>> [  449.801338] CPU: 3 PID: 55 Comm: kworker/3:1 Tainted: G           OE >>> >>>>> 4.18.0-15-generic #16~18.04.1-Ubuntu >>> >>>>>> [  449.802157] Hardware name: QEMU Standard PC (i440FX + PIIX, >>> >>>>>> 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 [  449.802944] >>> >>>>>> Workqueue: events drm_sched_job_timedout [amd_sched] [ >>> >>>>>> 449.803488] >>> >>> RIP: >>> >>>>> 0010:amdgpu_device_gpu_recover+0x1da/0xb60 [amdgpu] >>> >>>>>> [  449.804020] Code: dd ff ff 49 39 c5 48 89 55 a8 0f 85 56 ff ff >>> >>>>>> ff >>> >>>>>> 45 85 e4 0f >>> >>>>> 85 a1 00 00 00 48 8b 45 b0 48 85 c0 0f 84 60 01 00 00 48 8b 40 10 >>> >>>>> <48> 8b >>> >>> 98 >>> >>>>> c0 00         00 00 48 85 db 0f 84 4c 01 00 00 48 8b 43 48 a8 01 >>> >>>>>> [  449.805593] RSP: 0018:ffffb4c7c08f7d68 EFLAGS: 00010286 [ >>> >>>>>> 449.806032] RAX: 0000000000000000 RBX: 0000000000000000 RCX: >>> >>>>>> 0000000000000000 [ 449.806625] RDX: ffffb4c7c08f5ac0 RSI: >>> >>>>>> 0000000fffffffe0 RDI: 0000000000000246 [  449.807224] RBP: >>> >>>>>> ffffb4c7c08f7de0 R08: 00000068b9d54000 R09: 0000000000000000 [ >>> >>>>>> 449.807818] R10: 0000000000000000 R11: 0000000000000148 R12: >>> >>>>>> 0000000000000000 [ 449.808411] R13: ffffb4c7c08f7da0 R14: >>> >>>>>> ffff8d82b8525d40 R15: ffff8d82b8525d40 [  449.809004] FS: >>> >>>>>> 0000000000000000(0000) GS:ffff8d82bfd80000(0000) >>> >>>>>> knlGS:0000000000000000 [ 449.809674] CS:  0010 DS: 0000 ES: 0000 >>> >CR0: >>> >>>>>> 0000000080050033 [ 449.810153] CR2: 00000000000000c0 CR3: >>> >>>>>> 000000003cc0a001 CR4: 00000000003606e0 [  449.810747] DR0: >>> >>>>> 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ >>> >>>>> 449.811344] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: >>> >>>>> 0000000000000400 [  449.811937] Call Trace: >>> >>>>>> [  449.812206] amdgpu_job_timedout+0x114/0x140 [amdgpu] [ >>> >>>>>> 449.812635] drm_sched_job_timedout+0x44/0x90 [amd_sched] [ >>> >>>>>> 449.813139]  ? amdgpu_cgs_destroy_device+0x10/0x10 [amdgpu] [ >>> >>>>>> 449.813609]  ? drm_sched_job_timedout+0x44/0x90 [amd_sched] [ >>> >>>>>> 449.814077] process_one_work+0x1fd/0x3f0 [  449.814417] >>> >>>>>> worker_thread+0x34/0x410 [ 449.814728]  kthread+0x121/0x140 [ >>> >>>>>> 449.815004]  ? process_one_work+0x3f0/0x3f0 [  449.815374]  ? >>> >>>>>> kthread_create_worker_on_cpu+0x70/0x70 >>> >>>>>> [  449.815799] ret_from_fork+0x35/0x40 >>> >>>>>> >>> >>>>>>> -----Original Message----- >>> >>>>>>> From: Koenig, Christian >>> >>>>>>> Sent: Friday, November 8, 2019 5:43 PM >>> >>>>>>> To: Deng, Emily ; amd- >>> >>> gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for >>> >>>>>>> tdr >>> >>>>>>> >>> >>>>>>> Am 08.11.19 um 10:39 schrieb Deng, Emily: >>> >>>>>>>> Sorry, please take your time. >>> >>>>>>> Have you seen my other response a bit below? >>> >>>>>>> >>> >>>>>>> I can't follow how it would be possible for job->s_fence to be >>> >>>>>>> NULL without the job also being freed. >>> >>>>>>> >>> >>>>>>> So it looks like this patch is just papering over some bigger issues. >>> >>>>>>> >>> >>>>>>> Regards, >>> >>>>>>> Christian. >>> >>>>>>> >>> >>>>>>>> Best wishes >>> >>>>>>>> Emily Deng >>> >>>>>>>> >>> >>>>>>>> >>> >>>>>>>> >>> >>>>>>>>> -----Original Message----- >>> >>>>>>>>> From: Koenig, Christian >>> >>>>>>>>> Sent: Friday, November 8, 2019 5:08 PM >>> >>>>>>>>> To: Deng, Emily ; amd- >>> >>>>> gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >>>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for >>> >>>>>>>>> tdr >>> >>>>>>>>> >>> >>>>>>>>> Am 08.11.19 um 09:52 schrieb Deng, Emily: >>> >>>>>>>>>> Ping..... >>> >>>>>>>>> You need to give me at least enough time to wake up :) >>> >>>>>>>>> >>> >>>>>>>>>> Best wishes >>> >>>>>>>>>> Emily Deng >>> >>>>>>>>>> >>> >>>>>>>>>> >>> >>>>>>>>>> >>> >>>>>>>>>>> -----Original Message----- >>> >>>>>>>>>>> From: amd-gfx On >>> >>> Behalf >>> >>>>>>>>>>> Of Deng, Emily >>> >>>>>>>>>>> Sent: Friday, November 8, 2019 10:56 AM >>> >>>>>>>>>>> To: Koenig, Christian ; amd- >>> >>>>>>>>>>> gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >>>>>>>>>>> Subject: RE: [PATCH] drm/amdgpu: Fix the null pointer issue >>> >>>>>>>>>>> for tdr >>> >>>>>>>>>>> >>> >>>>>>>>>>>> -----Original Message----- >>> >>>>>>>>>>>> From: Christian König >>> >>>>>>>>>>>> Sent: Thursday, November 7, 2019 7:28 PM >>> >>>>>>>>>>>> To: Deng, Emily ; >>> >>>>>>>>>>>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >>>>>>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue >>> >>>>>>>>>>>> for tdr >>> >>>>>>>>>>>> >>> >>>>>>>>>>>> Am 07.11.19 um 11:25 schrieb Emily Deng: >>> >>>>>>>>>>>>> When the job is already signaled, the s_fence is freed. >>> >>>>>>>>>>>>> Then it will has null pointer in amdgpu_device_gpu_recover. >>> >>>>>>>>>>>> NAK, the s_fence is only set to NULL when the job is destroyed. >>> >>>>>>>>>>>> See drm_sched_job_cleanup(). >>> >>>>>>>>>>> I know it is set to NULL in drm_sched_job_cleanup. But in one >>> >>>>>>>>>>> case, when it enter into the amdgpu_device_gpu_recover, it >>> >>>>>>>>>>> already in drm_sched_job_cleanup, and at this time, it will >>> >>>>>>>>>>> go to free >>> >>>>> job. >>> >>>>>>>>>>> But the amdgpu_device_gpu_recover sometimes is faster. At >>> >>>>>>>>>>> that time, job is not freed, but s_fence is already NULL. >>> >>>>>>>>> No, that case can't happen. See here: >>> >>>>>>>>> >>> >>>>>>>>>> drm_sched_job_cleanup(s_job); >>> >>>>>>>>>> >>> >>>>>>>>>> amdgpu_ring_priority_put(ring, s_job->s_priority); >>> >>>>>>>>>> dma_fence_put(job->fence); >>> >>>>>>>>>> amdgpu_sync_free(&job->sync); >>> >>>>>>>>>> amdgpu_sync_free(&job->sched_sync); >>> >>>>>>>>>> kfree(job); >>> >>>>>>>>> The job itself is freed up directly after freeing the reference >>> >>>>>>>>> to the >>> >>>>> s_fence. >>> >>>>>>>>> So you are just papering over a much bigger problem here. This >>> >>>>>>>>> patch is a clear NAK. >>> >>>>>>>>> >>> >>>>>>>>> Regards, >>> >>>>>>>>> Christian. >>> >>>>>>>>> >>> >>>>>>>>>>>> When you see a job without an s_fence then that means the >>> >>>>>>>>>>>> problem is somewhere else. >>> >>>>>>>>>>>> >>> >>>>>>>>>>>> Regards, >>> >>>>>>>>>>>> Christian. >>> >>>>>>>>>>>> >>> >>>>>>>>>>>>> Signed-off-by: Emily Deng >>> >>>>>>>>>>>>> --- >>> >>>>>>>>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +- >>> >>>>>>>>>>>>> drivers/gpu/drm/scheduler/sched_main.c     | 11 ++++++--- >>> >-- >>> >>>>>>>>>>>>> 2 files changed, 7 insertions(+), 6 deletions(-) >>> >>>>>>>>>>>>> >>> >>>>>>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> >>>>>>>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> >>>>>>>>>>>>> index e6ce949..5a8f08e 100644 >>> >>>>>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> >>>>>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> >>>>>>>>>>>>> @@ -4075,7 +4075,7 @@ int >>> >>> amdgpu_device_gpu_recover(struct >>> >>>>>>>>>>>> amdgpu_device *adev, >>> >>>>>>>>>>>>> * >>> >>>>>>>>>>>>> * job->base holds a reference to parent fence >>> >>>>>>>>>>>>> */ >>> >>>>>>>>>>>>> -  if (job && job->base.s_fence->parent && >>> >>>>>>>>>>>>> +  if (job && job->base.s_fence && >>> >>>>>>>>>>>>> + job->base.s_fence->parent >>> >>>>>>> && >>> >>>>>>>>>>>>> dma_fence_is_signaled(job->base.s_fence->parent)) >>> >>>>>>>>>>>>> job_signaled = true; >>> >>>>>>>>>>>>> >>> >>>>>>>>>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c >>> >>>>>>>>>>>>> b/drivers/gpu/drm/scheduler/sched_main.c >>> >>>>>>>>>>>>> index 31809ca..56cc10e 100644 >>> >>>>>>>>>>>>> --- a/drivers/gpu/drm/scheduler/sched_main.c >>> >>>>>>>>>>>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c >>> >>>>>>>>>>>>> @@ -334,8 +334,8 @@ void >>> >drm_sched_increase_karma(struct >>> >>>>>>>>>>>> drm_sched_job >>> >>>>>>>>>>>>> *bad) >>> >>>>>>>>>>>>> >>> >>>>>>>>>>>>> spin_lock(&rq->lock); >>> >>>>>>>>>>>>> list_for_each_entry_safe(entity, >>> >>>>>>>>>>>>> tmp, >>> >>> &rq- >>> >>>>>>>> entities, >>> >>>>>>>>>>>> list) { >>> >>>>>>>>>>>>> -                          if (bad->s_fence->scheduled.context >>> >>>>>>> == >>> >>>>>>>>>>>>> -                              entity->fence_context) { >>> >>>>>>>>>>>>> +                          if (bad->s_fence && >>> >>>>>>>>>>>>> + (bad->s_fence- >>> >>>>>>>>>>>>> scheduled.context == >>> >>>>>>>>>>>>> + entity->fence_context)) { >>> >>>>>>>>>>>>> if >>> >>>>>>>>>>>>> (atomic_read(&bad- >>> >>>>>>>> karma) > >>> >>>>>>>>>>>>> bad->sched- >>> >>>> hang_limit) >>> >>>>>>>>>>>>> if >>> >>>>>>>>>>>>> (entity- >>> >>>> guilty) @@ -376,7 +376,7 @@ void >>> >>>>>>>>>>>>> drm_sched_stop(struct >>> >>>>>>> drm_gpu_scheduler >>> >>>>>>>>>>>> *sched, struct drm_sched_job *bad) >>> >>>>>>>>>>>>> * This iteration is thread safe as sched thread >>> >>>>>>>>>>>>> is >>> >>> stopped. >>> >>>>>>>>>>>>> */ >>> >>>>>>>>>>>>> list_for_each_entry_safe_reverse(s_job, tmp, >>> >>>>>>>>>>>>> &sched- ring_mirror_list, node) { >>> >>>>>>>>>>>>> -          if (s_job->s_fence->parent && >>> >>>>>>>>>>>>> +          if (s_job->s_fence && s_job->s_fence->parent && >>> >>>>>>>>>>>>> dma_fence_remove_callback(s_job- >>> >>>> s_fence- >>> >>>>>>>> parent, >>> >>>>>>>>>>>>> &s_job->cb)) { >>> >>>>>>>>>>>>> atomic_dec(&sched->hw_rq_count); >>> >>> @@ - >>> >>>>>>> 395,7 >>> >>>>>>>>>>> +395,8 @@ void >>> >>>>>>>>>>>>> drm_sched_stop(struct drm_gpu_scheduler >>> >>>>>>>>>>>> *sched, struct drm_sched_job *bad) >>> >>>>>>>>>>>>> * >>> >>>>>>>>>>>>> * Job is still alive so fence >>> >>>>>>>>>>>>> refcount at >>> >>> least 1 >>> >>>>>>>>>>>>> */ >>> >>>>>>>>>>>>> - dma_fence_wait(&s_job->s_fence->finished, >>> >>>>>>> false); >>> >>>>>>>>>>>>> +                  if (s_job->s_fence) >>> >>>>>>>>>>>>> + dma_fence_wait(&s_job->s_fence- >>> >>>>>>>> finished, >>> >>>>>>>>>>>> false); >>> >>>>>>>>>>>>> /* >>> >>>>>>>>>>>>> * We must keep bad job alive >>> >>>>>>>>>>>>> for later >>> >>> use >>> >>>>>>> during @@ >>> >>>>>>>>>>>> -438,7 >>> >>>>>>>>>>>>> +439,7 @@ void drm_sched_start(struct drm_gpu_scheduler >>> >>>>> *sched, >>> >>>>>>>>>>>>> +bool >>> >>>>>>>>>>>> full_recovery) >>> >>>>>>>>>>>>> * GPU recovers can't run in parallel. >>> >>>>>>>>>>>>> */ >>> >>>>>>>>>>>>> list_for_each_entry_safe(s_job, tmp, >>> >>>>>>>>>>>>> &sched->ring_mirror_list, >>> >>>>>>>>>>>>> node) >>> >>>>>>>>>>>> { >>> >>>>>>>>>>>>> -          struct dma_fence *fence = s_job->s_fence->parent; >>> >>>>>>>>>>>>> +          struct dma_fence *fence = s_job->s_fence ? >>> >>>>>>>>>>>>> + s_job- >>> >>>>>>>> s_fence- >>> >>>>>>>>>>>>> parent : >>> >>>>>>>>>>>>> +NULL; >>> >>>>>>>>>>>>> >>> >>>>>>>>>>>>> atomic_inc(&sched->hw_rq_count); >>> >>>>>>>>>>>>> >>> >>>>>>>>>>> _______________________________________________ >>> >>>>>>>>>>> amd-gfx mailing list >>> >>>>>>>>>>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >>>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>> >>> > >>> >_______________________________________________ >>> >amd-gfx mailing list >>> >amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> >https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> > > _______________________________________________ > amd-gfx mailing list > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx --------------76F2BAA5ED3003DCF53B1335 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
The question is where do we rearm the timer for this problem to occur?

Regards,
Christian.

Am 12.11.19 um 20:21 schrieb Andrey Grodzovsky:

I was able to reproduce the crash by using the attached simulate_crash.patch - waiting on guilty job to signal in reset work and artificially rearming the timeout timer just before the check for !cancel_delayed_work(&sched->work_tdr)  in drm_sched_cleanup_jobs - crash log attached in crash.log. This I think confirms my theory i described earlier in this thread.

basic_fix.patch handles this by testing whether another timer already armed ob this scheduler or is there a timeout work in execution right now (see documentation for work_busy) - obviously  this is not a full solution as this will not protect from races if for example there is immediate work scheduling such as in drm_sched_fault -  so we probably need to account for this by making drm_sched_cleanup_jobs (at least in the part where it iterates ring mirror list and frees jobs) and GPU reset really mutually exclusive and not like now.

Andrey


On 11/11/19 4:11 PM, Christian König wrote:
Hi Emily,

you need to print which scheduler instance is freeing the jobs and which one is triggering the reset. The TID and PID is completely meaningless here since we are called from different worker threads and the TID/PID can change on each call.

Apart from that I will look into this a bit deeper when I have time.

Regards,
Christian.

Am 12.11.19 um 07:02 schrieb Deng, Emily:
Hi Christian,
    I add the follow print in function drm_sched_cleanup_jobs. From the log it shows that only use cancel_delayed_work could not avoid to free job when the sched is in reset. But don’t know exactly where it is wrong about the driver. Do you have any suggestion about this?
 
+       printk("Emily:drm_sched_cleanup_jobs:begin,tid:%lu, pid:%lu\n", current->tgid, current->pid);
 
        /*
         * Don't destroy jobs while the timeout worker is running  OR thread
         * is being parked and hence assumed to not touch ring_mirror_list
         */
         if ((sched->timeout != MAX_SCHEDULE_TIMEOUT &&
            !cancel_delayed_work(&sched->work_tdr)))
                return;
+       printk("Emily:drm_sched_cleanup_jobs,tid:%lu, pid:%lu\n", current->tgid, current->pid);
 
 
Best wishes
Emily Deng
 
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695091] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695104] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695105] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695107] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695107] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.222954] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled seq=78585, emitted seq=78587
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.224275] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process  pid 0 thread  pid 0, s_job:00000000fe75ab36,tid=15603, pid=15603
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225413] amdgpu 0000:00:08.0: GPU reset begin!
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225417] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225425] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225425] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225428] Emily:amdgpu_job_free_cb,Process information: process  pid 0 thread  pid 0, s_job:00000000fe75ab36, tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225429] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225430] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225473] Emily:drm_sched_cleanup_jobs:begin,tid:2253, pid:2253
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225486] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225489] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225494] Emily:amdgpu_job_free_cb,Process information: process  pid 0 thread  pid 0, s_job:00000000f086ec84, tid:2262, pid:2262
>-----Original Message-----
>Sent: Tuesday, November 12, 2019 11:28 AM
>To: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; Deng, Emily
>Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr
>
>Thinking more about this claim - we assume here that if cancel_delayed_work
>returned true it guarantees that timeout work is not running but, it merely
>means there was a pending timeout work which was removed from the
>workqueue before it's timer elapsed and so it didn't have a chance to be
>dequeued and executed, it doesn't cover already executing work. So there is a
>possibility where while timeout work started executing another timeout work
>already got enqueued (maybe through earlier cleanup jobs or through
>drm_sched_fault) and if at this point another drm_sched_cleanup_jobs runs
>cancel_delayed_work(&sched->work_tdr) will return true even while there is a
>timeout job in progress.
>Unfortunately we cannot change cancel_delayed_work to
>cancel_delayed_work_sync to flush the timeout work as timeout work itself
>waits for schedule thread  to be parked again when calling park_thread.
>
>Andrey
>
>________________________________________
>Sent: 08 November 2019 05:35:18
>Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr
>
>Hi Emily,
>
>exactly that can't happen. See here:
>
>>         /* Don't destroy jobs while the timeout worker is running */
>>         if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>>             !cancel_delayed_work(&sched->work_tdr))
>>                 return NULL;
>
>We never free jobs while the timeout working is running to prevent exactly
>that issue.
>
>Regards,
>Christian.
>
>Am 08.11.19 um 11:32 schrieb Deng, Emily:
>> Hi Christian,
>>       The drm_sched_job_timedout-> amdgpu_job_timedout call
>amdgpu_device_gpu_recover. I mean the main scheduler free the jobs while
>in amdgpu_device_gpu_recover, and before calling drm_sched_stop.
>>
>> Best wishes
>> Emily Deng
>>
>>
>>
>>> -----Original Message-----
>>> Sent: Friday, November 8, 2019 6:26 PM
>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr
>>>
>>> Hi Emily,
>>>
>>> well who is calling amdgpu_device_gpu_recover() in this case?
>>>
>>> When it's not the scheduler we shouldn't have a guilty job in the first place.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 08.11.19 um 11:22 schrieb Deng, Emily:
>>>> Hi Chrisitan,
>>>>        No, I am with the new branch and also has the patch. Even it
>>>> are freed by
>>> main scheduler, how we could avoid main scheduler to free jobs while
>>> enter to function amdgpu_device_gpu_recover?
>>>> Best wishes
>>>> Emily Deng
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> Sent: Friday, November 8, 2019 6:15 PM
>>>>> To: Deng, Emily <Emily.Deng-5C7GfCeVMHo@public.gmane.org>; amd-
>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr
>>>>>
>>>>> Hi Emily,
>>>>>
>>>>> in this case you are on an old code branch.
>>>>>
>>>>> Jobs are freed now by the main scheduler thread and only if no
>>>>> timeout handler is running.
>>>>>
>>>>> See this patch here:
>>>>>> commit 5918045c4ed492fb5813f980dcf89a90fefd0a4e
>>>>>> Author: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
>>>>>> Date:   Thu Apr 18 11:00:21 2019 -0400
>>>>>>
>>>>>>       drm/scheduler: rework job destruction
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>> Am 08.11.19 um 11:11 schrieb Deng, Emily:
>>>>>> Hi Christian,
>>>>>>         Please refer to follow log, when it enter to
>>>>>> amdgpu_device_gpu_recover
>>>>> function, the bad job 000000005086879e is freeing in function
>>>>> amdgpu_job_free_cb  at the same time, because of the hardware fence
>>> signal.
>>>>> But amdgpu_device_gpu_recover goes faster, at this case, the
>>>>> s_fence is already freed, but job is not freed in time. Then this issue
>occurs.
>>>>>> [  449.792189] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring
>>> sdma0
>>>>>> timeout, signaled seq=2481, emitted seq=2483 [  449.793202]
>>>>>> [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information:
>>>>> process  pid 0 thread  pid 0, s_job:000000005086879e [  449.794163]
>>>>> amdgpu
>>>>> 0000:00:08.0: GPU reset begin!
>>>>>> [  449.794175] Emily:amdgpu_job_free_cb,Process information:
>>>>>> process pid 0 thread  pid 0, s_job:000000005086879e [  449.794221]
>>>>>> Emily:amdgpu_job_free_cb,Process information: process  pid 0
>>>>>> thread pid 0, s_job:0000000066eb74ab [  449.794222]
>>>>>> Emily:amdgpu_job_free_cb,Process information: process  pid 0
>>>>>> thread pid 0, s_job:00000000d4438ad9 [  449.794255]
>>>>>> Emily:amdgpu_job_free_cb,Process information: process  pid 0
>>>>>> thread pid 0, s_job:00000000b6d69c65 [  449.794257]
>>>>>> Emily:amdgpu_job_free_cb,Process information: process  pid 0
>>>>>> thread pid 0,
>>>>> s_job:00000000ea85e922 [  449.794287]
>>>>> Emily:amdgpu_job_free_cb,Process
>>>>> information: process  pid 0 thread  pid 0, s_job:00000000ed3a5ac6 [
>>>>> 449.794366] BUG: unable to handle kernel NULL pointer dereference
>>>>> at
>>>>> 00000000000000c0 [  449.800818] PGD 0 P4D 0 [  449.801040] Oops:
>>>>> 0000 [#1] SMP PTI
>>>>>> [  449.801338] CPU: 3 PID: 55 Comm: kworker/3:1 Tainted: G           OE
>>>>> 4.18.0-15-generic #16~18.04.1-Ubuntu
>>>>>> [  449.802157] Hardware name: QEMU Standard PC (i440FX + PIIX,
>>>>>> 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 [  449.802944]
>>>>>> Workqueue: events drm_sched_job_timedout [amd_sched] [
>>>>>> 449.803488]
>>> RIP:
>>>>> 0010:amdgpu_device_gpu_recover+0x1da/0xb60 [amdgpu]
>>>>>> [  449.804020] Code: dd ff ff 49 39 c5 48 89 55 a8 0f 85 56 ff ff
>>>>>> ff
>>>>>> 45 85 e4 0f
>>>>> 85 a1 00 00 00 48 8b 45 b0 48 85 c0 0f 84 60 01 00 00 48 8b 40 10
>>>>> <48> 8b
>>> 98
>>>>> c0 00         00 00 48 85 db 0f 84 4c 01 00 00 48 8b 43 48 a8 01
>>>>>> [  449.805593] RSP: 0018:ffffb4c7c08f7d68 EFLAGS: 00010286 [
>>>>>> 449.806032] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
>>>>>> 0000000000000000 [  449.806625] RDX: ffffb4c7c08f5ac0 RSI:
>>>>>> 0000000fffffffe0 RDI: 0000000000000246 [  449.807224] RBP:
>>>>>> ffffb4c7c08f7de0 R08: 00000068b9d54000 R09: 0000000000000000 [
>>>>>> 449.807818] R10: 0000000000000000 R11: 0000000000000148 R12:
>>>>>> 0000000000000000 [  449.808411] R13: ffffb4c7c08f7da0 R14:
>>>>>> ffff8d82b8525d40 R15: ffff8d82b8525d40 [  449.809004] FS:
>>>>>> 0000000000000000(0000) GS:ffff8d82bfd80000(0000)
>>>>>> knlGS:0000000000000000 [  449.809674] CS:  0010 DS: 0000 ES: 0000
>CR0:
>>>>>> 0000000080050033 [  449.810153] CR2: 00000000000000c0 CR3:
>>>>>> 000000003cc0a001 CR4: 00000000003606e0 [  449.810747] DR0:
>>>>> 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [
>>>>> 449.811344] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
>>>>> 0000000000000400 [  449.811937] Call Trace:
>>>>>> [  449.812206]  amdgpu_job_timedout+0x114/0x140 [amdgpu] [
>>>>>> 449.812635]  drm_sched_job_timedout+0x44/0x90 [amd_sched] [
>>>>>> 449.813139]  ? amdgpu_cgs_destroy_device+0x10/0x10 [amdgpu] [
>>>>>> 449.813609]  ? drm_sched_job_timedout+0x44/0x90 [amd_sched] [
>>>>>> 449.814077]  process_one_work+0x1fd/0x3f0 [  449.814417]
>>>>>> worker_thread+0x34/0x410 [  449.814728]  kthread+0x121/0x140 [
>>>>>> 449.815004]  ? process_one_work+0x3f0/0x3f0 [  449.815374]  ?
>>>>>> kthread_create_worker_on_cpu+0x70/0x70
>>>>>> [  449.815799]  ret_from_fork+0x35/0x40
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
>>>>>>> Sent: Friday, November 8, 2019 5:43 PM
>>>>>>> To: Deng, Emily <Emily.Deng-5C7GfCeVMHo@public.gmane.org>; amd-
>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for
>>>>>>> tdr
>>>>>>>
>>>>>>> Am 08.11.19 um 10:39 schrieb Deng, Emily:
>>>>>>>> Sorry, please take your time.
>>>>>>> Have you seen my other response a bit below?
>>>>>>>
>>>>>>> I can't follow how it would be possible for job->s_fence to be
>>>>>>> NULL without the job also being freed.
>>>>>>>
>>>>>>> So it looks like this patch is just papering over some bigger issues.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Christian.
>>>>>>>
>>>>>>>> Best wishes
>>>>>>>> Emily Deng
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
>>>>>>>>> Sent: Friday, November 8, 2019 5:08 PM
>>>>>>>>> To: Deng, Emily <Emily.Deng-5C7GfCeVMHo@public.gmane.org>; amd-
>>>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for
>>>>>>>>> tdr
>>>>>>>>>
>>>>>>>>> Am 08.11.19 um 09:52 schrieb Deng, Emily:
>>>>>>>>>> Ping.....
>>>>>>>>> You need to give me at least enough time to wake up :)
>>>>>>>>>
>>>>>>>>>> Best wishes
>>>>>>>>>> Emily Deng
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>> Behalf
>>>>>>>>>>> Of Deng, Emily
>>>>>>>>>>> Sent: Friday, November 8, 2019 10:56 AM
>>>>>>>>>>> To: Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; amd-
>>>>>>>>>>> Subject: RE: [PATCH] drm/amdgpu: Fix the null pointer issue
>>>>>>>>>>> for tdr
>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> Sent: Thursday, November 7, 2019 7:28 PM
>>>>>>>>>>>> To: Deng, Emily <Emily.Deng-5C7GfCeVMHo@public.gmane.org>;
>>>>>>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue
>>>>>>>>>>>> for tdr
>>>>>>>>>>>>
>>>>>>>>>>>> Am 07.11.19 um 11:25 schrieb Emily Deng:
>>>>>>>>>>>>> When the job is already signaled, the s_fence is freed.
>>>>>>>>>>>>> Then it will has null pointer in amdgpu_device_gpu_recover.
>>>>>>>>>>>> NAK, the s_fence is only set to NULL when the job is destroyed.
>>>>>>>>>>>> See drm_sched_job_cleanup().
>>>>>>>>>>> I know it is set to NULL in drm_sched_job_cleanup. But in one
>>>>>>>>>>> case, when it enter into the amdgpu_device_gpu_recover, it
>>>>>>>>>>> already in drm_sched_job_cleanup, and at this time, it will
>>>>>>>>>>> go to free
>>>>> job.
>>>>>>>>>>> But the amdgpu_device_gpu_recover sometimes is faster. At
>>>>>>>>>>> that time, job is not freed, but s_fence is already NULL.
>>>>>>>>> No, that case can't happen. See here:
>>>>>>>>>
>>>>>>>>>>             drm_sched_job_cleanup(s_job);
>>>>>>>>>>
>>>>>>>>>>             amdgpu_ring_priority_put(ring, s_job->s_priority);
>>>>>>>>>>             dma_fence_put(job->fence);
>>>>>>>>>>             amdgpu_sync_free(&job->sync);
>>>>>>>>>>             amdgpu_sync_free(&job->sched_sync);
>>>>>>>>>>             kfree(job);
>>>>>>>>> The job itself is freed up directly after freeing the reference
>>>>>>>>> to the
>>>>> s_fence.
>>>>>>>>> So you are just papering over a much bigger problem here. This
>>>>>>>>> patch is a clear NAK.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Christian.
>>>>>>>>>
>>>>>>>>>>>> When you see a job without an s_fence then that means the
>>>>>>>>>>>> problem is somewhere else.
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Christian.
>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: Emily Deng <Emily.Deng-5C7GfCeVMHo@public.gmane.org>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>>        drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
>>>>>>>>>>>>>        drivers/gpu/drm/scheduler/sched_main.c     | 11 ++++++---
>--
>>>>>>>>>>>>>        2 files changed, 7 insertions(+), 6 deletions(-)
>>>>>>>>>>>>>
>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>>>>>>>> index e6ce949..5a8f08e 100644
>>>>>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>>>>>>>> @@ -4075,7 +4075,7 @@ int
>>> amdgpu_device_gpu_recover(struct
>>>>>>>>>>>> amdgpu_device *adev,
>>>>>>>>>>>>>             *
>>>>>>>>>>>>>             * job->base holds a reference to parent fence
>>>>>>>>>>>>>             */
>>>>>>>>>>>>> -  if (job && job->base.s_fence->parent &&
>>>>>>>>>>>>> +  if (job && job->base.s_fence &&
>>>>>>>>>>>>> + job->base.s_fence->parent
>>>>>>> &&
>>>>>>>>>>>>>                dma_fence_is_signaled(job->base.s_fence->parent))
>>>>>>>>>>>>>                    job_signaled = true;
>>>>>>>>>>>>>
>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
>>>>>>>>>>>>> b/drivers/gpu/drm/scheduler/sched_main.c
>>>>>>>>>>>>> index 31809ca..56cc10e 100644
>>>>>>>>>>>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>>>>>>>>>>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>>>>>>>>>>>> @@ -334,8 +334,8 @@ void
>drm_sched_increase_karma(struct
>>>>>>>>>>>> drm_sched_job
>>>>>>>>>>>>> *bad)
>>>>>>>>>>>>>
>>>>>>>>>>>>>                            spin_lock(&rq->lock);
>>>>>>>>>>>>>                            list_for_each_entry_safe(entity,
>>>>>>>>>>>>> tmp,
>>> &rq-
>>>>>>>> entities,
>>>>>>>>>>>> list) {
>>>>>>>>>>>>> -                          if (bad->s_fence->scheduled.context
>>>>>>> ==
>>>>>>>>>>>>> -                              entity->fence_context) {
>>>>>>>>>>>>> +                          if (bad->s_fence &&
>>>>>>>>>>>>> + (bad->s_fence-
>>>>>>>>>>>>> scheduled.context ==
>>>>>>>>>>>>> +                              entity->fence_context)) {
>>>>>>>>>>>>>                                            if
>>>>>>>>>>>>> (atomic_read(&bad-
>>>>>>>> karma) >
>>>>>>>>>>>>>                                                bad->sched-
>>>> hang_limit)
>>>>>>>>>>>>>                                                    if
>>>>>>>>>>>>> (entity-
>>>> guilty) @@ -376,7 +376,7 @@ void
>>>>>>>>>>>>> drm_sched_stop(struct
>>>>>>> drm_gpu_scheduler
>>>>>>>>>>>> *sched, struct drm_sched_job *bad)
>>>>>>>>>>>>>             * This iteration is thread safe as sched thread
>>>>>>>>>>>>> is
>>> stopped.
>>>>>>>>>>>>>             */
>>>>>>>>>>>>>            list_for_each_entry_safe_reverse(s_job, tmp,
>>>>>>>>>>>>> &sched- ring_mirror_list, node) {
>>>>>>>>>>>>> -          if (s_job->s_fence->parent &&
>>>>>>>>>>>>> +          if (s_job->s_fence && s_job->s_fence->parent &&
>>>>>>>>>>>>>                        dma_fence_remove_callback(s_job-
>>>> s_fence-
>>>>>>>> parent,
>>>>>>>>>>>>>                                                  &s_job->cb)) {
>>>>>>>>>>>>>                            atomic_dec(&sched->hw_rq_count);
>>> @@ -
>>>>>>> 395,7
>>>>>>>>>>> +395,8 @@ void
>>>>>>>>>>>>> drm_sched_stop(struct drm_gpu_scheduler
>>>>>>>>>>>> *sched, struct drm_sched_job *bad)
>>>>>>>>>>>>>                             *
>>>>>>>>>>>>>                             * Job is still alive so fence
>>>>>>>>>>>>> refcount at
>>> least 1
>>>>>>>>>>>>>                             */
>>>>>>>>>>>>> -                  dma_fence_wait(&s_job->s_fence->finished,
>>>>>>> false);
>>>>>>>>>>>>> +                  if (s_job->s_fence)
>>>>>>>>>>>>> +                          dma_fence_wait(&s_job->s_fence-
>>>>>>>> finished,
>>>>>>>>>>>> false);
>>>>>>>>>>>>>                            /*
>>>>>>>>>>>>>                             * We must keep bad job alive
>>>>>>>>>>>>> for later
>>> use
>>>>>>> during @@
>>>>>>>>>>>> -438,7
>>>>>>>>>>>>> +439,7 @@ void drm_sched_start(struct drm_gpu_scheduler
>>>>> *sched,
>>>>>>>>>>>>> +bool
>>>>>>>>>>>> full_recovery)
>>>>>>>>>>>>>             * GPU recovers can't run in parallel.
>>>>>>>>>>>>>             */
>>>>>>>>>>>>>            list_for_each_entry_safe(s_job, tmp,
>>>>>>>>>>>>> &sched->ring_mirror_list,
>>>>>>>>>>>>> node)
>>>>>>>>>>>> {
>>>>>>>>>>>>> -          struct dma_fence *fence = s_job->s_fence->parent;
>>>>>>>>>>>>> +          struct dma_fence *fence = s_job->s_fence ?
>>>>>>>>>>>>> + s_job-
>>>>>>>> s_fence-
>>>>>>>>>>>>> parent :
>>>>>>>>>>>>> +NULL;
>>>>>>>>>>>>>
>>>>>>>>>>>>>                    atomic_inc(&sched->hw_rq_count);
>>>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> amd-gfx mailing list
>
>_______________________________________________
>amd-gfx mailing list
 
 


_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

--------------76F2BAA5ED3003DCF53B1335-- --===============0761989273== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdmeCBt YWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4 --===============0761989273==-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI,MIME_HTML_MOSTLY,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 36C08C43331 for ; Wed, 13 Nov 2019 07:37:04 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1298F21A49 for ; Wed, 13 Nov 2019 07:37:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1298F21A49 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=amd-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1BBED6E11C; Wed, 13 Nov 2019 07:37:03 +0000 (UTC) Received: from mail-wr1-x443.google.com (mail-wr1-x443.google.com [IPv6:2a00:1450:4864:20::443]) by gabe.freedesktop.org (Postfix) with ESMTPS id 40BF76E11C for ; Wed, 13 Nov 2019 07:36:57 +0000 (UTC) Received: by mail-wr1-x443.google.com with SMTP id b3so1060086wrs.13 for ; Tue, 12 Nov 2019 23:36:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:reply-to:subject:to:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language; bh=YQh5aboevxXG3ZbeJ+pYOApV9OJwqGWm4BhKPzEBfGg=; b=gNSOUmn7dvDyADMBD5CziIqFKVt8ZRYTwJUXu+w0++OQlcbZQ1jGFDdRegxubeLUDk E7feQR+IfdV3D8X/sAM3pQThvw1oJSktySKw9Afivwoi7UOttyKPDLc8/0D/2V82u6L/ GBw3srgIK5xtXP/Rl/wsQDsDbw/khJsdJVT3gzOf9XOnWYSYyAKrqF1HVDLDOG8kjGMI LQwx0jw3BDvQYBHExcVnISwhAOLztByJRKgp8QbpkRUdYPApZwSdMUkXtC9Td3wDKG5Q 5RLBYyWMwIvde167PboIeOc9yTcvdogeeiFSr05QBUs4DHqAMjhezE2Mdo8az+tT7Mzs WfxA== X-Gm-Message-State: APjAAAUqSB3VzvxCEjOGBbt9QV+i7xE5g3r9o4TclkNwM6mnHDCo1DA2 V/pKTdvJKa3DSfW/GzWSyO4B/noD X-Google-Smtp-Source: APXvYqzWSYab8rsjiwtJXkG/kgp2crLkFJ5/vH4RDaeemaef4dEjJCOtKo4kniXgCluFGpxnvEBFHg== X-Received: by 2002:adf:f5c6:: with SMTP id k6mr1287215wrp.245.1573630615477; Tue, 12 Nov 2019 23:36:55 -0800 (PST) Received: from ?IPv6:2a02:908:1252:fb60:be8a:bd56:1f94:86e7? ([2a02:908:1252:fb60:be8a:bd56:1f94:86e7]) by smtp.gmail.com with ESMTPSA id f67sm1400223wme.16.2019.11.12.23.36.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Nov 2019 23:36:54 -0800 (PST) Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr To: Andrey Grodzovsky , =?UTF-8?Q?Christian_K=c3=b6nig?= , "Deng, Emily" , "amd-gfx@lists.freedesktop.org" References: <1573122349-22080-1-git-send-email-Emily.Deng@amd.com> <70c2c1cc-40b8-30da-7aee-f59fbc4d0d42@amd.com> <91f4a0c4-23e3-a399-5cb1-fb01da922784@amd.com> <2f035f22-4057-dd9e-27ef-0f5612113e29@amd.com> <9269d447-ed32-81f7-ab43-cb16139096e2@amd.com> From: =?UTF-8?Q?Christian_K=c3=b6nig?= Message-ID: <33ffe2f1-32b6-a238-3752-cee67cd9e141@gmail.com> Date: Wed, 13 Nov 2019 08:36:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <9269d447-ed32-81f7-ab43-cb16139096e2@amd.com> Content-Language: en-US X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=reply-to:subject:to:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=YQh5aboevxXG3ZbeJ+pYOApV9OJwqGWm4BhKPzEBfGg=; b=jfTlPay5boyUyH+x7s9AU4e7D1Far9UFVp6NkFasa6cZdzIINM5uQYGcfKEJhoGMHm myN05fxW9UVFy0qG5zaG44AomZpVlYp5NE0PFKWz0N0sm6QW+tDGFyDimie2hUyzIA0J wUf27x1YdrlFziCjxsU4+/IvcOEO0r4wXG8S0uHcR9t+rmuMDLywAQJmLjkv+dstYAq3 hwiLZEt5Z+d0UPfGaUQ8sqTJyS3RgKbcKr4Dpny/sOwnBHftErYkGr3vxub1xCAy5r/U crg4bdxovgDMe8zgaaM0X0tZl5WWw0w05TcyrjdNnTfYvx9knGX/cqarNxGsVsev8dyW B4YA== X-BeenThere: amd-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: christian.koenig@amd.com Content-Type: multipart/mixed; boundary="===============0761989273==" Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" Message-ID: <20191113073652.-KIvvV7is8fvsVYhwFy1qSCrZ1ensUd3Ua_To0RK-o4@z> This is a multi-part message in MIME format. --===============0761989273== Content-Type: multipart/alternative; boundary="------------76F2BAA5ED3003DCF53B1335" Content-Language: en-US This is a multi-part message in MIME format. --------------76F2BAA5ED3003DCF53B1335 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit The question is where do we rearm the timer for this problem to occur? Regards, Christian. Am 12.11.19 um 20:21 schrieb Andrey Grodzovsky: > > I was able to reproduce the crash by using the attached > simulate_crash.patch - waiting on guilty job to signal in reset work > and artificially rearming the timeout timer just before the check for > !cancel_delayed_work(&sched->work_tdr)  in drm_sched_cleanup_jobs - > crash log attached in crash.log. This I think confirms my theory i > described earlier in this thread. > > basic_fix.patch handles this by testing whether another timer already > armed ob this scheduler or is there a timeout work in execution right > now (see documentation for work_busy) - obviously  this is not a full > solution as this will not protect from races if for example there is > immediate work scheduling such as in drm_sched_fault -  so we probably > need to account for this by making drm_sched_cleanup_jobs (at least in > the part where it iterates ring mirror list and frees jobs) and GPU > reset really mutually exclusive and not like now. > > Andrey > > > On 11/11/19 4:11 PM, Christian König wrote: >> Hi Emily, >> >> you need to print which scheduler instance is freeing the jobs and >> which one is triggering the reset. The TID and PID is completely >> meaningless here since we are called from different worker threads >> and the TID/PID can change on each call. >> >> Apart from that I will look into this a bit deeper when I have time. >> >> Regards, >> Christian. >> >> Am 12.11.19 um 07:02 schrieb Deng, Emily: >>> Hi Christian, >>>     I add the follow print in function drm_sched_cleanup_jobs. From >>> the log it shows that only use cancel_delayed_work could not avoid >>> to free job when the sched is in reset. But don’t know exactly where >>> it is wrong about the driver. Do you have any suggestion about this? >>> + printk("Emily:drm_sched_cleanup_jobs:begin,tid:%lu, pid:%lu\n", >>> current->tgid, current->pid); >>>         /* >>>          * Don't destroy jobs while the timeout worker is running  >>> OR thread >>>          * is being parked and hence assumed to not touch >>> ring_mirror_list >>>          */ >>>          if ((sched->timeout != MAX_SCHEDULE_TIMEOUT && >>> !cancel_delayed_work(&sched->work_tdr))) >>>                 return; >>> +       printk("Emily:drm_sched_cleanup_jobs,tid:%lu, pid:%lu\n", >>> current->tgid, current->pid); >>> Best wishes >>> Emily Deng >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695091] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695104] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695105] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695107] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11380.695107] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.222954] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 >>> timeout, signaled seq=78585, emitted seq=78587 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.224275] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process >>> information: process  pid 0 thread  pid 0, >>> s_job:00000000fe75ab36,tid=15603, pid=15603 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225413] amdgpu 0000:00:08.0: GPU reset begin! >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225417] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225425] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225425] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225428] Emily:amdgpu_job_free_cb,Process information: >>> process  pid 0 thread  pid 0, s_job:00000000fe75ab36, tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225429] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225430] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225473] Emily:drm_sched_cleanup_jobs:begin,tid:2253, pid:2253 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225486] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225489] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262 >>> Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: >>> [11381.225494] Emily:amdgpu_job_free_cb,Process information: >>> process  pid 0 thread  pid 0, s_job:00000000f086ec84, tid:2262, pid:2262 >>> >-----Original Message----- >>> >From: Grodzovsky, Andrey >>> >Sent: Tuesday, November 12, 2019 11:28 AM >>> >To: Koenig, Christian ; Deng, Emily >>> >; amd-gfx@lists.freedesktop.org >>> >Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr >>> > >>> >Thinking more about this claim - we assume here that if cancel_delayed_work >>> >returned true it guarantees that timeout work is not running but, it merely >>> >means there was a pending timeout work which was removed from the >>> >workqueue before it's timer elapsed and so it didn't have a chance to be >>> >dequeued and executed, it doesn't cover already executing work. So there is a >>> >possibility where while timeout work started executing another timeout work >>> >already got enqueued (maybe through earlier cleanup jobs or through >>> >drm_sched_fault) and if at this point another drm_sched_cleanup_jobs runs >>> >cancel_delayed_work(&sched->work_tdr) will return true even while there is a >>> >timeout job in progress. >>> >Unfortunately we cannot change cancel_delayed_work to >>> >cancel_delayed_work_sync to flush the timeout work as timeout work itself >>> >waits for schedule thread  to be parked again when calling park_thread. >>> > >>> >Andrey >>> > >>> >________________________________________ >>> >From: amd-gfx on behalf of >>> >Koenig, Christian >>> >Sent: 08 November 2019 05:35:18 >>> >To: Deng, Emily; amd-gfx@lists.freedesktop.org >>> >Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr >>> > >>> >Hi Emily, >>> > >>> >exactly that can't happen. See here: >>> > >>> >>         /* Don't destroy jobs while the timeout worker is running */ >>> >>         if (sched->timeout != MAX_SCHEDULE_TIMEOUT && >>> >> !cancel_delayed_work(&sched->work_tdr)) >>> >>                 return NULL; >>> > >>> >We never free jobs while the timeout working is running to prevent exactly >>> >that issue. >>> > >>> >Regards, >>> >Christian. >>> > >>> >Am 08.11.19 um 11:32 schrieb Deng, Emily: >>> >> Hi Christian, >>> >>       The drm_sched_job_timedout-> amdgpu_job_timedout call >>> >amdgpu_device_gpu_recover. I mean the main scheduler free the jobs while >>> >in amdgpu_device_gpu_recover, and before calling drm_sched_stop. >>> >> >>> >> Best wishes >>> >> Emily Deng >>> >> >>> >> >>> >> >>> >>> -----Original Message----- >>> >>> From: Koenig, Christian >>> >>> Sent: Friday, November 8, 2019 6:26 PM >>> >>> To: Deng, Emily ; amd-gfx@lists.freedesktop.org >>> >>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr >>> >>> >>> >>> Hi Emily, >>> >>> >>> >>> well who is calling amdgpu_device_gpu_recover() in this case? >>> >>> >>> >>> When it's not the scheduler we shouldn't have a guilty job in the first place. >>> >>> >>> >>> Regards, >>> >>> Christian. >>> >>> >>> >>> Am 08.11.19 um 11:22 schrieb Deng, Emily: >>> >>>> Hi Chrisitan, >>> >>>>        No, I am with the new branch and also has the patch. Even it >>> >>>> are freed by >>> >>> main scheduler, how we could avoid main scheduler to free jobs while >>> >>> enter to function amdgpu_device_gpu_recover? >>> >>>> Best wishes >>> >>>> Emily Deng >>> >>>> >>> >>>> >>> >>>> >>> >>>>> -----Original Message----- >>> >>>>> From: Koenig, Christian >>> >>>>> Sent: Friday, November 8, 2019 6:15 PM >>> >>>>> To: Deng, Emily ; amd- >>> >gfx@lists.freedesktop.org >>> >>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr >>> >>>>> >>> >>>>> Hi Emily, >>> >>>>> >>> >>>>> in this case you are on an old code branch. >>> >>>>> >>> >>>>> Jobs are freed now by the main scheduler thread and only if no >>> >>>>> timeout handler is running. >>> >>>>> >>> >>>>> See this patch here: >>> >>>>>> commit 5918045c4ed492fb5813f980dcf89a90fefd0a4e >>> >>>>>> Author: Christian König >>> >>>>>> Date:   Thu Apr 18 11:00:21 2019 -0400 >>> >>>>>> >>> >>>>>>       drm/scheduler: rework job destruction >>> >>>>> Regards, >>> >>>>> Christian. >>> >>>>> >>> >>>>> Am 08.11.19 um 11:11 schrieb Deng, Emily: >>> >>>>>> Hi Christian, >>> >>>>>>         Please refer to follow log, when it enter to >>> >>>>>> amdgpu_device_gpu_recover >>> >>>>> function, the bad job 000000005086879e is freeing in function >>> >>>>> amdgpu_job_free_cb  at the same time, because of the hardware fence >>> >>> signal. >>> >>>>> But amdgpu_device_gpu_recover goes faster, at this case, the >>> >>>>> s_fence is already freed, but job is not freed in time. Then this issue >>> >occurs. >>> >>>>>> [  449.792189] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring >>> >>> sdma0 >>> >>>>>> timeout, signaled seq=2481, emitted seq=2483 [  449.793202] >>> >>>>>> [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: >>> >>>>> process  pid 0 thread  pid 0, s_job:000000005086879e [  449.794163] >>> >>>>> amdgpu >>> >>>>> 0000:00:08.0: GPU reset begin! >>> >>>>>> [  449.794175] Emily:amdgpu_job_free_cb,Process information: >>> >>>>>> process pid 0 thread  pid 0, s_job:000000005086879e [  449.794221] >>> >>>>>> Emily:amdgpu_job_free_cb,Process information: process pid 0 >>> >>>>>> thread pid 0, s_job:0000000066eb74ab [  449.794222] >>> >>>>>> Emily:amdgpu_job_free_cb,Process information: process pid 0 >>> >>>>>> thread pid 0, s_job:00000000d4438ad9 [  449.794255] >>> >>>>>> Emily:amdgpu_job_free_cb,Process information: process pid 0 >>> >>>>>> thread pid 0, s_job:00000000b6d69c65 [  449.794257] >>> >>>>>> Emily:amdgpu_job_free_cb,Process information: process pid 0 >>> >>>>>> thread pid 0, >>> >>>>> s_job:00000000ea85e922 [ 449.794287] >>> >>>>> Emily:amdgpu_job_free_cb,Process >>> >>>>> information: process  pid 0 thread  pid 0, s_job:00000000ed3a5ac6 [ >>> >>>>> 449.794366] BUG: unable to handle kernel NULL pointer dereference >>> >>>>> at >>> >>>>> 00000000000000c0 [  449.800818] PGD 0 P4D 0 [  449.801040] Oops: >>> >>>>> 0000 [#1] SMP PTI >>> >>>>>> [  449.801338] CPU: 3 PID: 55 Comm: kworker/3:1 Tainted: G           OE >>> >>>>> 4.18.0-15-generic #16~18.04.1-Ubuntu >>> >>>>>> [  449.802157] Hardware name: QEMU Standard PC (i440FX + PIIX, >>> >>>>>> 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 [  449.802944] >>> >>>>>> Workqueue: events drm_sched_job_timedout [amd_sched] [ >>> >>>>>> 449.803488] >>> >>> RIP: >>> >>>>> 0010:amdgpu_device_gpu_recover+0x1da/0xb60 [amdgpu] >>> >>>>>> [  449.804020] Code: dd ff ff 49 39 c5 48 89 55 a8 0f 85 56 ff ff >>> >>>>>> ff >>> >>>>>> 45 85 e4 0f >>> >>>>> 85 a1 00 00 00 48 8b 45 b0 48 85 c0 0f 84 60 01 00 00 48 8b 40 10 >>> >>>>> <48> 8b >>> >>> 98 >>> >>>>> c0 00         00 00 48 85 db 0f 84 4c 01 00 00 48 8b 43 48 a8 01 >>> >>>>>> [  449.805593] RSP: 0018:ffffb4c7c08f7d68 EFLAGS: 00010286 [ >>> >>>>>> 449.806032] RAX: 0000000000000000 RBX: 0000000000000000 RCX: >>> >>>>>> 0000000000000000 [ 449.806625] RDX: ffffb4c7c08f5ac0 RSI: >>> >>>>>> 0000000fffffffe0 RDI: 0000000000000246 [  449.807224] RBP: >>> >>>>>> ffffb4c7c08f7de0 R08: 00000068b9d54000 R09: 0000000000000000 [ >>> >>>>>> 449.807818] R10: 0000000000000000 R11: 0000000000000148 R12: >>> >>>>>> 0000000000000000 [ 449.808411] R13: ffffb4c7c08f7da0 R14: >>> >>>>>> ffff8d82b8525d40 R15: ffff8d82b8525d40 [  449.809004] FS: >>> >>>>>> 0000000000000000(0000) GS:ffff8d82bfd80000(0000) >>> >>>>>> knlGS:0000000000000000 [ 449.809674] CS:  0010 DS: 0000 ES: 0000 >>> >CR0: >>> >>>>>> 0000000080050033 [ 449.810153] CR2: 00000000000000c0 CR3: >>> >>>>>> 000000003cc0a001 CR4: 00000000003606e0 [  449.810747] DR0: >>> >>>>> 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ >>> >>>>> 449.811344] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: >>> >>>>> 0000000000000400 [  449.811937] Call Trace: >>> >>>>>> [  449.812206] amdgpu_job_timedout+0x114/0x140 [amdgpu] [ >>> >>>>>> 449.812635] drm_sched_job_timedout+0x44/0x90 [amd_sched] [ >>> >>>>>> 449.813139]  ? amdgpu_cgs_destroy_device+0x10/0x10 [amdgpu] [ >>> >>>>>> 449.813609]  ? drm_sched_job_timedout+0x44/0x90 [amd_sched] [ >>> >>>>>> 449.814077] process_one_work+0x1fd/0x3f0 [  449.814417] >>> >>>>>> worker_thread+0x34/0x410 [ 449.814728]  kthread+0x121/0x140 [ >>> >>>>>> 449.815004]  ? process_one_work+0x3f0/0x3f0 [  449.815374]  ? >>> >>>>>> kthread_create_worker_on_cpu+0x70/0x70 >>> >>>>>> [  449.815799] ret_from_fork+0x35/0x40 >>> >>>>>> >>> >>>>>>> -----Original Message----- >>> >>>>>>> From: Koenig, Christian >>> >>>>>>> Sent: Friday, November 8, 2019 5:43 PM >>> >>>>>>> To: Deng, Emily ; amd- >>> >>> gfx@lists.freedesktop.org >>> >>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for >>> >>>>>>> tdr >>> >>>>>>> >>> >>>>>>> Am 08.11.19 um 10:39 schrieb Deng, Emily: >>> >>>>>>>> Sorry, please take your time. >>> >>>>>>> Have you seen my other response a bit below? >>> >>>>>>> >>> >>>>>>> I can't follow how it would be possible for job->s_fence to be >>> >>>>>>> NULL without the job also being freed. >>> >>>>>>> >>> >>>>>>> So it looks like this patch is just papering over some bigger issues. >>> >>>>>>> >>> >>>>>>> Regards, >>> >>>>>>> Christian. >>> >>>>>>> >>> >>>>>>>> Best wishes >>> >>>>>>>> Emily Deng >>> >>>>>>>> >>> >>>>>>>> >>> >>>>>>>> >>> >>>>>>>>> -----Original Message----- >>> >>>>>>>>> From: Koenig, Christian >>> >>>>>>>>> Sent: Friday, November 8, 2019 5:08 PM >>> >>>>>>>>> To: Deng, Emily ; amd- >>> >>>>> gfx@lists.freedesktop.org >>> >>>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for >>> >>>>>>>>> tdr >>> >>>>>>>>> >>> >>>>>>>>> Am 08.11.19 um 09:52 schrieb Deng, Emily: >>> >>>>>>>>>> Ping..... >>> >>>>>>>>> You need to give me at least enough time to wake up :) >>> >>>>>>>>> >>> >>>>>>>>>> Best wishes >>> >>>>>>>>>> Emily Deng >>> >>>>>>>>>> >>> >>>>>>>>>> >>> >>>>>>>>>> >>> >>>>>>>>>>> -----Original Message----- >>> >>>>>>>>>>> From: amd-gfx On >>> >>> Behalf >>> >>>>>>>>>>> Of Deng, Emily >>> >>>>>>>>>>> Sent: Friday, November 8, 2019 10:56 AM >>> >>>>>>>>>>> To: Koenig, Christian ; amd- >>> >>>>>>>>>>> gfx@lists.freedesktop.org >>> >>>>>>>>>>> Subject: RE: [PATCH] drm/amdgpu: Fix the null pointer issue >>> >>>>>>>>>>> for tdr >>> >>>>>>>>>>> >>> >>>>>>>>>>>> -----Original Message----- >>> >>>>>>>>>>>> From: Christian König >>> >>>>>>>>>>>> Sent: Thursday, November 7, 2019 7:28 PM >>> >>>>>>>>>>>> To: Deng, Emily ; >>> >>>>>>>>>>>> amd-gfx@lists.freedesktop.org >>> >>>>>>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue >>> >>>>>>>>>>>> for tdr >>> >>>>>>>>>>>> >>> >>>>>>>>>>>> Am 07.11.19 um 11:25 schrieb Emily Deng: >>> >>>>>>>>>>>>> When the job is already signaled, the s_fence is freed. >>> >>>>>>>>>>>>> Then it will has null pointer in amdgpu_device_gpu_recover. >>> >>>>>>>>>>>> NAK, the s_fence is only set to NULL when the job is destroyed. >>> >>>>>>>>>>>> See drm_sched_job_cleanup(). >>> >>>>>>>>>>> I know it is set to NULL in drm_sched_job_cleanup. But in one >>> >>>>>>>>>>> case, when it enter into the amdgpu_device_gpu_recover, it >>> >>>>>>>>>>> already in drm_sched_job_cleanup, and at this time, it will >>> >>>>>>>>>>> go to free >>> >>>>> job. >>> >>>>>>>>>>> But the amdgpu_device_gpu_recover sometimes is faster. At >>> >>>>>>>>>>> that time, job is not freed, but s_fence is already NULL. >>> >>>>>>>>> No, that case can't happen. See here: >>> >>>>>>>>> >>> >>>>>>>>>> drm_sched_job_cleanup(s_job); >>> >>>>>>>>>> >>> >>>>>>>>>> amdgpu_ring_priority_put(ring, s_job->s_priority); >>> >>>>>>>>>> dma_fence_put(job->fence); >>> >>>>>>>>>> amdgpu_sync_free(&job->sync); >>> >>>>>>>>>> amdgpu_sync_free(&job->sched_sync); >>> >>>>>>>>>> kfree(job); >>> >>>>>>>>> The job itself is freed up directly after freeing the reference >>> >>>>>>>>> to the >>> >>>>> s_fence. >>> >>>>>>>>> So you are just papering over a much bigger problem here. This >>> >>>>>>>>> patch is a clear NAK. >>> >>>>>>>>> >>> >>>>>>>>> Regards, >>> >>>>>>>>> Christian. >>> >>>>>>>>> >>> >>>>>>>>>>>> When you see a job without an s_fence then that means the >>> >>>>>>>>>>>> problem is somewhere else. >>> >>>>>>>>>>>> >>> >>>>>>>>>>>> Regards, >>> >>>>>>>>>>>> Christian. >>> >>>>>>>>>>>> >>> >>>>>>>>>>>>> Signed-off-by: Emily Deng >>> >>>>>>>>>>>>> --- >>> >>>>>>>>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +- >>> >>>>>>>>>>>>> drivers/gpu/drm/scheduler/sched_main.c     | 11 ++++++--- >>> >-- >>> >>>>>>>>>>>>> 2 files changed, 7 insertions(+), 6 deletions(-) >>> >>>>>>>>>>>>> >>> >>>>>>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> >>>>>>>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> >>>>>>>>>>>>> index e6ce949..5a8f08e 100644 >>> >>>>>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> >>>>>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> >>>>>>>>>>>>> @@ -4075,7 +4075,7 @@ int >>> >>> amdgpu_device_gpu_recover(struct >>> >>>>>>>>>>>> amdgpu_device *adev, >>> >>>>>>>>>>>>> * >>> >>>>>>>>>>>>> * job->base holds a reference to parent fence >>> >>>>>>>>>>>>> */ >>> >>>>>>>>>>>>> -  if (job && job->base.s_fence->parent && >>> >>>>>>>>>>>>> +  if (job && job->base.s_fence && >>> >>>>>>>>>>>>> + job->base.s_fence->parent >>> >>>>>>> && >>> >>>>>>>>>>>>> dma_fence_is_signaled(job->base.s_fence->parent)) >>> >>>>>>>>>>>>> job_signaled = true; >>> >>>>>>>>>>>>> >>> >>>>>>>>>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c >>> >>>>>>>>>>>>> b/drivers/gpu/drm/scheduler/sched_main.c >>> >>>>>>>>>>>>> index 31809ca..56cc10e 100644 >>> >>>>>>>>>>>>> --- a/drivers/gpu/drm/scheduler/sched_main.c >>> >>>>>>>>>>>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c >>> >>>>>>>>>>>>> @@ -334,8 +334,8 @@ void >>> >drm_sched_increase_karma(struct >>> >>>>>>>>>>>> drm_sched_job >>> >>>>>>>>>>>>> *bad) >>> >>>>>>>>>>>>> >>> >>>>>>>>>>>>> spin_lock(&rq->lock); >>> >>>>>>>>>>>>> list_for_each_entry_safe(entity, >>> >>>>>>>>>>>>> tmp, >>> >>> &rq- >>> >>>>>>>> entities, >>> >>>>>>>>>>>> list) { >>> >>>>>>>>>>>>> -                          if (bad->s_fence->scheduled.context >>> >>>>>>> == >>> >>>>>>>>>>>>> -                              entity->fence_context) { >>> >>>>>>>>>>>>> +                          if (bad->s_fence && >>> >>>>>>>>>>>>> + (bad->s_fence- >>> >>>>>>>>>>>>> scheduled.context == >>> >>>>>>>>>>>>> + entity->fence_context)) { >>> >>>>>>>>>>>>> if >>> >>>>>>>>>>>>> (atomic_read(&bad- >>> >>>>>>>> karma) > >>> >>>>>>>>>>>>> bad->sched- >>> >>>> hang_limit) >>> >>>>>>>>>>>>> if >>> >>>>>>>>>>>>> (entity- >>> >>>> guilty) @@ -376,7 +376,7 @@ void >>> >>>>>>>>>>>>> drm_sched_stop(struct >>> >>>>>>> drm_gpu_scheduler >>> >>>>>>>>>>>> *sched, struct drm_sched_job *bad) >>> >>>>>>>>>>>>> * This iteration is thread safe as sched thread >>> >>>>>>>>>>>>> is >>> >>> stopped. >>> >>>>>>>>>>>>> */ >>> >>>>>>>>>>>>> list_for_each_entry_safe_reverse(s_job, tmp, >>> >>>>>>>>>>>>> &sched- ring_mirror_list, node) { >>> >>>>>>>>>>>>> -          if (s_job->s_fence->parent && >>> >>>>>>>>>>>>> +          if (s_job->s_fence && s_job->s_fence->parent && >>> >>>>>>>>>>>>> dma_fence_remove_callback(s_job- >>> >>>> s_fence- >>> >>>>>>>> parent, >>> >>>>>>>>>>>>> &s_job->cb)) { >>> >>>>>>>>>>>>> atomic_dec(&sched->hw_rq_count); >>> >>> @@ - >>> >>>>>>> 395,7 >>> >>>>>>>>>>> +395,8 @@ void >>> >>>>>>>>>>>>> drm_sched_stop(struct drm_gpu_scheduler >>> >>>>>>>>>>>> *sched, struct drm_sched_job *bad) >>> >>>>>>>>>>>>> * >>> >>>>>>>>>>>>> * Job is still alive so fence >>> >>>>>>>>>>>>> refcount at >>> >>> least 1 >>> >>>>>>>>>>>>> */ >>> >>>>>>>>>>>>> - dma_fence_wait(&s_job->s_fence->finished, >>> >>>>>>> false); >>> >>>>>>>>>>>>> +                  if (s_job->s_fence) >>> >>>>>>>>>>>>> + dma_fence_wait(&s_job->s_fence- >>> >>>>>>>> finished, >>> >>>>>>>>>>>> false); >>> >>>>>>>>>>>>> /* >>> >>>>>>>>>>>>> * We must keep bad job alive >>> >>>>>>>>>>>>> for later >>> >>> use >>> >>>>>>> during @@ >>> >>>>>>>>>>>> -438,7 >>> >>>>>>>>>>>>> +439,7 @@ void drm_sched_start(struct drm_gpu_scheduler >>> >>>>> *sched, >>> >>>>>>>>>>>>> +bool >>> >>>>>>>>>>>> full_recovery) >>> >>>>>>>>>>>>> * GPU recovers can't run in parallel. >>> >>>>>>>>>>>>> */ >>> >>>>>>>>>>>>> list_for_each_entry_safe(s_job, tmp, >>> >>>>>>>>>>>>> &sched->ring_mirror_list, >>> >>>>>>>>>>>>> node) >>> >>>>>>>>>>>> { >>> >>>>>>>>>>>>> -          struct dma_fence *fence = s_job->s_fence->parent; >>> >>>>>>>>>>>>> +          struct dma_fence *fence = s_job->s_fence ? >>> >>>>>>>>>>>>> + s_job- >>> >>>>>>>> s_fence- >>> >>>>>>>>>>>>> parent : >>> >>>>>>>>>>>>> +NULL; >>> >>>>>>>>>>>>> >>> >>>>>>>>>>>>> atomic_inc(&sched->hw_rq_count); >>> >>>>>>>>>>>>> >>> >>>>>>>>>>> _______________________________________________ >>> >>>>>>>>>>> amd-gfx mailing list >>> >>>>>>>>>>> amd-gfx@lists.freedesktop.org >>> >>>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>> >>> > >>> >_______________________________________________ >>> >amd-gfx mailing list >>> >amd-gfx@lists.freedesktop.org >>> >https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx --------------76F2BAA5ED3003DCF53B1335 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
The question is where do we rearm the timer for this problem to occur?

Regards,
Christian.

Am 12.11.19 um 20:21 schrieb Andrey Grodzovsky:

I was able to reproduce the crash by using the attached simulate_crash.patch - waiting on guilty job to signal in reset work and artificially rearming the timeout timer just before the check for !cancel_delayed_work(&sched->work_tdr)  in drm_sched_cleanup_jobs - crash log attached in crash.log. This I think confirms my theory i described earlier in this thread.

basic_fix.patch handles this by testing whether another timer already armed ob this scheduler or is there a timeout work in execution right now (see documentation for work_busy) - obviously  this is not a full solution as this will not protect from races if for example there is immediate work scheduling such as in drm_sched_fault -  so we probably need to account for this by making drm_sched_cleanup_jobs (at least in the part where it iterates ring mirror list and frees jobs) and GPU reset really mutually exclusive and not like now.

Andrey


On 11/11/19 4:11 PM, Christian König wrote:
Hi Emily,

you need to print which scheduler instance is freeing the jobs and which one is triggering the reset. The TID and PID is completely meaningless here since we are called from different worker threads and the TID/PID can change on each call.

Apart from that I will look into this a bit deeper when I have time.

Regards,
Christian.

Am 12.11.19 um 07:02 schrieb Deng, Emily:
Hi Christian,
    I add the follow print in function drm_sched_cleanup_jobs. From the log it shows that only use cancel_delayed_work could not avoid to free job when the sched is in reset. But don’t know exactly where it is wrong about the driver. Do you have any suggestion about this?
 
+       printk("Emily:drm_sched_cleanup_jobs:begin,tid:%lu, pid:%lu\n", current->tgid, current->pid);
 
        /*
         * Don't destroy jobs while the timeout worker is running  OR thread
         * is being parked and hence assumed to not touch ring_mirror_list
         */
         if ((sched->timeout != MAX_SCHEDULE_TIMEOUT &&
            !cancel_delayed_work(&sched->work_tdr)))
                return;
+       printk("Emily:drm_sched_cleanup_jobs,tid:%lu, pid:%lu\n", current->tgid, current->pid);
 
 
Best wishes
Emily Deng
 
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695091] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695104] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695105] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695107] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11380.695107] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.222954] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout, signaled seq=78585, emitted seq=78587
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.224275] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process  pid 0 thread  pid 0, s_job:00000000fe75ab36,tid=15603, pid=15603
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225413] amdgpu 0000:00:08.0: GPU reset begin!
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225417] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225425] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225425] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225428] Emily:amdgpu_job_free_cb,Process information: process  pid 0 thread  pid 0, s_job:00000000fe75ab36, tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225429] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225430] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225473] Emily:drm_sched_cleanup_jobs:begin,tid:2253, pid:2253
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225486] Emily:drm_sched_cleanup_jobs:begin,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225489] Emily:drm_sched_cleanup_jobs,tid:2262, pid:2262
Nov 12 12:58:20 ubuntu-drop-August-2018-rc2-gpu0-vf02 kernel: [11381.225494] Emily:amdgpu_job_free_cb,Process information: process  pid 0 thread  pid 0, s_job:00000000f086ec84, tid:2262, pid:2262
>-----Original Message-----
>From: Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com>
>Sent: Tuesday, November 12, 2019 11:28 AM
>To: Koenig, Christian <Christian.Koenig@amd.com>; Deng, Emily
>Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr
>
>Thinking more about this claim - we assume here that if cancel_delayed_work
>returned true it guarantees that timeout work is not running but, it merely
>means there was a pending timeout work which was removed from the
>workqueue before it's timer elapsed and so it didn't have a chance to be
>dequeued and executed, it doesn't cover already executing work. So there is a
>possibility where while timeout work started executing another timeout work
>already got enqueued (maybe through earlier cleanup jobs or through
>drm_sched_fault) and if at this point another drm_sched_cleanup_jobs runs
>cancel_delayed_work(&sched->work_tdr) will return true even while there is a
>timeout job in progress.
>Unfortunately we cannot change cancel_delayed_work to
>cancel_delayed_work_sync to flush the timeout work as timeout work itself
>waits for schedule thread  to be parked again when calling park_thread.
>
>Andrey
>
>________________________________________
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of
>Koenig, Christian <Christian.Koenig@amd.com>
>Sent: 08 November 2019 05:35:18
>Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr
>
>Hi Emily,
>
>exactly that can't happen. See here:
>
>>         /* Don't destroy jobs while the timeout worker is running */
>>         if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>>             !cancel_delayed_work(&sched->work_tdr))
>>                 return NULL;
>
>We never free jobs while the timeout working is running to prevent exactly
>that issue.
>
>Regards,
>Christian.
>
>Am 08.11.19 um 11:32 schrieb Deng, Emily:
>> Hi Christian,
>>       The drm_sched_job_timedout-> amdgpu_job_timedout call
>amdgpu_device_gpu_recover. I mean the main scheduler free the jobs while
>in amdgpu_device_gpu_recover, and before calling drm_sched_stop.
>>
>> Best wishes
>> Emily Deng
>>
>>
>>
>>> -----Original Message-----
>>> From: Koenig, Christian <Christian.Koenig@amd.com>
>>> Sent: Friday, November 8, 2019 6:26 PM
>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr
>>>
>>> Hi Emily,
>>>
>>> well who is calling amdgpu_device_gpu_recover() in this case?
>>>
>>> When it's not the scheduler we shouldn't have a guilty job in the first place.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 08.11.19 um 11:22 schrieb Deng, Emily:
>>>> Hi Chrisitan,
>>>>        No, I am with the new branch and also has the patch. Even it
>>>> are freed by
>>> main scheduler, how we could avoid main scheduler to free jobs while
>>> enter to function amdgpu_device_gpu_recover?
>>>> Best wishes
>>>> Emily Deng
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Koenig, Christian <Christian.Koenig@amd.com>
>>>>> Sent: Friday, November 8, 2019 6:15 PM
>>>>> To: Deng, Emily <Emily.Deng@amd.com>; amd-
>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for tdr
>>>>>
>>>>> Hi Emily,
>>>>>
>>>>> in this case you are on an old code branch.
>>>>>
>>>>> Jobs are freed now by the main scheduler thread and only if no
>>>>> timeout handler is running.
>>>>>
>>>>> See this patch here:
>>>>>> commit 5918045c4ed492fb5813f980dcf89a90fefd0a4e
>>>>>> Author: Christian König <christian.koenig@amd.com>
>>>>>> Date:   Thu Apr 18 11:00:21 2019 -0400
>>>>>>
>>>>>>       drm/scheduler: rework job destruction
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>> Am 08.11.19 um 11:11 schrieb Deng, Emily:
>>>>>> Hi Christian,
>>>>>>         Please refer to follow log, when it enter to
>>>>>> amdgpu_device_gpu_recover
>>>>> function, the bad job 000000005086879e is freeing in function
>>>>> amdgpu_job_free_cb  at the same time, because of the hardware fence
>>> signal.
>>>>> But amdgpu_device_gpu_recover goes faster, at this case, the
>>>>> s_fence is already freed, but job is not freed in time. Then this issue
>occurs.
>>>>>> [  449.792189] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring
>>> sdma0
>>>>>> timeout, signaled seq=2481, emitted seq=2483 [  449.793202]
>>>>>> [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information:
>>>>> process  pid 0 thread  pid 0, s_job:000000005086879e [  449.794163]
>>>>> amdgpu
>>>>> 0000:00:08.0: GPU reset begin!
>>>>>> [  449.794175] Emily:amdgpu_job_free_cb,Process information:
>>>>>> process pid 0 thread  pid 0, s_job:000000005086879e [  449.794221]
>>>>>> Emily:amdgpu_job_free_cb,Process information: process  pid 0
>>>>>> thread pid 0, s_job:0000000066eb74ab [  449.794222]
>>>>>> Emily:amdgpu_job_free_cb,Process information: process  pid 0
>>>>>> thread pid 0, s_job:00000000d4438ad9 [  449.794255]
>>>>>> Emily:amdgpu_job_free_cb,Process information: process  pid 0
>>>>>> thread pid 0, s_job:00000000b6d69c65 [  449.794257]
>>>>>> Emily:amdgpu_job_free_cb,Process information: process  pid 0
>>>>>> thread pid 0,
>>>>> s_job:00000000ea85e922 [  449.794287]
>>>>> Emily:amdgpu_job_free_cb,Process
>>>>> information: process  pid 0 thread  pid 0, s_job:00000000ed3a5ac6 [
>>>>> 449.794366] BUG: unable to handle kernel NULL pointer dereference
>>>>> at
>>>>> 00000000000000c0 [  449.800818] PGD 0 P4D 0 [  449.801040] Oops:
>>>>> 0000 [#1] SMP PTI
>>>>>> [  449.801338] CPU: 3 PID: 55 Comm: kworker/3:1 Tainted: G           OE
>>>>> 4.18.0-15-generic #16~18.04.1-Ubuntu
>>>>>> [  449.802157] Hardware name: QEMU Standard PC (i440FX + PIIX,
>>>>>> 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 [  449.802944]
>>>>>> Workqueue: events drm_sched_job_timedout [amd_sched] [
>>>>>> 449.803488]
>>> RIP:
>>>>> 0010:amdgpu_device_gpu_recover+0x1da/0xb60 [amdgpu]
>>>>>> [  449.804020] Code: dd ff ff 49 39 c5 48 89 55 a8 0f 85 56 ff ff
>>>>>> ff
>>>>>> 45 85 e4 0f
>>>>> 85 a1 00 00 00 48 8b 45 b0 48 85 c0 0f 84 60 01 00 00 48 8b 40 10
>>>>> <48> 8b
>>> 98
>>>>> c0 00         00 00 48 85 db 0f 84 4c 01 00 00 48 8b 43 48 a8 01
>>>>>> [  449.805593] RSP: 0018:ffffb4c7c08f7d68 EFLAGS: 00010286 [
>>>>>> 449.806032] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
>>>>>> 0000000000000000 [  449.806625] RDX: ffffb4c7c08f5ac0 RSI:
>>>>>> 0000000fffffffe0 RDI: 0000000000000246 [  449.807224] RBP:
>>>>>> ffffb4c7c08f7de0 R08: 00000068b9d54000 R09: 0000000000000000 [
>>>>>> 449.807818] R10: 0000000000000000 R11: 0000000000000148 R12:
>>>>>> 0000000000000000 [  449.808411] R13: ffffb4c7c08f7da0 R14:
>>>>>> ffff8d82b8525d40 R15: ffff8d82b8525d40 [  449.809004] FS:
>>>>>> 0000000000000000(0000) GS:ffff8d82bfd80000(0000)
>>>>>> knlGS:0000000000000000 [  449.809674] CS:  0010 DS: 0000 ES: 0000
>CR0:
>>>>>> 0000000080050033 [  449.810153] CR2: 00000000000000c0 CR3:
>>>>>> 000000003cc0a001 CR4: 00000000003606e0 [  449.810747] DR0:
>>>>> 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [
>>>>> 449.811344] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
>>>>> 0000000000000400 [  449.811937] Call Trace:
>>>>>> [  449.812206]  amdgpu_job_timedout+0x114/0x140 [amdgpu] [
>>>>>> 449.812635]  drm_sched_job_timedout+0x44/0x90 [amd_sched] [
>>>>>> 449.813139]  ? amdgpu_cgs_destroy_device+0x10/0x10 [amdgpu] [
>>>>>> 449.813609]  ? drm_sched_job_timedout+0x44/0x90 [amd_sched] [
>>>>>> 449.814077]  process_one_work+0x1fd/0x3f0 [  449.814417]
>>>>>> worker_thread+0x34/0x410 [  449.814728]  kthread+0x121/0x140 [
>>>>>> 449.815004]  ? process_one_work+0x3f0/0x3f0 [  449.815374]  ?
>>>>>> kthread_create_worker_on_cpu+0x70/0x70
>>>>>> [  449.815799]  ret_from_fork+0x35/0x40
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Koenig, Christian <Christian.Koenig@amd.com>
>>>>>>> Sent: Friday, November 8, 2019 5:43 PM
>>>>>>> To: Deng, Emily <Emily.Deng@amd.com>; amd-
>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for
>>>>>>> tdr
>>>>>>>
>>>>>>> Am 08.11.19 um 10:39 schrieb Deng, Emily:
>>>>>>>> Sorry, please take your time.
>>>>>>> Have you seen my other response a bit below?
>>>>>>>
>>>>>>> I can't follow how it would be possible for job->s_fence to be
>>>>>>> NULL without the job also being freed.
>>>>>>>
>>>>>>> So it looks like this patch is just papering over some bigger issues.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Christian.
>>>>>>>
>>>>>>>> Best wishes
>>>>>>>> Emily Deng
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Koenig, Christian <Christian.Koenig@amd.com>
>>>>>>>>> Sent: Friday, November 8, 2019 5:08 PM
>>>>>>>>> To: Deng, Emily <Emily.Deng@amd.com>; amd-
>>>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue for
>>>>>>>>> tdr
>>>>>>>>>
>>>>>>>>> Am 08.11.19 um 09:52 schrieb Deng, Emily:
>>>>>>>>>> Ping.....
>>>>>>>>> You need to give me at least enough time to wake up :)
>>>>>>>>>
>>>>>>>>>> Best wishes
>>>>>>>>>> Emily Deng
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On
>>> Behalf
>>>>>>>>>>> Of Deng, Emily
>>>>>>>>>>> Sent: Friday, November 8, 2019 10:56 AM
>>>>>>>>>>> To: Koenig, Christian <Christian.Koenig@amd.com>; amd-
>>>>>>>>>>> Subject: RE: [PATCH] drm/amdgpu: Fix the null pointer issue
>>>>>>>>>>> for tdr
>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: Christian König <ckoenig.leichtzumerken@gmail.com>
>>>>>>>>>>>> Sent: Thursday, November 7, 2019 7:28 PM
>>>>>>>>>>>> To: Deng, Emily <Emily.Deng@amd.com>;
>>>>>>>>>>>> Subject: Re: [PATCH] drm/amdgpu: Fix the null pointer issue
>>>>>>>>>>>> for tdr
>>>>>>>>>>>>
>>>>>>>>>>>> Am 07.11.19 um 11:25 schrieb Emily Deng:
>>>>>>>>>>>>> When the job is already signaled, the s_fence is freed.
>>>>>>>>>>>>> Then it will has null pointer in amdgpu_device_gpu_recover.
>>>>>>>>>>>> NAK, the s_fence is only set to NULL when the job is destroyed.
>>>>>>>>>>>> See drm_sched_job_cleanup().
>>>>>>>>>>> I know it is set to NULL in drm_sched_job_cleanup. But in one
>>>>>>>>>>> case, when it enter into the amdgpu_device_gpu_recover, it
>>>>>>>>>>> already in drm_sched_job_cleanup, and at this time, it will
>>>>>>>>>>> go to free
>>>>> job.
>>>>>>>>>>> But the amdgpu_device_gpu_recover sometimes is faster. At
>>>>>>>>>>> that time, job is not freed, but s_fence is already NULL.
>>>>>>>>> No, that case can't happen. See here:
>>>>>>>>>
>>>>>>>>>>             drm_sched_job_cleanup(s_job);
>>>>>>>>>>
>>>>>>>>>>             amdgpu_ring_priority_put(ring, s_job->s_priority);
>>>>>>>>>>             dma_fence_put(job->fence);
>>>>>>>>>>             amdgpu_sync_free(&job->sync);
>>>>>>>>>>             amdgpu_sync_free(&job->sched_sync);
>>>>>>>>>>             kfree(job);
>>>>>>>>> The job itself is freed up directly after freeing the reference
>>>>>>>>> to the
>>>>> s_fence.
>>>>>>>>> So you are just papering over a much bigger problem here. This
>>>>>>>>> patch is a clear NAK.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Christian.
>>>>>>>>>
>>>>>>>>>>>> When you see a job without an s_fence then that means the
>>>>>>>>>>>> problem is somewhere else.
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Christian.
>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: Emily Deng <Emily.Deng@amd.com>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>>        drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
>>>>>>>>>>>>>        drivers/gpu/drm/scheduler/sched_main.c     | 11 ++++++---
>--
>>>>>>>>>>>>>        2 files changed, 7 insertions(+), 6 deletions(-)
>>>>>>>>>>>>>
>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>>>>>>>> index e6ce949..5a8f08e 100644
>>>>>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>>>>>>>>>> @@ -4075,7 +4075,7 @@ int
>>> amdgpu_device_gpu_recover(struct
>>>>>>>>>>>> amdgpu_device *adev,
>>>>>>>>>>>>>             *
>>>>>>>>>>>>>             * job->base holds a reference to parent fence
>>>>>>>>>>>>>             */
>>>>>>>>>>>>> -  if (job && job->base.s_fence->parent &&
>>>>>>>>>>>>> +  if (job && job->base.s_fence &&
>>>>>>>>>>>>> + job->base.s_fence->parent
>>>>>>> &&
>>>>>>>>>>>>>                dma_fence_is_signaled(job->base.s_fence->parent))
>>>>>>>>>>>>>                    job_signaled = true;
>>>>>>>>>>>>>
>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
>>>>>>>>>>>>> b/drivers/gpu/drm/scheduler/sched_main.c
>>>>>>>>>>>>> index 31809ca..56cc10e 100644
>>>>>>>>>>>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>>>>>>>>>>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>>>>>>>>>>>> @@ -334,8 +334,8 @@ void
>drm_sched_increase_karma(struct
>>>>>>>>>>>> drm_sched_job
>>>>>>>>>>>>> *bad)
>>>>>>>>>>>>>
>>>>>>>>>>>>>                            spin_lock(&rq->lock);
>>>>>>>>>>>>>                            list_for_each_entry_safe(entity,
>>>>>>>>>>>>> tmp,
>>> &rq-
>>>>>>>> entities,
>>>>>>>>>>>> list) {
>>>>>>>>>>>>> -                          if (bad->s_fence->scheduled.context
>>>>>>> ==
>>>>>>>>>>>>> -                              entity->fence_context) {
>>>>>>>>>>>>> +                          if (bad->s_fence &&
>>>>>>>>>>>>> + (bad->s_fence-
>>>>>>>>>>>>> scheduled.context ==
>>>>>>>>>>>>> +                              entity->fence_context)) {
>>>>>>>>>>>>>                                            if
>>>>>>>>>>>>> (atomic_read(&bad-
>>>>>>>> karma) >
>>>>>>>>>>>>>                                                bad->sched-
>>>> hang_limit)
>>>>>>>>>>>>>                                                    if
>>>>>>>>>>>>> (entity-
>>>> guilty) @@ -376,7 +376,7 @@ void
>>>>>>>>>>>>> drm_sched_stop(struct
>>>>>>> drm_gpu_scheduler
>>>>>>>>>>>> *sched, struct drm_sched_job *bad)
>>>>>>>>>>>>>             * This iteration is thread safe as sched thread
>>>>>>>>>>>>> is
>>> stopped.
>>>>>>>>>>>>>             */
>>>>>>>>>>>>>            list_for_each_entry_safe_reverse(s_job, tmp,
>>>>>>>>>>>>> &sched- ring_mirror_list, node) {
>>>>>>>>>>>>> -          if (s_job->s_fence->parent &&
>>>>>>>>>>>>> +          if (s_job->s_fence && s_job->s_fence->parent &&
>>>>>>>>>>>>>                        dma_fence_remove_callback(s_job-
>>>> s_fence-
>>>>>>>> parent,
>>>>>>>>>>>>>                                                  &s_job->cb)) {
>>>>>>>>>>>>>                            atomic_dec(&sched->hw_rq_count);
>>> @@ -
>>>>>>> 395,7
>>>>>>>>>>> +395,8 @@ void
>>>>>>>>>>>>> drm_sched_stop(struct drm_gpu_scheduler
>>>>>>>>>>>> *sched, struct drm_sched_job *bad)
>>>>>>>>>>>>>                             *
>>>>>>>>>>>>>                             * Job is still alive so fence
>>>>>>>>>>>>> refcount at
>>> least 1
>>>>>>>>>>>>>                             */
>>>>>>>>>>>>> -                  dma_fence_wait(&s_job->s_fence->finished,
>>>>>>> false);
>>>>>>>>>>>>> +                  if (s_job->s_fence)
>>>>>>>>>>>>> +                          dma_fence_wait(&s_job->s_fence-
>>>>>>>> finished,
>>>>>>>>>>>> false);
>>>>>>>>>>>>>                            /*
>>>>>>>>>>>>>                             * We must keep bad job alive
>>>>>>>>>>>>> for later
>>> use
>>>>>>> during @@
>>>>>>>>>>>> -438,7
>>>>>>>>>>>>> +439,7 @@ void drm_sched_start(struct drm_gpu_scheduler
>>>>> *sched,
>>>>>>>>>>>>> +bool
>>>>>>>>>>>> full_recovery)
>>>>>>>>>>>>>             * GPU recovers can't run in parallel.
>>>>>>>>>>>>>             */
>>>>>>>>>>>>>            list_for_each_entry_safe(s_job, tmp,
>>>>>>>>>>>>> &sched->ring_mirror_list,
>>>>>>>>>>>>> node)
>>>>>>>>>>>> {
>>>>>>>>>>>>> -          struct dma_fence *fence = s_job->s_fence->parent;
>>>>>>>>>>>>> +          struct dma_fence *fence = s_job->s_fence ?
>>>>>>>>>>>>> + s_job-
>>>>>>>> s_fence-
>>>>>>>>>>>>> parent :
>>>>>>>>>>>>> +NULL;
>>>>>>>>>>>>>
>>>>>>>>>>>>>                    atomic_inc(&sched->hw_rq_count);
>>>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> amd-gfx mailing list
>
>_______________________________________________
>amd-gfx mailing list
 
 


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

--------------76F2BAA5ED3003DCF53B1335-- --===============0761989273== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdmeCBt YWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4 --===============0761989273==--