linux-kernel - Re: [PATCH v23 7/9] sched: Have try_to_wake_up() handle return-migration for PROXY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANDhNCq9iS55Y4x779NF+_w2=Uky-m1Jn5Ayb5MeR5Dw4u38Kg@mail.gmail.com>
Date: Wed, 19 Nov 2025 17:05:07 -0800
From: John Stultz <jstultz@...gle.com>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: LKML <linux-kernel@...r.kernel.org>, 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>, 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 v23 7/9] sched: Have try_to_wake_up() handle
 return-migration for PROXY_WAKING case

On Thu, Oct 30, 2025 at 9:28 PM K Prateek Nayak <kprateek.nayak@....com> wrote:
> On 10/30/2025 5:48 AM, John Stultz wrote:
> > +/*
> > + * Checks to see if task p has been proxy-migrated to another rq
> > + * and needs to be returned. If so, we deactivate the task here
> > + * so that it can be properly woken up on the p->wake_cpu
> > + * (or whichever cpu select_task_rq() picks at the bottom of
> > + * try_to_wake_up()
> > + */
> > +static inline bool proxy_needs_return(struct rq *rq, struct task_struct *p)
> > +{
> > +     bool ret = false;
> > +
> > +     if (!sched_proxy_exec())
> > +             return false;
> > +
> > +     raw_spin_lock(&p->blocked_lock);
> > +     if (p->blocked_on == PROXY_WAKING) {
> > +             if (!task_current(rq, p) && p->wake_cpu != cpu_of(rq)) {
> > +                     if (task_current_donor(rq, p))
> > +                             proxy_resched_idle(rq);
> > +
> > +                     deactivate_task(rq, p, DEQUEUE_NOCLOCK);
> > +                     ret = true;
> > +             }
> > +             __clear_task_blocked_on(p, PROXY_WAKING);
> > +             resched_curr(rq);
>
> Do we need to resched_curr() if we've simply dequeued a waiting
> owner who is neither the current, nor the donor? I would have

Hrm. Yeah, I guess really we only need to resched_curr() when we are
skipping the deactivation because already we're waking on the wake_cpu
rq.

I'll fix that up.

> preferred if this function was similar to ttwu_queue_cond() with
> each step explaining the the intent - something like:
>
> static inline bool
> proxy_needs_return(struct rq *rq, struct task_struct *p, int wake_flags)
> {
>         if (!sched_proxy_exec())
>                 return false;
>
>         guard(raw_spinlock)(&p->blocked_lock);
>
>         /* Task has been woken up / is running. */
>         if (p->blocked_on != PROXY_WAKING)
>                 return false;
>
>         __clear_task_blocked_on(p, PROXY_WAKING);

Ah, this is nicer! I'll rework the function in this style! Thanks for
that suggestion!

> > +     /*
> > +      * If we're PROXY_WAKING, we have deactivated on this cpu, so we should
> > +      * activate it here as well, to avoid IPI'ing a cpu that is stuck in
> > +      * task_rq_lock() spinning on p->on_rq, deadlocking that cpu.
> > +      */
>
> Sounds like block_task() would be better than deactivate_task() above
> in that case. Anything that is waiting on the task's state change takes
> the pi_lock afaik and the wakeup is always done with pi_lock held so
> blocking the task shouldn't cause any problems based on my reading.

So earlier I did try using block_task() but it always seemed to run
into crashes, which I assumed was because other cpus were picking the
task up as it wasn't on_rq (any references to a task after
block_task() in other situations often runs into this trouble).

But your point about the pi_lock being held is a good one, so I will
tinker and think a bit more on this.

Thanks as always for the review!
-john