Possible changes folded into this series.
(1) (I guess) no need to use _nested version.
(2) Use mutex_lock_killable() where possible.
(3) Move fput() to after mutex_unlock().
(4) Don't return 0 upon invalid loop_control_ioctl().
(5) No need to mutex_lock()/mutex_unlock() on each loop device at
unregister_transfer_cb() callback.
(6) No need to mutex_unlock()+mutex_lock() when calling __loop_clr_fd().
---
drivers/block/loop.c | 78 ++++++++++++++++++++++++----------------------------
1 file changed, 36 insertions(+), 42 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index a0fb7bf..395d1e9 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -678,11 +678,11 @@ static int loop_validate_file(struct file *file, struct block_device *bdev)
static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
unsigned int arg)
{
- struct file *file, *old_file;
+ struct file *file = NULL, *old_file;
int error;
bool partscan;
- error = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
+ error = mutex_lock_killable(&loop_ctl_mutex);
if (error)
return error;
error = -ENXIO;
@@ -701,7 +701,7 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
error = loop_validate_file(file, bdev);
if (error)
- goto out_putf;
+ goto out_unlock;
old_file = lo->lo_backing_file;
@@ -709,29 +709,31 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
/* size of the new backing store needs to be the same */
if (get_loop_size(lo, file) != get_loop_size(lo, old_file))
- goto out_putf;
+ goto out_unlock;
/* and ... switch */
blk_mq_freeze_queue(lo->lo_queue);
mapping_set_gfp_mask(old_file->f_mapping, lo->old_gfp_mask);
+ spin_lock_irq(&lo->lo_lock);
lo->lo_backing_file = file;
+ spin_unlock_irq(&lo->lo_lock);
lo->old_gfp_mask = mapping_gfp_mask(file->f_mapping);
mapping_set_gfp_mask(file->f_mapping,
lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
loop_update_dio(lo);
blk_mq_unfreeze_queue(lo->lo_queue);
- fput(old_file);
partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
mutex_unlock(&loop_ctl_mutex);
+ fput(old_file);
if (partscan)
loop_reread_partitions(lo, bdev);
return 0;
-out_putf:
- fput(file);
out_unlock:
mutex_unlock(&loop_ctl_mutex);
+ if (file)
+ fput(file);
return error;
}
@@ -916,7 +918,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
if (!file)
goto out;
- error = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
+ error = mutex_lock_killable(&loop_ctl_mutex);
if (error)
goto out_putf;
@@ -975,7 +977,8 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
lo->lo_flags |= LO_FLAGS_PARTSCAN;
partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
- /* Grab the block_device to prevent its destruction after we
+ /*
+ * Grab the block_device to prevent its destruction after we
* put /dev/loopXX inode. Later in __loop_clr_fd() we bdput(bdev).
*/
bdgrab(bdev);
@@ -1031,26 +1034,21 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
return err;
}
+/* loop_ctl_mutex is held by caller, and is released before return. */
static int __loop_clr_fd(struct loop_device *lo, bool release)
{
- struct file *filp = NULL;
+ /*
+ * filp != NULL because the caller checked lo->lo_state == Lo_bound
+ * under loop_ctl_mutex.
+ */
+ struct file *filp = lo->lo_backing_file;
gfp_t gfp = lo->old_gfp_mask;
struct block_device *bdev = lo->lo_device;
int err = 0;
bool partscan = false;
int lo_number;
- mutex_lock(&loop_ctl_mutex);
- if (WARN_ON_ONCE(lo->lo_state != Lo_rundown)) {
- err = -ENXIO;
- goto out_unlock;
- }
-
- filp = lo->lo_backing_file;
- if (filp == NULL) {
- err = -EINVAL;
- goto out_unlock;
- }
+ lo->lo_state = Lo_rundown;
/* freeze request queue during the transition */
blk_mq_freeze_queue(lo->lo_queue);
@@ -1091,13 +1089,12 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
module_put(THIS_MODULE);
blk_mq_unfreeze_queue(lo->lo_queue);
- partscan = lo->lo_flags & LO_FLAGS_PARTSCAN && bdev;
+ partscan = (lo->lo_flags & LO_FLAGS_PARTSCAN) && bdev;
lo_number = lo->lo_number;
lo->lo_flags = 0;
if (!part_shift)
lo->lo_disk->flags |= GENHD_FL_NO_PART_SCAN;
loop_unprepare_queue(lo);
-out_unlock:
mutex_unlock(&loop_ctl_mutex);
if (partscan) {
/*
@@ -1123,8 +1120,7 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
* lock dependency possibility warning as fput can take
* bd_mutex which is usually taken before loop_ctl_mutex.
*/
- if (filp)
- fput(filp);
+ fput(filp);
return err;
}
@@ -1132,7 +1128,7 @@ static int loop_clr_fd(struct loop_device *lo)
{
int err;
- err = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
+ err = mutex_lock_killable(&loop_ctl_mutex);
if (err)
return err;
if (lo->lo_state != Lo_bound) {
@@ -1154,9 +1150,6 @@ static int loop_clr_fd(struct loop_device *lo)
mutex_unlock(&loop_ctl_mutex);
return 0;
}
- lo->lo_state = Lo_rundown;
- mutex_unlock(&loop_ctl_mutex);
-
return __loop_clr_fd(lo, false);
}
@@ -1169,7 +1162,7 @@ static int loop_clr_fd(struct loop_device *lo)
struct block_device *bdev;
bool partscan = false;
- err = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
+ err = mutex_lock_killable(&loop_ctl_mutex);
if (err)
return err;
if (lo->lo_encrypt_key_size &&
@@ -1274,7 +1267,7 @@ static int loop_clr_fd(struct loop_device *lo)
struct kstat stat;
int ret;
- ret = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
+ ret = mutex_lock_killable(&loop_ctl_mutex);
if (ret)
return ret;
if (lo->lo_state != Lo_bound) {
@@ -1463,7 +1456,7 @@ static int lo_simple_ioctl(struct loop_device *lo, unsigned int cmd,
{
int err;
- err = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
+ err = mutex_lock_killable(&loop_ctl_mutex);
if (err)
return err;
switch (cmd) {
@@ -1712,8 +1705,6 @@ static void lo_release(struct gendisk *disk, fmode_t mode)
if (lo->lo_flags & LO_FLAGS_AUTOCLEAR) {
if (lo->lo_state != Lo_bound)
goto out_unlock;
- lo->lo_state = Lo_rundown;
- mutex_unlock(&loop_ctl_mutex);
/*
* In autoclear mode, stop the loop thread
* and remove configuration after last close.
@@ -1769,10 +1760,8 @@ static int unregister_transfer_cb(int id, void *ptr, void *data)
struct loop_device *lo = ptr;
struct loop_func_table *xfer = data;
- mutex_lock(&loop_ctl_mutex);
if (lo->lo_encryption == xfer)
loop_release_xfer(lo);
- mutex_unlock(&loop_ctl_mutex);
return 0;
}
@@ -1785,7 +1774,13 @@ int loop_unregister_transfer(int number)
return -EINVAL;
xfer_funcs[n] = NULL;
+ /*
+ * cleanup_cryptoloop() cannot handle errors because it is called
+ * from module_exit(). Thus, don't give up upon SIGKILL here.
+ */
+ mutex_lock(&loop_ctl_mutex);
idr_for_each(&loop_index_idr, &unregister_transfer_cb, xfer);
+ mutex_unlock(&loop_ctl_mutex);
return 0;
}
@@ -2031,7 +2026,9 @@ static struct kobject *loop_probe(dev_t dev, int *part, void *data)
struct kobject *kobj;
int err;
- mutex_lock(&loop_ctl_mutex);
+ *part = 0;
+ if (mutex_lock_killable(&loop_ctl_mutex))
+ return NULL;
err = loop_lookup(&lo, MINOR(dev) >> part_shift);
if (err < 0)
err = loop_add(&lo, MINOR(dev) >> part_shift);
@@ -2040,8 +2037,6 @@ static struct kobject *loop_probe(dev_t dev, int *part, void *data)
else
kobj = get_disk_and_module(lo->lo_disk);
mutex_unlock(&loop_ctl_mutex);
-
- *part = 0;
return kobj;
}
@@ -2049,12 +2044,11 @@ static long loop_control_ioctl(struct file *file, unsigned int cmd,
unsigned long parm)
{
struct loop_device *lo;
- int ret = -ENOSYS;
+ int ret = mutex_lock_killable(&loop_ctl_mutex);
- ret = mutex_lock_killable(&loop_ctl_mutex);
if (ret)
return ret;
-
+ ret = -ENOSYS;
switch (cmd) {
case LOOP_CTL_ADD:
ret = loop_lookup(&lo, parm);
--
1.8.3.1
