On Thu 24-03-22 08:51:18, Christoph Hellwig wrote:
> lo_refcount counts how many openers a loop device has, but that count
> is already provided by the block layer in the bd_openers field of the
> whole-disk block_device. Remove lo_refcount and allow opens to
> succeed even on devices beeing deleted - now that ->free_disk is
> implemented we can handle that race gracefull and all I/O on it will
> just fail. Similarly there is a small race window now where
> loop_control_remove does not synchronize the delete vs the remove
> due do bd_openers not being under lo_mutex protection, but we can
> handle that just as gracefully.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Looks good but I still think we need something like attached preparatory
patch to not regress e.g. filesystem probing triggered by udev events. What
do you think?
Otherwise feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> drivers/block/loop.c | 37 +++++++------------------------------
> drivers/block/loop.h | 1 -
> 2 files changed, 7 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index b3170e8cdbe95..e1eb925d3f855 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1244,7 +1244,7 @@ static int loop_clr_fd(struct loop_device *lo)
> * <dev>/do something like mkfs/losetup -d <dev> causing the losetup -d
> * command to fail with EBUSY.
> */
> - if (atomic_read(&lo->lo_refcnt) > 1) {
> + if (disk_openers(lo->lo_disk) > 1) {
> lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
> mutex_unlock(&lo->lo_mutex);
> return 0;
> @@ -1724,33 +1724,15 @@ static int lo_compat_ioctl(struct block_device *bdev, fmode_t mode,
> }
> #endif
>
> -static int lo_open(struct block_device *bdev, fmode_t mode)
> -{
> - struct loop_device *lo = bdev->bd_disk->private_data;
> - int err;
> -
> - err = mutex_lock_killable(&lo->lo_mutex);
> - if (err)
> - return err;
> - if (lo->lo_state == Lo_deleting)
> - err = -ENXIO;
> - else
> - atomic_inc(&lo->lo_refcnt);
> - mutex_unlock(&lo->lo_mutex);
> - return err;
> -}
> -
> static void lo_release(struct gendisk *disk, fmode_t mode)
> {
> struct loop_device *lo = disk->private_data;
>
> - mutex_lock(&lo->lo_mutex);
> - if (atomic_dec_return(&lo->lo_refcnt))
> - goto out_unlock;
> + if (disk_openers(disk) > 0)
> + return;
>
> - if (lo->lo_flags & LO_FLAGS_AUTOCLEAR) {
> - if (lo->lo_state != Lo_bound)
> - goto out_unlock;
> + mutex_lock(&lo->lo_mutex);
> + if (lo->lo_state == Lo_bound && (lo->lo_flags & LO_FLAGS_AUTOCLEAR)) {
> lo->lo_state = Lo_rundown;
> mutex_unlock(&lo->lo_mutex);
> /*
> @@ -1760,8 +1742,6 @@ static void lo_release(struct gendisk *disk, fmode_t mode)
> __loop_clr_fd(lo, true);
> return;
> }
> -
> -out_unlock:
> mutex_unlock(&lo->lo_mutex);
> }
>
> @@ -1775,7 +1755,6 @@ static void lo_free_disk(struct gendisk *disk)
>
> static const struct block_device_operations lo_fops = {
> .owner = THIS_MODULE,
> - .open = lo_open,
> .release = lo_release,
> .ioctl = lo_ioctl,
> #ifdef CONFIG_COMPAT
> @@ -2029,7 +2008,6 @@ static int loop_add(int i)
> */
> if (!part_shift)
> disk->flags |= GENHD_FL_NO_PART;
> - atomic_set(&lo->lo_refcnt, 0);
> mutex_init(&lo->lo_mutex);
> lo->lo_number = i;
> spin_lock_init(&lo->lo_lock);
> @@ -2119,13 +2097,12 @@ static int loop_control_remove(int idx)
> ret = mutex_lock_killable(&lo->lo_mutex);
> if (ret)
> goto mark_visible;
> - if (lo->lo_state != Lo_unbound ||
> - atomic_read(&lo->lo_refcnt) > 0) {
> + if (lo->lo_state != Lo_unbound || disk_openers(lo->lo_disk) > 0) {
> mutex_unlock(&lo->lo_mutex);
> ret = -EBUSY;
> goto mark_visible;
> }
> - /* Mark this loop device no longer open()-able. */
> + /* Mark this loop device as no more bound, but not quite unbound yet */
> lo->lo_state = Lo_deleting;
> mutex_unlock(&lo->lo_mutex);
>
> diff --git a/drivers/block/loop.h b/drivers/block/loop.h
> index 082d4b6bfc6a6..449d562738c52 100644
> --- a/drivers/block/loop.h
> +++ b/drivers/block/loop.h
> @@ -28,7 +28,6 @@ struct loop_func_table;
>
> struct loop_device {
> int lo_number;
> - atomic_t lo_refcnt;
> loff_t lo_offset;
> loff_t lo_sizelimit;
> int lo_flags;
> --
> 2.30.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
