Re: [PATCH V2 2/6] null_blk: give the file-scope mutex a descriptive name

[Zizhi Wo <wozizhi@huawei.com>]

在 2026/7/7 14:28, Damien Le Moal 写道:
> On 7/7/26 15:24, Zizhi Wo wrote:
>>
>>
>> 在 2026/7/7 12:16, Damien Le Moal 写道:
>>> On 7/7/26 11:55, Zizhi Wo wrote:
>>>> From: Zizhi Wo <wozizhi@huawei.com>
>>>>
>>>> The file-scope lock mutex serializes access to global null_blk state,
>>>> including the nullb_list and device creation/removal. Rename it to
>>>> "nullb_global_lock" to make its purpose clear. No functional change.
>>>>
>>>> Suggested-by: Bart Van Assche <bvanassche@acm.org>
>>>> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
>>>> ---
>>>>    drivers/block/null_blk/main.c | 32 ++++++++++++++++----------------
>>>>    1 file changed, 16 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>>> index eba204b27785..98f6935bb502 100644
>>>> --- a/drivers/block/null_blk/main.c
>>>> +++ b/drivers/block/null_blk/main.c
>>>> @@ -66,7 +66,7 @@ struct nullb_page {
>>>>    #define NULLB_PAGE_FREE (MAP_SZ - 2)
>>>>    
>>>>    static LIST_HEAD(nullb_list);
>>>> -static DEFINE_MUTEX(lock);
>>>> +static DEFINE_MUTEX(nullb_global_lock);
>>>
>>> Since this seem to protect only the device list, why not simply call this
>>> nullb_list_lock ?
>>>
>>>>    static int null_major;
>>>>    static DEFINE_IDA(nullb_indexes);
>>>>    static struct blk_mq_tag_set tag_set;
>>>> @@ -423,9 +423,9 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
>>>>    {
>>>>    	int ret;
>>>>    
>>>> -	mutex_lock(&lock);
>>>> +	mutex_lock(&nullb_global_lock);
>>>>    	ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
>>>> -	mutex_unlock(&lock);
>>>> +	mutex_unlock(&nullb_global_lock);
>>>
>>> Nothing in nullb_update_nr_hw_queues() touches the device list. So it is very
>>> odd that nullb_global_lock is used for serialization here instead of a
>>> nullb_device mutex. If you change this, why not a prep patch to introduce such a
>>> mutex ?
>>>
>>
>> Thanks for pointing this out. The rename really belongs together with
>> the locking rework, so I'll drop this patch from v3 and fold the rename
>> into a separate series that splits the locking (introducing a per-
>> nullb_device mutex for the updates).
> 
> Please carefully check if the use of the global lock in these function is not to
> serialize with power on/off. The code is a little (uselessly) complicated in
> that area and I may not be seeing something. But it seems that on/off control
> could also use a device lock, with proper ordering: device lock first, then list
> lock.

Thanks for the suggestion. At a first glance this does look doable, and
I will check it carefully. :)

Thanks,
Zizhi Wo

> 
>