Re: [PATCHv2 12/17] nvme: add Clang context annotations for nvme_queue::cq_poll_lock

[Marco Elver]

On Tue, 30 Jun 2026 at 11:26, Nilay Shroff <nilay@xxxxxxxxxxxxx> wrote:
>
> On 6/29/26 6:20 PM, Christoph Hellwig wrote:
> > On Fri, Jun 26, 2026 at 08:52:36PM +0530, Nilay Shroff wrote:
> >>
> >> That's because nvme_poll() and nvme_irq() both reuse nvme_poll_cq() to
> >> process completions, but they rely on different synchronization mechanisms.
> >
> > Well, we could split the function easily, but it still uses the same
> > data structure and thus annotations.
> >
> I see your point. The annotations describe only the polling synchronization model,
> while the same state is also accessed under interrupt serialization for IRQ queues.
> Since __guarded_by() cannot express alternative synchronization mechanisms today,
> the annotations end up requiring __context_unsafe() for the IRQ path.
>
> It sounds like your preference would be to leave these fields unannotated rather
> than partially annotate one synchronization model. Is that the direction you'd
> recommend until the annotation infrastructure can express multiple valid
> synchronization schemes?
>
>
> Marco,
> I know it's not currently supported, but are there any plan to extend the current
> capability model to support multiple valid synchronization mechanisms for a single
> object? For example, allowing a guarded field to be protected by either a lock or
> another abstract capability (such as interrupt serialization), instead of requiring
> a single __guarded_by() relationship." For instance,
>
> __guarded_by_any(&cq_poll_lock, IRQ_CONTEXT)
>
> Then irq handler would be annotated with __capability(irq_serialization).

I had planned to support IRQ context locks, which would be abstract
but not be backed by a real object (similar to how RCU is handled).
Just haven't gotten around to it.

> So now the compiler would know if the code executes in the interrupt/irq context
> then access to the guarded variable doesn't require any lock otherwise it needs
> ->cq_poll_lock when accessed. I don't know how easy it'd be to add such support
> but it may be explored.

Multi-arg __guarded_by() is supported by Clang, but perhaps not
exactly the way you imagined above. This has a brief summary:
https://patch.msgid.link/20260515124426.2227783-1-elver@xxxxxxxxxx

In short, you can hold any lock to get reader access, and must hold
all for writer access.