Optimized Donor Migration for Proxy Execution
Subsystems (4)
v29 cover Optimized Donor Migration for Proxy Execution
Hey All,
This is just the next step for Proxy Execution, allowing proxy-return-migration to occur from the try_to_wake_up() callpath. This however introduces some complexity as it means the rq->donor can change from a try_to_wake_up() call, and thus there are further changes needed to make this safe.
As always, I’m trying to submit this larger work in smallish digestible pieces, so in this portion of the series, I’m only submitting for review and consideration these patches which minimize the use of passing “prev” (ie: the rq->donor) down through the class schedulers since this cached value can become invalid across runqueue lock drops. From there, there are some fixups to the deadline class scheduler to avoid issues when the donor changes, then the actual patch to do return migration from ttwu() followed by the added is_blocked state to avoid the workqueue stall issue, and some optimization patches so we migrate the entire task chain in proxy_migrate_task().
Apologies for being sort of slow on recent replies, I was hoping to get this out last week, but caught whatever is going around and was out for a bit (but the series did get a few extra days of test runtime, without issue, so that’s good).
Big thanks to K Prateek and PeterZ for their feedback and suggestions on the new changes.
New in this iteration:
* Vineeth Pillai diagnosed and reported subtle workqueue stalls being caused by __schedule(SM_PREEMPT) calls racing with __schedule(SM_NONE) calls made after the task’s blocked_on is set, causing tasks to be deactivated without running task workqueue work. Peter suggested that we use an is_blocked flag to better track when a task should have been blocked (even if we don’t block and keep it on the rq for proxy-exec). This solution requires the ttwu() changes in this series, so it was included in this set.
* Use scoped_guard() in proxy_needs_return() to avoid calling block_task() while holding blocked_lock (a Sashiko suggestion)
* Minor cleanup in deadline changes
I’d love to get further feedback on any place where these patches are confusing, or could use additional clarifications.
Additionally I’d appreciate any testing or comments that folks have with the full Proxy Execution series! You can find the full Proxy Exec series here:
https://github.com/johnstultz-work/linux-dev/commits/proxy-exec-v29-7.1-rc3/
https://github.com/johnstultz-work/linux-dev.git proxy-exec-v29-7.1-rc3
New changes to the full patch series in this revision include:
* Pulled in Vineeth’s waitqueue stall reproducer
Issues still to address with the full series:
* Really excited to see Andrea’s series taking a stab at better getting sched_ext and Proxy Execution to play nice with each other. I hope to try to integrate his series into my full stack for testing:
https://lore.kernel.org/lkml/[email protected]/
* Reevaluate performance regression K Prateek Nayak found with the full series.
* The chain migration functionality needs further iterations and better validation to ensure it truly maintains the RT/DL load balancing invariants (despite this being broken in vanilla upstream with RT_PUSH_IPI currently)
Future work:
* Expand to more locking primitives: Figuring out pi-futexes would be good, using proxy for Binder PI is something else we’re exploring.
* Eventually: Work to replace rt_mutexes and get things happy with PREEMPT_RT
Credit/Disclaimer:
—--------------------
As always, this Proxy Execution series has a long history with lots of developers that deserve credit:
First described in a paper[1] by Watkins, Straub, Niehaus, then from patches from Peter Zijlstra, extended with lots of work by Juri Lelli, Valentin Schneider, and Connor O'Brien. (and thank you to Steven Rostedt for providing additional details here!). Thanks also to Joel Fernandes, Dietmar Eggemann, Metin Kaya, K Prateek Nayak and Suleiman Souhlal for their substantial review, suggestion, and patch contributions.
So again, many thanks to those above, as all the credit for this series really is due to them - while the mistakes are surely mine.
Thanks so much!
-john
[1] https://static.lwn.net/images/conf/rtlws11/papers/proc/p38.pdf
John Stultz (8):
sched: Rework pick_next_task() and prev_balance() to avoid stale prev
references
sched: deadline: Add some helper variables to cleanup deadline logic
sched: deadline: Add dl_rq->curr pointer to address issues with Proxy
Exec
sched: Rework block_task so it can be directly called
sched: Have try_to_wake_up() handle return-migration for PROXY_WAKING
case
sched: Add is_blocked task flag
sched: Break out core of attach_tasks() helper into sched.h
sched: Migrate whole chain in proxy_migrate_task()
Peter Zijlstra (1):
sched: Add blocked_donor link to task for smarter mutex handoffs
include/linux/sched.h | 10 +- init/init_task.c | 1 + kernel/fork.c | 1 + kernel/locking/mutex.c | 43 +++++- kernel/sched/core.c | 322 +++++++++++++++++++++------------------ kernel/sched/deadline.c | 46 ++++-- kernel/sched/fair.c | 25 +-- kernel/sched/idle.c | 2 +- kernel/sched/rt.c | 8 +- kernel/sched/sched.h | 30 +++- kernel/sched/stop_task.c | 2 +- 11 files changed, 300 insertions(+), 190 deletions(-)
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
PATCH v29 1/9 sched: Rework pick_next_task() and prev_balance() to avoid stale prev references
Historically, the prev value from __schedule() was the rq->curr. This prev value is passed down through numerous functions, and used in the class scheduler implementations. The fact that prev was on_cpu until the end of __schedule(), meant it was stable across the rq lock drops that the class->pick_next_task() and ->balance() implementations often do.
However, with proxy-exec, the prev passed to functions called by __schedule() is rq->donor, which may not be the same as rq->curr and may not be on_cpu, this makes the prev value potentially unstable across rq lock drops.
A recently found issue with proxy-exec, is when we begin doing return migration from try_to_wake_up(), its possible we may be waking up the rq->donor. When we do this, we proxy_resched_idle() to put_prev_set_next() setting the rq->donor to rq->idle, allowing the rq->donor to be return migrated and allowed to run.
This however runs into trouble, as on another cpu we might be in the middle of calling __schedule(). Conceptually the rq lock is held for the majority of the time, but in calling pick_next_task() its possible the class->pick_next_task() handler or the ->balance() call may briefly drop the rq lock. This opens a window for try_to_wake_up() to wake and return migrate the rq->donor before the class logic reacquires the rq lock.
Unfortunately pick_next_task() and prev_balance() pass in a prev argument, to which we pass rq->donor. However this prev value can now become stale and incorrect across a rq lock drop.
So, to correct this, rework the pick_next_task() and prev_balance() calls so that they do not take a "prev" argument.
Also rework the class ->pick_next_task() and ->balance() implementations to drop the prev argument, and in the cases where it was used, and have the class functions reference rq->donor directly, and not save the value across rq lock drops so that we don't end up with a stale references.
---
Cc: Joel Fernandes <[email protected]>
Cc: Qais Yousef <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Ben Segall <[email protected]>
Cc: Zimuzo Ezeozue <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Metin Kaya <[email protected]>
Cc: Xuewen Yan <[email protected]>
Cc: K Prateek Nayak <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Suleiman Souhlal <[email protected]>
Cc: kuyo chang <[email protected]>
Cc: hupu <[email protected]>
Cc: [email protected]
--- kernel/sched/core.c | 37 ++++++++++++++++++------------------- kernel/sched/deadline.c | 8 +++++++- kernel/sched/fair.c | 9 +++++++-- kernel/sched/idle.c | 2 +- kernel/sched/rt.c | 8 +++++++- kernel/sched/sched.h | 10 ++++------ kernel/sched/stop_task.c | 2 +- 7 files changed, 45 insertions(+), 31 deletions(-)
kernel/sched/core.c +18 -19
@@ -5972,10 +5972,9 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)schedstat_inc (this_rq ()->sched_count); } -static void prev_balance (struct rq *rq,struct task_struct *prev, -struct rq_flags *rf) +static void prev_balance (struct rq *rq,struct rq_flags *rf) { -const struct sched_class *start_class = prev->sched_class; +const struct sched_class *start_class = rq->donor->sched_class;const struct sched_class *class;/*
@@ -5987,7 +5986,7 @@ static void prev_balance(struct rq *rq, struct task_struct *prev,* a runnable task of @class priority or higher. */ for_active_class_range(class, start_class, &idle_sched_class) { -if (class->balance && class->balance (rq, prev, rf)) +if (class->balance && class->balance (rq, rf))break ; } }
@@ -5996,7 +5995,7 @@ static void prev_balance(struct rq *rq, struct task_struct *prev,* Pick up the highest-prio task: */ static inline struct task_struct * -__pick_next_task (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +__pick_next_task (struct rq *rq,struct rq_flags *rf)__must_hold (__rq_lockp (rq)) {const struct sched_class *class;
@@ -6013,28 +6012,28 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)* higher scheduling class, because otherwise those lose the * opportunity to pull in more work from other CPUs. */ -if (likely (!sched_class_above (prev->sched_class, &fair_sched_class) && +if (likely (!sched_class_above (rq->donor->sched_class, &fair_sched_class) && rq->nr_running == rq->cfs.h_nr_queued)) { - p =pick_next_task_fair (rq, prev, rf); + p =pick_next_task_fair (rq, rf);if (unlikely (p ==RETRY_TASK )) goto restart;/* Assume the next prioritized class is idle_sched_class */ if (!p) { p =pick_task_idle (rq, rf); -put_prev_set_next_task (rq, prev, p); +put_prev_set_next_task (rq, rq->donor, p); }return p; } restart: -prev_balance (rq, prev, rf); +prev_balance (rq, rf);for_each_active_class (class) {if (class->pick_next_task) { - p = class->pick_next_task (rq, prev, rf); + p = class->pick_next_task (rq, rf);if (unlikely (p ==RETRY_TASK )) goto restart;if (p)
@@ -6044,7 +6043,7 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)if (unlikely (p ==RETRY_TASK )) goto restart;if (p) { -put_prev_set_next_task (rq, prev, p); +put_prev_set_next_task (rq, rq->donor, p);return p; } }
@@ -6097,7 +6096,7 @@ extern void task_vruntime_update(struct rq *rq, struct task_struct *p, bool in_fstatic void queue_core_balance (struct rq *rq);static struct task_struct * -pick_next_task (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +pick_next_task (struct rq *rq,struct rq_flags *rf)__must_hold (__rq_lockp (rq)) {struct task_struct *next, *p, *max;
@@ -6110,7 +6109,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)bool need_sync;if (!sched_core_enabled (rq)) -return __pick_next_task (rq, prev, rf); +return __pick_next_task (rq, rf); cpu =cpu_of (rq);
@@ -6123,7 +6122,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)*/ rq->core_pick =NULL ; rq->core_dl_server =NULL ; -return __pick_next_task (rq, prev, rf); +return __pick_next_task (rq, rf); }/*
@@ -6147,7 +6146,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) goto out_set_next; } -prev_balance (rq, prev, rf); +prev_balance (rq, rf); smt_mask =cpu_smt_mask (cpu); need_sync = !!rq->core->core_cookie;
@@ -6329,7 +6328,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) } out_set_next: -put_prev_set_next_task (rq, prev, next); +put_prev_set_next_task (rq, rq->donor, next);if (rq->core->core_forceidle_count && next == rq->idle)queue_core_balance (rq);
@@ -6552,10 +6551,10 @@ static inline void sched_core_cpu_deactivate(unsigned int cpu) {}static inline void sched_core_cpu_dying (unsigned int cpu) {}static struct task_struct * -pick_next_task (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +pick_next_task (struct rq *rq,struct rq_flags *rf)__must_hold (__rq_lockp (rq)) { -return __pick_next_task (rq, prev, rf); +return __pick_next_task (rq, rf); }#endif /* !CONFIG_SCHED_CORE */
@@ -7102,7 +7101,7 @@ static void __sched notrace __schedule(int sched_mode) pick_again:assert_balance_callbacks_empty (rq); - next =pick_next_task (rq, rq->donor, &rf); + next =pick_next_task (rq, &rf); rq->next_class = next->sched_class;if (sched_proxy_exec ()) {struct task_struct *prev_donor = rq->donor;
kernel/sched/deadline.c +7 -1
@@ -2506,8 +2506,14 @@ static void check_preempt_equal_dl(struct rq *rq, struct task_struct *p)resched_curr (rq); } -static int balance_dl (struct rq *rq,struct task_struct *p,struct rq_flags *rf) +static int balance_dl (struct rq *rq,struct rq_flags *rf) { +/* +* Note, rq->donor may change during rq lock drops, +* so don't re-use prev across lock drops +*/ +struct task_struct *p = rq->donor; +if (!on_dl_rq (&p->dl) &&need_pull_dl_task (rq, p)) {/* * This is OK, because current is on_cpu, which avoids it being
kernel/sched/fair.c +7 -2
@@ -9231,14 +9231,19 @@ static void __set_next_task_fair(struct rq *rq, struct task_struct *p, bool firsstatic void set_next_task_fair (struct rq *rq,struct task_struct *p,bool first);struct task_struct * -pick_next_task_fair (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +pick_next_task_fair (struct rq *rq,struct rq_flags *rf)__must_hold (__rq_lockp (rq)) {struct sched_entity *se; -struct task_struct *p; +struct task_struct *p, *prev;int new_tasks; again: +/* +* Re-read rq->donor at the top as it may have +* changed across a rq lock drop +*/ + prev = rq->donor; p =pick_task_fair (rq, rf);if (!p) goto idle;
kernel/sched/idle.c +1 -1
@@ -462,7 +462,7 @@ select_task_rq_idle(struct task_struct *p, int cpu, int flags) }static int -balance_idle (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +balance_idle (struct rq *rq,struct rq_flags *rf) {return WARN_ON_ONCE (1 ); }
kernel/sched/rt.c +7 -1
@@ -1596,8 +1596,14 @@ static void check_preempt_equal_prio(struct rq *rq, struct task_struct *p)resched_curr (rq); } -static int balance_rt (struct rq *rq,struct task_struct *p,struct rq_flags *rf) +static int balance_rt (struct rq *rq,struct rq_flags *rf) { +/* +* Note, rq->donor may change during rq lock drops, +* so don't re-use p across lock drops +*/ +struct task_struct *p = rq->donor; +if (!on_rt_rq (&p->rt) &&need_pull_rt_task (rq, p)) {/* * This is OK, because current is on_cpu, which avoids it being
kernel/sched/sched.h +4 -6
@@ -2561,7 +2561,7 @@ struct sched_class {/* * schedule/pick_next_task/prev_balance: rq->lock */ -int (*balance)(struct rq *rq,struct task_struct *prev,struct rq_flags *rf); +int (*balance)(struct rq *rq,struct rq_flags *rf);/* * schedule/pick_next_task: rq->lock
@@ -2572,12 +2572,11 @@ struct sched_class {* * next = pick_task(); * if (next) { -* put_prev_task(prev); +* put_prev_task(rq->donor); * set_next_task_first(next); * } */ -struct task_struct *(*pick_next_task)(struct rq *rq,struct task_struct *prev, -struct rq_flags *rf); +struct task_struct *(*pick_next_task)(struct rq *rq,struct rq_flags *rf);/* * sched_change:
@@ -2801,8 +2800,7 @@ static inline bool sched_fair_runnable(struct rq *rq)return rq->cfs.nr_queued >0 ; } -extern struct task_struct *pick_next_task_fair (struct rq *rq,struct task_struct *prev, -struct rq_flags *rf); +extern struct task_struct *pick_next_task_fair (struct rq *rq,struct rq_flags *rf);extern struct task_struct *pick_task_idle (struct rq *rq,struct rq_flags *rf);#define SCA_CHECK 0x01
kernel/sched/stop_task.c +1 -1
@@ -16,7 +16,7 @@ select_task_rq_stop(struct task_struct *p, int cpu, int flags) }static int -balance_stop (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +balance_stop (struct rq *rq,struct rq_flags *rf) {return sched_stop_runnable (rq); }
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
Peter Zijlstra
maintainerRe: [PATCH v29 1/9] sched: Rework pick_next_task() and prev_balance() to avoid stale prev references
Quoted 33 lines · John Stultz
Historically, the prev value from
__schedule()was therq->curr. This prev value is passed down through numerous functions, and used in the class scheduler implementations. The fact that prev wason_cpuuntil the end of__schedule(), meant it was stable across the rq lock drops that theclass->pick_next_task()and ->balance() implementations often do.However, with proxy-exec, the prev passed to functions called by
__schedule()isrq->donor, which may not be the same asrq->currand may not beon_cpu, this makes the prev value potentially unstable across rq lock drops.A recently found issue with proxy-exec, is when we begin doing return migration from
try_to_wake_up(), its possible we may be waking up therq->donor. When we do this, weproxy_resched_idle()toput_prev_set_next()setting therq->donortorq->idle, allowing therq->donorto be return migrated and allowed to run.This however runs into trouble, as on another cpu we might be in the middle of calling
__schedule(). Conceptually the rq lock is held for the majority of the time, but in callingpick_next_task()its possible theclass->pick_next_task()handler or the ->balance() call may briefly drop the rq lock. This opens a window fortry_to_wake_up()to wake and return migrate therq->donorbefore the class logic reacquires the rq lock.Unfortunately
pick_next_task()andprev_balance()pass in a prev argument, to which we passrq->donor. However this prev value can now become stale and incorrect across a rq lock drop.So, to correct this, rework the
pick_next_task()andprev_balance()calls so that they do not take a "prev" argument.
Quoted 7 lines
Also rework the class ->pick_next_task() and ->balance() implementations to drop the prev argument, and in the cases where it was used, and have the class functions reference
rq->donordirectly, and not save the value across rq lock drops so that we don't end up with a stale references.Signed-off-by John Stultz <[email protected]>
Right, so I think I'll take patches 1,2,3,8,9 from the flat series and munge this on top. That gets rid of all that pick_next_task() nonsense and simplifies things a little.
Re: [PATCH v29 1/9] sched: Rework pick_next_task() and prev_balance() to avoid stale prev references
Quoted 40 lines · Peter Zijlstra
Quoted 39 lines · John Stultz
Historically, the prev value from
__schedule()was therq->curr. This prev value is passed down through numerous functions, and used in the class scheduler implementations. The fact that prev wason_cpuuntil the end of__schedule(), meant it was stable across the rq lock drops that theclass->pick_next_task()and ->balance() implementations often do.However, with proxy-exec, the prev passed to functions called by
__schedule()isrq->donor, which may not be the same asrq->currand may not beon_cpu, this makes the prev value potentially unstable across rq lock drops.A recently found issue with proxy-exec, is when we begin doing return migration from
try_to_wake_up(), its possible we may be waking up therq->donor. When we do this, weproxy_resched_idle()toput_prev_set_next()setting therq->donortorq->idle, allowing therq->donorto be return migrated and allowed to run.This however runs into trouble, as on another cpu we might be in the middle of calling
__schedule(). Conceptually the rq lock is held for the majority of the time, but in callingpick_next_task()its possible theclass->pick_next_task()handler or the ->balance() call may briefly drop the rq lock. This opens a window fortry_to_wake_up()to wake and return migrate therq->donorbefore the class logic reacquires the rq lock.Unfortunately
pick_next_task()andprev_balance()pass in a prev argument, to which we passrq->donor. However this prev value can now become stale and incorrect across a rq lock drop.So, to correct this, rework the
pick_next_task()andprev_balance()calls so that they do not take a "prev" argument.Also rework the class ->pick_next_task() and ->balance() implementations to drop the prev argument, and in the cases where it was used, and have the class functions reference
rq->donordirectly, and not save the value across rq lock drops so that we don't end up with a stale references.
Quoted 5 lines
Quoted 1 line
Signed-off-by John Stultz <[email protected]>Right, so I think I'll take patches 1,2,3,8,9 from the flat series and munge this on top. That gets rid of all that
pick_next_task()nonsense and simplifies things a little.
Sounds good to me! Let me know when you have a branch posted that I should use as a base for any further iterations on the rest of the proxy chagnes I submitted.
thanks
-john
Peter Zijlstra
maintainerRe: [PATCH v29 1/9] sched: Rework pick_next_task() and prev_balance() to avoid stale prev references
Quoted 43 lines · John Stultz
Quoted 42 lines · Peter Zijlstra
Quoted 40 lines · John Stultz
Historically, the prev value from
__schedule()was therq->curr. This prev value is passed down through numerous functions, and used in the class scheduler implementations. The fact that prev wason_cpuuntil the end of__schedule(), meant it was stable across the rq lock drops that theclass->pick_next_task()and ->balance() implementations often do.However, with proxy-exec, the prev passed to functions called by
__schedule()isrq->donor, which may not be the same asrq->currand may not beon_cpu, this makes the prev value potentially unstable across rq lock drops.A recently found issue with proxy-exec, is when we begin doing return migration from
try_to_wake_up(), its possible we may be waking up therq->donor. When we do this, weproxy_resched_idle()toput_prev_set_next()setting therq->donortorq->idle, allowing therq->donorto be return migrated and allowed to run.This however runs into trouble, as on another cpu we might be in the middle of calling
__schedule(). Conceptually the rq lock is held for the majority of the time, but in callingpick_next_task()its possible theclass->pick_next_task()handler or the ->balance() call may briefly drop the rq lock. This opens a window fortry_to_wake_up()to wake and return migrate therq->donorbefore the class logic reacquires the rq lock.Unfortunately
pick_next_task()andprev_balance()pass in a prev argument, to which we passrq->donor. However this prev value can now become stale and incorrect across a rq lock drop.So, to correct this, rework the
pick_next_task()andprev_balance()calls so that they do not take a "prev" argument.Also rework the class ->pick_next_task() and ->balance() implementations to drop the prev argument, and in the cases where it was used, and have the class functions reference
rq->donordirectly, and not save the value across rq lock drops so that we don't end up with a stale references.Signed-off-by John Stultz <[email protected]>
Quoted 7 lines
Quoted 3 lines
Right, so I think I'll take patches 1,2,3,8,9 from the flat series and munge this on top. That gets rid of all that
pick_next_task()nonsense and simplifies things a little.Sounds good to me! Let me know when you have a branch posted that I should use as a base for any further iterations on the rest of the proxy chagnes I submitted.
I have some random bits in queue:sched/proxy, this is more a work-in-progress branch and will be rebased. It doesn't (yet) include the full series you posted simply because I didn't get around to looking at the remaining few patches yet.
Aside from doing that blocked_donor review, I was also side-tracked by looking at doing some further cleanups wrt is_blocked.
[tip: sched/core] sched: Rework prev_balance() to avoid stale prev references
In reply to John StultzThe following commit has been merged into the sched/core branch of tip:
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 7a3a6bfbd62a2ba3e0ef1e92d6b71abb66890825 Gitweb: https://git.kernel.org/tip/7a3a6bfbd62a2ba3e0ef1e92d6b71abb66890825 Author: John Stultz <[email protected]> AuthorDate: Tue, 12 May 2026 02:56:11 Committer: Peter Zijlstra <[email protected]> CommitterDate: Tue, 02 Jun 2026 12:26:06 +02:00
sched: Rework prev_balance() to avoid stale prev references
Historically, the prev value from __schedule() was the rq->curr. This prev value is passed down through numerous functions, and used in the class scheduler implementations. The fact that prev was on_cpu until the end of __schedule(), meant it was stable across the rq lock drops that the class->balance() implementations often do.
However, with proxy-exec, the prev passed to functions called by __schedule() is rq->donor, which may not be the same as rq->curr and may not be on_cpu, this makes the prev value potentially unstable across rq lock drops.
A recently found issue with proxy-exec, is when we begin doing return migration from try_to_wake_up(), its possible we may be waking up the rq->donor. When we do this, we proxy_resched_idle() to put_prev_set_next() setting the rq->donor to rq->idle, allowing the rq->donor to be return migrated and allowed to run.
This however runs into trouble, as on another cpu we might be in the middle of calling __schedule(). Conceptually the rq lock is held for the majority of the time, but in calling prev_balance() its possible the class->balance() handler call may briefly drop the rq lock. This opens a window for try_to_wake_up() to wake and return migrate the rq->donor before the class logic reacquires the rq lock.
Unfortunately prev_balance() pass in a prev argument, to which we pass rq->donor. However this prev value can now become stale and incorrect across a rq lock drop.
So, to correct this, rework the prev_balance() call so that it does not take a "prev" argument.
--- kernel/sched/core.c | 33 ++++++++++++++++----------------- kernel/sched/deadline.c | 8 +++++++- kernel/sched/idle.c | 2 +- kernel/sched/rt.c | 8 +++++++- kernel/sched/sched.h | 2 +- kernel/sched/stop_task.c | 2 +- 6 files changed, 33 insertions(+), 22 deletions(-)
kernel/sched/core.c +16 -17
@@ -5986,10 +5986,9 @@ static inline void schedule_debug(struct task_struct *prev, bool preempt)schedstat_inc (this_rq ()->sched_count); } -static void prev_balance (struct rq *rq,struct task_struct *prev, -struct rq_flags *rf) +static void prev_balance (struct rq *rq,struct rq_flags *rf) { -const struct sched_class *start_class = prev->sched_class; +const struct sched_class *start_class = rq->donor->sched_class;const struct sched_class *class;/*
@@ -6001,7 +6000,7 @@ static void prev_balance(struct rq *rq, struct task_struct *prev,* a runnable task of @class priority or higher. */ for_active_class_range(class, start_class, &idle_sched_class) { -if (class->balance && class->balance (rq, prev, rf)) +if (class->balance && class->balance (rq, rf))break ; } }
@@ -6010,7 +6009,7 @@ static void prev_balance(struct rq *rq, struct task_struct *prev,* Pick up the highest-prio task: */ static inline struct task_struct * -__pick_next_task (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +__pick_next_task (struct rq *rq,struct rq_flags *rf)__must_hold (__rq_lockp (rq)) {const struct sched_class *class;
@@ -6027,7 +6026,7 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)* higher scheduling class, because otherwise those lose the * opportunity to pull in more work from other CPUs. */ -if (likely (!sched_class_above (prev->sched_class, &fair_sched_class) && +if (likely (!sched_class_above (rq->donor->sched_class, &fair_sched_class) && rq->nr_running == rq->cfs.h_nr_queued)) { p =pick_task_fair (rq, rf);
@@ -6038,19 +6037,19 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)if (!p) p =pick_task_idle (rq, rf); -put_prev_set_next_task (rq, prev, p); +put_prev_set_next_task (rq, rq->donor, p);return p; } restart: -prev_balance (rq, prev, rf); +prev_balance (rq, rf);for_each_active_class (class) { p = class->pick_task (rq, rf);if (unlikely (p ==RETRY_TASK )) goto restart;if (p) { -put_prev_set_next_task (rq, prev, p); +put_prev_set_next_task (rq, rq->donor, p);return p; } }
@@ -6102,7 +6101,7 @@ extern void task_vruntime_update(struct rq *rq, struct task_struct *p, bool in_fstatic void queue_core_balance (struct rq *rq);static struct task_struct * -pick_next_task (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +pick_next_task (struct rq *rq,struct rq_flags *rf)__must_hold (__rq_lockp (rq)) {struct task_struct *next, *p, *max;
@@ -6115,7 +6114,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)bool need_sync;if (!sched_core_enabled (rq)) -return __pick_next_task (rq, prev, rf); +return __pick_next_task (rq, rf); cpu =cpu_of (rq);
@@ -6128,7 +6127,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)*/ rq->core_pick =NULL ; rq->core_dl_server =NULL ; -return __pick_next_task (rq, prev, rf); +return __pick_next_task (rq, rf); }/*
@@ -6152,7 +6151,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) goto out_set_next; } -prev_balance (rq, prev, rf); +prev_balance (rq, rf); smt_mask =cpu_smt_mask (cpu); need_sync = !!rq->core->core_cookie;
@@ -6334,7 +6333,7 @@ restart_multi: } out_set_next: -put_prev_set_next_task (rq, prev, next); +put_prev_set_next_task (rq, rq->donor, next);if (rq->core->core_forceidle_count && next == rq->idle)queue_core_balance (rq);
@@ -6557,10 +6556,10 @@ static inline void sched_core_cpu_deactivate(unsigned int cpu) {}static inline void sched_core_cpu_dying (unsigned int cpu) {}static struct task_struct * -pick_next_task (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +pick_next_task (struct rq *rq,struct rq_flags *rf)__must_hold (__rq_lockp (rq)) { -return __pick_next_task (rq, prev, rf); +return __pick_next_task (rq, rf); }#endif /* !CONFIG_SCHED_CORE */
@@ -7108,7 +7107,7 @@ static void __sched notrace __schedule(int sched_mode) pick_again:assert_balance_callbacks_empty (rq); - next =pick_next_task (rq, rq->donor, &rf); + next =pick_next_task (rq, &rf); rq->next_class = next->sched_class;if (sched_proxy_exec ()) {struct task_struct *prev_donor = rq->donor;
kernel/sched/deadline.c +7 -1
@@ -2698,8 +2698,14 @@ static void check_preempt_equal_dl(struct rq *rq, struct task_struct *p)resched_curr (rq); } -static int balance_dl (struct rq *rq,struct task_struct *p,struct rq_flags *rf) +static int balance_dl (struct rq *rq,struct rq_flags *rf) { +/* +* Note, rq->donor may change during rq lock drops, +* so don't re-use prev across lock drops +*/ +struct task_struct *p = rq->donor; +if (!on_dl_rq (&p->dl) &&need_pull_dl_task (rq, p)) {/* * This is OK, because current is on_cpu, which avoids it being
kernel/sched/idle.c +1 -1
@@ -462,7 +462,7 @@ select_task_rq_idle(struct task_struct *p, int cpu, int flags) }static int -balance_idle (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +balance_idle (struct rq *rq,struct rq_flags *rf) {return WARN_ON_ONCE (1 ); }
kernel/sched/rt.c +7 -1
@@ -1596,8 +1596,14 @@ static void check_preempt_equal_prio(struct rq *rq, struct task_struct *p)resched_curr (rq); } -static int balance_rt (struct rq *rq,struct task_struct *p,struct rq_flags *rf) +static int balance_rt (struct rq *rq,struct rq_flags *rf) { +/* +* Note, rq->donor may change during rq lock drops, +* so don't re-use p across lock drops +*/ +struct task_struct *p = rq->donor; +if (!on_rt_rq (&p->rt) &&need_pull_rt_task (rq, p)) {/* * This is OK, because current is on_cpu, which avoids it being
kernel/sched/sched.h +1 -1
@@ -2587,7 +2587,7 @@ struct sched_class {/* * schedule/pick_next_task/prev_balance: rq->lock */ -int (*balance)(struct rq *rq,struct task_struct *prev,struct rq_flags *rf); +int (*balance)(struct rq *rq,struct rq_flags *rf);/* * schedule/pick_next_task: rq->lock
kernel/sched/stop_task.c +1 -1
@@ -16,7 +16,7 @@ select_task_rq_stop(struct task_struct *p, int cpu, int flags) }static int -balance_stop (struct rq *rq,struct task_struct *prev,struct rq_flags *rf) +balance_stop (struct rq *rq,struct rq_flags *rf) {return sched_stop_runnable (rq); }
PATCH v29 2/9 sched: deadline: Add some helper variables to cleanup deadline logic
In reply to John StultzAs part of an improvement to handling pushable deadline tasks, Peter suggested this cleanup[1], to use helper values for dl_entity and dl_rq in the enqueue_task_dl() and put_prev_task_dl() functions. There should be no functional change from this patch.
To make sure this cleanup change doesn't obscure later logic changes, I've split it into its own patch.
[1]: https://lore.kernel.org/lkml/[email protected]/
---
v29:
* Replace a missed p->dl usage with dl_se
--- kernel/sched/deadline.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
kernel/sched/deadline.c +14 -11
@@ -2292,7 +2292,10 @@ static void dequeue_dl_entity(struct sched_dl_entity *dl_se, int flags)static void enqueue_task_dl (struct rq *rq,struct task_struct *p,int flags) { -if (is_dl_boosted (&p->dl)) { +struct sched_dl_entity *dl_se = &p->dl; +struct dl_rq *dl_rq = &rq->dl; + +if (is_dl_boosted (dl_se)) {/* * Because of delays in the detection of the overrun of a * thread's runtime, it might be the case that a thread
@@ -2305,14 +2308,14 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)* * In this case, the boost overrides the throttle. */ -if (p->dl.dl_throttled) { +if (dl_se->dl_throttled) {/* * The replenish timer needs to be canceled. No * problem if it fires concurrently: boosted threads * are ignored in dl_task_timer(). */ -cancel_replenish_timer (&p->dl); - p->dl.dl_throttled =0 ; +cancel_replenish_timer (dl_se); + dl_se->dl_throttled =0 ; } }else if (!dl_prio (p->normal_prio)) {/*
@@ -2324,7 +2327,7 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)* being boosted again with no means to replenish the runtime and clear * the throttle. */ - p->dl.dl_throttled =0 ; + dl_se->dl_throttled =0 ;if (!(flags &ENQUEUE_REPLENISH ))printk_deferred_once ("sched: DL de-boosted task PID %d: REPLENISH flag missing\n" ,task_pid_nr (p));
@@ -2333,20 +2336,20 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags) }check_schedstat_required (); -update_stats_wait_start_dl (dl_rq_of_se (&p->dl), &p->dl); +update_stats_wait_start_dl (dl_rq, dl_se);if (p->on_rq ==TASK_ON_RQ_MIGRATING ) flags |=ENQUEUE_MIGRATING ; -enqueue_dl_entity (&p->dl, flags); +enqueue_dl_entity (dl_se, flags); -if (dl_server (&p->dl)) +if (dl_server (dl_se))return ;if (task_is_blocked (p))return ; -if (!task_current (rq, p) && !p->dl.dl_throttled && p->nr_cpus_allowed >1 ) +if (!task_current (rq, p) && !dl_se->dl_throttled && p->nr_cpus_allowed >1 )enqueue_pushable_dl_task (rq, p); }
@@ -2643,7 +2646,7 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_sstruct sched_dl_entity *dl_se = &p->dl;struct dl_rq *dl_rq = &rq->dl; -if (on_dl_rq (&p->dl)) +if (on_dl_rq (dl_se))update_stats_wait_start_dl (dl_rq, dl_se);update_curr_dl (rq);
@@ -2653,7 +2656,7 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_sif (task_is_blocked (p))return ; -if (on_dl_rq (&p->dl) && p->nr_cpus_allowed >1 ) +if (on_dl_rq (dl_se) && p->nr_cpus_allowed >1 )enqueue_pushable_dl_task (rq, p); }
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
[tip: sched/core] sched: deadline: Add some helper variables to cleanup deadline logic
The following commit has been merged into the sched/core branch of tip:
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 96a6988fb595ab1d77f60b33ea392b2e15b68605 Gitweb: https://git.kernel.org/tip/96a6988fb595ab1d77f60b33ea392b2e15b68605 Author: John Stultz <[email protected]> AuthorDate: Tue, 12 May 2026 02:56:12 Committer: Peter Zijlstra <[email protected]> CommitterDate: Tue, 02 Jun 2026 12:26:06 +02:00
sched: deadline: Add some helper variables to cleanup deadline logic
As part of an improvement to handling pushable deadline tasks, Peter suggested this cleanup[1], to use helper values for dl_entity and dl_rq in the enqueue_task_dl() and put_prev_task_dl() functions. There should be no functional change from this patch.
To make sure this cleanup change doesn't obscure later logic changes, I've split it into its own patch.
[1]: https://lore.kernel.org/lkml/[email protected]/
--- kernel/sched/deadline.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
kernel/sched/deadline.c +14 -11
@@ -2484,7 +2484,10 @@ static void dequeue_dl_entity(struct sched_dl_entity *dl_se, int flags)static void enqueue_task_dl (struct rq *rq,struct task_struct *p,int flags) { -if (is_dl_boosted (&p->dl)) { +struct sched_dl_entity *dl_se = &p->dl; +struct dl_rq *dl_rq = &rq->dl; + +if (is_dl_boosted (dl_se)) {/* * Because of delays in the detection of the overrun of a * thread's runtime, it might be the case that a thread
@@ -2497,14 +2500,14 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)* * In this case, the boost overrides the throttle. */ -if (p->dl.dl_throttled) { +if (dl_se->dl_throttled) {/* * The replenish timer needs to be canceled. No * problem if it fires concurrently: boosted threads * are ignored in dl_task_timer(). */ -cancel_replenish_timer (&p->dl); - p->dl.dl_throttled =0 ; +cancel_replenish_timer (dl_se); + dl_se->dl_throttled =0 ; } }else if (!dl_prio (p->normal_prio)) {/*
@@ -2516,7 +2519,7 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)* being boosted again with no means to replenish the runtime and clear * the throttle. */ - p->dl.dl_throttled =0 ; + dl_se->dl_throttled =0 ;if (!(flags &ENQUEUE_REPLENISH ))printk_deferred_once ("sched: DL de-boosted task PID %d: REPLENISH flag missing\n" ,task_pid_nr (p));
@@ -2525,20 +2528,20 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags) }check_schedstat_required (); -update_stats_wait_start_dl (dl_rq_of_se (&p->dl), &p->dl); +update_stats_wait_start_dl (dl_rq, dl_se);if (p->on_rq ==TASK_ON_RQ_MIGRATING ) flags |=ENQUEUE_MIGRATING ; -enqueue_dl_entity (&p->dl, flags); +enqueue_dl_entity (dl_se, flags); -if (dl_server (&p->dl)) +if (dl_server (dl_se))return ;if (task_is_blocked (p))return ; -if (!task_current (rq, p) && !p->dl.dl_throttled && p->nr_cpus_allowed >1 ) +if (!task_current (rq, p) && !dl_se->dl_throttled && p->nr_cpus_allowed >1 )enqueue_pushable_dl_task (rq, p); }
@@ -2835,7 +2838,7 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_sstruct sched_dl_entity *dl_se = &p->dl;struct dl_rq *dl_rq = &rq->dl; -if (on_dl_rq (&p->dl)) +if (on_dl_rq (dl_se))update_stats_wait_start_dl (dl_rq, dl_se);update_curr_dl (rq);
@@ -2845,7 +2848,7 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_sif (task_is_blocked (p))return ; -if (on_dl_rq (&p->dl) && p->nr_cpus_allowed >1 ) +if (on_dl_rq (dl_se) && p->nr_cpus_allowed >1 )enqueue_pushable_dl_task (rq, p); }
PATCH v29 3/9 sched: deadline: Add dl_rq->curr pointer to address issues with Proxy Exec
In reply to John StultzThe DL scheduler keeps the current task in the rbtree, since the deadline value isn't usually chagned while the task is runnable.
This results in set_next_task() and put_prev_task() being simpler, but unfortunately this causes complexity elsewhere.
Specifically when update_curr_dl() updates the deadline, it has to dequeue and then enqueue the task.
From put_prev_task_dl(), we first call update_curr_dl(), and then call enqueue_pushable_dl_task().
However, with Proxy Exec this goes awry. Since when a mutex is released, we might wake the waiting rq->donor. This will cause put_prev_task() to be called on the donor to take it off the cpu for return migration.
At that point, from put_prev_task_dl() the update_curr_dl() logic will dequeue & enqueue the task, and the enqueue function will call enqueue_pushable_dl_task() (since the task_current() check won't prevent it). Then back up the callstack in put_prev_task_dl() we'll end up calling enqueue_pushable_dl_task() again, tripping the !RB_EMPTY_NODE(&p->pushable_dl_tasks) warning.
So to avoid this, use Peter's suggested[1] approach, and add a dl_rq->curr pointer that is set/cleared from set_next_task()/ put_prev_task(), which effectively tracks the rq->donor. We can then use this to avoid adding the active donor to the pushable list from enqueue_task_dl().
[1]: https://lore.kernel.org/lkml/[email protected]/
---
Cc: Joel Fernandes <[email protected]>
Cc: Qais Yousef <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Ben Segall <[email protected]>
Cc: Zimuzo Ezeozue <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Metin Kaya <[email protected]>
Cc: Xuewen Yan <[email protected]>
Cc: K Prateek Nayak <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Suleiman Souhlal <[email protected]>
Cc: kuyo chang <[email protected]>
Cc: hupu <[email protected]>
Cc: [email protected]
--- kernel/sched/deadline.c | 13 +++++++++++++ kernel/sched/sched.h | 1 + 2 files changed, 14 insertions(+)
kernel/sched/deadline.c +13 -0
@@ -2349,6 +2349,9 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)if (task_is_blocked (p))return ; +if (dl_rq->curr == dl_se) +return ; +if (!task_current (rq, p) && !dl_se->dl_throttled && p->nr_cpus_allowed >1 )enqueue_pushable_dl_task (rq, p); }
@@ -2571,6 +2574,10 @@ static void start_hrtick_dl(struct rq *rq, struct sched_dl_entity *dl_se) }#endif /* !CONFIG_SCHED_HRTICK */ +/* +* DL keeps current in tree, because ->deadline is not typically changed while +* a task is runnable. +*/ static void set_next_task_dl (struct rq *rq,struct task_struct *p,bool first) {struct sched_dl_entity *dl_se = &p->dl;
@@ -2583,6 +2590,9 @@ static void set_next_task_dl(struct rq *rq, struct task_struct *p, bool first)/* You can't push away the running task */ dequeue_pushable_dl_task (rq, p); +WARN_ON_ONCE (dl_rq->curr); + dl_rq->curr = dl_se; +if (!first)return ;
@@ -2653,6 +2663,9 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_supdate_dl_rq_load_avg (rq_clock_pelt (rq), rq,1 ); +WARN_ON_ONCE (dl_rq->curr != dl_se); + dl_rq->curr =NULL ; +if (task_is_blocked (p))return ;
kernel/sched/sched.h +1 -0
@@ -889,6 +889,7 @@ struct dl_rq {bool overloaded; +struct sched_dl_entity *curr;/* * Tasks on this rq that can be pushed away. They are kept in * an rb-tree, ordered by tasks' deadlines, with caching
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
[tip: sched/core] sched: deadline: Add dl_rq->curr pointer to address issues with Proxy Exec
The following commit has been merged into the sched/core branch of tip:
The following commit has been merged into the sched/core branch of tip:
Commit-ID: cd8e62c85861bcfbbefedce11a6f8eb00c774312 Gitweb: https://git.kernel.org/tip/cd8e62c85861bcfbbefedce11a6f8eb00c774312 Author: John Stultz <[email protected]> AuthorDate: Tue, 12 May 2026 02:56:13 Committer: Peter Zijlstra <[email protected]> CommitterDate: Tue, 02 Jun 2026 12:26:06 +02:00
sched: deadline: Add dl_rq->curr pointer to address issues with Proxy Exec
The DL scheduler keeps the current task in the rbtree, since the deadline value isn't usually chagned while the task is runnable.
This results in set_next_task() and put_prev_task() being simpler, but unfortunately this causes complexity elsewhere.
Specifically when update_curr_dl() updates the deadline, it has to dequeue and then enqueue the task.
Quoted 1 line
From
put_prev_task_dl(), we first callupdate_curr_dl(), and
then call enqueue_pushable_dl_task().
However, with Proxy Exec this goes awry. Since when a mutex is released, we might wake the waiting rq->donor. This will cause put_prev_task() to be called on the donor to take it off the cpu for return migration.
At that point, from put_prev_task_dl() the update_curr_dl() logic will dequeue & enqueue the task, and the enqueue function will call enqueue_pushable_dl_task() (since the task_current() check won't prevent it). Then back up the callstack in put_prev_task_dl() we'll end up calling enqueue_pushable_dl_task() again, tripping the !RB_EMPTY_NODE(&p->pushable_dl_tasks) warning.
So to avoid this, use Peter's suggested[1] approach, and add a dl_rq->curr pointer that is set/cleared from set_next_task()/ put_prev_task(), which effectively tracks the rq->donor. We can then use this to avoid adding the active donor to the pushable list from enqueue_task_dl().
[1]: https://lore.kernel.org/lkml/[email protected]/
--- kernel/sched/deadline.c | 13 +++++++++++++ kernel/sched/sched.h | 1 + 2 files changed, 14 insertions(+)
kernel/sched/deadline.c +13 -0
@@ -2541,6 +2541,9 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)if (task_is_blocked (p))return ; +if (dl_rq->curr == dl_se) +return ; +if (!task_current (rq, p) && !dl_se->dl_throttled && p->nr_cpus_allowed >1 )enqueue_pushable_dl_task (rq, p); }
@@ -2763,6 +2766,10 @@ static void start_hrtick_dl(struct rq *rq, struct sched_dl_entity *dl_se) }#endif /* !CONFIG_SCHED_HRTICK */ +/* +* DL keeps current in tree, because ->deadline is not typically changed while +* a task is runnable. +*/ static void set_next_task_dl (struct rq *rq,struct task_struct *p,bool first) {struct sched_dl_entity *dl_se = &p->dl;
@@ -2775,6 +2782,9 @@ static void set_next_task_dl(struct rq *rq, struct task_struct *p, bool first)/* You can't push away the running task */ dequeue_pushable_dl_task (rq, p); +WARN_ON_ONCE (dl_rq->curr); + dl_rq->curr = dl_se; +if (!first)return ;
@@ -2845,6 +2855,9 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_supdate_dl_rq_load_avg (rq_clock_pelt (rq), rq,1 ); +WARN_ON_ONCE (dl_rq->curr != dl_se); + dl_rq->curr =NULL ; +if (task_is_blocked (p))return ;
kernel/sched/sched.h +1 -0
@@ -893,6 +893,7 @@ struct dl_rq {bool overloaded; +struct sched_dl_entity *curr;/* * Tasks on this rq that can be pushed away. They are kept in * an rb-tree, ordered by tasks' deadlines, with caching
PATCH v29 4/9 sched: Rework block_task so it can be directly called
In reply to John StultzPull most of the logic out of try_to_block_task() and put it into block_task() directly, so that we can call block_task() and not have to worry about the failing cases in try_to_block_task()
---
Cc: Joel Fernandes <[email protected]>
Cc: Qais Yousef <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Ben Segall <[email protected]>
Cc: Zimuzo Ezeozue <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Metin Kaya <[email protected]>
Cc: Xuewen Yan <[email protected]>
Cc: K Prateek Nayak <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Suleiman Souhlal <[email protected]>
Cc: kuyo chang <[email protected]>
Cc: hupu <[email protected]>
Cc: [email protected]
--- kernel/sched/core.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-)
kernel/sched/core.c +23 -22
@@ -2223,8 +2223,29 @@ void deactivate_task(struct rq *rq, struct task_struct *p, int flags)dequeue_task (rq, p, flags); } -static void block_task (struct rq *rq,struct task_struct *p,int flags) +static void block_task (struct rq *rq,struct task_struct *p,unsigned long task_state) { +int flags =DEQUEUE_NOCLOCK ; + + p->sched_contributes_to_load = + (task_state &TASK_UNINTERRUPTIBLE ) && + !(task_state &TASK_NOLOAD ) && + !(task_state &TASK_FROZEN ); + +if (unlikely (is_special_task_state (task_state))) + flags |=DEQUEUE_SPECIAL ; + +/* +* __schedule() ttwu() +* prev_state = prev->state; if (p->on_rq && ...) +* if (prev_state) goto out; +* p->on_rq = 0; smp_acquire__after_ctrl_dep(); +* p->state = TASK_WAKING +* +* Where __schedule() and ttwu() have matching control dependencies. +* +* After this, schedule() must not care about p->state any more. +*/ if (dequeue_task (rq, p,DEQUEUE_SLEEP | flags))__block_task (rq, p); }
@@ -6582,7 +6603,6 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p, unsigned long *task_state_p, bool should_block) {unsigned long task_state = *task_state_p; -int flags =DEQUEUE_NOCLOCK ;if (signal_pending_state (task_state, p)) {WRITE_ONCE (p->__state,TASK_RUNNING );
@@ -6602,26 +6622,7 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,if (!should_block)return false; - p->sched_contributes_to_load = - (task_state &TASK_UNINTERRUPTIBLE ) && - !(task_state &TASK_NOLOAD ) && - !(task_state &TASK_FROZEN ); - -if (unlikely (is_special_task_state (task_state))) - flags |=DEQUEUE_SPECIAL ; - -/* -* __schedule() ttwu() -* prev_state = prev->state; if (p->on_rq && ...) -* if (prev_state) goto out; -* p->on_rq = 0; smp_acquire__after_ctrl_dep(); -* p->state = TASK_WAKING -* -* Where __schedule() and ttwu() have matching control dependencies. -* -* After this, schedule() must not care about p->state any more. -*/ -block_task (rq, p, flags); +block_task (rq, p, task_state);return true; }
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
[tip: sched/core] sched: Rework block_task so it can be directly called
The following commit has been merged into the sched/core branch of tip:
The following commit has been merged into the sched/core branch of tip:
Commit-ID: f0c1ecde6447079505f1d4557d30401136218ae0 Gitweb: https://git.kernel.org/tip/f0c1ecde6447079505f1d4557d30401136218ae0 Author: John Stultz <[email protected]> AuthorDate: Tue, 12 May 2026 02:56:14 Committer: Peter Zijlstra <[email protected]> CommitterDate: Tue, 02 Jun 2026 12:26:06 +02:00
sched: Rework block_task so it can be directly called
Pull most of the logic out of try_to_block_task() and put it into block_task() directly, so that we can call block_task() and not have to worry about the failing cases in try_to_block_task()
--- kernel/sched/core.c | 45 ++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-)
kernel/sched/core.c +23 -22
@@ -2236,8 +2236,29 @@ void deactivate_task(struct rq *rq, struct task_struct *p, int flags)dequeue_task (rq, p, flags); } -static void block_task (struct rq *rq,struct task_struct *p,int flags) +static void block_task (struct rq *rq,struct task_struct *p,unsigned long task_state) { +int flags =DEQUEUE_NOCLOCK ; + + p->sched_contributes_to_load = + (task_state &TASK_UNINTERRUPTIBLE ) && + !(task_state &TASK_NOLOAD ) && + !(task_state &TASK_FROZEN ); + +if (unlikely (is_special_task_state (task_state))) + flags |=DEQUEUE_SPECIAL ; + +/* +* __schedule() ttwu() +* prev_state = prev->state; if (p->on_rq && ...) +* if (prev_state) goto out; +* p->on_rq = 0; smp_acquire__after_ctrl_dep(); +* p->state = TASK_WAKING +* +* Where __schedule() and ttwu() have matching control dependencies. +* +* After this, schedule() must not care about p->state any more. +*/ if (dequeue_task (rq, p,DEQUEUE_SLEEP | flags))__block_task (rq, p); }
@@ -6587,7 +6608,6 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p, unsigned long *task_state_p, bool should_block) {unsigned long task_state = *task_state_p; -int flags =DEQUEUE_NOCLOCK ;if (signal_pending_state (task_state, p)) {WRITE_ONCE (p->__state,TASK_RUNNING );
@@ -6607,26 +6627,7 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,if (!should_block)return false; - p->sched_contributes_to_load = - (task_state &TASK_UNINTERRUPTIBLE ) && - !(task_state &TASK_NOLOAD ) && - !(task_state &TASK_FROZEN ); - -if (unlikely (is_special_task_state (task_state))) - flags |=DEQUEUE_SPECIAL ; - -/* -* __schedule() ttwu() -* prev_state = prev->state; if (p->on_rq && ...) -* if (prev_state) goto out; -* p->on_rq = 0; smp_acquire__after_ctrl_dep(); -* p->state = TASK_WAKING -* -* Where __schedule() and ttwu() have matching control dependencies. -* -* After this, schedule() must not care about p->state any more. -*/ -block_task (rq, p, flags); +block_task (rq, p, task_state);return true; }
PATCH v29 5/9 sched: Have try_to_wake_up() handle return-migration for PROXY_WAKING case
In reply to John StultzThis patch adds logic so try_to_wake_up() will notice if we are waking a task where blocked_on == PROXY_WAKING, and if necessary dequeue the task so the wakeup will naturally return-migrate the donor task back to a cpu it can run on.
This helps performance as we do the dequeue and wakeup under the locks normally taken in the try_to_wake_up() and avoids having to do proxy_force_return() from __schedule(), which has to re-take similar locks and then force a pick again loop.
This was split out from the larger proxy patch, and significantly reworked.
Credits for the original patch go to:
Peter Zijlstra (Intel) <[email protected]>
Juri Lelli <[email protected]>
Valentin Schneider <[email protected]>
Connor O'Brien <[email protected]>
---
v24:
* Reworked proxy_needs_return() so its less nested as suggested by K Prateek
* Switch to using block_task with DEQUEUE_SPECIAL as suggested by K Prateek
* Fix edge case to reset wake_cpu if select_task_rq() chooses the current rq and we skip set_task_cpu()
v26:
* Handle both blocked and PROXY_WAKING tasks in proxy_needs_return(), as suggested by K Prateek
* Try to handle signal edge case in ttwu that K Prateek pointed out
v27:
* Integrate simplifications to proxy_needs_return() suggested by K Prateek
* Rework ttwu_runnable() to align with ACQUIRE(__task_rq_lock, guard)(p) usage as suggested by Peter
* Major rework suggested by Peter to get rid of proxy_force_return() completely, using proxy_deactivate() and allow ttwu to handle all the return migration. Lots of helpful improvements suggested by K Prateek included as well here.
v28:
* Folded in change suggested by K Prateek to introduce proxy_reset_donor() to reset the donor to current task when the donor is woken up.
* Drop an unnecessary PROXY_WAKING assignment which was noted by K Prateek
v29:
* Use scoped_guard in proxy_needs_return() to avoid block_task being called with blocked_lock held.
--- include/linux/sched.h | 2 +- kernel/sched/core.c | 195 +++++++++++++++++++++--------------------- 2 files changed, 97 insertions(+), 100 deletions(-)
include/linux/sched.h +1 -1
@@ -161,7 +161,7 @@ struct user_event_mm;*/ #define is_special_task_state (state) \ ((state) & (__TASK_STOPPED | __TASK_TRACED | TASK_PARKED | \ - TASK_DEAD | TASK_FROZEN)) + TASK_DEAD | TASK_WAKING | TASK_FROZEN))#ifdef CONFIG_DEBUG_ATOMIC_SLEEP # define debug_normal_state_change (state_value) \
kernel/sched/core.c +96 -99
284 lines not on this page · raw message
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
Peter Zijlstra
maintainerRe: [PATCH v29 5/9] sched: Have try_to_wake_up() handle return-migration for PROXY_WAKING case
kernel/sched/core.c static void __sched notrace __schedule(int sched_mode) · line 7109
Quoted 12 lines · John Stultz
migrate_task:kernel/sched/core.c +3
-0proxy_migrate_task (rq, rf, p, owner_cpu);@@ -7107,6 +7101,9 @@ static void __sched notrace __schedule(int sched_mode)if (sched_proxy_exec ()) {struct task_struct *prev_donor = rq->donor; +if (!prev_state && prev->blocked_on) +clear_task_blocked_on (prev,NULL ); +rq_set_donor (rq, next);if (unlikely (next->blocked_on)) { next =find_proxy_task (rq, next, &rf);
This adds a @prev dereference, after you've just explained how we should not use @prev after a lock break.
Peter Zijlstra
maintainerRe: [PATCH v29 5/9] sched: Have try_to_wake_up() handle return-migration for PROXY_WAKING case
Quoted 8 lines · Peter Zijlstra
Quoted 6 lines · John Stultz
migrate_task:kernel/sched/core.c +0
-0proxy_migrate_task (rq, rf, p, owner_cpu);@@ -7107,6 +7101,9 @@ static void __sched notrace __schedule(int sched_mode)if (sched_proxy_exec ()) {struct task_struct *prev_donor = rq->donor;
Quoted 9 lines
kernel/sched/core.c static void __sched notrace __schedule(int sched_mode) · line 7109
Quoted 6 lines
kernel/sched/core.c +3
-0+if (!prev_state && prev->blocked_on) +clear_task_blocked_on (prev,NULL ); +rq_set_donor (rq, next);if (unlikely (next->blocked_on)) { next =find_proxy_task (rq, next, &rf);This adds a @prev dereference, after you've just explained how we should not use @prev after a lock break.
Ah, n/m. It is only those prev that really were rq->donor. prev itself is in fact current and will have on_cpu set.
[tip: sched/core] sched: Have try_to_wake_up() handle return-migration for PROXY_WAKING case
In reply to John StultzThe following commit has been merged into the sched/core branch of tip:
The following commit has been merged into the sched/core branch of tip:
Commit-ID: f13beb010e4ab0735c9e46802cbcc820a8bd6467 Gitweb: https://git.kernel.org/tip/f13beb010e4ab0735c9e46802cbcc820a8bd6467 Author: John Stultz <[email protected]> AuthorDate: Tue, 12 May 2026 02:56:15 Committer: Peter Zijlstra <[email protected]> CommitterDate: Tue, 02 Jun 2026 12:26:07 +02:00
sched: Have try_to_wake_up() handle return-migration for PROXY_WAKING case
This patch adds logic so try_to_wake_up() will notice if we are waking a task where blocked_on == PROXY_WAKING, and if necessary dequeue the task so the wakeup will naturally return-migrate the donor task back to a cpu it can run on.
This helps performance as we do the dequeue and wakeup under the locks normally taken in the try_to_wake_up() and avoids having to do proxy_force_return() from __schedule(), which has to re-take similar locks and then force a pick again loop.
This was split out from the larger proxy patch, and significantly reworked.
Credits for the original patch go to:
Peter Zijlstra (Intel) <[email protected]>
Juri Lelli <[email protected]>
Valentin Schneider <[email protected]>
Connor O'Brien <[email protected]>
--- include/linux/sched.h | 2 +- kernel/sched/core.c | 195 ++++++++++++++++++++--------------------- 2 files changed, 97 insertions(+), 100 deletions(-)
include/linux/sched.h +1 -1
@@ -161,7 +161,7 @@ struct user_event_mm;*/ #define is_special_task_state (state) \ ((state) & (__TASK_STOPPED | __TASK_TRACED | TASK_PARKED | \ - TASK_DEAD | TASK_FROZEN)) + TASK_DEAD | TASK_WAKING | TASK_FROZEN))#ifdef CONFIG_DEBUG_ATOMIC_SLEEP # define debug_normal_state_change (state_value) \
kernel/sched/core.c +96 -99
284 lines not on this page · raw message
PATCH v29 6/9 sched: Add is_blocked task flag
In reply to John StultzAdd a new is_blocked flag to the task struct. This flag is set by try_to_block_task() and cleared by ttwu_do_wakeup() and tracks if the task is blocked.
Traditionally this would mirror !p->on_rq, however due things like DELAY_DEQUEUE and PROXY_EXEC, this can diverge, so its useful to manage separately.
Additionally with this, we might be able to get rid of the p->se.sched_delayed (ab)use in the core code (eventually).
Taken whole cloth from Peter's email:
https://lore.kernel.org/lkml/[email protected]/
With a few additional p->is_blocked = 0 in a few cases where we return current if blocked_on gets zeroed or there is no owner. This may hint that these current special cases might be dropped eventually.
This change also helps resolve wait-queue stalls seen with proxy-execution. See previous patch attempts for details:
https://lore.kernel.org/lkml/[email protected]/
---
Cc: Joel Fernandes <[email protected]>
Cc: Qais Yousef <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Ben Segall <[email protected]>
Cc: Zimuzo Ezeozue <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Metin Kaya <[email protected]>
Cc: Xuewen Yan <[email protected]>
Cc: K Prateek Nayak <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Suleiman Souhlal <[email protected]>
Cc: kuyo chang <[email protected]>
Cc: hupu <[email protected]>
Cc: [email protected]
--- include/linux/sched.h | 7 +++++-- kernel/sched/core.c | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-)
include/linux/sched.h +5 -2
@@ -846,7 +846,11 @@ struct task_struct {struct alloc_tag *alloc_tag;#endif -int on_cpu; +u8 on_cpu; +u8 on_rq; +u8 is_blocked; +u8 __pad; +struct __call_single_node wake_entry;unsigned int wakee_flips;unsigned long wakee_flip_decay_ts;
@@ -861,7 +865,6 @@ struct task_struct {*/ int recent_used_cpu;int wake_cpu; -int on_rq;int prio;int static_prio;
kernel/sched/core.c +15 -1
@@ -615,6 +615,12 @@ EXPORT_SYMBOL(__trace_set_current_state);* [ The astute reader will observe that it is possible for two tasks on one * CPU to have ->on_cpu = 1 at the same time. ] * +* p->is_blocked <- { 0, 1 }: +* +* is set by try_to_block_task() and cleared by ttwu_do_wakeup() and tracks +* if the task is blocked. Traditionally this would mirror p->on_rq, however +* due things like DELAY_DEQUEUE and PROXY_EXEC, this can diverge. +* * task_cpu(p): is changed by set_task_cpu(), the rules are: * * - Don't call set_task_cpu() on a blocked task:
@@ -3706,6 +3712,7 @@ ttwu_stat(struct task_struct *p, int cpu, int wake_flags)*/ static inline void ttwu_do_wakeup (struct task_struct *p) { + p->is_blocked =0 ;WRITE_ONCE (p->__state,TASK_RUNNING );trace_sched_wakeup (p); }
@@ -4239,6 +4246,7 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)* it disabling IRQs (this allows not taking ->pi_lock). */ WARN_ON_ONCE (p->se.sched_delayed); +WARN_ON_ONCE (p->is_blocked);/* If p is current, we know we can run here, so clear blocked_on */ clear_task_blocked_on (p,NULL );if (!ttwu_state_match (p, state, &success))
@@ -4550,6 +4558,7 @@ static void __sched_fork(u64 clone_flags, struct task_struct *p)/* A delayed task cannot be in clone(). */ WARN_ON_ONCE (p->se.sched_delayed); +WARN_ON_ONCE (p->is_blocked);#ifdef CONFIG_FAIR_GROUP_SCHED p->se.cfs_rq =NULL ;
@@ -6671,6 +6680,7 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,unsigned long task_state = *task_state_p;if (signal_pending_state (task_state, p)) { + p->is_blocked =0 ;WRITE_ONCE (p->__state,TASK_RUNNING ); *task_state_p =TASK_RUNNING ;clear_task_blocked_on (p,NULL );
@@ -6678,6 +6688,8 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,return false; } + p->is_blocked =1 ; +/* * We check should_block after signal_pending because we * will want to wake the task in that case. But if
@@ -6837,6 +6849,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)/* if its PROXY_WAKING, do return migration or run if current */ if (mutex ==PROXY_WAKING ) {if (task_current (rq, p)) { + p->is_blocked =0 ;clear_task_blocked_on (p,PROXY_WAKING );return p; }
@@ -6872,6 +6885,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)* just run on this rq), or return-migrate the task. */ if (task_current (rq, p)) { + p->is_blocked =0 ;__clear_task_blocked_on (p,NULL );return p; }
@@ -7105,7 +7119,7 @@ static void __sched notrace __schedule(int sched_mode)clear_task_blocked_on (prev,NULL );rq_set_donor (rq, next); -if (unlikely (next->blocked_on)) { +if (unlikely (next->is_blocked && next->blocked_on)) { next =find_proxy_task (rq, next, &rf);if (!next) {zap_balance_callbacks (rq);
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
[tip: sched/core] sched: Add is_blocked task flag
The following commit has been merged into the sched/core branch of tip:
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 4c2a20413d7fb3fc3dd7adf233a4f82bb203fb58 Gitweb: https://git.kernel.org/tip/4c2a20413d7fb3fc3dd7adf233a4f82bb203fb58 Author: John Stultz <[email protected]> AuthorDate: Tue, 12 May 2026 02:56:16 Committer: Peter Zijlstra <[email protected]> CommitterDate: Tue, 02 Jun 2026 12:26:07 +02:00
sched: Add is_blocked task flag
Add a new is_blocked flag to the task struct. This flag is set by try_to_block_task() and cleared by ttwu_do_wakeup() and tracks if the task is blocked.
Traditionally this would mirror !p->on_rq, however due things like DELAY_DEQUEUE and PROXY_EXEC, this can diverge, so its useful to manage separately.
Additionally with this, we might be able to get rid of the p->se.sched_delayed (ab)use in the core code (eventually).
Taken whole cloth from Peter's email:
https://lore.kernel.org/lkml/[email protected]/
With a few additional p->is_blocked = 0 in a few cases where we return current if blocked_on gets zeroed or there is no owner. This may hint that these current special cases might be dropped eventually.
This change also helps resolve wait-queue stalls seen with proxy-execution. See previous patch attempts for details:
https://lore.kernel.org/lkml/[email protected]/
--- include/linux/sched.h | 7 +++++-- kernel/sched/core.c | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-)
include/linux/sched.h +5 -2
@@ -852,7 +852,11 @@ struct task_struct {struct alloc_tag *alloc_tag;#endif -int on_cpu; +u8 on_cpu; +u8 on_rq; +u8 is_blocked; +u8 __pad; +struct __call_single_node wake_entry;unsigned int wakee_flips;unsigned long wakee_flip_decay_ts;
@@ -867,7 +871,6 @@ struct task_struct {*/ int recent_used_cpu;int wake_cpu; -int on_rq;int prio;int static_prio;
kernel/sched/core.c +15 -1
@@ -624,6 +624,12 @@ int task_llc(const struct task_struct *p)* [ The astute reader will observe that it is possible for two tasks on one * CPU to have ->on_cpu = 1 at the same time. ] * +* p->is_blocked <- { 0, 1 }: +* +* is set by try_to_block_task() and cleared by ttwu_do_wakeup() and tracks +* if the task is blocked. Traditionally this would mirror p->on_rq, however +* due things like DELAY_DEQUEUE and PROXY_EXEC, this can diverge. +* * task_cpu(p): is changed by set_task_cpu(), the rules are: * * - Don't call set_task_cpu() on a blocked task:
@@ -3719,6 +3725,7 @@ ttwu_stat(struct task_struct *p, int cpu, int wake_flags)*/ static inline void ttwu_do_wakeup (struct task_struct *p) { + p->is_blocked =0 ;WRITE_ONCE (p->__state,TASK_RUNNING );trace_sched_wakeup (p); }
@@ -4252,6 +4259,7 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)* it disabling IRQs (this allows not taking ->pi_lock). */ WARN_ON_ONCE (p->se.sched_delayed); +WARN_ON_ONCE (p->is_blocked);/* If p is current, we know we can run here, so clear blocked_on */ clear_task_blocked_on (p,NULL );if (!ttwu_state_match (p, state, &success))
@@ -4563,6 +4571,7 @@ static void __sched_fork(u64 clone_flags, struct task_struct *p)/* A delayed task cannot be in clone(). */ WARN_ON_ONCE (p->se.sched_delayed); +WARN_ON_ONCE (p->is_blocked);#ifdef CONFIG_FAIR_GROUP_SCHED p->se.cfs_rq =NULL ;
@@ -6676,6 +6685,7 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,unsigned long task_state = *task_state_p;if (signal_pending_state (task_state, p)) { + p->is_blocked =0 ;WRITE_ONCE (p->__state,TASK_RUNNING ); *task_state_p =TASK_RUNNING ;clear_task_blocked_on (p,NULL );
@@ -6683,6 +6693,8 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,return false; } + p->is_blocked =1 ; +/* * We check should_block after signal_pending because we * will want to wake the task in that case. But if
@@ -6843,6 +6855,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)/* if its PROXY_WAKING, do return migration or run if current */ if (mutex ==PROXY_WAKING ) {if (task_current (rq, p)) { + p->is_blocked =0 ;clear_task_blocked_on (p,PROXY_WAKING );return p; }
@@ -6878,6 +6891,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)* just run on this rq), or return-migrate the task. */ if (task_current (rq, p)) { + p->is_blocked =0 ;__clear_task_blocked_on (p,NULL );return p; }
@@ -7111,7 +7125,7 @@ pick_again:clear_task_blocked_on (prev,NULL );rq_set_donor (rq, next); -if (unlikely (next->blocked_on)) { +if (unlikely (next->is_blocked && next->blocked_on)) { next =find_proxy_task (rq, next, &rf);if (!next) {zap_balance_callbacks (rq);
PATCH v29 7/9 sched: Add blocked_donor link to task for smarter mutex handoffs
In reply to John StultzFrom: Peter Zijlstra <[email protected]>
Add link to the task this task is proxying for, and use it so the mutex owner can do an intelligent hand-off of the mutex to the task that the owner is running on behalf.
---
v5:
* Split out from larger proxy patch
v6:
* Moved proxied value from earlier patch to this one where it is actually used
* Rework logic to check sched_proxy_exec() instead of using ifdefs
* Moved comment change to this patch where it makes sense v7:
* Use more descriptive term then "us" in comments, as suggested by Metin Kaya.
* Minor typo fixup from Metin Kaya
* Reworked proxied variable to prev_not_proxied to simplify usage v8:
* Use helper for donor blocked_on_state transition
v9:
* Re-add mutex lock handoff in the unlock path, but only when we have a blocked donor
* Slight reword of commit message suggested by Metin v18:
* Add task_init initialization for blocked_donor, suggested by Suleiman
v23:
* Reworks for PROXY_WAKING approach suggested by PeterZ v25:
* Simplified some logic now we don't have proxy_tag_curr() v28:
* Remove sched_proxy_exec() conditionalized blocked_lock usage as it was hitting llvm's "'blocked_lock' is not held on every path through here [-Wthread-safety-analysis]" errors.
--- include/linux/sched.h | 1 + init/init_task.c | 1 + kernel/fork.c | 1 + kernel/locking/mutex.c | 43 +++++++++++++++++++++++++++++++++++++++--- kernel/sched/core.c | 14 +++++++++++++- 5 files changed, 56 insertions(+), 4 deletions(-)
include/linux/sched.h +1 -0
@@ -1242,6 +1242,7 @@ struct task_struct {#endif struct mutex *blocked_on;/* lock we're blocked on */ +struct task_struct *blocked_donor;/* task that is boosting this task */ raw_spinlock_t blocked_lock;#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
init/init_task.c +1 -0
@@ -200,6 +200,7 @@ struct task_struct init_task __aligned(L1_CACHE_BYTES) = { .mems_allowed_seq = SEQCNT_SPINLOCK_ZERO(init_task.mems_allowed_seq, &init_task.alloc_lock), #endif + .blocked_donor =NULL , #ifdef CONFIG_RT_MUTEXES .pi_waiters =RB_ROOT_CACHED , .pi_top_task = NULL,
kernel/fork.c +1 -0
@@ -2218,6 +2218,7 @@ __latent_entropy struct task_struct *copy_process(lockdep_init_task (p); p->blocked_on =NULL ;/* not blocked yet */ + p->blocked_donor =NULL ;/* nobody is boosting p yet */ #ifdef CONFIG_BCACHE p->sequential_io =0 ;
kernel/locking/mutex.c +40 -3
72 lines not on this page · raw message
kernel/sched/core.c +13 -1
@@ -6821,7 +6821,17 @@ static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,* Find runnable lock owner to proxy for mutex blocked donor * * Follow the blocked-on relation: -* task->blocked_on -> mutex->owner -> task... +* +* ,-> task +* | | blocked-on +* | v +* blocked_donor | mutex +* | | owner +* | v +* `-- task +* +* and set the blocked_donor relation, this latter is used by the mutex +* code to find which (blocked) task to hand-off to. * * Lock order: *
@@ -6963,6 +6973,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)* rq, therefore holding @rq->lock is sufficient to * guarantee its existence, as per ttwu_remote(). */ + owner->blocked_donor = p; }WARN_ON_ONCE (owner && !owner->on_rq);return owner;
@@ -7119,6 +7130,7 @@ static void __sched notrace __schedule(int sched_mode)clear_task_blocked_on (prev,NULL );rq_set_donor (rq, next); + next->blocked_donor =NULL ;if (unlikely (next->is_blocked && next->blocked_on)) { next =find_proxy_task (rq, next, &rf);if (!next) {
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
Peter Zijlstra
maintainerRe: [PATCH v29 7/9] sched: Add blocked_donor link to task for smarter mutex handoffs
Quoted 25 lines · John Stultz
From: Peter Zijlstra <[email protected]>
Add link to the task this task is proxying for, and use it so the mutex owner can do an intelligent hand-off of the mutex to the task that the owner is running on behalf.
Signed-off-by Peter Zijlstra (Intel) <[email protected]>Signed-off-by Juri Lelli <[email protected]>Signed-off-by Valentin Schneider <[email protected]>Signed-off-by Connor O'Brien <[email protected]>[jstultz: This patch was split out from larger proxy patch]Signed-off-by John Stultz <[email protected]>--- include/linux/sched.h | 1 + init/init_task.c | 1 + kernel/fork.c | 1 + kernel/locking/mutex.c | 43 +++++++++++++++++++++++++++++++++++++++--- kernel/sched/core.c | 14 +++++++++++++- 5 files changed, 56 insertions(+), 4 deletions(-)include/linux/sched.h +0
-0
include/linux/sched.h struct task_struct { · line 1246
Quoted 5 lines
include/linux/sched.h +1
-0@@ -1242,6 +1242,7 @@ struct task_struct {#endif struct mutex *blocked_on;/* lock we're blocked on */ +struct task_struct *blocked_donor;/* task that is boosting this task */ raw_spinlock_t blocked_lock;
The placement suggests this new field is also serialized by blocked_lock, but that is not, in fact, the case AFAICT.
It is set in schedule(), while holding:
rq->lock
mutex->wait_lock
p->blocked_lock
But p != owner, so we don't hold owner->blocked_lock.
Quoted 32 lines
kernel/sched/core.c +12
-1@@ -6821,7 +6821,17 @@ static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,* Find runnable lock owner to proxy for mutex blocked donor * * Follow the blocked-on relation: -* task->blocked_on -> mutex->owner -> task... +* +* ,-> task +* | | blocked-on +* | v +* blocked_donor | mutex +* | | owner +* | v +* `-- task +* +* and set the blocked_donor relation, this latter is used by the mutex +* code to find which (blocked) task to hand-off to. * * Lock order: * @@ -6963,6 +6973,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)* rq, therefore holding @rq->lock is sufficient to * guarantee its existence, as per ttwu_remote(). */ + owner->blocked_donor = p; }WARN_ON_ONCE (owner && !owner->on_rq);return owner;
kernel/sched/core.c static void __sched notrace __schedule(int sched_mode) · line 7136
Quoted 7 lines
kernel/sched/core.c +1
-0@@ -7119,6 +7130,7 @@ static void __sched notrace __schedule(int sched_mode)clear_task_blocked_on (prev,NULL );rq_set_donor (rq, next); + next->blocked_donor =NULL ;if (unlikely (next->is_blocked && next->blocked_on)) { next =find_proxy_task (rq, next, &rf);if (!next) {
Notably, blocked_donor is a back link that is specific to the current schedule() call / donor pick cycle.
This means it is stable when either: holding rq->lock or disabling preemption -- because when preemption is disabled.
This then brings us to the consumer side of things:
Quoted 16 lines
kernel/locking/mutex.c +1
-1@@ -980,7 +980,7 @@ EXPORT_SYMBOL_GPL(ww_mutex_lock_interruptible); static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigned long ip)__releases (lock) { -struct task_struct *next =NULL ; +struct task_struct *donor, *next =NULL ;struct mutex_waiter *waiter;DEFINE_WAKE_Q (wake_q);unsigned long owner;@@ -1001,6 +1001,12 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigneMUTEX_WARN_ON (__owner_task (owner) != current);MUTEX_WARN_ON (owner &MUTEX_FLAG_PICKUP );
kernel/locking/mutex.c static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · line 1012
Quoted 10 lines
kernel/locking/mutex.c +6
-0+if (sched_proxy_exec () && current->blocked_donor) { +/* force handoff if we have a blocked_donor */ + owner =MUTEX_FLAG_HANDOFF ; +break ; + } +if (owner &MUTEX_FLAG_HANDOFF )break ;
AFAICT this is racy since we don't have preemption disabled.
So we can observe ->blocked_donor (A) set, or (B) unset. If (A) we can schedule() right after this (and before taking ->wait_lock) and it can be unset when we resume running this task. Or (B), the exact opposite.
Now, (A) is harmless, because if ->blocked_donor becomes NULL, the hand off code falls back to picking the first on the wait list and things just get on.
*However*, (B) might be a problem, because then we will not have the HANDOFF bit set even though there is in fact a donor we need to hand off to.
Quoted 42 lines
kernel/locking/mutex.c +31
-2@@ -1013,19 +1019,50 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne }raw_spin_lock_irqsave (&lock->wait_lock, flags); +raw_spin_lock (¤t->blocked_lock);debug_mutex_unlock (lock); + +if (sched_proxy_exec ()) { +/* +* If we have a task boosting current, and that task was boosting +* current through this lock, hand the lock to that task, as that +* is the highest waiter, as selected by the scheduling function. +*/ + donor = current->blocked_donor; +if (donor) { +struct mutex *next_lock; + +raw_spin_lock_nested (&donor->blocked_lock,SINGLE_DEPTH_NESTING ); + next_lock =__get_task_blocked_on (donor); +if (next_lock == lock) { + next = donor; +__set_task_blocked_on_waking (donor, next_lock); +wake_q_add (&wake_q, donor); + current->blocked_donor =NULL ; + } +raw_spin_unlock (&donor->blocked_lock); + } + } + +/* +* Failing that, pick first on the wait list. +*/ waiter = lock->first_waiter; -if (waiter) { +if (!next && waiter) { next = waiter->task; +raw_spin_lock_nested (&next->blocked_lock,SINGLE_DEPTH_NESTING );debug_mutex_wake_waiter (lock, waiter); -set_task_blocked_on_waking (next, lock); +__set_task_blocked_on_waking (next, lock); +raw_spin_unlock (&next->blocked_lock);
kernel/locking/mutex.c static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · line 1067
Quoted 10 lines
kernel/locking/mutex.c +2
-0wake_q_add (&wake_q, next); + }if (owner &MUTEX_FLAG_HANDOFF )__mutex_handoff (lock, next); +raw_spin_unlock (¤t->blocked_lock);raw_spin_unlock_irqrestore_wake (&lock->wait_lock, flags, &wake_q); }
That is, we need this on top, no?
---
include/linux/sched.h +7 -1
@@ -1242,8 +1242,14 @@ struct task_struct {#endif struct mutex *blocked_on;/* lock we're blocked on */ -struct task_struct *blocked_donor;/* task that is boosting this task */ raw_spinlock_t blocked_lock; + +/* +* The task that is boosting this task; a back link for the current +* donor stack. Set in schedule() -> find_proxy_task() and only stable +* under preempt_disable(). +*/ +struct task_struct *blocked_donor;#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER /*
kernel/locking/mutex.c +10 -1
@@ -990,6 +990,14 @@ static noinline void __sched __mutex_unl__release (lock);/* +* Ensures the proxy donor stack is stable across unlock and handoff. +* Specifically, it avoids the case where current->blocked_donor is +* NULL when it is inspected while doing the unlock, but a preemption +* before taking the wake_lock would make it set and a hand-off is +* missed. +*/ +guard (preempt)(); +/* * Release the lock before (potentially) taking the spinlock such that * other contenders can get on with things ASAP. *
@@ -1063,7 +1071,8 @@ static noinline void __sched __mutex_unl__mutex_handoff (lock, next);raw_spin_unlock (¤t->blocked_lock); -raw_spin_unlock_irqrestore_wake (&lock->wait_lock, flags, &wake_q); +raw_spin_unlock_irqrestore (&lock->wait_lock, flags); +wake_up_q (&wake_q); }#ifndef CONFIG_DEBUG_LOCK_ALLOC
Peter Zijlstra
maintainerRe: [PATCH v29 7/9] sched: Add blocked_donor link to task for smarter mutex handoffs
Quoted 5 lines · Peter Zijlstra
Notably,
blocked_donoris a back link that is specific to the currentschedule()call / donor pick cycle.This means it is stable when either: holding
rq->lockor disabling preemption -- because when preemption is disabled.
... you cannot schedule().
K Prateek Nayak
reviewerRe: [PATCH v29 7/9] sched: Add blocked_donor link to task for smarter mutex handoffs
In reply to Peter ZijlstraHello Peter,
Quoted 36 lines · Peter Zijlstra
This then brings us to the consumer side of things:
Quoted 16 lines
kernel/locking/mutex.c +1
-1@@ -980,7 +980,7 @@ EXPORT_SYMBOL_GPL(ww_mutex_lock_interruptible); static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigned long ip)__releases (lock) { -struct task_struct *next =NULL ; +struct task_struct *donor, *next =NULL ;struct mutex_waiter *waiter;DEFINE_WAKE_Q (wake_q);unsigned long owner;@@ -1001,6 +1001,12 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigneMUTEX_WARN_ON (__owner_task (owner) != current);MUTEX_WARN_ON (owner &MUTEX_FLAG_PICKUP );kernel/locking/mutex.c static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · line 1012
Quoted 10 lines
kernel/locking/mutex.c +6
-0+if (sched_proxy_exec () && current->blocked_donor) { +/* force handoff if we have a blocked_donor */ + owner =MUTEX_FLAG_HANDOFF ; +break ; + } +if (owner &MUTEX_FLAG_HANDOFF )break ;AFAICT this is racy since we don't have preemption disabled.
So we can observe ->blocked_donor (A) set, or (B) unset. If (A) we can
schedule()right after this (and before taking ->wait_lock) and it can be unset when we resume running this task. Or (B), the exact opposite.
Quoted 7 lines
Now, (A) is harmless, because if ->blocked_donor becomes NULL, the hand off code falls back to picking the first on the wait list and things just get on.
*However*, (B) might be a problem, because then we will not have the HANDOFF bit set even though there is in fact a donor we need to hand off to.
I don't think it is that big of a problem: __mutex_unlock_slowpath() sees current->blocked_donor as NULL but just before it can do:
atomic_long_try_cmpxchg_release(&lock->owner, &owner, __owner_flags(owner))
it is preempted and comes back with a ->blocked_donor.
Once we grab the wait_lock, we realize there is a ->blocked_donor link and we wake it up instead of first waiter which goes and grabs the lock once it hits mutex_trylock() for !owner case.
If a concurrent mutex_lock() races with this sequence, the ->blocked_donor that was woken up sees the new owner and goes to proxy that.
With guard(preempt)(), we see a stable ->blocked_donor as NULL, and we wake up the first waiter after clearing the "owner" from lock->owner.
If a concurrent mutex_lock() races, it still grabs the mutex and now it is just the first waiter that realizes it has to go proxy the task that stole the lock.
One wasted wakeup is incurred in both cases but it seems like no biggie. Maybe I don't understand the subtleties but stabilizing it against preemption is definitely a good move since it saves this debate :-)
Quoted 39 lines
That is, we need this on top, no?
---
include/linux/sched.h +7
-1@@ -1242,8 +1242,14 @@ struct task_struct {#endif struct mutex *blocked_on;/* lock we're blocked on */ -struct task_struct *blocked_donor;/* task that is boosting this task */ raw_spinlock_t blocked_lock; + +/* +* The task that is boosting this task; a back link for the current +* donor stack. Set in schedule() -> find_proxy_task() and only stable +* under preempt_disable(). +*/ +struct task_struct *blocked_donor;#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER /* kernel/locking/mutex.c +8
-0@@ -990,6 +990,14 @@ static noinline void __sched __mutex_unl__release (lock);/* +* Ensures the proxy donor stack is stable across unlock and handoff. +* Specifically, it avoids the case where current->blocked_donor is +* NULL when it is inspected while doing the unlock, but a preemption +* before taking the wake_lock would make it set and a hand-off is +* missed. +*/ +guard (preempt)(); +/* * Release the lock before (potentially) taking the spinlock such that * other contenders can get on with things ASAP. *
kernel/locking/mutex.c static noinline void __sched __mutex_unl · line 1075
Quoted 6 lines
kernel/locking/mutex.c +2
-1@@ -1063,7 +1071,8 @@ static noinline void __sched __mutex_unl__mutex_handoff (lock, next);raw_spin_unlock (¤t->blocked_lock); -raw_spin_unlock_irqrestore_wake (&lock->wait_lock, flags, &wake_q); +raw_spin_unlock_irqrestore (&lock->wait_lock, flags); +wake_up_q (&wake_q);
In that case, can't we simplify this to:
if (next)
wake_up_process(next);and save on the wake_q enqueue dequeue overheads?
Quoted 3 lines
kernel/locking/mutex.c +0
-0}#ifndef CONFIG_DEBUG_LOCK_ALLOC
Signature · 2 lines
Thanks and Regards, Prateek
Re: [PATCH v29 7/9] sched: Add blocked_donor link to task for smarter mutex handoffs
Quoted 11 lines · K Prateek Nayak
kernel/locking/mutex.c static noinline void __sched __mutex_unl · line 1075
Quoted 7 lines · Peter Zijlstra
kernel/locking/mutex.c +2
-1@@ -1063,7 +1071,8 @@ static noinline void __sched __mutex_unl__mutex_handoff (lock, next);raw_spin_unlock (¤t->blocked_lock); -raw_spin_unlock_irqrestore_wake (&lock->wait_lock, flags, &wake_q); +raw_spin_unlock_irqrestore (&lock->wait_lock, flags); +wake_up_q (&wake_q);In that case, can't we simplify this to:
Quoted 5 lines
if (next) wake_up_process(next);and save on the
wake_qenqueue dequeue overheads?
Ah, good point. I'll add that cleanup as well to my tree and start testing. Appreciate you pointing this out!
thanks
-john
Peter Zijlstra
maintainerRe: [PATCH v29 7/9] sched: Add blocked_donor link to task for smarter mutex handoffs
In reply to K Prateek NayakQuoted 47 lines · K Prateek Nayak
Quoted 21 lines
Review static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · line 1012
Quoted 13 lines
(quoted diff) +6
-0@@ -1001,6 +1001,12 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne MUTEX_WARN_ON(__owner_task(owner) != current); MUTEX_WARN_ON(owner & MUTEX_FLAG_PICKUP); + if (sched_proxy_exec() && current->blocked_donor) { + /* force handoff if we have a blocked_donor */ + owner = MUTEX_FLAG_HANDOFF; + break; + } + if (owner & MUTEX_FLAG_HANDOFF) break;AFAICT this is racy since we don't have preemption disabled.
So we can observe ->blocked_donor (A) set, or (B) unset. If (A) we can
schedule()right after this (and before taking ->wait_lock) and it can be unset when we resume running this task. Or (B), the exact opposite.Quoted 7 lines
Now, (A) is harmless, because if ->blocked_donor becomes NULL, the hand off code falls back to picking the first on the wait list and things just get on.
*However*, (B) might be a problem, because then we will not have the HANDOFF bit set even though there is in fact a donor we need to hand off to.
I don't think it is that big of a problem:
__mutex_unlock_slowpath()seescurrent->blocked_donoras NULL but just before it can do:atomic_long_try_cmpxchg_release(&lock->owner, &owner, __owner_flags(owner))it is preempted and comes back with a ->blocked_donor.
Once we grab the
wait_lock, we realize there is a ->blocked_donor link and we wake it up instead of first waiter which goes and grabs the lock once it hitsmutex_trylock()for !owner case.If a concurrent
mutex_lock()races with this sequence, the ->blocked_donor that was woken up sees the new owner and goes to proxy that.With guard(preempt)(), we see a stable ->blocked_donor as NULL, and we wake up the first waiter after clearing the "owner" from
lock->owner.
Quoted 8 lines
If a concurrent
mutex_lock()races, it still grabs the mutex and now it is just the first waiter that realizes it has to go proxy the task that stole the lock.One wasted wakeup is incurred in both cases but it seems like no biggie. Maybe I don't understand the subtleties but stabilizing it against preemption is definitely a good move since it saves this debate :-)
The preemption can happen after the cmpxchg loop completes and before we take the wake_lock.
Quoted 10 lines
kernel/locking/mutex.c static noinline void __sched __mutex_unl · line 1075
Quoted 7 lines
kernel/locking/mutex.c +2
-1@@ -1063,7 +1071,8 @@ static noinline void __sched __mutex_unl__mutex_handoff (lock, next);raw_spin_unlock (¤t->blocked_lock); -raw_spin_unlock_irqrestore_wake (&lock->wait_lock, flags, &wake_q); +raw_spin_unlock_irqrestore (&lock->wait_lock, flags); +wake_up_q (&wake_q);In that case, can't we simplify this to:
Quoted 4 lines
if (next) wake_up_process(next);and save on the
wake_qenqueue dequeue overheads?
Sorta, it still needs the get/put_task_struct() dance on.
Re: [PATCH v29 7/9] sched: Add blocked_donor link to task for smarter mutex handoffs
In reply to Peter ZijlstraQuoted 30 lines · Peter Zijlstra
Review static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne · line 1012
Quoted 13 lines · John Stultz
(quoted diff) +6
-0@@ -1001,6 +1001,12 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne MUTEX_WARN_ON(__owner_task(owner) != current); MUTEX_WARN_ON(owner & MUTEX_FLAG_PICKUP); + if (sched_proxy_exec() && current->blocked_donor) { + /* force handoff if we have a blocked_donor */ + owner = MUTEX_FLAG_HANDOFF; + break; + } + if (owner & MUTEX_FLAG_HANDOFF) break;AFAICT this is racy since we don't have preemption disabled.
So we can observe ->blocked_donor (A) set, or (B) unset. If (A) we can
schedule()right after this (and before taking ->wait_lock) and it can be unset when we resume running this task. Or (B), the exact opposite.Now, (A) is harmless, because if ->blocked_donor becomes NULL, the hand off code falls back to picking the first on the wait list and things just get on.
*However*, (B) might be a problem, because then we will not have the HANDOFF bit set even though there is in fact a donor we need to hand off to.
...
Quoted 40 lines
That is, we need this on top, no?
---
include/linux/sched.h +7
-1@@ -1242,8 +1242,14 @@ struct task_struct {#endif struct mutex *blocked_on;/* lock we're blocked on */ -struct task_struct *blocked_donor;/* task that is boosting this task */ raw_spinlock_t blocked_lock; + +/* +* The task that is boosting this task; a back link for the current +* donor stack. Set in schedule() -> find_proxy_task() and only stable +* under preempt_disable(). +*/ +struct task_struct *blocked_donor;#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER /* kernel/locking/mutex.c +8
-0@@ -990,6 +990,14 @@ static noinline void __sched __mutex_unl__release (lock);/* +* Ensures the proxy donor stack is stable across unlock and handoff. +* Specifically, it avoids the case where current->blocked_donor is +* NULL when it is inspected while doing the unlock, but a preemption +* before taking the wake_lock would make it set and a hand-off is +* missed. +*/ +guard (preempt)(); +/* * Release the lock before (potentially) taking the spinlock such that * other contenders can get on with things ASAP. *
kernel/locking/mutex.c static noinline void __sched __mutex_unl · line 1078
Quoted 9 lines
kernel/locking/mutex.c +2
-1@@ -1063,7 +1071,8 @@ static noinline void __sched __mutex_unl__mutex_handoff (lock, next);raw_spin_unlock (¤t->blocked_lock); -raw_spin_unlock_irqrestore_wake (&lock->wait_lock, flags, &wake_q); +raw_spin_unlock_irqrestore (&lock->wait_lock, flags); +wake_up_q (&wake_q); }#ifndef CONFIG_DEBUG_LOCK_ALLOC
Thanks for the review and bugfix here, Peter! I've integrated this into my tree, and will work out a similar fix for the rwsem logic in my full series.
I know you have a tree with some of these patches that your tinkering with, so I'll wait to see how that settles down before rebasing and sending another iteration of my set.
thanks
-john
[tip: sched/core] sched: Add blocked_donor link to task for smarter mutex handoffs
In reply to John StultzThe following commit has been merged into the sched/core branch of tip:
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 1628b25248d0742b2ce9c7cfa59cd183e35f37e1 Gitweb: https://git.kernel.org/tip/1628b25248d0742b2ce9c7cfa59cd183e35f37e1 Author: Peter Zijlstra <[email protected]> AuthorDate: Tue, 12 May 2026 02:56:17 Committer: Peter Zijlstra <[email protected]> CommitterDate: Tue, 02 Jun 2026 12:26:07 +02:00
sched: Add blocked_donor link to task for smarter mutex handoffs
Add link to the task this task is proxying for, and use it so the mutex owner can do an intelligent hand-off of the mutex to the task that the owner is running on behalf.
--- include/linux/sched.h | 7 +++++- init/init_task.c | 1 +- kernel/fork.c | 1 +- kernel/locking/mutex.c | 60 ++++++++++++++++++++++++++++++++++++----- kernel/sched/core.c | 14 +++++++++- 5 files changed, 75 insertions(+), 8 deletions(-)
include/linux/sched.h +7 -0
@@ -1250,6 +1250,13 @@ struct task_struct {struct mutex *blocked_on;/* lock we're blocked on */ raw_spinlock_t blocked_lock; +/* +* The task that is boosting this task; a back link for the current +* donor stack. Set in schedule() -> find_proxy_task() and only stable +* under preempt_disable(). +*/ +struct task_struct *blocked_donor; +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER /* * Encoded lock address causing task block (lower 2 bits = type from
init/init_task.c +1 -0
@@ -200,6 +200,7 @@ struct task_struct init_task __aligned(L1_CACHE_BYTES) = { .mems_allowed_seq = SEQCNT_SPINLOCK_ZERO(init_task.mems_allowed_seq, &init_task.alloc_lock), #endif + .blocked_donor =NULL , #ifdef CONFIG_RT_MUTEXES .pi_waiters =RB_ROOT_CACHED , .pi_top_task = NULL,
kernel/fork.c +1 -0
@@ -2224,6 +2224,7 @@ __latent_entropy struct task_struct *copy_process(lockdep_init_task (p); p->blocked_on =NULL ;/* not blocked yet */ + p->blocked_donor =NULL ;/* nobody is boosting p yet */ #ifdef CONFIG_BCACHE p->sequential_io =0 ;
kernel/locking/mutex.c +53 -7
94 lines not on this page · raw message
kernel/sched/core.c +13 -1
@@ -6827,7 +6827,17 @@ static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,* Find runnable lock owner to proxy for mutex blocked donor * * Follow the blocked-on relation: -* task->blocked_on -> mutex->owner -> task... +* +* ,-> task +* | | blocked-on +* | v +* blocked_donor | mutex +* | | owner +* | v +* `-- task +* +* and set the blocked_donor relation, this latter is used by the mutex +* code to find which (blocked) task to hand-off to. * * Lock order: *
@@ -6969,6 +6979,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)* rq, therefore holding @rq->lock is sufficient to * guarantee its existence, as per ttwu_remote(). */ + owner->blocked_donor = p; }WARN_ON_ONCE (owner && !owner->on_rq);return owner;
@@ -7125,6 +7136,7 @@ pick_again:clear_task_blocked_on (prev,NULL );rq_set_donor (rq, next); + next->blocked_donor =NULL ;if (unlikely (next->is_blocked && next->blocked_on)) { next =find_proxy_task (rq, next, &rf);if (!next) {
PATCH v29 8/9 sched: Break out core of attach_tasks() helper into sched.h
In reply to John StultzPull the core of attach_tasks() out into sched.h so it can be used more generically.
---
Cc: Joel Fernandes <[email protected]>
Cc: Qais Yousef <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Ben Segall <[email protected]>
Cc: Zimuzo Ezeozue <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Metin Kaya <[email protected]>
Cc: Xuewen Yan <[email protected]>
Cc: K Prateek Nayak <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: Suleiman Souhlal <[email protected]>
Cc: kuyo chang <[email protected]>
Cc: hupu <[email protected]>
Cc: [email protected]
--- kernel/sched/fair.c | 16 +--------------- kernel/sched/sched.h | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 15 deletions(-)
kernel/sched/fair.c +1 -15
@@ -10047,21 +10047,7 @@ static int detach_tasks(struct lb_env *env)*/ static void attach_tasks (struct lb_env *env) { -struct list_head *tasks = &env->tasks; -struct task_struct *p; -struct rq_flags rf; - -rq_lock (env->dst_rq, &rf); -update_rq_clock (env->dst_rq); - -while (!list_empty (tasks)) { - p = list_first_entry(tasks, struct task_struct, se.group_node); -list_del_init (&p->se.group_node); - -attach_task (env->dst_rq, p); - } - -rq_unlock (env->dst_rq, &rf); +__attach_tasks (env->dst_rq, &env->tasks); }#ifdef CONFIG_NO_HZ_COMMON
kernel/sched/sched.h +19 -0
@@ -3045,6 +3045,25 @@ static inline void attach_one_task(struct rq *rq, struct task_struct *p)attach_task (rq, p); } +/* +* __attach_tasks() - attaches a list of tasks (using se.group_node) to +* the new rq +*/ +static inline void __attach_tasks (struct rq *rq,struct list_head *tasks) +{ +guard (rq_lock)(rq); +update_rq_clock (rq); + +while (!list_empty (tasks)) { +struct task_struct *p; + + p = list_first_entry(tasks, struct task_struct, se.group_node); +list_del_init (&p->se.group_node); + +attach_task (rq, p); + } +} +#ifdef CONFIG_PREEMPT_RT # define SCHED_NR_MIGRATE_BREAK 8#else
Signature · 1 line
2.54.0.563.g4f69b47b94-goog
PATCH v29 9/9 sched: Migrate whole chain in proxy_migrate_task()
In reply to John StultzInstead of migrating one task each time through find_proxy_task(), we can walk up the blocked_donor ptrs and migrate the entire current chain in one go.
This was broken out of earlier patches and held back while the series was being stabilized, but I wanted to re-introduce it.
---
v12:
* Earlier this was re-using blocked_node, but I hit
a race with activating blocked entities, and to avoid it introduced a new migration_node listhead
v18:
* Add init_task initialization of migration_node as suggested by Suleiman
v22:
* Move migration_node under CONFIG_SCHED_PROXY_EXEC as suggested by K Prateek
v25:
* Use se.group_node instead of adding migration_node, as suggsested by K Prateek
* Integrated attach_tasks() cleanups suggested by K Prateek
--- kernel/sched/core.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
kernel/sched/core.c +13 -6
@@ -6791,9 +6791,9 @@ static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,__must_hold (__rq_lockp (rq )) {struct rq *target_rq =cpu_rq (target_cpu); +LIST_HEAD (migrate_list);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
@@ -6806,13 +6806,20 @@ static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,* before we release the lock. */ proxy_resched_idle (rq); - -deactivate_task (rq, p,DEQUEUE_NOCLOCK ); -proxy_set_task_cpu (p, target_cpu); - +for (; p; p = p->blocked_donor) { +WARN_ON (p == rq->curr); +deactivate_task (rq, p,DEQUEUE_NOCLOCK ); +proxy_set_task_cpu (p, target_cpu); +/* +* We can re-use se.group_node to migrate the thing, +* because @p is deactivated (won't be balanced) and +* we hold the rq_lock. +*/ +list_add (&p->se.group_node, &migrate_list); + }proxy_release_rq_lock (rq, rf); -attach_one_task (target_rq, p); +__attach_tasks (target_rq, &migrate_list);proxy_reacquire_rq_lock (rq, rf); }
Signature · 1 line
2.54.0.563.g4f69b47b94-goog