[PATCH] drm/scheduler: signal fences of jobs pushed to a stopped entity

[PATCH] drm/scheduler: signal fences of jobs pushed to a stopped entity

[Olivier Kaloudoff]

drm_sched_entity_push_job() queues the job on entity->job_queue first,
then drm_sched_rq_add_entity() notices entity->stopped under
entity->lock, prints "Trying to push to a killed entity" and returns
NULL. The scheduler is never woken, the job strands on the spsc queue
forever, and its fences never signal, hanging any task that waits on
them.

One way to hit this is process kill racing a late submission from the
same address space: an amdkfd SVM migration job submitted from the
fault path of a dying process (SIGBUS on device unplug -> coredump ->
get_dump_page -> svm_migrate_to_ram) lands on the process's already
killed entity and is never completed, which then wedges the entire
PCIe hot-unplug removal thread.

Check entity->stopped under entity->lock before queueing; since
drm_sched_entity_kill() only sets ->stopped while holding that same
lock, this cannot race. A job pushed to a stopped entity is never
queued; instead it is run through the normal kill-jobs work so its
finished fence completes with -ESRCH, exactly like the jobs drained by
drm_sched_entity_kill() itself. The sched->score increment is balanced
explicitly since drm_sched_job_done() only decrements it for jobs that
actually run.

An earlier attempt [1] checked ->stopped without holding entity->lock
and was rejected as racy; this performs the check under the lock where
->stopped is written.

Note: Tvrtko Ursulin's entity run-queue consolidation series [2]
moves this code around, making drm_sched_entity_push_job() lockless
and taking entity->lock inside drm_sched_rq_add_entity() instead.
The orphan survives that series unchanged -- the job is queued before
the NULL return and nothing completes it -- so this fix stays needed
whichever lands first; the loser's side just needs a context rebase.

[1] https://lore.kernel.org/dri-devel/20250811072050.3838241-1-Tong.Liu01@amd.com/
[2] https://lists.freedesktop.org/archives/amd-gfx/2026-April/142980.html

Fixes: 62347a33001c ("drm/scheduler: Add stopped flag to drm_sched_entity")
Signed-off-by: Olivier Kaloudoff <olivier.kaloudoff@gmail.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Philipp Stanner <phasta@kernel.org>
Cc: "Christian König" <christian.koenig@amd.com>
---
 drivers/gpu/drm/scheduler/sched_entity.c | 21 ++
 1 file changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
index 673ca9cbf..22ecc9bfd 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -651,6 +651,27 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job)
 
 	spin_lock(&entity->lock);
 
+	/*
+	 * A stopped entity is never put back on a run queue, so a job
+	 * queued on it would be stranded forever and its fences would
+	 * never signal, hanging any task waiting on them. Don't queue
+	 * the job at all; run the normal kill-jobs work on it so its
+	 * finished fence completes with -ESRCH.
+	 */
+	if (unlikely(entity->stopped)) {
+		spin_unlock(&entity->lock);
+		/*
+		 * Balance the atomic_inc(&sched->score) above; the
+		 * decrement in drm_sched_job_done() only happens when
+		 * the job actually runs on a scheduler.
+		 */
+		atomic_dec(entity->rq->sched->score);
+		DRM_ERROR("Trying to push to a killed entity\n");
+		INIT_WORK(&sched_job->work, drm_sched_entity_kill_jobs_work);
+		schedule_work(&sched_job->work);
+		return;
+	}
+
 	first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
 
 	/* first job wakes up scheduler */
-- 
2.39.0

Re: [PATCH] drm/scheduler: signal fences of jobs pushed to a stopped entity

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] drm/sched: out-of-order fence signaling in drm_sched_entity_push_job()
--

commit 94b284ecd7cfe529710e74fba166d9065b9197ff
Author: Olivier Kaloudoff <olivier.kaloudoff@gmail.com>

drm/scheduler: signal fences of jobs pushed to a stopped entity

