[PATCH RFC v3] sched/proxy: Defer donor commit until after proxy resolution

[PATCH RFC v3] sched/proxy: Defer donor commit until after proxy resolution

[Xukai Wang]

pick_next_task() currently couples selecting a task with committing that
selection through put_prev_set_next_task().

With proxy execution, the selected task may be a blocked donor which
still has to go through find_proxy_task(). If proxy resolution returns
NULL, __schedule() retries through pick_again, and the following pick may
select a different task. In that case, the put_prev_task()/set_next_task()
work done for the previous pick is followed by another
put_prev_set_next_task() for the new pick, even though the previous pick
was ultimately abandoned.

Temporary instrumentation was added to the kernel without this patch to
measure how often this happens. In one 60 second proxy-mutex stress run:
	spec_commit_blocked 5407
	spec_commit_then_null 3224
	spec_commit_then_idle 1660
	spec_commit_then_success 523

For the 3224 NULL cases, the following retry selected:
	null_retry_same_donor 0
	null_retry_diff_donor 2258
	null_retry_to_idle 966

The temporary counters mean:
- spec_commit_blocked: pick_next_task() selected a blocked donor, and in
the baseline code that donor had already gone through
put_prev_set_next_task() before proxy-chain resolution.

- spec_commit_then_null: the blocked donor had already been committed,
but find_proxy_task() returned NULL and __schedule() retried.

- spec_commit_then_idle: the blocked donor had already been committed,
but find_proxy_task() returned rq->idle.

- spec_commit_then_success: the blocked donor had already been committed,
and find_proxy_task() successfully found a task to run.

- null_retry_same_donor: after find_proxy_task() returned NULL, the next
pick selected the same donor again.

- null_retry_diff_donor: after find_proxy_task() returned NULL, the next
pick selected a different non-idle donor.

- null_retry_to_idle: after find_proxy_task() returned NULL, the next
pick selected idle.

Thus, in this run, none of the NULL retries selected the same donor
again. 2258 selected a different non-idle donor and 966 selected idle.

These measurements only characterize how frequently the redundant
put_prev/set_next case occurs. The cycle cost of those callbacks and
the end-to-end performance impact were not measured.

Separate candidate selection from donor commit. Make pick_next_task()
return only the selected candidate and move put_prev_set_next_task() to
__schedule(), after proxy-chain resolution.

Keep the selected donor separate from the task that will actually run:
	donor = pick_next_task(rq, &rf);
	next = donor;
	...
	next = find_proxy_task(rq, donor, &rf);
	...
	put_prev_set_next_task(rq, rq->donor, donor);
	rq_set_donor(rq, donor);

If proxy resolution abandons the candidate, the candidate is discarded
without first doing the sched-class put_prev/set_next work for that
pick.

Deferring put_prev_set_next_task() also changes the rq->dl_server state
seen by find_proxy_task(). Before this change, put_prev_set_next_task()
has already consumed and cleared rq->dl_server by the time
find_proxy_task() is entered. Preserve that behavior by saving and
clearing rq->dl_server around proxy resolution and restoring it only
when resolution succeeds.

A proxy candidate is also no longer necessarily the committed
rq->donor. Update proxy_deactivate() accordingly: only switch to idle
before blocking the candidate when it is still the committed donor.
proxy_migrate_task() continues to switch the committed donor to idle
before dropping the rq lock.

Move zap_balance_callbacks() into proxy_resched_idle(), next to the idle
commit. Since the remaining callers of zap_balance_callbacks() are
proxy-exec specific, guard its definition with CONFIG_SCHED_PROXY_EXEC.

Signed-off-by: Xukai Wang <kingxukai@zohomail.com>
---
Changes in v3:
- Add the motivation and retry-frequency measurements previously posted
in the v1 discussion.
- Preserve rq->dl_server == NULL while find_proxy_task() is running by
saving and clearing the candidate DL-server state around proxy
resolution and restoring it only on success.
- Make the donor/execution-task distinction explicit in __schedule().
- Preserve the existing DL-server/accounting ordering by doing the
common put_prev_set_next_task() before the same-donor sched-class
callback refresh.
- Fix a W=1 !CONFIG_SCHED_PROXY_EXEC unused-function warning reported by
the kernel test robot.
- Link to v2: https://patch.msgid.link/20260713-sched-proxy-v2-0-729170082633@zohomail.com

Changes in v2:
- Move the final put_prev_set_next_task()/rq_set_donor() after the
  proxy/non-proxy branches.
- Move zap_balance_callbacks() from the NULL/idle proxy-resolution
  paths into proxy_resched_idle(), next to the idle commit.
- Link to v1: https://patch.msgid.link/20260707-sched-proxy-v1-0-5928bf6dedf0@zohomail.com

To: Ingo Molnar <mingo@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
To: Juri Lelli <juri.lelli@redhat.com>
To: Vincent Guittot <vincent.guittot@linaro.org>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Steven Rostedt <rostedt@goodmis.org>
To: Ben Segall <bsegall@google.com>
To: Mel Gorman <mgorman@suse.de>
To: Valentin Schneider <vschneid@redhat.com>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/sched/core.c | 136 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 78 insertions(+), 58 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2e7cde033a31..bd6c0252b820 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5090,6 +5090,7 @@ static inline void finish_task(struct task_struct *prev)
 	smp_store_release(&prev->on_cpu, 0);
 }
 
+#ifdef CONFIG_SCHED_PROXY_EXEC
 /*
  * Only called from __schedule context
  *
@@ -5117,6 +5118,7 @@ static void zap_balance_callbacks(struct rq *rq)
 	}
 	rq->balance_callback = found ? &balance_push_callback : NULL;
 }
+#endif
 
 static void do_balance_callbacks(struct rq *rq, struct balance_callback *head)
 {
@@ -6146,7 +6148,6 @@ __pick_next_task(struct rq *rq, struct rq_flags *rf)
 		if (!p)
 			p = pick_task_idle(rq, rf);
 
-		put_prev_set_next_task(rq, rq->donor, p);
 		return p;
 	}
 
@@ -6157,10 +6158,8 @@ __pick_next_task(struct rq *rq, struct rq_flags *rf)
 		p = class->pick_task(rq, rf);
 		if (unlikely(p == RETRY_TASK))
 			goto restart;
-		if (p) {
-			put_prev_set_next_task(rq, rq->donor, p);
+		if (p)
 			return p;
-		}
 	}
 
 	BUG(); /* The idle class should always have a runnable task. */
@@ -6257,7 +6256,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 		rq->dl_server = rq->core_dl_server;
 		rq->core_pick = NULL;
 		rq->core_dl_server = NULL;
-		goto out_set_next;
+		goto out_return_next;
 	}
 
 	prev_balance(rq, rf);
@@ -6311,7 +6310,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 			 */
 			WARN_ON_ONCE(fi_before);
 			task_vruntime_update(rq, next, false);
-			goto out_set_next;
+			goto out_return_next;
 		}
 	}
 
@@ -6441,8 +6440,7 @@ pick_next_task(struct rq *rq, struct rq_flags *rf)
 		resched_curr(rq_i);
 	}
 
-out_set_next:
-	put_prev_set_next_task(rq, rq->donor, next);
+out_return_next:
 	if (rq->core->core_forceidle_count && next == rq->idle)
 		queue_core_balance(rq);
 
@@ -6745,6 +6743,14 @@ static inline struct task_struct *proxy_resched_idle(struct rq *rq)
 	rq->next_class = &idle_sched_class;
 	rq_set_donor(rq, rq->idle);
 	set_tsk_need_resched(rq->idle);
+
+	/*
+	 * This helper performs a real idle commit. Some callers return to
+	 * __schedule() without dropping rq->lock, so clear callbacks
+	 * generated by the idle commit here. Paths that later drop
+	 * rq->lock may zap again.
+	 */
+	zap_balance_callbacks(rq);
 	return rq->idle;
 }
 
@@ -6755,15 +6761,23 @@ static void proxy_deactivate(struct rq *rq, struct task_struct *donor)
 	WARN_ON_ONCE(state == TASK_RUNNING);
 	WARN_ON_ONCE(donor->blocked_on);
 	/*
-	 * Because we got donor from pick_next_task(), it is *crucial*
-	 * that we call proxy_resched_idle() before we deactivate it.
-	 * As once we deactivate donor, donor->on_rq is set to zero,
-	 * which allows ttwu() to immediately try to wake the task on
-	 * another rq. So we cannot use *any* references to donor
-	 * after that point. So things like cfs_rq->curr or rq->donor
-	 * need to be changed from next *before* we deactivate.
+	 * A proxy candidate is not necessarily the committed rq->donor.
+	 * pick_next_task() only selected it; the class current state is
+	 * updated later, after proxy-chain resolution.
+	 *
+	 * If @donor is still the committed donor, the rq and the scheduling
+	 * class may hold current references to it, such as rq->donor or
+	 * cfs_rq->curr/h_curr. Drop those references before block_task(),
+	 * because block_task() clears donor->on_rq and a concurrent wakeup
+	 * may then move the task elsewhere.
+	 *
+	 * If @donor is only an uncommitted proxy candidate, it is still a
+	 * queued task, not the class current task, so it can be blocked
+	 * directly.
 	 */
-	proxy_resched_idle(rq);
+	if (donor == rq->donor)
+		proxy_resched_idle(rq);
+
 	block_task(rq, donor, state);
 }
 
@@ -6816,15 +6830,17 @@ static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,
 	lockdep_assert_rq_held(rq);
 	WARN_ON(p == rq->curr);
 	/*
-	 * Since we are migrating a blocked donor, it could be rq->donor,
-	 * and we want to make sure there aren't any references from this
-	 * rq to it before we drop the lock. This avoids another cpu
-	 * jumping in and grabbing the rq lock and referencing rq->donor
-	 * or cfs_rq->curr, etc after we have migrated it to another cpu,
-	 * and before we pick_again in __schedule.
+	 * Migrating a task found in the proxy chain abandons the current proxy
+	 * pick attempt and drops this rq's lock.
+	 *
+	 * The picked proxy candidate has not necessarily been committed, so
+	 * @p is not necessarily rq->donor. Switch the currently committed
+	 * donor to idle before dropping the lock, leaving rq->donor and the
+	 * class current state in a well-defined state while the chain is
+	 * modified and @p is attached elsewhere.
 	 *
-	 * So call proxy_resched_idle() to drop the rq->donor references
-	 * before we release the lock.
+	 * If @p is rq->donor, this also drops the direct rq/class-current
+	 * references to @p before it leaves this rq.
 	 */
 	proxy_resched_idle(rq);
 
