AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: phasta@kernel.org, "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>
Subject: Re: [PATCH] drm/scheduler: Fix sched hang when killing app with dependent jobs
Date: Fri, 11 Jul 2025 15:24:01 +0200	[thread overview]
Message-ID: <398f044d-7716-49aa-bfa1-cdfd3ce188bf@amd.com> (raw)
In-Reply-To: <d3999026bba8ace350c6e226881b98f901fa474b.camel@mailbox.org>

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. */
>>
> 


      reply	other threads:[~2025-07-11 13:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=398f044d-7716-49aa-bfa1-cdfd3ce188bf@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Emily.Deng@amd.com \
    --cc=ZhenGuo.Yin@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=lin.cao@amd.com \
    --cc=matthew.brost@intel.com \
    --cc=phasta@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox