[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CANDhNCrgxP+Ujvgpvd_0e5TM5Q-UTGfNmv=Lu3kn2MZ6wdS4Kw@mail.gmail.com>
Date: Fri, 1 Aug 2025 11:34:26 -0700
From: John Stultz <jstultz@...gle.com>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: LKML <linux-kernel@...r.kernel.org>,
syzbot+602c4720aed62576cd79@...kaller.appspotmail.com,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 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 <valentin.schneider@....com>, Suleiman Souhlal <suleiman@...gle.com>, airlied@...il.com,
mripard@...nel.org, simona@...ll.ch, tzimmermann@...e.de,
dri-devel@...ts.freedesktop.org, kernel-team@...roid.com
Subject: Re: [RFC][PATCH] locking: Fix __clear_task_blocked_on() warning from
__ww_mutex_wound() path
On Thu, Jul 31, 2025 at 10:09 PM K Prateek Nayak <kprateek.nayak@....com> wrote:
> At the very least I think we should make a local copy of "p->blocked_on"
> to see a consistent view throughout __clear_task_blocked_on() - task either
> sees it is blocked on the mutex and clear "p->blocked_on", or it sees it is
> blocked on nothing and still clears "p->blocked_on".
>
> (Tested lightly with syzbot's C reproducer)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 02c340450469..f35d93cca64f 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2165,6 +2165,8 @@ static inline void set_task_blocked_on(struct task_struct *p, struct mutex *m)
> static inline void __clear_task_blocked_on(struct task_struct *p, struct mutex *m)
> {
> if (m) {
> + struct mutex *blocked_on = p->blocked_on;
> +
> /* Currently we serialize blocked_on under the mutex::wait_lock */
> lockdep_assert_held_once(&m->wait_lock);
> /*
> @@ -2172,7 +2174,7 @@ static inline void __clear_task_blocked_on(struct task_struct *p, struct mutex *
> * blocked_on relationships, but make sure we are not
> * clearing the relationship with a different lock.
> */
> - WARN_ON_ONCE(m && p->blocked_on && p->blocked_on != m);
> + WARN_ON_ONCE(m && blocked_on && blocked_on != m);
> }
> p->blocked_on = NULL;
> }
> ---
>
> End result is the same, only that we avoid an unnecessary splat in this
> very unlikely case and save ourselves some head scratching later :)
Good point. Thanks for suggesting this! I'll rework to include both
this and Maarten's suggestions.
Thank you for the feedback!
-john
Powered by blists - more mailing lists