@@ -7057,7 +7073,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
  */
 static void __sched notrace __schedule(int sched_mode)
 {
-	struct task_struct *prev, *next;
+	struct task_struct *prev, *next, *donor;
 	/*
 	 * On PREEMPT_RT kernel, SM_RTLOCK_WAIT is noted
 	 * as a preemption by schedule_debug() and RCU.
@@ -7143,45 +7159,49 @@ static void __sched notrace __schedule(int sched_mode)
 
 pick_again:
 	assert_balance_callbacks_empty(rq);
-	next = pick_next_task(rq, &rf);
-	rq->next_class = next->sched_class;
+	donor = pick_next_task(rq, &rf);
+	rq->next_class = donor->sched_class;
+	next = donor;
 	if (sched_proxy_exec()) {
-		struct task_struct *prev_donor = rq->donor;
-
-		rq_set_donor(rq, next);
-		next->blocked_donor = NULL;
-		if (unlikely(next->is_blocked)) {
-			next = find_proxy_task(rq, next, &rf);
-			if (!next) {
-				zap_balance_callbacks(rq);
+		donor->blocked_donor = NULL;
+		if (unlikely(donor->is_blocked)) {
+			struct sched_dl_entity *donor_dl_server = rq->dl_server;
+
+			rq->dl_server = NULL;
+
+			next = find_proxy_task(rq, donor, &rf);
+			if (!next)
 				goto pick_again;
-			}
-			if (next == rq->idle) {
-				zap_balance_callbacks(rq);
+			if (next == rq->idle)
 				goto keep_resched;
-			}
-		}
-		if (rq->donor == prev_donor && prev != next) {
-			struct task_struct *donor = rq->donor;
-			/*
-			 * When transitioning like:
-			 *
-			 *         prev         next
-			 * donor:    B            B
-			 * curr:     A          B or C
-			 *
-			 * then put_prev_set_next_task() will not have done
-			 * anything, since B == B. However, A might have
-			 * missed a RT/DL balance opportunity due to being
-			 * on_cpu.
-			 */
-			donor->sched_class->put_prev_task(rq, donor, donor);
-			donor->sched_class->set_next_task(rq, donor, true);
+
+			rq->dl_server = donor_dl_server;
 		}
-	} else {
-		rq_set_donor(rq, next);
+
 	}
 
+	put_prev_set_next_task(rq, rq->donor, donor);
+
+	if (sched_proxy_exec() &&
+	    donor == rq->donor && prev != next) {
+		/*
+		 * When transitioning like:
+		 *
+		 *         prev         next
+		 * donor:    B            B
+		 * curr:     A          B or C
+		 *
+		 * then put_prev_set_next_task() will not have called
+		 * the class->put_prev_task()/set_next_task() callbacks,
+		 * since B == B. However, A might have missed a RT/DL
+		 * balance opportunity due to being on_cpu.
+		 */
+		donor->sched_class->put_prev_task(rq, donor, donor);
+		donor->sched_class->set_next_task(rq, donor, true);
+	}
+
+	rq_set_donor(rq, donor);
+
 picked:
 	clear_tsk_need_resched(prev);
 	clear_preempt_need_resched();

---
base-commit: 68e37487810a3da43c48340fab7a55b3b6efdae3
change-id: 20260707-sched-proxy-4404b6e97ad9

Best regards,
--  
Xukai Wang <kingxukai@zohomail.com>

Re: [PATCH RFC v3] sched/proxy: Defer donor commit until after proxy resolution

[Xukai Wang]

Hi Prateek, John, all,

Quick ping on the v3 defer-donor-commit patch. Following up on John's
questions from the v2 review -- specifically what the real impact of this
change is and whether it's measurable -- I've collected targeted
measurements of the put_prev_set_next_task() overhead on the donor commit
path.

1. Test setup

I ran John's priority-inversion-demo workload [1] 3 times per kernel on a
bare-metal x86 system.

I instrumented the two scheduling-class callbacks invoked by
put_prev_set_next_task() on the donor commit path:

/* around put_prev_task() */
t0 = local_clock();
prev->sched_class->put_prev_task(...);
t1 = local_clock();

put_calls++;
put_total_ns += t1 - t0;

/* around set_next_task() */
t0 = local_clock();
next->sched_class->set_next_task(...);
t1 = local_clock();

set_calls++;
set_total_ns += t1 - t0;

I also calibrated the overhead of an empty local_clock() pair (minimum
15ns observed on this machine) and subtracted that per-call from the
measured totals.

t0 = local_clock();
t1 = local_clock();

The adjusted times below subtract that value once per measured callback:

adjusted_total = raw_total - nr_calls * 15 ns

2. Results
2.1 callback invocations

Combined across 3 runs:

                  Baseline    v3 patch
  put_prev_task   49037       8968
  set_next_task   49037       8968

That is an 81.7% reduction in scheduling-class callback invocations
from the donor commit path.


2.2 callback execution time

Adjusted total time spent in these callbacks per workload run:

            Baseline    v3 patch
  Run 1     11.131 ms   2.361 ms
  Run 2     11.157 ms   2.548 ms
  Run 3     11.578 ms   2.642 ms
  Average   ~11.29 ms   ~2.52 ms

For reference, a single idle -> fair transition (the common case in these
runs) costs ~0.69 µs on baseline:
  put_prev_task(idle):  142-154 ns
  set_next_task(fair):  503-557 ns

3.End-to-End latency

I also ran uninstrumented kernels to check full workload end-to-end
impact. I did not observe a consistent stable shift:
  p99  latency: ~5.00 ms on both baseline and v3
  p999 latency: ~9.00 ms on both baseline and v3

