Add a find_proxy_task() function which doesn't do much.
When we select a blocked task to run, we will just deactivate it
and pick again. The exception being if it has become unblocked
after find_proxy_task() was called.
Greatly simplified from patch by:
Peter Zijlstra (Intel) <peterz@infradead.org>
Juri Lelli <juri.lelli@redhat.com>
Valentin Schneider <valentin.schneider@arm.com>
Connor O'Brien <connoro@google.com>
Cc: Joel Fernandes <joelagnelf@nvidia.com>
Cc: Qais Yousef <qyousef@layalina.io>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ben Segall <bsegall@google.com>
Cc: Zimuzo Ezeozue <zezeozue@google.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Will Deacon <will@kernel.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Metin Kaya <Metin.Kaya@arm.com>
Cc: Xuewen Yan <xuewen.yan94@gmail.com>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Suleiman Souhlal <suleiman@google.com>
Cc: kernel-team@android.com
[jstultz: Split out from larger proxy patch and simplified
for review and testing.]
Signed-off-by: John Stultz <jstultz@google.com>
---
v5:
* Split out from larger proxy patch
v7:
* Fixed unused function arguments, spelling nits, and tweaks for
clarity, pointed out by Metin Kaya
* Fix build warning Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202311081028.yDLmCWgr-lkp@intel.com/
v8:
* Fixed case where we might return a blocked task from find_proxy_task()
* Continued tweaks to handle avoiding returning blocked tasks
v9:
* Add zap_balance_callbacks helper to unwind balance_callbacks
when we will re-call pick_next_task() again.
* Add extra comment suggested by Metin
* Typo fixes from Metin
* Moved adding proxy_resched_idle earlier in the series, as suggested
by Metin
* Fix to call proxy_resched_idle() *prior* to deactivating next, to avoid
crashes caused by stale references to next
* s/PROXY/SCHED_PROXY_EXEC/ as suggested by Metin
* Number of tweaks and cleanups suggested by Metin
* Simplify proxy_deactivate as suggested by Metin
v11:
* Tweaks for earlier simplification in try_to_deactivate_task
v13:
* Rename rename "next" to "donor" in find_proxy_task() for clarity
* Similarly use "donor" instead of next in proxy_deactivate
* Refactor/simplify proxy_resched_idle
* Moved up a needed fix from later in the series
v15:
* Tweaked some comments to better explain the initial sketch of
find_proxy_task(), suggested by Qais
* Build fixes for !CONFIG_SMP
* Slight rework for blocked_on_state being added later in the
series.
* Move the zap_balance_callbacks to later in the patch series
---
kernel/sched/core.c | 103 +++++++++++++++++++++++++++++++++++++++++--
kernel/sched/rt.c | 15 ++++++-
kernel/sched/sched.h | 10 ++++-
3 files changed, 122 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3968c3967ec38..b4f7b14f62a24 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6600,7 +6600,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
* Otherwise marks the task's __state as RUNNING
*/
static bool try_to_block_task(struct rq *rq, struct task_struct *p,
- unsigned long task_state)
+ unsigned long task_state, bool deactivate_cond)
{
int flags = DEQUEUE_NOCLOCK;
@@ -6609,6 +6609,9 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
return false;
}
+ if (!deactivate_cond)
+ return false;
+
p->sched_contributes_to_load =
(task_state & TASK_UNINTERRUPTIBLE) &&
!(task_state & TASK_NOLOAD) &&
@@ -6632,6 +6635,93 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
return true;
}
+#ifdef CONFIG_SCHED_PROXY_EXEC
+
+static inline struct task_struct *
+proxy_resched_idle(struct rq *rq)
+{
+ put_prev_task(rq, rq->donor);
+ rq_set_donor(rq, rq->idle);
+ set_next_task(rq, rq->idle);
+ set_tsk_need_resched(rq->idle);
+ return rq->idle;
+}
+
+static bool proxy_deactivate(struct rq *rq, struct task_struct *donor)
+{
+ unsigned long state = READ_ONCE(donor->__state);
+
+ /* Don't deactivate if the state has been changed to TASK_RUNNING */
+ if (state == TASK_RUNNING)
+ return false;
+ /*
+ * Because we got donor from pick_next_task, it is *crucial*
+ * that we call proxy_resched_idle before we deactivate it.
+ * As once we deactivate donor, donor->on_rq is set to zero,
+ * which allows ttwu to immediately try to wake the task on
+ * another rq. So we cannot use *any* references to donor
+ * after that point. So things like cfs_rq->curr or rq->donor
+ * need to be changed from next *before* we deactivate.
+ */
+ proxy_resched_idle(rq);
+ return try_to_block_task(rq, donor, state, true);
+}
+
+/*
+ * Initial simple sketch that just deactivates the blocked task
+ * chosen by pick_next_task() so we can then pick something that
+ * isn't blocked.
+ */
+static struct task_struct *
+find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
+{
+ struct task_struct *p = donor;
+ struct mutex *mutex;
+
+ mutex = p->blocked_on;
+ /* Something changed in the chain, so pick again */
+ if (!mutex)
+ return NULL;
+ /*
+ * By taking mutex->wait_lock we hold off concurrent mutex_unlock()
+ * and ensure @owner sticks around.
+ */
+ raw_spin_lock(&mutex->wait_lock);
+
+ /* Check again that p is blocked with blocked_lock held */
+ if (!task_is_blocked(p) || mutex != __get_task_blocked_on(p)) {
+ /*
+ * Something changed in the blocked_on chain and
+ * we don't know if only at this level. So, let's
+ * just bail out completely and let __schedule
+ * figure things out (pick_again loop).
+ */
+ goto out;
+ }
+
+ if (!proxy_deactivate(rq, donor)) {
+ /*
+ * XXX: For now, if deactivation failed, set donor
+ * as not blocked, as we aren't doing proxy-migrations
+ * yet (more logic will be needed then).
+ */
+ __clear_task_blocked_on(donor, mutex);
+ raw_spin_unlock(&mutex->wait_lock);
+ return NULL;
+ }
+out:
+ raw_spin_unlock(&mutex->wait_lock);
+ return NULL; /* do pick_next_task again */
+}
+#else /* SCHED_PROXY_EXEC */
+static struct task_struct *
+find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
+{
+ WARN_ONCE(1, "This should never be called in the !SCHED_PROXY_EXEC case\n");
+ return donor;
+}
+#endif /* SCHED_PROXY_EXEC */
+
/*
* __schedule() is the main scheduler function.
*
@@ -6739,12 +6829,19 @@ static void __sched notrace __schedule(int sched_mode)
goto picked;
}
} else if (!preempt && prev_state) {
- try_to_block_task(rq, prev, prev_state);
+ try_to_block_task(rq, prev, prev_state,
+ !task_is_blocked(prev));
switch_count = &prev->nvcsw;
}
- next = pick_next_task(rq, prev, &rf);
+pick_again:
+ next = pick_next_task(rq, rq->donor, &rf);
rq_set_donor(rq, next);
+ if (unlikely(task_is_blocked(next))) {
+ next = find_proxy_task(rq, next, &rf);
+ if (!next)
+ goto pick_again;
+ }
picked:
clear_tsk_need_resched(prev);
clear_preempt_need_resched();
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 4b8e33c615b12..2d418e0efecc5 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1479,8 +1479,19 @@ enqueue_task_rt(struct rq *rq, struct task_struct *p, int flags)
enqueue_rt_entity(rt_se, flags);
- if (!task_current(rq, p) && p->nr_cpus_allowed > 1)
- enqueue_pushable_task(rq, p);
+ /*
+ * Current can't be pushed away. Selected is tied to current,
+ * so don't push it either.
+ */
+ if (task_current(rq, p) || task_current_donor(rq, p))
+ return;
+ /*
+ * Pinned tasks can't be pushed.
+ */
+ if (p->nr_cpus_allowed == 1)
+ return;
+
+ enqueue_pushable_task(rq, p);
}
static bool dequeue_task_rt(struct rq *rq, struct task_struct *p, int flags)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 05d2122533619..3e49d77ce2cdd 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2311,6 +2311,14 @@ static inline int task_current_donor(struct rq *rq, struct task_struct *p)
return rq->donor == p;
}
+static inline bool task_is_blocked(struct task_struct *p)
+{
+ if (!sched_proxy_exec())
+ return false;
+
+ return !!p->blocked_on;
+}
+
static inline int task_on_cpu(struct rq *rq, struct task_struct *p)
{
#ifdef CONFIG_SMP
@@ -2520,7 +2528,7 @@ static inline void put_prev_set_next_task(struct rq *rq,
struct task_struct *prev,
struct task_struct *next)
{
- WARN_ON_ONCE(rq->curr != prev);
+ WARN_ON_ONCE(rq->donor != prev);
__put_prev_set_next_dl_server(rq, prev, next);
--
2.49.0.rc0.332.g42c0ae87b1-goog
On Wed, Mar 12, 2025 at 03:11:35PM -0700, John Stultz wrote: > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > index 4b8e33c615b12..2d418e0efecc5 100644 > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -1479,8 +1479,19 @@ enqueue_task_rt(struct rq *rq, struct task_struct *p, int flags) > > enqueue_rt_entity(rt_se, flags); > > - if (!task_current(rq, p) && p->nr_cpus_allowed > 1) > - enqueue_pushable_task(rq, p); > + /* > + * Current can't be pushed away. Selected is tied to current, > + * so don't push it either. > + */ > + if (task_current(rq, p) || task_current_donor(rq, p)) > + return; > + /* > + * Pinned tasks can't be pushed. > + */ > + if (p->nr_cpus_allowed == 1) > + return; > + > + enqueue_pushable_task(rq, p); > } > As per always, deadline.c will have the exact same logic.
Hello John,
On 3/13/2025 3:41 AM, John Stultz wrote:
> Add a find_proxy_task() function which doesn't do much.
>
> When we select a blocked task to run, we will just deactivate it
> and pick again. The exception being if it has become unblocked
> after find_proxy_task() was called.
>
> Greatly simplified from patch by:
> Peter Zijlstra (Intel) <peterz@infradead.org>
> Juri Lelli <juri.lelli@redhat.com>
> Valentin Schneider <valentin.schneider@arm.com>
> Connor O'Brien <connoro@google.com>
>
> Cc: Joel Fernandes <joelagnelf@nvidia.com>
> Cc: Qais Yousef <qyousef@layalina.io>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ben Segall <bsegall@google.com>
> Cc: Zimuzo Ezeozue <zezeozue@google.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Will Deacon <will@kernel.org>
> Cc: Waiman Long <longman@redhat.com>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Metin Kaya <Metin.Kaya@arm.com>
> Cc: Xuewen Yan <xuewen.yan94@gmail.com>
> Cc: K Prateek Nayak <kprateek.nayak@amd.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Suleiman Souhlal <suleiman@google.com>
> Cc: kernel-team@android.com
> [jstultz: Split out from larger proxy patch and simplified
> for review and testing.]
> Signed-off-by: John Stultz <jstultz@google.com>
> ---
> v5:
> * Split out from larger proxy patch
> v7:
> * Fixed unused function arguments, spelling nits, and tweaks for
> clarity, pointed out by Metin Kaya
> * Fix build warning Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202311081028.yDLmCWgr-lkp@intel.com/
> v8:
> * Fixed case where we might return a blocked task from find_proxy_task()
> * Continued tweaks to handle avoiding returning blocked tasks
> v9:
> * Add zap_balance_callbacks helper to unwind balance_callbacks
> when we will re-call pick_next_task() again.
> * Add extra comment suggested by Metin
> * Typo fixes from Metin
> * Moved adding proxy_resched_idle earlier in the series, as suggested
> by Metin
> * Fix to call proxy_resched_idle() *prior* to deactivating next, to avoid
> crashes caused by stale references to next
> * s/PROXY/SCHED_PROXY_EXEC/ as suggested by Metin
> * Number of tweaks and cleanups suggested by Metin
> * Simplify proxy_deactivate as suggested by Metin
> v11:
> * Tweaks for earlier simplification in try_to_deactivate_task
> v13:
> * Rename rename "next" to "donor" in find_proxy_task() for clarity
> * Similarly use "donor" instead of next in proxy_deactivate
> * Refactor/simplify proxy_resched_idle
> * Moved up a needed fix from later in the series
> v15:
> * Tweaked some comments to better explain the initial sketch of
> find_proxy_task(), suggested by Qais
> * Build fixes for !CONFIG_SMP
> * Slight rework for blocked_on_state being added later in the
> series.
> * Move the zap_balance_callbacks to later in the patch series
> ---
> kernel/sched/core.c | 103 +++++++++++++++++++++++++++++++++++++++++--
> kernel/sched/rt.c | 15 ++++++-
> kernel/sched/sched.h | 10 ++++-
> 3 files changed, 122 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 3968c3967ec38..b4f7b14f62a24 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6600,7 +6600,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
> * Otherwise marks the task's __state as RUNNING
> */
> static bool try_to_block_task(struct rq *rq, struct task_struct *p,
> - unsigned long task_state)
> + unsigned long task_state, bool deactivate_cond)
> {
> int flags = DEQUEUE_NOCLOCK;
>
> @@ -6609,6 +6609,9 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
> return false;
> }
>
> + if (!deactivate_cond)
> + return false;
> +
> p->sched_contributes_to_load =
> (task_state & TASK_UNINTERRUPTIBLE) &&
> !(task_state & TASK_NOLOAD) &&
> @@ -6632,6 +6635,93 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
> return true;
> }
>
> +#ifdef CONFIG_SCHED_PROXY_EXEC
> +
> +static inline struct task_struct *
> +proxy_resched_idle(struct rq *rq)
nit. Any reason why this was put in the next line?
> +{
> + put_prev_task(rq, rq->donor);
Any reason we cannot do a:
put_prev_set_next_task(rq, rq->donor, rq->idle)
here? I don't see any dependency on rq->donor in set_next_task_idle()
and it should be safe.
> + rq_set_donor(rq, rq->idle);
> + set_next_task(rq, rq->idle);
> + set_tsk_need_resched(rq->idle);
> + return rq->idle;
> +}
> +
> +static bool proxy_deactivate(struct rq *rq, struct task_struct *donor)
> +{
> + unsigned long state = READ_ONCE(donor->__state);
> +
> + /* Don't deactivate if the state has been changed to TASK_RUNNING */
> + if (state == TASK_RUNNING)
> + return false;
> + /*
> + * Because we got donor from pick_next_task, it is *crucial*
> + * that we call proxy_resched_idle before we deactivate it.
> + * As once we deactivate donor, donor->on_rq is set to zero,
> + * which allows ttwu to immediately try to wake the task on
> + * another rq. So we cannot use *any* references to donor
> + * after that point. So things like cfs_rq->curr or rq->donor
> + * need to be changed from next *before* we deactivate.
> + */
> + proxy_resched_idle(rq);
> + return try_to_block_task(rq, donor, state, true);
> +}
> +
> +/*
> + * Initial simple sketch that just deactivates the blocked task
> + * chosen by pick_next_task() so we can then pick something that
> + * isn't blocked.
> + */
> +static struct task_struct *
> +find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
> +{
> + struct task_struct *p = donor;
> + struct mutex *mutex;
> +
> + mutex = p->blocked_on;
> + /* Something changed in the chain, so pick again */
> + if (!mutex)
> + return NULL;
> + /*
> + * By taking mutex->wait_lock we hold off concurrent mutex_unlock()
> + * and ensure @owner sticks around.
> + */
> + raw_spin_lock(&mutex->wait_lock);
> +
> + /* Check again that p is blocked with blocked_lock held */
> + if (!task_is_blocked(p) || mutex != __get_task_blocked_on(p)) {
> + /*
> + * Something changed in the blocked_on chain and
> + * we don't know if only at this level. So, let's
> + * just bail out completely and let __schedule
> + * figure things out (pick_again loop).
> + */
> + goto out;
> + }
> +
> + if (!proxy_deactivate(rq, donor)) {
> + /*
> + * XXX: For now, if deactivation failed, set donor
> + * as not blocked, as we aren't doing proxy-migrations
> + * yet (more logic will be needed then).
> + */
> + __clear_task_blocked_on(donor, mutex);
> + raw_spin_unlock(&mutex->wait_lock);
> + return NULL;
> + }
> +out:
> + raw_spin_unlock(&mutex->wait_lock);
> + return NULL; /* do pick_next_task again */
> +}
> +#else /* SCHED_PROXY_EXEC */
> +static struct task_struct *
> +find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
> +{
> + WARN_ONCE(1, "This should never be called in the !SCHED_PROXY_EXEC case\n");
> + return donor;
> +}
> +#endif /* SCHED_PROXY_EXEC */
> +
> /*
> * __schedule() is the main scheduler function.
> *
> @@ -6739,12 +6829,19 @@ static void __sched notrace __schedule(int sched_mode)
> goto picked;
> }
> } else if (!preempt && prev_state) {
> - try_to_block_task(rq, prev, prev_state);
> + try_to_block_task(rq, prev, prev_state,
> + !task_is_blocked(prev));
> switch_count = &prev->nvcsw;
> }
>
> - next = pick_next_task(rq, prev, &rf);
> +pick_again:
> + next = pick_next_task(rq, rq->donor, &rf);
> rq_set_donor(rq, next);
> + if (unlikely(task_is_blocked(next))) {
> + next = find_proxy_task(rq, next, &rf);
> + if (!next)
> + goto pick_again;
> + }
> picked:
> clear_tsk_need_resched(prev);
> clear_preempt_need_resched();
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 4b8e33c615b12..2d418e0efecc5 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1479,8 +1479,19 @@ enqueue_task_rt(struct rq *rq, struct task_struct *p, int flags)
>
> enqueue_rt_entity(rt_se, flags);
>
> - if (!task_current(rq, p) && p->nr_cpus_allowed > 1)
> - enqueue_pushable_task(rq, p);
> + /*
> + * Current can't be pushed away. Selected is tied to current,
> + * so don't push it either.
> + */
> + if (task_current(rq, p) || task_current_donor(rq, p))
> + return;
> + /*
> + * Pinned tasks can't be pushed.
> + */
> + if (p->nr_cpus_allowed == 1)
> + return;
> +
> + enqueue_pushable_task(rq, p);
> }
>
> static bool dequeue_task_rt(struct rq *rq, struct task_struct *p, int flags)
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 05d2122533619..3e49d77ce2cdd 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2311,6 +2311,14 @@ static inline int task_current_donor(struct rq *rq, struct task_struct *p)
> return rq->donor == p;
> }
>
> +static inline bool task_is_blocked(struct task_struct *p)
> +{
> + if (!sched_proxy_exec())
> + return false;
> +
> + return !!p->blocked_on;
> +}
> +
> static inline int task_on_cpu(struct rq *rq, struct task_struct *p)
> {
> #ifdef CONFIG_SMP
> @@ -2520,7 +2528,7 @@ static inline void put_prev_set_next_task(struct rq *rq,
> struct task_struct *prev,
> struct task_struct *next)
> {
> - WARN_ON_ONCE(rq->curr != prev);
> + WARN_ON_ONCE(rq->donor != prev);
>
> __put_prev_set_next_dl_server(rq, prev, next);
>
--
Thanks and Regards,
Prateek
© 2016 - 2025 Red Hat, Inc.