Skip to contentSkip to conversation

Optimized Donor Migration for Proxy Execution

PATCH Superseded John Stultz 2 participants 13 messages Apr 4

Subsystems (4)

v27 of 29 Superseded Latest is v29 →

lore
Share & tools
lore
#0 lore raw

v27 cover Optimized Donor Migration for Proxy Execution

Hey All,

Since Peter queued my last chunk of the Proxy Execution series (simple donor migration) I wanted to send out the next chunk of the series for discussion: Optimized Donor Migration.

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 yield and the deadline class scheduler to avoid issues when the donor changes, then the actual patch to do return migration from ttwu() followed by some optimization patches so we migrate the entire task chain in proxy_migrate_task().

I really want to share my appreciation for recent feedback and changes provided by Peter, Steven, and K Prateek!

New in this iteration:
* Major late rework, getting rid of proxy_force_return() and handling all of the return migrations from try_to_wake_up suggested by Peter along with help from K Prateek. This is a pretty significant change, but it’s survived some initial testing but I still need to run more thorough testing to make sure we haven’t introduced any troubles.

* 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

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 (based on current tip/sched/core):
https://github.com/johnstultz-work/linux-dev/commits/proxy-exec-v27-7.0-rc6-tip-sched-core/
https://github.com/johnstultz-work/linux-dev.git proxy-exec-v27-7.0-rc6-tip-sched-core

Issues still to address with the full series:
* Continue testing with the recent ttwu rework to ensure the whole stack is working as intended.

* Continue working to get sched_ext to be ok with Proxy Execution enabled.

* 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

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 Mel Gorman <[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]>

John Stultz (8):
sched: Rework pick_next_task() and prev_balance() to avoid stale prev
references
sched: Avoid donor->sched_class->yield_task() null traversal
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: Break out core of attach_tasks() helper into sched.h
sched: Migrate whole chain in proxy_migrate_task()

K Prateek Nayak (1):
sched/core: Reset the donor to current task when donor is woken

Peter Zijlstra (1):
sched: Add blocked_donor link to task for smarter mutex handoffs

 include/linux/sched.h    |   3 +-
 init/init_task.c         |   1 +
 kernel/fork.c            |   1 +
 kernel/locking/mutex.c   |  44 +++++-
 kernel/sched/core.c      | 315 +++++++++++++++++++++------------------
 kernel/sched/deadline.c  |  44 ++++--
 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 +-
 kernel/sched/syscalls.c  |   3 +-
 12 files changed, 291 insertions(+), 187 deletions(-)
Signature · 1 line
2.53.0.1213.gd9a14994de-goog
#1 lore raw

