[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aWFt6hcLaCjQQu2c@elver.google.com>
Date: Fri, 9 Jan 2026 22:06:50 +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 20/36] locking/ww_mutex: Support Clang's context
analysis
On Fri, Jan 09, 2026 at 12:16PM -0800, Bart Van Assche wrote:
> On 12/19/25 8:40 AM, Marco Elver wrote:
> > Add support for Clang's context analysis for ww_mutex.
> >
> > The programming model for ww_mutex is subtly more complex than other
> > locking primitives when using ww_acquire_ctx. Encoding the respective
> > pre-conditions for ww_mutex lock/unlock based on ww_acquire_ctx state
> > using Clang's context analysis makes incorrect use of the API harder.
>
> That's a very short description. It should have been explained in the
> patch description how the ww_acquire_ctx changes affect callers of the
> ww_acquire_{init,done,fini}() functions.
How so? The API is the same (now statically enforced), and there's no
functional change at runtime. Or did I miss something?
> > static inline void ww_acquire_init(struct ww_acquire_ctx *ctx,
> > struct ww_class *ww_class)
> > + __acquires(ctx) __no_context_analysis
> > [ ... ]
> > static inline void ww_acquire_done(struct ww_acquire_ctx *ctx)
> > + __releases(ctx) __acquires_shared(ctx) __no_context_analysis
> > {
> > [ ... ]
> > static inline void ww_acquire_fini(struct ww_acquire_ctx *ctx)
> > + __releases_shared(ctx) __no_context_analysis
>
> The above changes make it mandatory to call ww_acquire_done() before
> calling ww_acquire_fini(). In Documentation/locking/ww-mutex-design.rst
> there is an example where there is no ww_acquire_done() call between
> ww_acquire_init() and ww_acquire_fini() (see also line 202).
It might be worth updating the example with what the kernel-doc
documentation recommends (below).
> The
> function dma_resv_lockdep() in drivers/dma-buf/dma-resv.c doesn't call
> ww_acquire_done() at all. Does this mean that the above annotations are
> wrong?
If there's 1 out of N ww_mutex users that missed ww_acquire_done()
there's a good chance that 1 case is wrong.
But generally, depends if we want to enforce ww_acquire_done() or not
which itself is no-op in non-lockdep builds, however, with
DEBUG_WW_MUTEXES it's no longer no-op so it might be a good idea to
enforce it to get proper lockdep checking.
> Is there a better solution than removing the __acquire() and
> __release() annotations from the above three functions?
The kernel-doc comment for ww_acquire_done() says:
/**
* ww_acquire_done - marks the end of the acquire phase
* @ctx: the acquire context
*
>> * Marks the end of the acquire phase, any further w/w mutex lock calls using
>> * this context are forbidden.
>> *
>> * Calling this function is optional, it is just useful to document w/w mutex
>> * code and clearly designated the acquire phase from actually using the locked
>> * data structures.
*/
static inline void ww_acquire_done(struct ww_acquire_ctx *ctx)
__releases(ctx) __acquires_shared(ctx) __no_context_analysis
{
#ifdef DEBUG_WW_MUTEXES
lockdep_assert_held(ctx);
DEBUG_LOCKS_WARN_ON(ctx->done_acquire);
ctx->done_acquire = 1;
#endif
}
It states it's optional, but it's unclear if that's true with
DEBUG_WW_MUTEXES builds. I'd vote for enforcing use of
ww_acquire_done(). If there's old code that's not using it, it should be
added there to get proper lockdep checking.
Powered by blists - more mailing lists