Re: loop cleanups

[Christoph Hellwig <hch@lst.de>]

On Tue, Jun 22, 2021 at 08:15:27PM +0900, Tetsuo Handa wrote:
> On 2021/06/21 19:15, Christoph Hellwig wrote:
> > Hi Jens,
> > 
> > this series contains a bunch of cleanups for the loop driver,
> > mostly related to probing and the control device.
> > 
> 
> Please fold
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index a4a5466b998f..6c10400d4d38 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -2163,8 +2163,9 @@ static int loop_add(int i)
>  	disk->queue		= lo->lo_queue;
>  	sprintf(disk->disk_name, "loop%d", i);
>  	add_disk(disk);
> +	err = lo->lo_number;
>  	mutex_unlock(&loop_ctl_mutex);
> -	return lo->lo_number;
> +	return err;
>  
>  out_free_queue:
>  	blk_cleanup_queue(lo->lo_queue);
> @@ -2253,8 +2254,9 @@ static int loop_control_get_free(int idx)
>  	mutex_unlock(&loop_ctl_mutex);
>  	return loop_add(-1);
>  found:
> +	ret = lo->lo_number;
>  	mutex_unlock(&loop_ctl_mutex);
> -	return lo->lo_number;
> +	return ret;
>  }

Good find.  But we already have local variables holding the index
in both functions, so we can just use those.

> By the way, how can we fix a regression introduced by commit 6cc8e7430801fa23
> ("loop: scale loop device by introducing per device lock") ?
> Conditionally holding global lock like below untested diff?

It would be nice to factor the global locking into helpers, but otherwise
this looks ok.  Maybe also rename loop_configure_mutex into
loop_validate_mutex