Re: [PATCH v2 05/12] nbd: Enable lock context analysis

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

On 8/3/26 11:33 PM, Bart Van Assche wrote:
> On 8/3/26 6:26 AM, Nilay Shroff wrote:
>> On 7/31/26 1:28 AM, Bart Van Assche wrote:
>>> @@ -2773,7 +2776,11 @@ static void __exit nbd_cleanup(void)
>>>       /* Also wait for nbd_dev_remove_work() completes */
>>>       destroy_workqueue(nbd_del_wq);
>>> -    idr_destroy(&nbd_index_idr);
>>> +    {
>>> +        __assume_ctx_lock(&nbd_index_mutex);
>>> +        idr_destroy(&nbd_index_idr);
>>> +    }
>>> +
>>
>> Could we instead introduce a scoped __assume_ctx_lock() helper so we
>> don't need the explicit scope here? Alternatively, would annotating
>> nbd_cleanup() with __context_unsafe be a better fit?
> There is code in nbd_cleanup() that must be protected by a mutex so I
> think that we shouldn't annotate nbd_cleanup() with __context_unsafe().
> 
That's fair enough.

> Introducing a scoped __assume_ctx_lock() might be overkill.
> 
Adding an explicit scope around __assume_ctx_lock() just to suppress a
false-positive context warning still looks a bit awkward to me. At this
point we know that the IDR is being destroyed and that there can no longer
be any concurrent users, so holding nbd_index_mutex is unnecessary.
If a scoped __assume_ctx_lock() helper is considered overkill, perhaps we
should have an IDR teardown helper for this case, e.g. idr_destroy_unlocked()
or similar, with the implementation annotated with __context_unsafe. That
would make the teardown semantics explicit without having individual callers
pretend that the protecting lock is held.

Thanks,
--Nilay