Re: [PATCH 03/23] nvmet: add context annotation for nvmet_ns::file

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

On 11/10/26 3:23 AM, Bart Van Assche wrote:
> On 11/8/26 7:28 PM, Nilay Shroff wrote:
>  > @@ -258,6 +258,19 @@ static void nvmet_file_flush_work(struct work_struct *w)
>  >  {
>  >      struct nvmet_req *req = container_of(w, struct nvmet_req, f.work);
>  >
>  > +    /*
>  > +     * The flush work is scheduled from the I/O hotpath after acquiring
>  > +     * a reference to the namespace. This ensures that the namespace
>  > +     * cannot be modified while the I/O is in flight.
>  > +     *
>  > +     * nvmet_file_flush() requires callers to hold ns->subsys->lock.
>  > +     * Since we already hold a namespace reference, explicitly assert
>  > +     * the lock capability here to satisfy the context analyzer.
>  > +     *
>  > +     * Other callers of nvmet_file_flush() that do not hold a namespace
>  > +     * reference must acquire ns->subsys->lock before invoking it.
>  > +     */
>  > +    __assume_ctx_lock(&req->ns->subsys->lock);
>  >      nvmet_req_complete(req, nvmet_file_flush(req));
>  >  }
> 
> This assertion is incorrect. __assume_ctx_lock() informs the compiler
> that a capability is held when it is actually held (e.g. through an alias).
> Here, nvmet_file_flush_work() runs in a workqueue without holding
> subsys->lock. If lockdep_assert_held() were used here, Lockdep would
> trigger a warning.
> 
> nvmet_file_flush() should not be annotated with __must_hold(&req->ns->subsys->lock).
> This was a tricky case because nvmet_file_flush() is called from both the I/O
hotpath and the control path. The I/O hotpath holds a namespace reference, so
it does not necessarily need to hold subsys->lock. However, the control path
invokes nvmet_file_flush() while holding subsys->lock.

I agree that using __assume_ctx_lock() in nvmet_file_flush_work() is incorrect,
since the workqueue callback does not hold subsys->lock. I'll remove the
__must_hold() annotation from nvmet_file_flush() and update the code accordingly
in the next revision.

Thanks,
--Nilay