Re: [PATCHv2 07/17] nvme: add Clang context annotations for nvme_subsystem::lock
From: Marco Elver
Date: Mon Jun 29 2026 - 19:13:36 EST
On Mon, 29 Jun 2026 at 14:47, Christoph Hellwig <hch@xxxxxx> wrote:
>
> On Fri, Jun 26, 2026 at 08:09:52PM +0530, Nilay Shroff wrote:
> >>
> > yes correct, but this one is tricky as the list has to be inited
> > with non-zero value. Another way to fix this is by adding
> > context_unsafe wrapper around INIT_LIST_HEAD(). Again overuse of
> > those unsafe wrappers hinders readability.
> >
> > Would it make sense to introduce a helper, e.g. INIT_LIST_HEAD_UNSAFE(),
> > that simply wraps INIT_LIST_HEAD() with context_unsafe()? It would document
> > that this should only be used in cases where the caller knows the object
> > has not yet been published and therefore no concurrent access is possible,
> > such as during object initialization.
>
> I'll leave that to Marco and other core people, but I think this
> would be a lot better than the current version.
Initialization of guarded objects has gone through a few iterations,
so I don't want to open that can of worms again. I think the
infrastructure we have now provides various options (the scoped guard
machinery isn't the only way). You could just write:
/* Initializes unpublished lock-guarded variables. */
context_unsafe(
INIT_LIST_HEAD(&subsys->nsheads);
// ... other guarded var init in same block ...
);
context_unsafe() has a statement expression inside, so you can have
multiple statements in one context_unsafe() block.
And leave the mutex_init() where it was if you don't like the scoped_guard().
Although here you just have 1 INIT_LIST_HEAD() that needs that
treatment, so "context_unsafe(INIT_LIST_HEAD(...))" would do -- it's
just 9 chars more vs INIT_LIST_HEAD_UNSAFE(). And as soon as you have
several guarded variable initializations in a block, you can group
them as above.
If you introduce INIT_LIST_HEAD_UNSAFE(), that'll create more ways to
initialize guarded variables. I'm not against it if none of the other
ways work for you, but expanding the API surface also incurs a
complexity cost.
The simplest option is just adding __context_unsafe(init) as an
attribute to these init functions, although I see they do more than
just trivial initialization, so keeping context analysis enabled might
catch real bugs.