This commit checks entity->stopped under entity->lock in
drm_sched_entity_push_job() before pushing a job to the entity queue. When
the entity is stopped, it decrements the scheduler score and schedules
drm_sched_entity_kill_jobs_work() to signal the job fences.

> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index 7be0387bac3ce..37adb82292088 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -644,6 +644,27 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job)
>  
>  	spin_lock(&entity->lock);
>  
> +	/*
> +	 * A stopped entity is never put back on a run queue, so a job
> +	 * queued on it would be stranded forever and its fences would
> +	 * never signal, hanging any task waiting on them. Don't queue
> +	 * the job at all; run the normal kill-jobs work on it so its
> +	 * finished fence completes with -ESRCH.
> +	 */
> +	if (unlikely(entity->stopped)) {
> +		spin_unlock(&entity->lock);
> +		/*
> +		 * Balance the atomic_inc(&sched->score) above; the
> +		 * decrement in drm_sched_job_done() only happens when
> +		 * the job actually runs on a scheduler.
> +		 */
> +		atomic_dec(entity->rq->sched->score);
> +		DRM_ERROR("Trying to push to a killed entity\n");
> +		INIT_WORK(&sched_job->work, drm_sched_entity_kill_jobs_work);
> +		schedule_work(&sched_job->work);
> +		return;
> +	}
> +
>  	first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
>  
>  	/* first job wakes up scheduler */

[Severity: Critical]
Can scheduling drm_sched_entity_kill_jobs_work() directly here cause
s_fence->scheduled and s_fence->finished to signal out of order relative to
earlier jobs on the same entity?

Before calling drm_sched_entity_push_job(), the caller arms the job via
drm_sched_job_arm() -> drm_sched_fence_init(), which assigns the next
sequence number on entity->fence_context and entity->fence_context + 1.

In drivers/gpu/drm/scheduler/sched_entity.c:drm_sched_entity_kill(), after
setting entity->stopped = true, the function waits for entity->entity_idle
and chains each drained job behind entity->last_scheduled and prior queued
jobs:

	spin_lock(&entity->lock);
	entity->stopped = true;
	drm_sched_rq_remove_entity(entity->rq, entity);
	spin_unlock(&entity->lock);

	/* Make sure this entity is not used by the scheduler at the moment */
	wait_for_completion(&entity->entity_idle);

	spin_lock(&entity->lock);
	prev = dma_fence_get(entity->last_scheduled);
	spin_unlock(&entity->lock);
	while ((job = drm_sched_entity_queue_pop(entity))) {
		struct drm_sched_fence *s_fence = job->s_fence;

		dma_fence_get(&s_fence->finished);
		if (!prev ||
		    dma_fence_add_callback(prev, &job->finish_cb,
					   drm_sched_entity_kill_jobs_cb)) {
			...
			drm_sched_entity_kill_jobs_cb(NULL, &job->finish_cb);
		}

		prev = &s_fence->finished;
	}
	dma_fence_put(prev);

In contrast, when drm_sched_entity_push_job() sees entity->stopped, it
immediately schedules drm_sched_entity_kill_jobs_work() without waiting for
entity->entity_idle, entity->last_scheduled, or the tail of the chain in
drm_sched_entity_kill() (which is only kept in the local variable prev).

Since drm_sched_entity_kill_jobs_work() only waits on fences in
sched_job->dependencies, and jobs on the same entity normally rely on the
entity queue for ordering rather than explicit same-entity dependencies,
this job's fences can signal while an earlier job from the same entity is
still executing on the hardware.

For example, drivers/dma-buf/dma-resv.c:dma_resv_add_fence() replaces an
earlier unsignaled fence from the same context when a later sequence number
is added:

	for (i = 0; i < count; ++i) {
		enum dma_resv_usage old_usage;

		dma_resv_list_entry(fobj, i, obj, &old, &old_usage);
		if ((old->context == fence->context && old_usage >= usage &&
		     dma_fence_is_later_or_same(fence, old)) ||
		    dma_fence_is_signaled(old)) {
			dma_resv_list_set(fobj, i, fence, usage);
			dma_fence_put(old);
			return;
		}
	}

