linux-kernel - Re: [PATCH v16 2/7] locking/mutex: Rework task_struct::blocked

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: <CANDhNCr8sza8fhdc1vxoRXuojdPsO4ZG8RZzvyfjSc7S45uhRw@mail.gmail.com>
Date: Wed, 16 Apr 2025 14:28:26 -0700
From: John Stultz <jstultz@...gle.com>
To: Juri Lelli <juri.lelli@...hat.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Peter Zijlstra <peterz@...radead.org>, 
	Joel Fernandes <joelagnelf@...dia.com>, Qais Yousef <qyousef@...alina.io>, 
	Ingo Molnar <mingo@...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>, K Prateek Nayak <kprateek.nayak@....com>, 
	Thomas Gleixner <tglx@...utronix.de>, Daniel Lezcano <daniel.lezcano@...aro.org>, 
	Suleiman Souhlal <suleiman@...gle.com>, kernel-team@...roid.com, 
	"Connor O'Brien" <connoro@...gle.com>
Subject: Re: [PATCH v16 2/7] locking/mutex: Rework task_struct::blocked_on

On Mon, Apr 14, 2025 at 2:00 AM Juri Lelli <juri.lelli@...hat.com> wrote:
>
> Hi John,
>
> On 11/04/25 23:02, John Stultz wrote:
> > From: Peter Zijlstra <peterz@...radead.org>
> >
> > Track the blocked-on relation for mutexes, to allow following this
> > relation at schedule time.
> >
> >    task
> >      | blocked-on
> >      v
> >    mutex
> >      | owner
> >      v
> >    task
> >
> > This all will be used for tracking blocked-task/mutex chains
> > with the prox-execution patch in a similar fashion to how
> > priority inheritance is done with rt_mutexes.
> >
> > For serialization, blocked-on is only set by the task itself
> > (current). And both when setting or clearing (potentially by
> > others), is done while holding the mutex::wait_lock.
> >
> > 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: 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: kernel-team@...roid.com
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> > [minor changes while rebasing]
> > Signed-off-by: Juri Lelli <juri.lelli@...hat.com>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> > Signed-off-by: Connor O'Brien <connoro@...gle.com>
> > [jstultz: Fix blocked_on tracking in __mutex_lock_common in error paths]
> > Signed-off-by: John Stultz <jstultz@...gle.com>
> > ---
>
> ...
>
> > @@ -940,6 +954,14 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
> >               next = waiter->task;
> >
> >               debug_mutex_wake_waiter(lock, waiter);
> > +             /*
> > +              * Unlock wakeups can be happening in parallel
> > +              * (when optimistic spinners steal and release
> > +              * the lock), so blocked_on may already be
> > +              * cleared here.
> > +              */
> > +             WARN_ON(next->blocked_on && next->blocked_on != lock);
> > +             next->blocked_on = NULL;
>
> Here and below, why the WARN_ON() if the fact that blocked_on has been
> cleared already it's an OK situation? Ah, maybe it's catching the more
> worrying situation that the lock has changed since the task blocked?

Right. Just trying to make sure we aren't getting into any mismatched
usage of the lock.
If it has been cleared already, that's ok, but I want to make sure
otherwise that it is the lock we are expecting.

Appreciate the review. Let me know if you have any further questions
or clarifications!

thanks
-john