Re: [PATCHv2 11/17] nvme: add Clang context annotations for nvme_queue::sq_lock

[Nilay Shroff <nilay@linux.ibm.com>]

On 6/26/26 3:20 PM, Marco Elver wrote:
> On Fri, 26 Jun 2026 at 08:47, Christoph Hellwig <hch@lst.de> wrote:
>>
>> On Sun, Jun 14, 2026 at 06:45:26PM +0530, Nilay Shroff wrote:
>>> Accesses to sq_tail used solely for tracing are annotated with
>>> context_unsafe(), as they only require a lockless snapshot of the
> 
> This sounds like an intentional "benign" data race...
> 
Yes, agreed. For the tracing case this is an intentional data race,
so it can simply be written as:

trace_nvme_sq(req, cqe->sq_head, data_race(nvmeq->sq_tail));

>>> value. Likewise, nvme_init_queue() and nvme_free_queue() operate on
>>> queues that have not yet been published or are no longer reachable,
>>> and therefore do not require sq_lock protection. Similarly,
>>> nvme_alloc_sq_cmds() allocates memory for nvme_queue::sq_cmds for
>>> the queue which is not yet published or in use and hence it's safe
>>> to annotate all these helpers using context_unsafe.
>>
>> Accessing scalar fields without lock is pretty common.  Don't we
>> have an annotation that only requires the guarding lock for
>> writes?
> 
> If the writes can happen concurrently with readers these are data
> races. I'd probably annotate the particular reads with data_race(...):
> this will both tell Context Analysis this is ok without a lock, and
> also if KCSAN is enabled, not to report these data races when actually
> observed at runtime
> (https://docs.kernel.org/dev-tools/lkmm/docs/access-marking.html).

I don't think data_race() is appropriate for the initialization paths.
There we're only initializing scalar fields before the object is published,
so there is no concurrent reader or writer and hence no actual data race.

The remaining issue is simply convincing the context analyzer that these
accesses are safe. As far as I can tell, the only way to suppress those
warnings today is to use context_unsafe(), either around the helper itself
or around the individual scalar field accesses.

Thanks,
--Nilay