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