The reduction above amounts to about 8.8 ms of scheduling-class callback
time per workload run. This local reduction did not translate into a
stable end-to-end latency shift in this workload.

So the measurable effect is at the donor commit point itself: deferring
the commit reduced the number of put_prev_task()/set_next_task() callback
invocations by 81.7%, with the measured callback time at that point
dropping from about 11.29 ms to 2.52 ms per workload run.

Separately from the measured cost, this also places the scheduling-class
state commit after the donor has survived proxy resolution, rather than
committing state for a candidate that may subsequently be abandoned.

I'd appreciate your thoughts on whether these measurements address the
concern from v2, and whether there are any changes you'd like to see for
v4.

LINK: https://github.com/johnstultz-work/priority-inversion-demo [1]

Re: [PATCH RFC v3] sched/proxy: Defer donor commit until after proxy resolution

[K Prateek Nayak]

Hello Xukai,

On 10/10/2026 6:27 PM, Xukai Wang wrote:
> So the measurable effect is at the donor commit point itself: deferring
> the commit reduced the number of put_prev_task()/set_next_task() callback
> invocations by 81.7%, with the measured callback time at that point
> dropping from about 11.29 ms to 2.52 ms per workload run.
> 
> Separately from the measured cost, this also places the scheduling-class
> state commit after the donor has survived proxy resolution, rather than
> committing state for a candidate that may subsequently be abandoned.
> 
> I'd appreciate your thoughts on whether these measurements address the
> concern from v2, and whether there are any changes you'd like to see for
> v4.

I don't have any real concern except you'll have to rebase it on latest
tip and handle the scx changes. Also in your v3 ...

