<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=Windows-1252"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> [AMD Official Use Only - AMD Internal Distribution Only] <div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> Hi Philipp,</div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> </div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> Thanks for your review, let me try to clarify why I added drm_sched_wakeup() to drm_sched_entity_kill_jobs_work():</div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> </div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 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()):</div> <div style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 1. a1 gets popped from the entity by the scheduler</div> <div style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 2. run_job(a1) executes</div> <div style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 3. a1's scheduled fence gets signaled </div> <div style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 4. drm_sched_run_job_work() calls drm_sched_run_job_queue() at the end</div> <div style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 5. The scheduler wakes up and re-selects entities to pop jobs</div> <div style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 6. Since b1's dependency is cleared, the scheduler can select b1 and continue the same flow</div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> </div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 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 say step 4 in the normal flow is missing, and the scheduler enters sleep state, which causes application B to hang.</div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> 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.</div> <div id="appendonsend"></div> <div> </div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> Thanks,</div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof"> Lin</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <hr style="display: inline-block; width: 98%;"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> From: Philipp Stanner <phasta@mailbox.org> Sent: Tuesday, July 15, 2025 17:12 To: cao, lin <lin.cao@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; phasta@kernel.org <phasta@kernel.org>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org> Cc: Yin, ZhenGuo (Chris) <ZhenGuo.Yin@amd.com>; Deng, Emily <Emily.Deng@amd.com>; dakr@kernel.org <dakr@kernel.org>; matthew.brost@intel.com <matthew.brost@intel.com> Subject: Re: [PATCH v2] drm/scheduler: Fix sched hang when killing app with dependent jobs </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-size: 11pt;">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@amd.com> > Sent: Monday, July 14, 2025 21:39 > To: phasta@kernel.org <phasta@kernel.org>; cao, lin <lin.cao@amd.com>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org> > Cc: Yin, ZhenGuo (Chris) <ZhenGuo.Yin@amd.com>; Deng, Emily <Emily.Deng@amd.com>; dakr@kernel.org <dakr@kernel.org>; matthew.brost@intel.com <matthew.brost@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@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); > > > > > > > > > > </div> </div> </body> </html>