Skip to content

Commit f893366

Browse files
Ming Leiaxboe
Ming Lei
authored and
axboe
committed
block: loop: don't hold lo_ctl_mutex in lo_open
The lo_ctl_mutex is held for running all ioctl handlers, and in some ioctl handlers, ioctl_by_bdev(BLKRRPART) is called for rereading partitions, which requires bd_mutex. So it is easy to cause failure because trylock(bd_mutex) may fail inside blkdev_reread_part(), and follows the lock context: blkid or other application: ->open() ->mutex_lock(bd_mutex) ->lo_open() ->mutex_lock(lo_ctl_mutex) losetup(set fd ioctl): ->mutex_lock(lo_ctl_mutex) ->ioctl_by_bdev(BLKRRPART) ->trylock(bd_mutex) This patch trys to eliminate the ABBA lock dependency by removing lo_ctl_mutext in lo_open() with the following approach: 1) make lo_refcnt as atomic_t and avoid acquiring lo_ctl_mutex in lo_open(): - for open vs. add/del loop, no any problem because of loop_index_mutex - freeze request queue during clr_fd, so I/O can't come until clearing fd is completed, like the effect of holding lo_ctl_mutex in lo_open - both open() and release() have been serialized by bd_mutex already 2) don't hold lo_ctl_mutex for decreasing/checking lo_refcnt in lo_release(), then lo_ctl_mutex is only required for the last release. Reviewed-by: Christoph Hellwig <hch@lst.de> Tested-by: Jarod Wilson <jarod@redhat.com> Acked-by: Jarod Wilson <jarod@redhat.com> Signed-off-by: Ming Lei <ming.lei@canonical.com> Signed-off-by: Jens Axboe <axboe@fb.com>
1 parent b816d45 commit f893366

2 files changed

Lines changed: 13 additions & 10 deletions

File tree

‎drivers/block/loop.c‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ static int loop_clr_fd(struct loop_device *lo)
831831
* <dev>/do something like mkfs/losetup -d <dev> causing the losetup -d
832832
* command to fail with EBUSY.
833833
*/
834-
if (lo->lo_refcnt > 1) {
834+
if (atomic_read(&lo->lo_refcnt) > 1) {
835835
lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
836836
mutex_unlock(&lo->lo_ctl_mutex);
837837
return 0;
@@ -840,6 +840,9 @@ static int loop_clr_fd(struct loop_device *lo)
840840
if (filp == NULL)
841841
return -EINVAL;
842842

843+
/* freeze request queue during the transition */
844+
blk_mq_freeze_queue(lo->lo_queue);
845+
843846
spin_lock_irq(&lo->lo_lock);
844847
lo->lo_state = Lo_rundown;
845848
lo->lo_backing_file = NULL;
@@ -871,6 +874,8 @@ static int loop_clr_fd(struct loop_device *lo)
871874
lo->lo_state = Lo_unbound;
872875
/* This is safe: open() is still holding a reference. */
873876
module_put(THIS_MODULE);
877+
blk_mq_unfreeze_queue(lo->lo_queue);
878+
874879
if (lo->lo_flags & LO_FLAGS_PARTSCAN && bdev)
875880
ioctl_by_bdev(bdev, BLKRRPART, 0);
876881
lo->lo_flags = 0;
@@ -1330,9 +1335,7 @@ static int lo_open(struct block_device *bdev, fmode_t mode)
13301335
goto out;
13311336
}
13321337

1333-
mutex_lock(&lo->lo_ctl_mutex);
1334-
lo->lo_refcnt++;
1335-
mutex_unlock(&lo->lo_ctl_mutex);
1338+
atomic_inc(&lo->lo_refcnt);
13361339
out:
13371340
mutex_unlock(&loop_index_mutex);
13381341
return err;
@@ -1343,11 +1346,10 @@ static void lo_release(struct gendisk *disk, fmode_t mode)
13431346
struct loop_device *lo = disk->private_data;
13441347
int err;
13451348

1346-
mutex_lock(&lo->lo_ctl_mutex);
1347-
1348-
if (--lo->lo_refcnt)
1349-
goto out;
1349+
if (atomic_dec_return(&lo->lo_refcnt))
1350+
return;
13501351

1352+
mutex_lock(&lo->lo_ctl_mutex);
13511353
if (lo->lo_flags & LO_FLAGS_AUTOCLEAR) {
13521354
/*
13531355
* In autoclear mode, stop the loop thread
@@ -1601,6 +1603,7 @@ static int loop_add(struct loop_device **l, int i)
16011603
disk->flags |= GENHD_FL_NO_PART_SCAN;
16021604
disk->flags |= GENHD_FL_EXT_DEVT;
16031605
mutex_init(&lo->lo_ctl_mutex);
1606+
atomic_set(&lo->lo_refcnt, 0);
16041607
lo->lo_number = i;
16051608
spin_lock_init(&lo->lo_lock);
16061609
disk->major = LOOP_MAJOR;
@@ -1718,7 +1721,7 @@ static long loop_control_ioctl(struct file *file, unsigned int cmd,
17181721
mutex_unlock(&lo->lo_ctl_mutex);
17191722
break;
17201723
}
1721-
if (lo->lo_refcnt > 0) {
1724+
if (atomic_read(&lo->lo_refcnt) > 0) {
17221725
ret = -EBUSY;
17231726
mutex_unlock(&lo->lo_ctl_mutex);
17241727
break;

‎drivers/block/loop.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ struct loop_func_table;
2828

2929
struct loop_device {
3030
int lo_number;
31-
int lo_refcnt;
31+
atomic_t lo_refcnt;
3232
loff_t lo_offset;
3333
loff_t lo_sizelimit;
3434
int lo_flags;

0 commit comments

Comments
 (0)