PATCH v27 1/10 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: Mel Gorman <[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
index c15c9865299e7..9c8a769a6d109 100644
@@ -5907,10 +5907,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;
 
 	/*
@@ -5922,7 +5921,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;
 	}
 }
@@ -5931,7 +5930,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;
@@ -5948,28 +5947,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)
@@ -5979,7 +5978,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;
 			}
 		}
@@ -6032,7 +6031,7 @@ extern void task_vruntime_update(struct rq *rq, struct task_struct *p, bool in_f
 static 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;
@@ -6045,7 +6044,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);
 
@@ -6058,7 +6057,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);
 	}
 
 	/*
@@ -6082,7 +6081,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;
@@ -6264,7 +6263,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);
 
@@ -6487,10 +6486,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 */
@@ -7038,7 +7037,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
index 27359a1e995f9..7352506208287 100644
@@ -2509,8 +2509,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
index 597ce5b718d26..4a6669c517dae 100644
@@ -9153,14 +9153,19 @@ static void __set_next_task_fair(struct rq *rq, struct task_struct *p, bool firs
 static 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
index a83be0c834ddb..ff39120d723a9 100644
@@ -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
index 4e5f1957b91b1..3fd03a836731e 100644
@@ -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
index 9594355a36811..8ee82b03a8a10 100644
@@ -2550,7 +2550,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
@@ -2561,12 +2561,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:
@@ -2790,8 +2789,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
index f95798baddebb..c909ca0d8c87c 100644
@@ -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.53.0.1213.gd9a14994de-goog
#2 lore raw

PATCH v27 2/10 sched: Avoid donor->sched_class->yield_task() null traversal

In reply to John Stultz

With proxy-exec once we do return migration from ttwu(), if a task is proxying for a waiting donor, and the donor is woken up, we switch the rq->donor to point to idle briefly until we can re-enter __schedule().

However, if a task that was acting as a proxy calls into yield() right after the donor is switched to idle, it may trip a null pointer traversal, because the idle task doesn't have a yield_task() pointer.

So add a conditional to ensure we don't try to call the yield_task() pointer in that case.

This was only recently found because prior to commit 127b90315ca07 ("sched/proxy: Yield the donor task") do_sched_yield() incorrectly called current->sched_class_yield_task() instead of using rq->donor.

---
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: Mel Gorman <[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/syscalls.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/sched/syscalls.c +2 -1
index b215b0ead9a60..e3e4fd674ed63 100644
@@ -1340,7 +1340,8 @@ static void do_sched_yield(void)
 	rq = this_rq_lock_irq(&rf);
 
 	schedstat_inc(rq->yld_count);
-	rq->donor->sched_class->yield_task(rq);
+	if (rq->donor->sched_class->yield_task)
+		rq->donor->sched_class->yield_task(rq);
 
 	preempt_disable();
 	rq_unlock_irq(rq, &rf);
Signature · 1 line
2.53.0.1213.gd9a14994de-goog
#3 lore raw

Re: [PATCH v27 02/10] sched: Avoid donor->sched_class->yield_task() null traversal

Hello John,

Quoted 9 lines · John Stultz

With proxy-exec once we do return migration from ttwu(), if a task is proxying for a waiting donor, and the donor is woken up, we switch the rq->donor to point to idle briefly until we can re-enter __schedule().

However, if a task that was acting as a proxy calls into yield() right after the donor is switched to idle, it may trip a null pointer traversal, because the idle task doesn't have a yield_task() pointer.

After introduction of proxy_resched_idle(), we shouldn't have any case of running with rq->idle as donor with a different rq->curr.

I think we can drop this fix now and squash the changes from Patch 7 into Patch 6 to even avoid any transient cases during bisection where this might be possible

Thoughts?

Signature · 2 lines
Thanks and Regards,
Prateek
#4 lore raw

Re: [PATCH v27 02/10] sched: Avoid donor->sched_class->yield_task() null traversal

Quoted 9 lines · K Prateek Nayak

Hello John,

Quoted 5 lines · John Stultz

With proxy-exec once we do return migration from ttwu(), if a task is proxying for a waiting donor, and the donor is woken up, we switch the rq->donor to point to idle briefly until we can re-enter __schedule().

Quoted 8 lines
Quoted 4 lines

However, if a task that was acting as a proxy calls into yield() right after the donor is switched to idle, it may trip a null pointer traversal, because the idle task doesn't have a yield_task() pointer.

After introduction of proxy_resched_idle(), we shouldn't have any case of running with rq->idle as donor with a different rq->curr.

Oh good point.

Quoted 5 lines

I think we can drop this fix now and squash the changes from Patch 7 into Patch 6 to even avoid any transient cases during bisection where this might be possible

Thoughts?

That sounds fine by me. Next week.

thanks
-john

#5 lore raw

PATCH v27 3/10 sched: deadline: Add some helper variables to cleanup deadline logic

In reply to John Stultz

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]/

---
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: Mel Gorman <[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 | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
kernel/sched/deadline.c +13 -10
index 7352506208287..60ccb492c4427 100644
@@ -2295,7 +2295,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
@@ -2308,14 +2311,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)) {
 		/*
@@ -2327,7 +2330,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));
@@ -2336,14 +2339,14 @@ 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))
@@ -2646,7 +2649,7 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_s
 	struct 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);
@@ -2656,7 +2659,7 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_s
 	if (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.53.0.1213.gd9a14994de-goog
#6 lore raw

PATCH v27 4/10 sched: deadline: Add dl_rq->curr pointer to address issues with Proxy Exec

In reply to John Stultz

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.

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: Mel Gorman <[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
index 60ccb492c4427..3ba5f8deb3687 100644
@@ -2352,6 +2352,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) && !p->dl.dl_throttled && p->nr_cpus_allowed > 1)
 		enqueue_pushable_dl_task(rq, p);
 }
@@ -2574,6 +2577,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;
@@ -2586,6 +2593,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;
 
@@ -2656,6 +2666,9 @@ static void put_prev_task_dl(struct rq *rq, struct task_struct *p, struct task_s
 
 	update_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
index 8ee82b03a8a10..adefea777e0a5 100644
@@ -880,6 +880,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.53.0.1213.gd9a14994de-goog
#7 lore raw

PATCH v27 5/10 sched: Rework block_task so it can be directly called

In reply to John Stultz

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

---
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: Mel Gorman <[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
index 9c8a769a6d109..8f1b14a830851 100644
@@ -2160,8 +2160,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);
 }
@@ -6517,7 +6538,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);
@@ -6537,26 +6557,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.53.0.1213.gd9a14994de-goog
#8 lore raw

PATCH v27 6/10 sched: Have try_to_wake_up() handle return-migration for PROXY_WAKING case

In reply to John Stultz

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]>

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

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 Mel Gorman <[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]>
---
 include/linux/sched.h |   2 +-
 kernel/sched/core.c   | 194 +++++++++++++++++++++---------------------
 2 files changed, 96 insertions(+), 100 deletions(-)
include/linux/sched.h +1 -1
index 8ec3b6d7d718b..3ae1330801157 100644
@@ -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 +95 -99

283 lines not on this page · raw message

Signature · 1 line
2.53.0.1213.gd9a14994de-goog
#9 lore raw

PATCH v27 7/10 sched/core: Reset the donor to current task when donor is woken

In reply to John Stultz

From: K Prateek Nayak <[email protected]>

Introduce proxy_reset_donor() to reset the donor to current task when the donor is woken up.

This avoids needing to run with rq->idle as the donor when proxy_needs_return() hits the donor which can result in another different set of headaches.

---
XXX: Confirm with Juri if there is any side-effect of temporarily doing a set_next_task() on a throttled DL 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 Mel Gorman <[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]>
---
 kernel/sched/core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
kernel/sched/core.c +12 -2
index 2b5f9f905afe1..a0d55225a62c3 100644
@@ -3660,7 +3660,17 @@ void update_rq_avg_idle(struct rq *rq)
 }
 
 #ifdef CONFIG_SCHED_PROXY_EXEC
-static inline struct task_struct *proxy_resched_idle(struct rq *rq);
+static void zap_balance_callbacks(struct rq *rq);
+
+static inline void proxy_reset_donor(struct rq *rq)
+{
+	WARN_ON_ONCE(rq->donor == rq->curr);
+
+	put_prev_set_next_task(rq, rq->donor, rq->curr);
+	rq_set_donor(rq, rq->curr);
+	zap_balance_callbacks(rq);
+	resched_curr(rq);
+}
 
 /*
  * Checks to see if task p has been proxy-migrated to another rq
@@ -3685,7 +3695,7 @@ static inline bool proxy_needs_return(struct rq *rq, struct task_struct *p)
 
 	/* If we're return migrating the rq->donor, switch it out for idle */
 	if (task_current_donor(rq, p))
-		proxy_resched_idle(rq);
+		proxy_reset_donor(rq);
 
 	block_task(rq, p, TASK_WAKING);
 	return true;
Signature · 1 line
2.53.0.1213.gd9a14994de-goog
#10 lore raw

PATCH v27 8/10 sched: Add blocked_donor link to task for smarter mutex handoffs

In reply to 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.

[jstultz: This patch was split out from larger proxy patch]

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

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 Mel Gorman <[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]>
---
 include/linux/sched.h  |  1 +
 init/init_task.c       |  1 +
 kernel/fork.c          |  1 +
 kernel/locking/mutex.c | 44 +++++++++++++++++++++++++++++++++++++++---
 kernel/sched/core.c    | 14 +++++++++++++-
 5 files changed, 57 insertions(+), 4 deletions(-)
include/linux/sched.h +1 -0
index 3ae1330801157..18665b4b973e2 100644
@@ -1238,6 +1238,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
index b5f48ebdc2b6e..41c19670c8f6b 100644
@@ -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
index 079802cb61002..a3d2cd4395791 100644
@@ -2178,6 +2178,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 +41 -3

75 lines not on this page · raw message

kernel/sched/core.c +13 -1
index a0d55225a62c3..9197b4274de8c 100644
@@ -6753,7 +6753,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:
  *
@@ -6893,6 +6903,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;
@@ -7050,6 +7061,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->blocked_on)) {
 			next = find_proxy_task(rq, next, &rf);
 			if (!next) {
Signature · 1 line
2.53.0.1213.gd9a14994de-goog
#11 lore raw

PATCH v27 9/10 sched: Break out core of attach_tasks() helper into sched.h

In reply to John Stultz

Pull 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: Mel Gorman <[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
index 4a6669c517dae..9964670621d8a 100644
@@ -9969,21 +9969,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
index adefea777e0a5..02e28fd211b76 100644
@@ -3034,6 +3034,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.53.0.1213.gd9a14994de-goog
#12 lore raw

PATCH v27 10/10 sched: Migrate whole chain in proxy_migrate_task()

In reply to John Stultz

Instead 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

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 Mel Gorman <[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]>
---
 kernel/sched/core.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
kernel/sched/core.c +13 -6
index 9197b4274de8c..164429de8dc1f 100644
@@ -6723,9 +6723,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
@@ -6738,13 +6738,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.53.0.1213.gd9a14994de-goog