dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs
@ 2025-07-10  6:36 Lin.Cao
  2025-07-10  7:52 ` Christian König
  0 siblings, 1 reply; 5+ messages in thread
From: Lin.Cao @ 2025-07-10  6:36 UTC (permalink / raw)
  To: dri-devel, amd-gfx; +Cc: zhenguo.yin, Emily.Deng, Christian König, Lin.Cao

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, due to missing work_run_job or
work_free_job in entity_kill_job_work(), the scheduler enters sleep
state, causing application B hang.

Add drm_sched_wakeup() when entity_kill_job_work() to preventing
scheduler sleep and subsequent application hangs.

Signed-off-by: Lin.Cao <lincao12@amd.com>
---
 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..a22b0f65558a 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -180,6 +180,7 @@ static void drm_sched_entity_kill_jobs_work(struct work_struct *wrk)
 	drm_sched_fence_finished(job->s_fence, -ESRCH);
 	WARN_ON(job->s_fence->parent);
 	job->sched->ops->free_job(job);
+	drm_sched_wakeup(job->sched);
 }
 
 /* Signal the scheduler finished fence when the entity in question is killed. */
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs
  2025-07-10  6:36 [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs Lin.Cao
@ 2025-07-10  7:52 ` Christian König
  2025-07-10  8:33   ` cao, lin
  0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2025-07-10  7:52 UTC (permalink / raw)
  To: Lin.Cao, dri-devel, amd-gfx
  Cc: zhenguo.yin, Emily.Deng, Matthew Brost, Danilo Krummrich,
	Philipp Stanner

First of all you need to CC the scheduler maintainers, try to use the get_maintainer.pl script. Adding them on CC.

On 10.07.25 08:36, Lin.Cao wrote:
> 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, due to missing work_run_job or
> work_free_job in entity_kill_job_work(), the scheduler enters sleep
> state, causing application B hang.

Ah! Because of optimizing the dependency when submitting to the same scheduler in drm_sched_entity_add_dependency_cb().

Yeah that suddenly starts to make sense.

> Add drm_sched_wakeup() when entity_kill_job_work() to preventing
> scheduler sleep and subsequent application hangs.
> 
> Signed-off-by: Lin.Cao <lincao12@amd.com>
> ---
>  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..a22b0f65558a 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -180,6 +180,7 @@ static void drm_sched_entity_kill_jobs_work(struct work_struct *wrk)
>  	drm_sched_fence_finished(job->s_fence, -ESRCH);
>  	WARN_ON(job->s_fence->parent);
>  	job->sched->ops->free_job(job);
> +	drm_sched_wakeup(job->sched);

That should probably be after drm_sched_fence_scheduled().

Alternatively we could also drop the optimization in drm_sched_entity_add_dependency_cb(), scheduling the work item again has only minimal overhead.

Apart from that looks good to me.

Regards,
Christian.

>  }
>  
>  /* Signal the scheduler finished fence when the entity in question is killed. */


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs
  2025-07-10  7:52 ` Christian König
@ 2025-07-10  8:33   ` cao, lin
  2025-07-11 13:13     ` Philipp Stanner
  0 siblings, 1 reply; 5+ messages in thread
From: cao, lin @ 2025-07-10  8:33 UTC (permalink / raw)
  To: Koenig, Christian, dri-devel@lists.freedesktop.org,
	amd-gfx@lists.freedesktop.org
  Cc: Yin, ZhenGuo (Chris), Deng, Emily, Matthew Brost,
	Danilo Krummrich, Philipp Stanner

[-- Attachment #1: Type: text/plain, Size: 3217 bytes --]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Christian,

Thanks for your suggestion, I modified the patch as:

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);
--

Thanks,
Lin