On 8/27/2026 2:48 PM, Xukai Wang wrote:
> @@ -7143,45 +7159,49 @@ static void __sched notrace __schedule(int sched_mode)
>  
>  pick_again:
>  	assert_balance_callbacks_empty(rq);
> -	next = pick_next_task(rq, &rf);
> -	rq->next_class = next->sched_class;
> +	donor = pick_next_task(rq, &rf);
> +	rq->next_class = donor->sched_class;
> +	next = donor;
>  	if (sched_proxy_exec()) {
> -		struct task_struct *prev_donor = rq->donor;
> -
> -		rq_set_donor(rq, next);
> -		next->blocked_donor = NULL;
> -		if (unlikely(next->is_blocked)) {
> -			next = find_proxy_task(rq, next, &rf);
> -			if (!next) {
> -				zap_balance_callbacks(rq);
> +		donor->blocked_donor = NULL;
> +		if (unlikely(donor->is_blocked)) {
> +			struct sched_dl_entity *donor_dl_server = rq->dl_server;
> +
> +			rq->dl_server = NULL;

I'm not able to understand this special dl_server handling.

In case of "pick_again", pick_next_task() will clear the rq->dl_server
before the pick and for "keep_resched" with idle task, I don't think the
dl_server from pick makes any difference since we'll come back into
schedule right after the switch.

For rest of find_proxy_task() we don't really care about dl_server afaict
unless I'm missing something. did you trip something that required this
change?

> +
> +			next = find_proxy_task(rq, donor, &rf);
> +			if (!next)
>  				goto pick_again;
> -			}
> -			if (next == rq->idle) {
> -				zap_balance_callbacks(rq);
> +			if (next == rq->idle)
>  				goto keep_resched;
> -			}
> -		}
> -		if (rq->donor == prev_donor && prev != next) {
> -			struct task_struct *donor = rq->donor;
> -			/*
> -			 * When transitioning like:
> -			 *
> -			 *         prev         next
> -			 * donor:    B            B
> -			 * curr:     A          B or C
> -			 *
> -			 * then put_prev_set_next_task() will not have done
> -			 * anything, since B == B. However, A might have
> -			 * missed a RT/DL balance opportunity due to being
> -			 * on_cpu.
> -			 */
> -			donor->sched_class->put_prev_task(rq, donor, donor);
> -			donor->sched_class->set_next_task(rq, donor, true);
> +
> +			rq->dl_server = donor_dl_server;
>  		}
> -	} else {
> -		rq_set_donor(rq, next);
> +
>  	}
>  
> +	put_prev_set_next_task(rq, rq->donor, donor);
> +
> +	if (sched_proxy_exec() &&
> +	    donor == rq->donor && prev != next) {

nit. That condition is too short to need splitting into two.

> +		/*
> +		 * When transitioning like:
> +		 *
> +		 *         prev         next
> +		 * donor:    B            B
> +		 * curr:     A          B or C
> +		 *
> +		 * then put_prev_set_next_task() will not have called
> +		 * the class->put_prev_task()/set_next_task() callbacks,
> +		 * since B == B. However, A might have missed a RT/DL
> +		 * balance opportunity due to being on_cpu.
> +		 */
> +		donor->sched_class->put_prev_task(rq, donor, donor);
> +		donor->sched_class->set_next_task(rq, donor, true);
> +	}
> +
> +	rq_set_donor(rq, donor);


(+ Andrea for sched-ext)

On current tip + sched-ext changes lined for v7.4 from
https://lore.kernel.org/lkml/20260922165445.943315-9-arighi@nvidia.com/
some sched-ext bits will need rq->donor to already reflect the next
selection before they are called and three "if (sched_proxy_exec() ...)"
splits in schedule() looks all over the place.

I'm not sure if 8ms overhead is that visible in a real world workload
especially where SCHED_PROXY_EXEC brings much more benefit to balance
that overhead.

I'll let John, Peter, and Andrea comment further - last I remember,
Peter preferred keeping all the sched_proxy_exec() related bits in a
single place in __schedule().

> +
>  picked:
>  	clear_tsk_need_resched(prev);
>  	clear_preempt_need_resched();
> 

-- 
Thanks and Regards,
Prateek

Re: [PATCH RFC v3] sched/proxy: Defer donor commit until after proxy resolution

[Xukai Wang]

On 2026/10/12 12:05, K Prateek Nayak wrote:
> On 8/27/2026 2:48 PM, Xukai Wang wrote:
>> @@ -7143,45 +7159,49 @@ static void __sched notrace __schedule(int sched_mode)
>>  
>>  pick_again:
>>  	assert_balance_callbacks_empty(rq);
>> -	next = pick_next_task(rq, &rf);
>> -	rq->next_class = next->sched_class;
>> +	donor = pick_next_task(rq, &rf);
>> +	rq->next_class = donor->sched_class;
>> +	next = donor;
>>  	if (sched_proxy_exec()) {
>> -		struct task_struct *prev_donor = rq->donor;
>> -
>> -		rq_set_donor(rq, next);
>> -		next->blocked_donor = NULL;
>> -		if (unlikely(next->is_blocked)) {
>> -			next = find_proxy_task(rq, next, &rf);
>> -			if (!next) {
>> -				zap_balance_callbacks(rq);
>> +		donor->blocked_donor = NULL;
>> +		if (unlikely(donor->is_blocked)) {
>> +			struct sched_dl_entity *donor_dl_server = rq->dl_server;
>> +
>> +			rq->dl_server = NULL;
> I'm not able to understand this special dl_server handling.
>
> In case of "pick_again", pick_next_task() will clear the rq->dl_server
> before the pick and for "keep_resched" with idle task, I don't think the
> dl_server from pick makes any difference since we'll come back into
> schedule right after the switch.
>
> For rest of find_proxy_task() we don't really care about dl_server afaict
> unless I'm missing something. did you trip something that required this
> change?
>
>> +
>> +			next = find_proxy_task(rq, donor, &rf);
>> +			if (!next)
>>  				goto pick_again;
>> -			}
>> -			if (next == rq->idle) {
>> -				zap_balance_callbacks(rq);
>> +			if (next == rq->idle)
>>  				goto keep_resched;
>> -			}
>> -		}
>> -		if (rq->donor == prev_donor && prev != next) {
>> -			struct task_struct *donor = rq->donor;
>> -			/*
>> -			 * When transitioning like:
>> -			 *
>> -			 *         prev         next
>> -			 * donor:    B            B
>> -			 * curr:     A          B or C
>> -			 *
>> -			 * then put_prev_set_next_task() will not have done
>> -			 * anything, since B == B. However, A might have
>> -			 * missed a RT/DL balance opportunity due to being
>> -			 * on_cpu.
>> -			 */
>> -			donor->sched_class->put_prev_task(rq, donor, donor);
>> -			donor->sched_class->set_next_task(rq, donor, true);
>> +
>> +			rq->dl_server = donor_dl_server;
>>  		}
>> -	} else {
>> -		rq_set_donor(rq, next);
>> +
>>  	}
>>  
>> +	put_prev_set_next_task(rq, rq->donor, donor);
>> +
>> +	if (sched_proxy_exec() &&
>> +	    donor == rq->donor && prev != next) {

Hi Prateek,

The case I had in mind is when pick_next_task() selects the candidate
through a DL server, and find_proxy_task() later goes through
proxy_resched_idle().

1. Before this patch, the sequence is effectively:

|-> pick_next_task()

|---|-> rq->dl_server = server;

|---|-> put_prev_set_next_task(rq, rq->donor, donor);

|---|---|-> donor->dl_server = rq->dl_server;

|---|---|-> rq->dl_server = NULL;

|-> find_proxy_task(...)

|---|-> proxy_resched_idle(rq);

|---|---|->put_prev_set_next_task(rq, rq->donor, rq->idle);

|---|---|---|-> rq->idle->dl_server = rq->dl_server; /* NULL */

|---|---|---|-> rq->dl_server = NULL;


So find_proxy_task() is entered with rq->dl_server already consumed by
the donor commit, i.e. rq->dl_server == NULL.

2. After deferring put_prev_set_next_task(), without the save/clear, the
sequence can instead become:

|-> pick_next_task()

|---|-> rq->dl_server = server;

|-> next = find_proxy_task(...)

|---|-> proxy_resched_idle(rq);

|---|---|->put_prev_set_next_task(rq, rq->donor, rq->idle);

|---|---|---|-> rq->idle->dl_server = rq->dl_server; /* server */

|---|---|---|-> rq->dl_server = NULL;

|->if (next == rq->idle) goto keep_resched;

...


In that case the DL server selected for the uncommitted donor gets
attached to rq->idle.

The next pick clearing rq->dl_server does not undo that assignment,
since the pointer has already moved to rq->idle->dl_server. It is only
cleared later when idle becomes the prev task of another
put_prev_set_next_task() transition.

3. That is what the save/clear/restore in v3 was intended to avoid:

pick_again:

|-> pick_next_task()

|---|-> rq->dl_server = server;

|->donor_dl_server = rq->dl_server;

|-> rq->dl_server = NULL;

|-> next = find_proxy_task(...)

|---|-> proxy_resched_idle(rq);

|---|---|->put_prev_set_next_task(rq, rq->donor, rq->idle);

|---|---|---|-> rq->idle->dl_server = rq->dl_server; /* NULL */

|---|---|---|-> rq->dl_server = NULL;

|-> if (!next) goto pick_again;

|-> if (next == rq->idle) goto keep_resched;

|-> rq->dl_server = donor_dl_server;

...

|-> put_prev_set_next_task(rq, rq->donor, donor);


If proxy resolution returns NULL or idle, the saved value is not
restored into rq->dl_server for a donor commit that will not happen.

So the intent is to keep the DL-server selection made for the candidate
out of rq->dl_server while proxy resolution is in progress, and restore
it only if that donor is actually going to be committed, while keeping
the state seen by find_proxy_task() consistent with the pre-patch path.

Do you think rq->idle->dl_server being set in the second sequence above
is harmless, or is keeping the save/clear/restore the right way to avoid
that state?

I’ll look at the sched-ext interaction and the __schedule() structure
separately while rebasing onto current tip.

-- 
Best regards,
Xukai

Re: [PATCH RFC v3] sched/proxy: Defer donor commit until after proxy resolution

[K Prateek Nayak]

Hello Xukai,

On 10/12/2026 1:09 PM, Xukai Wang wrote:
> 
> On 2026/10/12 12:05, K Prateek Nayak wrote:
>> On 8/27/2026 2:48 PM, Xukai Wang wrote:
>>> @@ -7143,45 +7159,49 @@ static void __sched notrace __schedule(int sched_mode)
>>>  
>>>  pick_again:
>>>  	assert_balance_callbacks_empty(rq);
>>> -	next = pick_next_task(rq, &rf);
>>> -	rq->next_class = next->sched_class;
>>> +	donor = pick_next_task(rq, &rf);
>>> +	rq->next_class = donor->sched_class;
>>> +	next = donor;
>>>  	if (sched_proxy_exec()) {
>>> -		struct task_struct *prev_donor = rq->donor;
>>> -
>>> -		rq_set_donor(rq, next);
>>> -		next->blocked_donor = NULL;
>>> -		if (unlikely(next->is_blocked)) {
>>> -			next = find_proxy_task(rq, next, &rf);
>>> -			if (!next) {
>>> -				zap_balance_callbacks(rq);
>>> +		donor->blocked_donor = NULL;
>>> +		if (unlikely(donor->is_blocked)) {
>>> +			struct sched_dl_entity *donor_dl_server = rq->dl_server;
>>> +
>>> +			rq->dl_server = NULL;
>> I'm not able to understand this special dl_server handling.
>>
>> In case of "pick_again", pick_next_task() will clear the rq->dl_server
>> before the pick and for "keep_resched" with idle task, I don't think the
>> dl_server from pick makes any difference since we'll come back into
>> schedule right after the switch.
>>
>> For rest of find_proxy_task() we don't really care about dl_server afaict
>> unless I'm missing something. did you trip something that required this
>> change?
>>
>>> +
>>> +			next = find_proxy_task(rq, donor, &rf);
>>> +			if (!next)
>>>  				goto pick_again;
>>> -			}
>>> -			if (next == rq->idle) {
>>> -				zap_balance_callbacks(rq);
>>> +			if (next == rq->idle)
>>>  				goto keep_resched;
>>> -			}
>>> -		}
>>> -		if (rq->donor == prev_donor && prev != next) {
>>> -			struct task_struct *donor = rq->donor;
>>> -			/*
>>> -			 * When transitioning like:
>>> -			 *
>>> -			 *         prev         next
>>> -			 * donor:    B            B
>>> -			 * curr:     A          B or C
>>> -			 *
>>> -			 * then put_prev_set_next_task() will not have done
>>> -			 * anything, since B == B. However, A might have
>>> -			 * missed a RT/DL balance opportunity due to being
>>> -			 * on_cpu.
>>> -			 */
>>> -			donor->sched_class->put_prev_task(rq, donor, donor);
>>> -			donor->sched_class->set_next_task(rq, donor, true);
>>> +
>>> +			rq->dl_server = donor_dl_server;
>>>  		}
>>> -	} else {
>>> -		rq_set_donor(rq, next);
>>> +
>>>  	}
>>>  
>>> +	put_prev_set_next_task(rq, rq->donor, donor);
>>> +
>>> +	if (sched_proxy_exec() &&
>>> +	    donor == rq->donor && prev != next) {
> 
> Hi Prateek,
> 
> The case I had in mind is when pick_next_task() selects the candidate
> through a DL server, and find_proxy_task() later goes through
> proxy_resched_idle().
> 
> 1. Before this patch, the sequence is effectively:
> 
> |-> pick_next_task()
> 
> |---|-> rq->dl_server = server;
> 
> |---|-> put_prev_set_next_task(rq, rq->donor, donor);
> 
> |---|---|-> donor->dl_server = rq->dl_server;
> 
> |---|---|-> rq->dl_server = NULL;
> 
> |-> find_proxy_task(...)
> 
> |---|-> proxy_resched_idle(rq);
> 
> |---|---|->put_prev_set_next_task(rq, rq->donor, rq->idle);
> 
> |---|---|---|-> rq->idle->dl_server = rq->dl_server; /* NULL */
> 
> |---|---|---|-> rq->dl_server = NULL;
> 
> 
> So find_proxy_task() is entered with rq->dl_server already consumed by
> the donor commit, i.e. rq->dl_server == NULL.
> 
> 2. After deferring put_prev_set_next_task(), without the save/clear, the
> sequence can instead become:
> 
> |-> pick_next_task()
> 
> |---|-> rq->dl_server = server;
> 
> |-> next = find_proxy_task(...)
> 
> |---|-> proxy_resched_idle(rq);
> 
> |---|---|->put_prev_set_next_task(rq, rq->donor, rq->idle);
> 
> |---|---|---|-> rq->idle->dl_server = rq->dl_server; /* server */
> 
> |---|---|---|-> rq->dl_server = NULL;
> 
> |->if (next == rq->idle) goto keep_resched;
> 
> ...
> 
> 
> In that case the DL server selected for the uncommitted donor gets
> attached to rq->idle.

I see. I guess, proxy_reshed_idle() would be a better spot to
consolidate it rather than juggling it in __schedule().

> 
> The next pick clearing rq->dl_server does not undo that assignment,
> since the pointer has already moved to rq->idle->dl_server. It is only
> cleared later when idle becomes the prev task of another
> put_prev_set_next_task() transition.

Which should be the very next __schedule() since we retain NEED_RESCHED
except if all the tasks migrate away and we genuinely select rq->idle,
then the dl_server lingers around.

> 
> 3. That is what the save/clear/restore in v3 was intended to avoid:
> 
> pick_again:
> 
> |-> pick_next_task()
> 
> |---|-> rq->dl_server = server;
> 
> |->donor_dl_server = rq->dl_server;
> 
> |-> rq->dl_server = NULL;
> 
> |-> next = find_proxy_task(...)
> 
> |---|-> proxy_resched_idle(rq);
> 
> |---|---|->put_prev_set_next_task(rq, rq->donor, rq->idle);
> 
> |---|---|---|-> rq->idle->dl_server = rq->dl_server; /* NULL */
> 
> |---|---|---|-> rq->dl_server = NULL;
> 
> |-> if (!next) goto pick_again;
> 
> |-> if (next == rq->idle) goto keep_resched;
> 
> |-> rq->dl_server = donor_dl_server;
> 
> ...
> 
> |-> put_prev_set_next_task(rq, rq->donor, donor);
> 
> 
> If proxy resolution returns NULL or idle, the saved value is not
> restored into rq->dl_server for a donor commit that will not happen.
> 
> So the intent is to keep the DL-server selection made for the candidate
> out of rq->dl_server while proxy resolution is in progress, and restore
> it only if that donor is actually going to be committed, while keeping
> the state seen by find_proxy_task() consistent with the pre-patch path.
> 
> Do you think rq->idle->dl_server being set in the second sequence above
> is harmless, or is keeping the save/clear/restore the right way to avoid
> that state?

I assume it may take the dl_defer_idle path but I'm not super sure. I
personally prefer resetting state in proxy_resched_idle() since it is
all in one place to refer at any point.

> 
> I’ll look at the sched-ext interaction and the __schedule() structure
> separately while rebasing onto current tip.

Thank you but I'd wait for Peter and John to chime in regarding the added
code complexity given the visible benefits of this optimization isn't
too much.

-- 
Thanks and Regards,
Prateek

Re: [PATCH RFC v3] sched/proxy: Defer donor commit until after proxy resolution

[Xukai Wang]

Hi Prateek,

Thanks for the detailed reply.

On 2026/10/12 17:21, K Prateek Nayak wrote:
> I see. I guess, proxy_reshed_idle() would be a better spot to
> consolidate it rather than juggling it in __schedule().
>
>> The next pick clearing rq->dl_server does not undo that assignment,
>> since the pointer has already moved to rq->idle->dl_server. It is only
>> cleared later when idle becomes the prev task of another
>> put_prev_set_next_task() transition.
> Which should be the very next __schedule() since we retain NEED_RESCHED
> except if all the tasks migrate away and we genuinely select rq->idle,
> then the dl_server lingers around.
...
>> If proxy resolution returns NULL or idle, the saved value is not
>> restored into rq->dl_server for a donor commit that will not happen.
>>
>> So the intent is to keep the DL-server selection made for the candidate
>> out of rq->dl_server while proxy resolution is in progress, and restore
>> it only if that donor is actually going to be committed, while keeping
>> the state seen by find_proxy_task() consistent with the pre-patch path.
>>
>> Do you think rq->idle->dl_server being set in the second sequence above
>> is harmless, or is keeping the save/clear/restore the right way to avoid
>> that state?
> I assume it may take the dl_defer_idle path but I'm not super sure. I
> personally prefer resetting state in proxy_resched_idle() since it is
> all in one place to refer at any point.

I agree with your suggestion. So I removed the save/clear/restore around
find_proxy_task() and now clear rq->dl_server in proxy_resched_idle()
before committing idle.

This leaves the successful-resolution path untouched, while a NULL
retry lets the next pick_next_task() reset/recompute rq->dl_server.

> On current tip + sched-ext changes lined for v7.4 from
> https://lore.kernel.org/lkml/20260922165445.943315-9-arighi@nvidia.com/
> some sched-ext bits will need rq->donor to already reflect the next
> selection before they are called

I rebased this onto the latest tip, and also applied the patch on top of
Andrea's sched_ext proxy-exec changes through the curr/donor split to
check the resulting ordering.

The rebased scheduling path now looks roughly like this:

```

pick_again:
        donor = pick_next_task(rq, &rf);
        rq->next_class = donor->sched_class;
        next = donor;

        if (sched_proxy_exec()) {
                struct task_struct *prev_donor = rq->donor;

                donor->blocked_donor = NULL;

                if (unlikely(donor->is_blocked)) {
                        next = find_proxy_task(rq, donor, &rf);
                        if (!next)
                                goto pick_again;

                        if (next == rq->idle) {
                                scx_proxy_reenqueue_retry(rq, next);
                                goto keep_resched;
                        }
                }

                put_prev_set_next_task(rq, prev_donor, donor);

                if (donor == prev_donor && prev != next) {
                        donor->sched_class->put_prev_task(rq, donor, donor);
                        donor->sched_class->set_next_task(rq, donor,
SNT_PICK);
                }

                rq_set_donor(rq, donor);

                scx_proxy_donor_start(rq);
                scx_proxy_reenqueue_retry(rq, next);
        } else {
                put_prev_set_next_task(rq, rq->donor, donor);
                rq_set_donor(rq, donor);
        }
```

and proxy_resched_idle() now does:

```

proxy_resched_idle()

{

    rq->dl_server = NULL;

    put_prev_set_next_task(...);

    ...

}

```

So the proxy-specific handling stays in one place in __schedule(), and
the existing sched_ext hooks remain in their current order.

I also looked more closely at the rq->donor dependency
in do_pick_task_scx():

```

static struct task_struct *do_pick_task_scx(...)

{

    struct task_struct *prev = rq->donor;

    ...

}

```

Suppose A is the currently committed donor and the first SCX pick
selects blocked donor D.

With the current early-commit ordering, the sequence is roughly:

```

    next = pick_next_task(); /* rq->donor is A */

        |-> pick_next_task_scx(); /* SCX pick with rq->donor
is A, selects D */

        |-> set_next_task_scx(D); /* D is removed from the local DSQ */

    rq_set_donor(rq, D); /*rq->donor is D */;

    next = find_proxy_task(D); /* if next == NULL */

    if (!next) goto pick_again;


pick_again:    

    pick_next_task(); /* rq->donor is D */

        |-> pick_next_task_scx(); /* SCX pick with prev is D, and D may
be retained through SCX_DSP_PREV. */

```


With the deferred ordering:

```

    pick_next_task(); /* rq->donor is A */

        |-> pick_next_task_scx(prev=A); /* selects D */

    next = find_proxy_task(D); /* if next == NULL */

    if (!next) goto pick_again;


pick_again:    

    pick_next_task(); /* rq->donor is A */

        |-> pick_next_task_scx(prev=A); /* D is still SCX_TASK_QUEUED
and still on the local DSQ, because set_next_task_scx() has not run yet. */

```

If find_proxy_task() did not otherwise change the relevant runqueue/SCX
state, the retry therefore starts from the same committed prev=A, while
D remains the local candidate selected by the first pass. Since the
first pass did not retain A through SCX_DSP_PREV, the retry reaches the
local DSQ again and selects D through SCX_DSP_LOCAL.

So the internal retry mechanism changes from `provisional D ->
SCX_DSP_PREV` to `committed A + queued D -> SCX_DSP_LOCAL`  without
changing the candidate in the unchanged-state case.

If find_proxy_task() deactivates or migrates D, then the runqueue state
has genuinely changed and the retry should of course select against that
new state. In that case it seems preferable for rq->donor to continue
describing the last committed scheduling context rather than committing
D provisionally and repairing that state afterwards.

Similarly, if SCX dispatch drops the rq lock, the existing
lock_drop_seq / SCX_DSP_RETRY handling already restarts selection
against the new state, so I don't see a need for an early rq->donor
update there either.

For a successful proxy resolution, the existing sched_ext ordering still
works unchanged:

```

find_proxy_task(D);

put_prev_set_next_task(..., D);

rq_set_donor(D);

scx_proxy_donor_start(); /* ops.running(D) is still issued only after D
has become the committed rq->donor. */

```

Given this, I don't see an additional sched_ext complexity introduced by
deferring the commit; the main difference is that rq->donor continues to
describe the committed scheduling context while proxy resolution is
still provisional.

Also, I probably wouldn't characterize the main benefit of this change
as a performance optimization. The end-to-end performance impact I
measured is indeed small.

The main benefit I am after is the scheduling-state semantics. The
invariant I am trying to preserve is that rq->donor and the
scheduler-class current state describe a scheduling context which has
survived proxy resolution, rather than a provisional candidate which
may still be deactivated, migrated, or cause the proxy walk to retry.

In other words, the intended ordering is:

1. pick candidate

2. resolve proxy chain

3. commit donor / scheduler-class state

rather than:

1. pick candidate

    -> commit donor / scheduler-class state

2. resolve proxy chain

    -> possibly discard that candidate and retry 1.


The reduction in redundant put_prev_task()/set_next_task() callbacks is
a consequence of keeping that commit boundary after proxy resolution,
rather than the primary reason for the change.

The sched_ext integration also seems to illustrate why keeping the
commit boundary after proxy resolution is useful. In Andrea's
'sched_ext: Fix ops.running/stopping() pairing for proxy-exec donors'
commit (5f882051efabaf19f9b4dbe4b50cee1d250ac19e), set_next_task_scx()
has to treat a blocked donor specially and defer ops.running() to
scx_proxy_donor_start(), specifically because proxy resolution can still
fail or migrate the donor after set_next_task_scx() has run.

> Thank you but I'd wait for Peter and John to chime in regarding the added
> code complexity given the visible benefits of this optimization isn't
> too much.

Thanks. I'll wait for Peter/John's thoughts before sending v4.

-- 
Best regards,
Xukai