From: Peter Zijlstra <peterz@infradead.org>
To: Marco Elver <elver@google.com>
Cc: Bart Van Assche <bvanassche@acm.org>,
Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
Boqun Feng <boqun@kernel.org>, Waiman Long <longman@redhat.com>,
linux-kernel@vger.kernel.org,
"Paul E . McKenney" <paulmck@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Nathan Chancellor <nathan@kernel.org>
Subject: Re: [PATCH 08/14] lockdep: Introduce lockdep_assume_ctx_held()
Date: Thu, 5 Nov 2026 15:58:44 +0100 [thread overview]
Message-ID: <20261105145844.GQ1813469@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <CANpmjNMaDwcm+tQ-cOUjnE3n+5x8WgB5CZ037uurbwVa4Jn0BA@mail.gmail.com>
On Thu, Nov 05, 2026 at 03:39:33PM +0100, Marco Elver wrote:
> On Thu, 5 Nov 2026 at 09:37, Peter Zijlstra <peterz@infradead.org> 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'.
next prev parent reply other threads:[~2026-11-05 14:58 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-02 21:34 [PATCH 00/14] Stop lockdep_assert_held*() from assuming lock ownership Bart Van Assche
2026-11-02 21:34 ` [PATCH 01/14] bit_spinlock: Fix the __bitlock() definition Bart Van Assche
2026-11-02 21:34 ` [PATCH 02/14] cleanup: Disable context analysis for __free() helpers Bart Van Assche
2026-11-02 21:34 ` [PATCH 03/14] tracing: Disable branch profiling for COMPILE_TEST builds Bart Van Assche
2026-11-02 21:34 ` [PATCH 04/14] srcu: Remove check from srcu_dereference_check() Bart Van Assche
2026-11-02 21:34 ` [PATCH 05/14] futex: Annotate __attach_to_pi_owner() and fixup_pi_*owner() Bart Van Assche
2026-11-02 21:34 ` [PATCH 06/14] locking/rwsem: Annotate rwsem internal functions with __must_hold() Bart Van Assche
2026-11-02 21:34 ` [PATCH 07/14] nvme-fabrics: Annotate nvmf_lookup_transport() with __must_hold_shared() Bart Van Assche
2026-11-03 7:51 ` Christoph Hellwig
2026-11-03 20:37 ` Bart Van Assche
2026-11-04 7:13 ` Christoph Hellwig
2026-11-02 21:34 ` [PATCH 08/14] lockdep: Introduce lockdep_assume_ctx_held() Bart Van Assche
2026-11-05 8:37 ` Peter Zijlstra
2026-11-05 14:39 ` Marco Elver
2026-11-05 14:58 ` Peter Zijlstra [this message]
2026-11-05 17:00 ` Bart Van Assche
2026-11-05 16:52 ` Bart Van Assche
2026-11-02 21:34 ` [PATCH 09/14] kfence: Use lockdep_assume_ctx_held() in kfence_report_error() Bart Van Assche
2026-11-02 21:34 ` [PATCH 10/14] Bluetooth: Add lock context annotations for connection aliases Bart Van Assche
2026-11-02 21:34 ` [PATCH 11/14] futex: Verify exit_mutex ownership in futex_exit_recursive() Bart Van Assche
2026-11-02 21:34 ` [PATCH 12/14] lockdep: Stop lockdep_assert_held*() from assuming lock ownership Bart Van Assche
2026-11-02 21:34 ` [PATCH 13/14] compiler-context-analysis: Split the RCU assertion tests Bart Van Assche
2026-11-02 21:34 ` [PATCH 14/14] rcu: Introduce lockdep_assume_in_rcu_*() Bart Van Assche
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261105145844.GQ1813469@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=akpm@linux-foundation.org \
--cc=boqun@kernel.org \
--cc=bvanassche@acm.org \
--cc=elver@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mingo@redhat.com \
--cc=nathan@kernel.org \
--cc=paulmck@kernel.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®