Similarly, in the amdkfd migration path
svm_migrate_vma_to_ram() -> svm_migrate_copy_memory_gart() in
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c, multiple copy jobs are submitted
to the same entity and *mfence is overwritten with the latest job's fence:

		r = amdgpu_copy_buffer(adev, entity,
				       gart_s, gart_d, size * PAGE_SIZE,
				       NULL, &next, true, 0);
		if (r) {
			dev_err(adev->dev, "fail %d to copy memory\n", r);
			goto out_unlock;
		}

		dma_fence_put(*mfence);
		*mfence = next;

If the entity is killed while an earlier job is still in flight and a
subsequent job hits the entity->stopped path in drm_sched_entity_push_job(),
wouldn't the later job's fence signal first and cause callers waiting on
*mfence or dma_resv to unmap and free memory while the earlier job is still
accessing it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022105454.60267-1-olivier.kaloudoff@gmail.com?part=1

Re: [PATCH] drm/scheduler: signal fences of jobs pushed to a stopped entity

[Philipp Stanner]

+Cc Danilo

Please use scripts/get_maintainer.pl


On Thu, 2026-10-22 at 12:54 +0200, Olivier Kaloudoff wrote:
> An earlier attempt [1] checked ->stopped without holding entity->lock
> and was rejected as racy; this performs the check under the lock where
> ->stopped is written.

I think it was also rejected because this as an attempt to fix a driver
issue in the shared infrastructure:

https://lore.kernel.org/dri-devel/61787d6cdbfbcb8acdd28c9b6ead50dfb0641899.camel@mailbox.org/


Who stops an entity?

It is amdgpu, which does so by calling drm_sched_entity_kill(). This is
the only place where an entity gets stopped.

Thus, amdgpu runs into an issue because it tries to push to an entity
which it has stopped itself. So the logical solution is to just not do
that :)

> 
> Note: Tvrtko Ursulin's entity run-queue consolidation series [2]
> moves this code around, making drm_sched_entity_push_job() lockless
> and taking entity->lock inside drm_sched_rq_add_entity() instead.
> The orphan survives that series unchanged -- the job is queued before
> the NULL return and nothing completes it -- so this fix stays needed
> whichever lands first; the loser's side just needs a context rebase.

That is long outdated, Tvrtko has now agreed to getting us rid of the
spsc_queue, which means that _push_job() and _pop_job() will almost be
fully locked, with a nice clean linked list below.

> 

[…]