________________________________
From: Koenig, Christian <Christian.Koenig@amd.com>
Sent: Thursday, July 10, 2025 15:52
To: cao, lin <lin.cao@amd.com>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Yin, ZhenGuo (Chris) <ZhenGuo.Yin@amd.com>; Deng, Emily <Emily.Deng@amd.com>; Matthew Brost <matthew.brost@intel.com>; Danilo Krummrich <dakr@kernel.org>; Philipp Stanner <phasta@kernel.org>
Subject: Re: [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs

First of all you need to CC the scheduler maintainers, try to use the get_maintainer.pl script. Adding them on CC.

On 10.07.25 08:36, Lin.Cao wrote:
> 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, due to missing work_run_job or
> work_free_job in entity_kill_job_work(), the scheduler enters sleep
> state, causing application B hang.

Ah! Because of optimizing the dependency when submitting to the same scheduler in drm_sched_entity_add_dependency_cb().

Yeah that suddenly starts to make sense.

> Add drm_sched_wakeup() when entity_kill_job_work() to preventing
> scheduler sleep and subsequent application hangs.
>
> Signed-off-by: Lin.Cao <lincao12@amd.com>
> ---
>  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..a22b0f65558a 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -180,6 +180,7 @@ static void drm_sched_entity_kill_jobs_work(struct work_struct *wrk)
>        drm_sched_fence_finished(job->s_fence, -ESRCH);
>        WARN_ON(job->s_fence->parent);
>        job->sched->ops->free_job(job);
> +     drm_sched_wakeup(job->sched);

That should probably be after drm_sched_fence_scheduled().

Alternatively we could also drop the optimization in drm_sched_entity_add_dependency_cb(), scheduling the work item again has only minimal overhead.

Apart from that looks good to me.

Regards,
Christian.

>  }
>
>  /* Signal the scheduler finished fence when the entity in question is killed. */


