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

[Damien Le Moal]

On 7/7/26 11:55, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@xxxxxxxxxx>
> 
> 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@xxxxxxx>
> Signed-off-by: Zizhi Wo <wozizhi@xxxxxxxxxx>
> ---
>  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 ?

>  
>  	return ret;
>  }
> @@ -435,9 +435,9 @@ static int nullb_apply_poll_queues(struct nullb_device *dev,
>  {
>  	int ret;
>  
> -	mutex_lock(&lock);
> +	mutex_lock(&nullb_global_lock);
>  	ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
> -	mutex_unlock(&lock);
> +	mutex_unlock(&nullb_global_lock);

Same here.

>  
>  	return ret;
>  }


-- 
Damien Le Moal
Western Digital Research