dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Philipp Stanner <phasta@mailbox.org>
To: "Lin.Cao" <lincao12@amd.com>, dri-devel@lists.freedesktop.org
Cc: zhenguo.yin@amd.com, Emily.Deng@amd.com,
	"Christian König" <christian.koenig@amd.com>,
	phasta@kernel.org, dakr@kernel.org, matthew.brost@intel.com
Subject: Re: [PATCH v2] drm/scheduler: Fix sched hang when killing app with dependent jobs
Date: Mon, 14 Jul 2025 14:46:00 +0200	[thread overview]
Message-ID: <87d61f5b7809828a55caf779b10a90be802fe83a.camel@mailbox.org> (raw)
In-Reply-To: <20250714062349.588235-1-lincao12@amd.com>

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.

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


  parent reply	other threads:[~2025-07-14 12:46 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-14  6:23 [PATCH v2] drm/scheduler: Fix sched hang when killing app with dependent jobs Lin.Cao
2025-07-14 11:03 ` Christian König
2025-07-14 12:46 ` Philipp Stanner [this message]
2025-07-14 13:08   ` Christian König
2025-07-14 13:27     ` Philipp Stanner
2025-07-14 13:39       ` Christian König
2025-07-15  3:38         ` cao, lin
2025-07-15  9:12           ` Philipp Stanner
2025-07-15  9:51             ` cao, lin
2025-07-15 10:27               ` Philipp Stanner
2025-07-15 10:52                 ` Christian König
2025-07-15 12:20                   ` Philipp Stanner
2025-07-15 12:32                     ` Christian König
2025-07-15 13:07                       ` Philipp Stanner
2025-07-15 13:20                         ` cao, lin

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=87d61f5b7809828a55caf779b10a90be802fe83a.camel@mailbox.org \
    --to=phasta@mailbox.org \
    --cc=Emily.Deng@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=lincao12@amd.com \
    --cc=matthew.brost@intel.com \
    --cc=phasta@kernel.org \
    --cc=zhenguo.yin@amd.com \
    /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;
as well as URLs for NNTP newsgroup(s).