Re: [PATCH 08/14] lockdep: Introduce lockdep_assume_ctx_held()

[Peter Zijlstra]

On Thu, Nov 05, 2026 at 03:39:33PM +0100, Marco Elver wrote:
> On Thu, 5 Nov 2026 at 09:37, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > On Mon, Nov 02, 2026 at 01:34:38PM -0800, Bart Van Assche wrote:
> > > Introduce lockdep_assume_ctx_held() and lockdep_assume_shared_ctx_held()
> > > for code that needs to inform Clang's context analysis that a lock is
> > > held in code paths where lock ownership cannot be proven at compile
> > > time, e.g. because the lock is accessed through a pointer alias that
> > > the analysis cannot resolve. These macros call respectively
> > > __assume_ctx_lock() and __assume_shared_ctx_lock() and also verify at
> > > runtime with lockdep that the lock is held. Store the macro argument in
> > > a local variable with a unique name such that it is evaluated only once
> > > and such that nested use of these macros does not cause shadowing. Rely
> > > on Clang's context analysis resolving that local variable to the
> > > expression it has been initialized with. Document both macros with
> > > kernel-doc comments, including that __assume_ctx_lock() and
> > > __assume_shared_ctx_lock() must be used for context locks without
> > > lockdep support, e.g. bit spinlocks and the RCU context tokens.
> > >
> > > Convert the lockdep_assert_held() tests in lib/test_context-analysis.c
> > > into lockdep_assume_ctx_held() tests and add tests for
> > > lockdep_assume_shared_ctx_held().
> > >
> > > This patch prepares for removing the context assumptions from
> > > lockdep_assert_held*().
> >
> > So the only convincing rationale from 0/n for doing this was that the
> > current annotation can hide the need, and thus cause a lacking of
> > __must_hold() on the function.
> >
> > I'm not sure I understand how this alternative form of the same avoids
> > that same problem, and the above doesn't seem to mention the whole
> > problem -- either that, or I just can't read this morning.
> 
> There's no problem if you're dealing with new code. But we have a lot
> of lockdep_assert_held*() sprinkled all over existing code, which when
> compiling the kernel with WARN_CONTEXT_ANALYSIS_ALL will not warn
> where we would be better off with a __must_hold(). I believe Bart had
> been working on enabling this more broadly and doesn't want
> lockdep_assert_held*() hide warnings.
> 
> This latest iteration is a compromise for that problem, rather than
> the previous proposals of removing __assume_ctx_lock() from lockdep
> primitives entirely. The programming model is unchanged: we delegate
> to dynamic checks where the static analysis fails, and that's the main
> purpose of __assume_ctx_lock(). The only difference is that we'd have
> to be more explicit about it, just because of legacy use of
> lockdep_assert_held*().
> 
> I'm somewhat on the fence still, although would be willing to accept
> this compromise if it does help catch more bugs, and is neutral to
> only mildly more mentally taxing in writing new code.

Aaah, so the 'solution' is to rename lockdep_assert_held(), such that
the pre-existing annotation doesn't pre-pollute things? But if you start
by converting existing lockdep_assert_held() to the new
lockdep_assume_*_ctx_held() you'll run into the exact same problems?

That is a very minor and very subtle 'fix'.