Re: [PATCHv2 11/17] nvme: add Clang context annotations for nvme_queue::sq_lock
From: Nilay Shroff
Date: Fri Jun 26 2026 - 11:22:38 EST
On 6/26/26 3:20 PM, Marco Elver wrote:
On Fri, 26 Jun 2026 at 08:47, Christoph Hellwig <hch@xxxxxx> wrote:Yes, agreed. For the tracing case this is an intentional data race,
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...
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