[-- Attachment #2: Type: text/html, Size: 8463 bytes --]

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs
  2025-07-10  8:33   ` cao, lin
@ 2025-07-11 13:13     ` Philipp Stanner
  2025-07-11 13:24       ` Christian König
  0 siblings, 1 reply; 5+ messages in thread
From: Philipp Stanner @ 2025-07-11 13:13 UTC (permalink / raw)
  To: cao, lin, Koenig, Christian, dri-devel@lists.freedesktop.org,
	amd-gfx@lists.freedesktop.org
  Cc: Yin, ZhenGuo (Chris), Deng, Emily, Matthew Brost,
	Danilo Krummrich, Philipp Stanner

On Thu, 2025-07-10 at 08:33 +0000, cao, lin wrote:
> 
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> 
> 
> Hi Christian,
> 
> 
> Thanks for your suggestion, I modified the patch as:

Looks promising. You'll send a v2 I guess :)

P.

> 
> 
> 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);
> --
> 
> 
> Thanks,
> Lin
> 
> 
> 
> 
> 
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Thursday, July 10, 2025 15:52
> To: cao, lin <lin.cao@amd.com>; dri-devel@lists.freedesktop.org
> <dri-devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org
> <amd-gfx@lists.freedesktop.org>
> Cc: Yin, ZhenGuo (Chris) <ZhenGuo.Yin@amd.com>; Deng, Emily
> <Emily.Deng@amd.com>; Matthew Brost <matthew.brost@intel.com>; Danilo
> Krummrich <dakr@kernel.org>; Philipp Stanner <phasta@kernel.org>
> Subject: Re: [PATCH] drm/scheduler: Fix sched hang when killing app
> with dependent jobs
> 
> 
> First of all you need to CC the scheduler maintainers, try to use the
> get_maintainer.pl script. Adding them on CC.
> 
> On 10.07.25 08:36, Lin.Cao wrote:
> > 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, due to missing work_run_job
> > or
> > work_free_job in entity_kill_job_work(), the scheduler enters sleep
> > state, causing application B hang.
> 
> Ah! Because of optimizing the dependency when submitting to the same
> scheduler in drm_sched_entity_add_dependency_cb().
> 
> Yeah that suddenly starts to make sense.
> 
> > Add drm_sched_wakeup() when entity_kill_job_work() to preventing
> > scheduler sleep and subsequent application hangs.
> > 
> > Signed-off-by: Lin.Cao <lincao12@amd.com>
> > ---
> >   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..a22b0f65558a 100644
> > --- a/drivers/gpu/drm/scheduler/sched_entity.c
> > +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> > @@ -180,6 +180,7 @@ static void
> > drm_sched_entity_kill_jobs_work(struct work_struct *wrk)
> >         drm_sched_fence_finished(job->s_fence, -ESRCH);
> >         WARN_ON(job->s_fence->parent);
> >         job->sched->ops->free_job(job);
> > +     drm_sched_wakeup(job->sched);
> 
> That should probably be after drm_sched_fence_scheduled().
> 
> Alternatively we could also drop the optimization in
> drm_sched_entity_add_dependency_cb(), scheduling the work item again
> has only minimal overhead.
> 
> Apart from that looks good to me.
> 
> Regards,
> Christian.
> 
> >   }
> >  
> >   /* Signal the scheduler finished fence when the entity in
> > question is killed. */
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs
  2025-07-11 13:13     ` Philipp Stanner
@ 2025-07-11 13:24       ` Christian König
  0 siblings, 0 replies; 5+ messages in thread
From: Christian König @ 2025-07-11 13:24 UTC (permalink / raw)
  To: phasta, cao, lin, dri-devel@lists.freedesktop.org,
	amd-gfx@lists.freedesktop.org
  Cc: Yin, ZhenGuo (Chris), Deng, Emily, Matthew Brost,
	Danilo Krummrich

On 11.07.25 15:13, Philipp Stanner wrote:
> On Thu, 2025-07-10 at 08:33 +0000, cao, lin wrote:
>>
>> [AMD Official Use Only - AMD Internal Distribution Only]
>>
>>
>>
>> Hi Christian,
>>
>>
>> Thanks for your suggestion, I modified the patch as:
> 
> Looks promising. You'll send a v2 I guess :)

Well I was just about to reply that a proper v2 should be send out and not just the change fragment :)

So Lin please send a properly formated v2 patch.

Regards,
Christian.

> 
> P.
> 
>>
>>
>> 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);
>> --
>>
>>
>> Thanks,
>> Lin
>>
>>
>>
>>
>>
>> From: Koenig, Christian <Christian.Koenig@amd.com>
>> Sent: Thursday, July 10, 2025 15:52
>> To: cao, lin <lin.cao@amd.com>; dri-devel@lists.freedesktop.org
>> <dri-devel@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org
>> <amd-gfx@lists.freedesktop.org>
>> Cc: Yin, ZhenGuo (Chris) <ZhenGuo.Yin@amd.com>; Deng, Emily
>> <Emily.Deng@amd.com>; Matthew Brost <matthew.brost@intel.com>; Danilo
>> Krummrich <dakr@kernel.org>; Philipp Stanner <phasta@kernel.org>
>> Subject: Re: [PATCH] drm/scheduler: Fix sched hang when killing app
>> with dependent jobs
>>
>>
>> First of all you need to CC the scheduler maintainers, try to use the
>> get_maintainer.pl script. Adding them on CC.
>>
>> On 10.07.25 08:36, Lin.Cao wrote:
>>> 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, due to missing work_run_job
>>> or
>>> work_free_job in entity_kill_job_work(), the scheduler enters sleep
>>> state, causing application B hang.
>>
>> Ah! Because of optimizing the dependency when submitting to the same
>> scheduler in drm_sched_entity_add_dependency_cb().
>>
>> Yeah that suddenly starts to make sense.
>>
>>> Add drm_sched_wakeup() when entity_kill_job_work() to preventing
>>> scheduler sleep and subsequent application hangs.
>>>
>>> Signed-off-by: Lin.Cao <lincao12@amd.com>
>>> ---
>>>   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..a22b0f65558a 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>> @@ -180,6 +180,7 @@ static void
>>> drm_sched_entity_kill_jobs_work(struct work_struct *wrk)
>>>         drm_sched_fence_finished(job->s_fence, -ESRCH);
>>>         WARN_ON(job->s_fence->parent);
>>>         job->sched->ops->free_job(job);
>>> +     drm_sched_wakeup(job->sched);
>>
>> That should probably be after drm_sched_fence_scheduled().
>>
>> Alternatively we could also drop the optimization in
>> drm_sched_entity_add_dependency_cb(), scheduling the work item again
>> has only minimal overhead.
>>
>> Apart from that looks good to me.
>>
>> Regards,
>> Christian.
>>
>>>   }
>>>  
>>>   /* Signal the scheduler finished fence when the entity in
>>> question is killed. */
>>
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-07-11 13:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-10  6:36 [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs Lin.Cao
2025-07-10  7:52 ` Christian König
2025-07-10  8:33   ` cao, lin
2025-07-11 13:13     ` Philipp Stanner
2025-07-11 13:24       ` Christian König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).