[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANpmjNPJXVtZgT96PP--eNAkHNOvw1MrYzWt5f2aA0LUeK8iGA@mail.gmail.com>
Date: Fri, 19 Dec 2025 22:16:36 +0100
From: Marco Elver <elver@...gle.com>
To: Bart Van Assche <bvanassche@....org>
Cc: Peter Zijlstra <peterz@...radead.org>, Boqun Feng <boqun.feng@...il.com>,
Ingo Molnar <mingo@...nel.org>, Will Deacon <will@...nel.org>,
"David S. Miller" <davem@...emloft.net>, Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
Chris Li <sparse@...isli.org>, "Paul E. McKenney" <paulmck@...nel.org>,
Alexander Potapenko <glider@...gle.com>, Arnd Bergmann <arnd@...db.de>, Christoph Hellwig <hch@....de>,
Dmitry Vyukov <dvyukov@...gle.com>, Eric Dumazet <edumazet@...gle.com>,
Frederic Weisbecker <frederic@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Herbert Xu <herbert@...dor.apana.org.au>, Ian Rogers <irogers@...gle.com>,
Jann Horn <jannh@...gle.com>, Joel Fernandes <joelagnelf@...dia.com>,
Johannes Berg <johannes.berg@...el.com>, Jonathan Corbet <corbet@....net>,
Josh Triplett <josh@...htriplett.org>, Justin Stitt <justinstitt@...gle.com>,
Kees Cook <kees@...nel.org>, Kentaro Takeda <takedakn@...data.co.jp>,
Lukas Bulwahn <lukas.bulwahn@...il.com>, Mark Rutland <mark.rutland@....com>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Miguel Ojeda <ojeda@...nel.org>,
Nathan Chancellor <nathan@...nel.org>, Neeraj Upadhyay <neeraj.upadhyay@...nel.org>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Steven Rostedt <rostedt@...dmis.org>,
Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>, Thomas Gleixner <tglx@...utronix.de>,
Thomas Graf <tgraf@...g.ch>, Uladzislau Rezki <urezki@...il.com>, Waiman Long <longman@...hat.com>,
kasan-dev@...glegroups.com, linux-crypto@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kbuild@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
linux-security-module@...r.kernel.org, linux-sparse@...r.kernel.org,
linux-wireless@...r.kernel.org, llvm@...ts.linux.dev, rcu@...r.kernel.org
Subject: Re: [PATCH v5 07/36] lockdep: Annotate lockdep assertions for context analysis
On Fri, 19 Dec 2025 at 21:54, 'Bart Van Assche' via kasan-dev
<kasan-dev@...glegroups.com> wrote:
>
> On 12/19/25 7:39 AM, Marco Elver wrote:
> > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> > index dd634103b014..621566345406 100644
> > --- a/include/linux/lockdep.h
> > +++ b/include/linux/lockdep.h
> > @@ -282,16 +282,16 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie);
> > do { WARN_ON_ONCE(debug_locks && !(cond)); } while (0)
> >
> > #define lockdep_assert_held(l) \
> > - lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
> > + do { lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD); __assume_ctx_lock(l); } while (0)
> >
> > #define lockdep_assert_not_held(l) \
> > lockdep_assert(lockdep_is_held(l) != LOCK_STATE_HELD)
> >
> > #define lockdep_assert_held_write(l) \
> > - lockdep_assert(lockdep_is_held_type(l, 0))
> > + do { lockdep_assert(lockdep_is_held_type(l, 0)); __assume_ctx_lock(l); } while (0)
> >
> > #define lockdep_assert_held_read(l) \
> > - lockdep_assert(lockdep_is_held_type(l, 1))
> > + do { lockdep_assert(lockdep_is_held_type(l, 1)); __assume_shared_ctx_lock(l); } while (0)
> >
> > #define lockdep_assert_held_once(l) \
> > lockdep_assert_once(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
> > @@ -389,10 +389,10 @@ extern int lockdep_is_held(const void *);
> > #define lockdep_assert(c) do { } while (0)
> > #define lockdep_assert_once(c) do { } while (0)
> >
> > -#define lockdep_assert_held(l) do { (void)(l); } while (0)
> > +#define lockdep_assert_held(l) __assume_ctx_lock(l)
> > #define lockdep_assert_not_held(l) do { (void)(l); } while (0)
> > -#define lockdep_assert_held_write(l) do { (void)(l); } while (0)
> > -#define lockdep_assert_held_read(l) do { (void)(l); } while (0)
> > +#define lockdep_assert_held_write(l) __assume_ctx_lock(l)
> > +#define lockdep_assert_held_read(l) __assume_shared_ctx_lock(l)
> > #define lockdep_assert_held_once(l) do { (void)(l); } while (0)
> > #define lockdep_assert_none_held_once() do { } while (0)
>
> I think these macros should use __must_hold() instead of __assume...().
> lockdep_assert_held() emits a runtime warning if 'l' is not held. Hence,
> I think that code where lockdep_assert_held() is used should not compile
> if it cannot be verified at compile time that 'l' is held.
That's not the purpose of this - if a function or variable should have
a lock held, we mark them explicitly with __must_hold() or
__guarded_by(), and we don't really need to use lockdep_assert,
because the compiler helped us out. In an ideal world, every function
or variable that requires a lock held is annotated, and we don't need
to ever worry about explicitly checking if a lock is held (but we'll
be far from that for a while).
The purpose is described in the commit message:
> Presence of these annotations causes the analysis to assume the context
> lock is held after calls to the annotated function, and avoid false
> positives with complex control-flow; [...]
It's basically an escape hatch to defer to dynamic analysis where the
limits of the static analysis are reached. This is also the original
purpose of the "assert"/"assume" attributes:
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#assert-capability-and-assert-shared-capability
Without this escape hatch, and deferral to dynamic analysis, we'd be
stuck in some cases.
Powered by blists - more mailing lists