>  
> +	/*
> +	 * A stopped entity is never put back on a run queue, so a job
> +	 * queued on it would be stranded forever and its fences would
> +	 * never signal, hanging any task waiting on them. Don't queue
> +	 * the job at all; run the normal kill-jobs work on it so its
> +	 * finished fence completes with -ESRCH.
> +	 */
> +	if (unlikely(entity->stopped)) {
> +		spin_unlock(&entity->lock);
> +		/*
> +		 * Balance the atomic_inc(&sched->score) above; the
> +		 * decrement in drm_sched_job_done() only happens when
> +		 * the job actually runs on a scheduler.
> +		 */
> +		atomic_dec(entity->rq->sched->score);
> +		DRM_ERROR("Trying to push to a killed entity\n");

Having a warning around here could be okay-ish I guess, but actual
operational code is not, because it fixes around the driver being
broken.

The warning would then merely be there to inform the driver that it
needs to be repaired.


P.

> +		INIT_WORK(&sched_job->work, drm_sched_entity_kill_jobs_work);
> +		schedule_work(&sched_job->work);
> +		return;

Re: [PATCH] drm/scheduler: signal fences of jobs pushed to a stopped entity

[Christian König]

On 10/22/26 12:54, Olivier Kaloudoff wrote:
> drm_sched_entity_push_job() queues the job on entity->job_queue first,
> then drm_sched_rq_add_entity() notices entity->stopped under
> entity->lock, prints "Trying to push to a killed entity" and returns
> NULL. The scheduler is never woken, the job strands on the spsc queue
> forever, and its fences never signal, hanging any task that waits on
> them.
> 
> One way to hit this is process kill racing a late submission from the
> same address space: an amdkfd SVM migration job submitted from the
> fault path of a dying process (SIGBUS on device unplug -> coredump ->
> get_dump_page -> svm_migrate_to_ram) lands on the process's already
> killed entity and is never completed, which then wedges the entire
> PCIe hot-unplug removal thread.
> 
> Check entity->stopped under entity->lock before queueing; since
> drm_sched_entity_kill() only sets ->stopped while holding that same
> lock, this cannot race. A job pushed to a stopped entity is never
> queued; instead it is run through the normal kill-jobs work so its
> finished fence completes with -ESRCH, exactly like the jobs drained by
> drm_sched_entity_kill() itself. The sched->score increment is balanced
> explicitly since drm_sched_job_done() only decrements it for jobs that
> actually run.
> 
> An earlier attempt [1] checked ->stopped without holding entity->lock
> and was rejected as racy; this performs the check under the lock where
> ->stopped is written.
> 
> Note: Tvrtko Ursulin's entity run-queue consolidation series [2]
> moves this code around, making drm_sched_entity_push_job() lockless
> and taking entity->lock inside drm_sched_rq_add_entity() instead.
> The orphan survives that series unchanged -- the job is queued before
> the NULL return and nothing completes it -- so this fix stays needed
> whichever lands first; the loser's side just needs a context rebase.

A non signaled fence can deadlock the kernel, but signaling a fence when there are eventually operations still running can cause random memory corruption. So I think that patch here makes things even worse.

Whatever we do we can only loose and the most reasonable thing to do is to call BUG().

A stopped entity is an entity which was killed and eventually freed up, the only reason that is not an use after free is because the backing memory stays around a bit longer than the entity itself.

This here is something we really need to fix inside the caller, in this particular case here amdgpu's hot remove handling.

Regards,
Christian.

> 
> [1] https://lore.kernel.org/dri-devel/20250811072050.3838241-1-Tong.Liu01@amd.com/
> [2] https://lists.freedesktop.org/archives/amd-gfx/2026-April/142980.html
> 
> Fixes: 62347a33001c ("drm/scheduler: Add stopped flag to drm_sched_entity")
> Signed-off-by: Olivier Kaloudoff <olivier.kaloudoff@gmail.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Philipp Stanner <phasta@kernel.org>
> Cc: "Christian König" <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/scheduler/sched_entity.c | 21 ++
>  1 file changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index 673ca9cbf..22ecc9bfd 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -651,6 +651,27 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job)
>  
>  	spin_lock(&entity->lock);
>  
> +	/*
> +	 * A stopped entity is never put back on a run queue, so a job
> +	 * queued on it would be stranded forever and its fences would
> +	 * never signal, hanging any task waiting on them. Don't queue
> +	 * the job at all; run the normal kill-jobs work on it so its
> +	 * finished fence completes with -ESRCH.
> +	 */
> +	if (unlikely(entity->stopped)) {
> +		spin_unlock(&entity->lock);
> +		/*
> +		 * Balance the atomic_inc(&sched->score) above; the
> +		 * decrement in drm_sched_job_done() only happens when
> +		 * the job actually runs on a scheduler.
> +		 */
> +		atomic_dec(entity->rq->sched->score);
> +		DRM_ERROR("Trying to push to a killed entity\n");
> +		INIT_WORK(&sched_job->work, drm_sched_entity_kill_jobs_work);
> +		schedule_work(&sched_job->work);
> +		return;
> +	}
> +
>  	first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
>  
>  	/* first job wakes up scheduler */