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

Nilay Shroff <[email protected]> Tue, 4 Aug 2026 16:57:51 +0530
Newsgroups org.kernel.vger.linux-block
Message-ID <[email protected]>
On 8/4/26 2:55 PM, Marco Elver wrote:
> On Mon, 3 Aug 2026 at 20:03, Bart Van Assche <[email protected]> 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().
> 
> Just 'context_unsafe(idr_destroy(&nbd_index_idr)); /* comment */'
> would be fine here I think. I've used that elsewhere for destructors.
> It makes sure that the rest of the code is still checked and is as
> simple as it gets.
> 
>> Introducing a scoped __assume_ctx_lock() might be overkill.
> 
> Agree. Note that __assume_ctx_lock() leaks outside the scope, so a
> real scoped __assume_ctx_lock() would just do no-op __acquire() and
> __release() using the cleanup.h infrastructure (similar to the init
> guards I suppose).

Yes correct, I think if we ever implement scoped __assume_ctx_lock() then
it should just do no-op acquire capability and then release capability
like init guard.

Thanks,
--Nilay