Message-ID: <20251121115443.GB71307@cmpxchg.org>
Date: Fri, 21 Nov 2025 06:54:43 -0500
From: Johannes Weiner <hannes@...xchg.org>
To: John Stultz <jstultz@...gle.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
	K Prateek Nayak <kprateek.nayak@....com>,
	Joel Fernandes <joelagnelf@...dia.com>,
	Qais Yousef <qyousef@...alina.io>, Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Juri Lelli <juri.lelli@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Valentin Schneider <vschneid@...hat.com>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ben Segall <bsegall@...gle.com>,
	Zimuzo Ezeozue <zezeozue@...gle.com>, Mel Gorman <mgorman@...e.de>,
	Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>,
	Boqun Feng <boqun.feng@...il.com>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Metin Kaya <Metin.Kaya@....com>,
	Xuewen Yan <xuewen.yan94@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Suleiman Souhlal <suleiman@...gle.com>,
	kuyo chang <kuyo.chang@...iatek.com>, hupu <hupu.gm@...il.com>,
	kernel-team@...roid.com
Subject: Re: [PATCH] sched: Fix psi_dequeue for Proxy Execution

On Tue, Nov 18, 2025 at 05:52:23AM +0000, John Stultz wrote:
> Currently, if the sleep flag is set, psi_dequeue() doesn't
> change any of the psi_flags.
> 
> This is because psi_switch_task() will clear TSK_ONCPU as well
> as other potential flags (TSK_RUNNING), and the assumption is
> that a voluntary sleep always consists of a task being dequeued
> followed shortly there after with a psi_sched_switch() call.
> 
> Proxy Execution changes this expectation, as mutex-blocked tasks
> that would normally sleep stay on the runqueue. But in the case
> where the mutex-owning task goes to sleep, or the owner is on a
> remote cpu, we will then deactivate the blocked task shortly
> after.
> 
> In that situation, the mutex-blocked task will have had its
> TSK_ONCPU cleared when it was switched off the cpu, but it will
> stay TSK_RUNNING. Then if we later dequeue it (as currently done
> if we hit a case find_proxy_task() can't yet handle, such as the
> case of the owner being on another rq or a sleeping owner)
> psi_dequeue() won't change any state (leaving it TSK_RUNNING),
> as it incorrectly expects a psi_task_switch() call to
> immediately follow.
> 
> Later on when the task get woken/re-enqueued, and psi_flags are
> set for TSK_RUNNING, we hit an error as the task is already
> TSK_RUNNING:
>   psi: inconsistent task state! task=188:kworker/28:0 cpu=28 psi_flags=4 clear=0 set=4
> 
> To resolve this, extend the logic in psi_dequeue() so that
> if the sleep flag is set, we also check if psi_flags have
> TSK_ONCPU set (meaning the psi_task_switch is imminent) before
> we do the shortcut return.
> 
> If TSK_ONCPU is not set, that means we've already switched away,
> and this psi_dequeue call needs to clear the flags.
> 
> Fixes: be41bde4c3a8 ("sched: Add an initial sketch of the find_proxy_task() function")
> Reported-by: K Prateek Nayak <kprateek.nayak@....com>
> Closes: https://lore.kernel.org/lkml/20251117185550.365156-1-kprateek.nayak@amd.com/
> Signed-off-by: John Stultz <jstultz@...gle.com>
> Tested-by: K Prateek Nayak <kprateek.nayak@....com>
> ---
> v13:
> * Reworked for collision
> v15:
> * Fixed commit message typo noticed by Todd Kjos
> v24:
> * Reworded commit message in response to K Prateek pointing
>   out this issue can affect us earlier in the full proxy
>   series then I had anticipated.
> 
> Cc: Joel Fernandes <joelagnelf@...dia.com>
> Cc: Qais Yousef <qyousef@...alina.io>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Juri Lelli <juri.lelli@...hat.com>
> Cc: Vincent Guittot <vincent.guittot@...aro.org>
> Cc: Dietmar Eggemann <dietmar.eggemann@....com>
> Cc: Valentin Schneider <vschneid@...hat.com>
> Cc: Johannes Weiner <hannes@...xchg.org>
> Cc: Suren Baghdasaryan <surenb@...gle.com>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Ben Segall <bsegall@...gle.com>
> Cc: Zimuzo Ezeozue <zezeozue@...gle.com>
> Cc: Mel Gorman <mgorman@...e.de>
> Cc: Will Deacon <will@...nel.org>
> Cc: Waiman Long <longman@...hat.com>
> Cc: Boqun Feng <boqun.feng@...il.com>
> Cc: "Paul E. McKenney" <paulmck@...nel.org>
> Cc: Metin Kaya <Metin.Kaya@....com>
> Cc: Xuewen Yan <xuewen.yan94@...il.com>
> Cc: K Prateek Nayak <kprateek.nayak@....com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Daniel Lezcano <daniel.lezcano@...aro.org>
> Cc: Suleiman Souhlal <suleiman@...gle.com>
> Cc: kuyo chang <kuyo.chang@...iatek.com>
> Cc: hupu <hupu.gm@...il.com>
> Cc: kernel-team@...roid.com
> ---
>  kernel/sched/stats.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
> index 26f3fd4d34cea..a38459813b537 100644
> --- a/kernel/sched/stats.h
> +++ b/kernel/sched/stats.h
> @@ -180,8 +180,12 @@ static inline void psi_dequeue(struct task_struct *p, int flags)
>  	 * avoid walking all ancestors twice, psi_task_switch() handles
>  	 * TSK_RUNNING and TSK_IOWAIT for us when it moves TSK_ONCPU.
>  	 * Do nothing here.

Newline here for new paragraph?

> +	 * In the SCHED_PROXY_EXECUTION case we may do sleeping
> +	 * dequeues that are not followed by a task switch, so check
> +	 * TSK_ONCPU is set to ensure the task switch is imminent.
> +	 * Otherwise clear the flags as usual.
>  	 */
> -	if (flags & DEQUEUE_SLEEP)
> +	if ((flags & DEQUEUE_SLEEP) && (p->psi_flags & TSK_ONCPU))
>  		return;

Otherwise, looks good to me. Thanks for the detailed explanation in
the changelog!

Acked-by: Johannes Weiner <hannes@...xchg.org>

Powered by blists - more mailing lists