[PATCH v2] drm/scheduler: Fix sched hang when killing app with dependent jobs
Christian König
christian.koenig at amd.com
Tue Jul 15 10:52:46 UTC 2025
On 15.07.25 12:27, Philipp Stanner wrote:
> On Tue, 2025-07-15 at 09:51 +0000, cao, lin wrote:
>>
>> [AMD Official Use Only - AMD Internal Distribution Only]
>>
>>
>>
>> Hi Philipp,
>>
>>
>> Thanks for your review, let me try to clarify why I added drm_sched_wakeup() to drm_sched_entity_kill_jobs_work():
>>
>>
>> When application A submits jobs (a1, a2, a3) and application B submits job b1 with a dependency on a1's scheduled fence, the normal execution flow is (function drm_sched_run_job_work()):
>> 1. a1 gets popped from the entity by the scheduler
>> 2. run_job(a1) executes
>> 3. a1's scheduled fence gets signaled
>> 4. drm_sched_run_job_work() calls drm_sched_run_job_queue() at the end
>> 5. The scheduler wakes up and re-selects entities to pop jobs
>> 6. Since b1's dependency is cleared, the scheduler can select b1 and continue the same flow
>>
>>
>> However, if application A is killed before a1 gets popped by the scheduler, then a1, a2, a3 are killed sequentially by drm_sched_entity_kill_jobs_cb(). During the kill process, their scheduled fences are still signaled, but the kill process itself lacks work_run_job. This means b1's dependency gets cleared, but there's no work_run_job to drive the scheduler to continue running, or we can saystep 4 in the normal flow is missing, and the scheduler enters sleep state, which causes application B to hang.
>> So if we add drm_sched_wakeup() in drm_sched_entity_kill_jobs_work() after drm_sched_fence_scheduled(), the scheduler can be woken up, and application B can continue running after a1's scheduled fence is force signaled.
>
> Thanks for the detailed explanation. Makes sense.
> Maybe you can detail in v3 that this "optimization" consists of the
> work item not being scheduled.
Yeah, removing this "optimization" would also be a valid solution to the problem.
Christian.
> I think that was the piece of the puzzle
> I was missing.
>
> I / DRM tools will also include a link to this thread, so I think that
> will then be sufficient.
>
> Thx
> P.
>
>>
>> Thanks,
>> Lin
>>
>>
>>
>>
>>
>> From: Philipp Stanner <phasta at mailbox.org>
>> Sent: Tuesday, July 15, 2025 17:12
>> To: cao, lin <lin.cao at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; phasta at kernel.org <phasta at kernel.org>; dri-devel at lists.freedesktop.org <dri-devel at lists.freedesktop.org>
>> Cc: Yin, ZhenGuo (Chris) <ZhenGuo.Yin at amd.com>; Deng, Emily <Emily.Deng at amd.com>; dakr at kernel.org <dakr at kernel.org>; matthew.brost at intel.com <matthew.brost at intel.com>
>> Subject: Re: [PATCH v2] drm/scheduler: Fix sched hang when killing app with dependent jobs
>>
>>
>> On Tue, 2025-07-15 at 03:38 +0000, cao, lin wrote:
>>>
>>> [AMD Official Use Only - AMD Internal Distribution Only]
>>>
>>>
>>>
>>> Hi Christian, Philipp,
>>>
>>>
>>> I modified the commit msg, and I hope it makes more sense:
>>>
>>>
>>> drm/sched: wake up scheduler when killing jobs to prevent hang
>>
>> nit:
>> s/wake/Wake
>>
>>>
>>>
>>> When application A submits jobs (a1, a2, a3) and application B submits
>>> job b1 with a dependency on a2's scheduler fence, killing application A
>>> before run_job(a1) causes drm_sched_entity_kill_jobs_work() to force
>>> signal all jobs sequentially. However, the optimization in
>>> drm_sched_entity_add_dependency_cb() waits for the fence to be scheduled
>>> by adding a callback (drm_sched_entity_clear_dep) instead of immediately
>>> waking up the scheduler. When A is killed before its jobs can run, the
>>> callback gets triggered but drm_sched_entity_clear_dep() only clears the
>>> dependency without waking up the scheduler, causing the scheduler to enter
>>> sleep state and application B to hang.
>>
>> That now reads as if drm_sched_entity_clear_dep() is supposed to wake
>> up the scheduler. But you add the wakeup to a different function (the
>> work item).
>>
>> Also the code actually looks like that:
>>
>>
>> xa_erase(&job->dependencies, index);
>> if (f && !dma_fence_add_callback(f, &job->finish_cb,
>> drm_sched_entity_kill_jobs_cb))
>> return;
>>
>> dma_fence_put(f);
>> }
>>
>> INIT_WORK(&job->work, drm_sched_entity_kill_jobs_work);
>> schedule_work(&job->work);
>> }
>>
>> So if adding that callback succeeds we.. return immediately but we.. do
>> that for the first dependency, not for all of them?
>>
>> Oh boy. That code base. We(tm) should look into pimping that up. Also
>> lacks documentation everywhere.
>>
>>
>> Anyways. If this solves a bug for you the patch looks fine (i.e., not
>> potentially harmful) by me and if the tests succeed we can merge it.
>> However, I'd feel better if you could clarify more why that function is
>> the right place to solve the bug.
>>
>>
>> Thanks,
>> P.
>>
>>
>>>
>>>
>>> Add drm_sched_wakeup() in entity_kill_job_work() to prevent scheduler
>>> sleep and subsequent application hangs.
>>>
>>>
>>> v2:
>>> - Move drm_sched_wakeup() to after drm_sched_fence_scheduled()
>>>
>>>
>>> Thanks,
>>> Lin
>>>
>>>
>>> From: Koenig, Christian <Christian.Koenig at amd.com>
>>> Sent: Monday, July 14, 2025 21:39
>>> To: phasta at kernel.org <phasta at kernel.org>; cao, lin <lin.cao at amd.com>; dri-devel at lists.freedesktop.org <dri-devel at lists.freedesktop.org>
>>> Cc: Yin, ZhenGuo (Chris) <ZhenGuo.Yin at amd.com>; Deng, Emily <Emily.Deng at amd.com>; dakr at kernel.org <dakr at kernel.org>; matthew.brost at intel.com <matthew.brost at intel.com>
>>> Subject: Re: [PATCH v2] drm/scheduler: Fix sched hang when killing app with dependent jobs
>>>
>>>
>>>
>>>
>>> On 14.07.25 15:27, Philipp Stanner wrote:
>>>> On Mon, 2025-07-14 at 15:08 +0200, Christian König wrote:
>>>>>
>>>>>
>>>>> On 14.07.25 14:46, Philipp Stanner wrote:
>>>>>> regarding the patch subject: the prefix we use for the scheduler
>>>>>> is:
>>>>>> drm/sched:
>>>>>>
>>>>>>
>>>>>> On Mon, 2025-07-14 at 14:23 +0800, Lin.Cao wrote:
>>>>>>
>>>>>>> When Application A submits jobs (a1, a2, a3) and application B
>>>>>>> submits
>>>>>>
>>>>>> s/Application/application
>>>>>>
>>>>>>> job b1 with a dependency on a2's scheduler fence, killing
>>>>>>> application A
>>>>>>> before run_job(a1) causes drm_sched_entity_kill_jobs_work() to
>>>>>>> force
>>>>>>> signal all jobs sequentially. However, due to missing
>>>>>>> work_run_job or
>>>>>>> work_free_job
>>>>>>>
>>>>>>
>>>>>> You probably mean "due to the work items work_run_job and
>>>>>> work_free_job
>>>>>> not being started […]".
>>>>>>
>>>>>> However, the call you add, drm_sched_wakeup(), immediately only
>>>>>> triggers work_run_job. It's not clear to me why you mention
>>>>>> work_free_job, because drm_sched_entity_kill_jobs_work() directly
>>>>>> invokes the free_job() callback.
>>>>>
>>>>> Yeah the description is rather confusing, took me more than one try
>>>>> to understand what's going on here as well. Let me try to explain it
>>>>> in my words:
>>>>>
>>>>> When an application A is killed the pending submissions from it are
>>>>> not executed, but rather torn down by
>>>>> drm_sched_entity_kill_jobs_work().
>>>>>
>>>>> If now a submission from application B depends on something
>>>>> application A wanted to do on the same scheduler instance the
>>>>> function drm_sched_entity_add_dependency_cb() would have optimized
>>>>> this dependency and assumed that the scheduler work would already run
>>>>> and try to pick a job.
>>>>>
>>>>> But that isn't the case when the submissions from application A are
>>>>> all killed. So make sure to start the scheduler work from
>>>>> drm_sched_entity_kill_jobs_work() to handle that case.
>>>>
>>>> Alright, so the bug then depends on B setting a dependency on A _after_
>>>> A was killed, doesn't it? Because the optimization in
>>>> _add_dependency_cb() can only have an effect after A's jobs have been
>>>> torn down.
>>>
>>> No, application A and application B submit right behind each other. Application A is then killed later on.
>>>
>>> The optimization in _add_dependency_cb() just waits for As submission to be handled by the scheduler, but that never happens because it was killed.
>>>
>>>> If there is such a timing order problem, the commit message should
>>>> mention it.
>>>>
>>>> The commit message definitely also needs to mention this optimization
>>>> in drm_sched_entity_add_dependency_cb() since it's essential for
>>>> understanding the bug.
>>>
>>> +1
>>>
>>> Christian.
>>>
>>>>
>>>>
>>>> Danke
>>>> P.
>>>>
>>>>
>>>>>
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>>>
>>>>>>> in entity_kill_job_work(), the scheduler enters sleep
>>>>>>> state, causing application B hang.
>>>>>>
>>>>>> So the issue is that if a1 never ran, the scheduler will not
>>>>>> continue
>>>>>> with the jobs of application B, correct?
>>>>>>
>>>>>>>
>>>>>>> Add drm_sched_wakeup() when entity_kill_job_work() to preventing
>>>>>>
>>>>>> s/to preventing/to prevent
>>>>>>
>>>>>>> scheduler sleep and subsequent application hangs.
>>>>>>>
>>>>>>> v2:
>>>>>>> - Move drm_sched_wakeup() to after drm_sched_fence_scheduled()
>>>>>>
>>>>>> Changelogs are cool and useful, but move them below the Signed-off
>>>>>> area
>>>>>> with another --- please, like so:
>>>>>>
>>>>>> Signed-off by: …
>>>>>> ---
>>>>>> v2:
>>>>>> …
>>>>>> ---
>>>>>> drivers/gpu/drm/scheduler/sched_entity.c | 1 +
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Lin.Cao <lincao12 at amd.com>
>>>>>>
>>>>>> This fixes a bug. Even a racy bug, it seems. So we need a Fixes tag
>>>>>> and
>>>>>> put the stable kernel in CC.
>>>>>>
>>>>>> Please figure out with git blame, git log and git tag --contains
>>>>>> which
>>>>>> commit your patch fixes and which is the oldest kernel that
>>>>>> contains
>>>>>> the bug. Then add a Fixes: tag and Cc: the stable kernel. You'll
>>>>>> find
>>>>>> lots of examples for that in git log.
>>>>>>
>>>>>>
>>>>>> Thx
>>>>>> P.
>>>>>>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/scheduler/sched_entity.c | 1 +
>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>>>>>>> b/drivers/gpu/drm/scheduler/sched_entity.c
>>>>>>> index e671aa241720..66f2a43c58fd 100644
>>>>>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>>>>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>>>>>> @@ -177,6 +177,7 @@ static void
>>>>>>> drm_sched_entity_kill_jobs_work(struct work_struct *wrk)
>>>>>>> struct drm_sched_job *job = container_of(wrk,
>>>>>>> typeof(*job), work);
>>>>>>>
>>>>>>> drm_sched_fence_scheduled(job->s_fence, NULL);
>>>>>>> + drm_sched_wakeup(job->sched);
>>>>>>> drm_sched_fence_finished(job->s_fence, -ESRCH);
>>>>>>> WARN_ON(job->s_fence->parent);
>>>>>>> job->sched->ops->free_job(job);
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the dri-devel
mailing list