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

[Zizhi Wo]

在 2026/7/7 12:16, 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 ?

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).

Thanks,
Zizhi Wo

> >   
> >   	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;
> >   }