[PATCH 01/23] nvmet: add context annotation for nvmet_ns::bdev

[Nilay Shroff]

On 11/10/26 3:16 AM, Bart Van Assche wrote:
> On 11/8/26 7:28 PM, Nilay Shroff wrote:
>  > The nvmet_ns::bdev field is protected by nvmet_subsys::lock. Annotate
>  > it with __guarded_by(&subsys->lock) so that the clang context analyzer
>  > can validate accesses to this field.
> 
> This patch introduces a compiler error in nvmet_get_smart_log_all() when
> built with CONFIG_WARN_CONTEXT_ANALYSIS=y:
> 
>    drivers/nvme/target/admin-cmd.c: error: reading variable 'bdev' requires
>    holding mutex 'ns->subsys->lock' [-Werror,-Wthread-safety-precise]
> 
> This is only resolved in patch 02. Please make sure this series is
> bisectable.
> 
Enabling CONFIG_WARN_CONTEXT_ANALYSIS=y only enables the context-analysis
infrastructure. The analysis is opt-in by default, and individual subsystems
or source files need to explicitly enable it through their Makefiles.

I haven't enabled context analysis for the NVMe target subsystem yet in this
series. My plan is to enable it by adding CONTEXT_ANALYSIS=y to the NVMe target
Makefile in a subsequent series, after completing the context annotations
throughout the subsystem.

Therefore, with only CONFIG_WARN_CONTEXT_ANALYSIS=y enabled (and without
CONFIG_WARN_CONTEXT_ANALYSIS_ALL=y), this patch should not trigger the reported error.
Could you check whether your build has CONFIG_WARN_CONTEXT_ANALYSIS_ALL enabled?

>  > +    /*
>  > +     * Both ctrl->subsys->lock and ns->subsys->lock refers to the same lock.
> 
> "refers to" -> "refer to".
Okay>
>  > +     * However there's no way for us to annotate such locking semantics
>  > +     * using clang context annotation. So explicitly assert the capability.
>  > +     */
>  > +    __assume_ctx_lock(&ns->subsys->lock);
> 
> __assume_ctx_lock() wouldn't be necessary if subsys would be passed
> as a function argument, isn't it? In general, I strongly prefer to
> add a function argument rather than using __assume_ctx_lock().
> 

I agree that passing the lock owner as a function argument is preferable whenever
possible. For this particular case, I tried passing subsys down the call chain to
nvmet_ns_dev() and annotating the function with __must_hold(&subsys->lock). However,
Clang still reports a locking violation when accessing ns->bdev:

drivers/nvme/target/nvmet.h:213:13: error: reading variable 'bdev'
requires holding mutex 'ns->subsys->lock'
[-Werror,-Wthread-safety-precise]
213 |         return ns->bdev ? disk_to_dev(ns->bdev->bd_disk) : NULL;
|                    ^
drivers/nvme/target/nvmet.h:213:13: note: found near match 'subsys->lock'

It appears that the context analyzer cannot infer that subsys->lock and ns->subsys->lock
refer to the same mutex, even though subsys and ns->subsys refer to the same subsystem.
Consequently, I couldn't avoid using __assume_ctx_lock() in this case to explicitly
establish the lock capability expected by the field annotation.

Thanks,
--Nilay