[PATCH v2 02/12] loop: Remove the "bool global" function argument

Move the code that is protected by a mutex from loop_change_fd() into
__loop_change_fd(). Move the code that is protected by a mutex from
loop_configure() into __loop_configure(). Keep the behavior in
loop_global_lock_killable() for the global == true case. Expand
loop_global_lock_killable(lo, false) calls into a mutex_lock_killable()
and a mutex_unlock() call. This patch prepares for adding lock context
annotations.

Cc: Haris Iqbal <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
---
 drivers/block/loop.c | 249 ++++++++++++++++++++++++-------------------
 1 file changed, 138 insertions(+), 111 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 1faecef33009..a71fe763c933 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -98,25 +98,22 @@ static DEFINE_MUTEX(loop_validate_mutex);
  * loop_global_lock_killable() - take locks for safe loop_validate_file() test
  *
  * @lo: struct loop_device
- * @global: true if @lo is about to bind another "struct loop_device", false otherwise
  *
  * Returns 0 on success, -EINTR otherwise.
  *
- * Since loop_validate_file() traverses on other "struct loop_device" if
- * is_loop_device() is true, we need a global lock for serializing concurrent
+ * Since loop_validate_file() traverses on other "struct loop_device", we need a
+ * global lock for serializing concurrent
  * loop_configure()/loop_change_fd()/__loop_clr_fd() calls.
  */
-static int loop_global_lock_killable(struct loop_device *lo, bool global)
+static int loop_global_lock_killable(struct loop_device *lo)
 {
 	int err;
 
-	if (global) {
-		err = mutex_lock_killable(&loop_validate_mutex);
-		if (err)
-			return err;
-	}
+	err = mutex_lock_killable(&loop_validate_mutex);
+	if (err)
+		return err;
 	err = mutex_lock_killable(&lo->lo_mutex);
-	if (err && global)
+	if (err)
 		mutex_unlock(&loop_validate_mutex);
 	return err;
 }
@@ -125,13 +122,11 @@ static int loop_global_lock_killable(struct loop_device *lo, bool global)
  * loop_global_unlock() - release locks taken by loop_global_lock_killable()
  *
  * @lo: struct loop_device
- * @global: true if @lo was about to bind another "struct loop_device", false otherwise
  */
-static void loop_global_unlock(struct loop_device *lo, bool global)
+static void loop_global_unlock(struct loop_device *lo)
 {
 	mutex_unlock(&lo->lo_mutex);
-	if (global)
-		mutex_unlock(&loop_validate_mutex);
+	mutex_unlock(&loop_validate_mutex);
 }
 
 static int max_part;
@@ -523,6 +518,50 @@ static int loop_check_backing_file(struct file *file)
 	return 0;
 }
 
+static int __loop_change_fd(struct loop_device *lo, struct block_device *bdev,
+			    struct file *file, struct file **old_file,
+			    bool *partscan)
+	__must_hold(&lo->lo_mutex)
+{
+	unsigned int memflags;
+	int error;
+
+	if (lo->lo_state != Lo_bound)
+		return -ENXIO;
+
+	/* the loop device has to be read-only */
+	if (!(lo->lo_flags & LO_FLAGS_READ_ONLY))
+		return -EINVAL;
+
+	error = loop_validate_file(file, bdev);
+	if (error)
+		return error;
+
+	*old_file = lo->lo_backing_file;
+
+	/* size of the new backing store needs to be the same */
+	if (lo_calculate_size(lo, file) != lo_calculate_size(lo, *old_file))
+		return -EINVAL;
+
+	/*
+	 * We might switch to direct I/O mode for the loop device, write back
+	 * all dirty data the page cache now that so that the individual I/O
+	 * operations don't have to do that.
+	 */
+	vfs_fsync(file, 0);
+
+	/* and ... switch */
+	disk_force_media_change(lo->lo_disk);
+	memflags = blk_mq_freeze_queue(lo->lo_queue);
+	mapping_set_gfp_mask((*old_file)->f_mapping, lo->old_gfp_mask);
+	loop_assign_backing_file(lo, file);
+	loop_update_dio(lo);
+	blk_mq_unfreeze_queue(lo->lo_queue, memflags);
+	*partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
+
+	return 0;
+}
+
 /*
  * loop_change_fd switched the backing store of a loopback device to
  * a new file. This is useful for operating system installers to free up
@@ -536,7 +575,6 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 {
 	struct file *file = fget(arg);
 	struct file *old_file;
-	unsigned int memflags;
 	int error;
 	bool partscan;
 	bool is_loop;
@@ -554,46 +592,21 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1);
 
 	is_loop = is_loop_device(file);
-	error = loop_global_lock_killable(lo, is_loop);
+	if (is_loop) {
+		error = loop_global_lock_killable(lo);
+		if (error)
+			goto out_putf;
+		error = __loop_change_fd(lo, bdev, file, &old_file, &partscan);
+		loop_global_unlock(lo);
+	} else {
+		error = mutex_lock_killable(&lo->lo_mutex);
+		if (error)
+			goto out_putf;
+		error = __loop_change_fd(lo, bdev, file, &old_file, &partscan);
+		mutex_unlock(&lo->lo_mutex);
+	}
 	if (error)
 		goto out_putf;
-	error = -ENXIO;
-	if (lo->lo_state != Lo_bound)
-		goto out_err;
-
-	/* the loop device has to be read-only */
-	error = -EINVAL;
-	if (!(lo->lo_flags & LO_FLAGS_READ_ONLY))
-		goto out_err;
-
-	error = loop_validate_file(file, bdev);
-	if (error)
-		goto out_err;
-
-	old_file = lo->lo_backing_file;
-
-	error = -EINVAL;
-
-	/* size of the new backing store needs to be the same */
-	if (lo_calculate_size(lo, file) != lo_calculate_size(lo, old_file))
-		goto out_err;
-
-	/*
-	 * We might switch to direct I/O mode for the loop device, write back
-	 * all dirty data the page cache now that so that the individual I/O
-	 * operations don't have to do that.
-	 */
-	vfs_fsync(file, 0);
-
-	/* and ... switch */
-	disk_force_media_change(lo->lo_disk);
-	memflags = blk_mq_freeze_queue(lo->lo_queue);
-	mapping_set_gfp_mask(old_file->f_mapping, lo->old_gfp_mask);
-	loop_assign_backing_file(lo, file);
-	loop_update_dio(lo);
-	blk_mq_unfreeze_queue(lo->lo_queue, memflags);
-	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
-	loop_global_unlock(lo, is_loop);
 
 	/*
 	 * Flush loop_validate_file() before fput(), for l->lo_backing_file
@@ -618,8 +631,6 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 	kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE);
 	return error;
 
-out_err:
-	loop_global_unlock(lo, is_loop);
 out_putf:
 	fput(file);
 	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
@@ -974,61 +985,29 @@ static void loop_update_limits(struct loop_device *lo, struct queue_limits *lim,
 		lim->discard_granularity = 0;
 }
 
-static int loop_configure(struct loop_device *lo, blk_mode_t mode,
-			  struct block_device *bdev,
-			  const struct loop_config *config)
+static int __loop_configure(struct loop_device *lo, blk_mode_t mode,
+			    struct block_device *bdev,
+			    const struct loop_config *config, struct file *file,
+			    bool *partscan)
+	__must_hold(&lo->lo_mutex)
 {
-	struct file *file = fget(config->fd);
 	struct queue_limits lim;
-	int error;
 	loff_t size;
-	bool partscan;
-	bool is_loop;
-
-	if (!file)
-		return -EBADF;
-
-	error = loop_check_backing_file(file);
-	if (error) {
-		fput(file);
-		return error;
-	}
-
-	is_loop = is_loop_device(file);
-
-	/* This is safe, since we have a reference from open(). */
-	__module_get(THIS_MODULE);
-
-	/*
-	 * If we don't hold exclusive handle for the device, upgrade to it
-	 * here to avoid changing device under exclusive owner.
-	 */
-	if (!(mode & BLK_OPEN_EXCL)) {
-		error = bd_prepare_to_claim(bdev, loop_configure, NULL);
-		if (error)
-			goto out_putf;
-	}
-
-	error = loop_global_lock_killable(lo, is_loop);
-	if (error)
-		goto out_bdev;
+	int error;
 
-	error = -EBUSY;
 	if (lo->lo_state != Lo_unbound)
-		goto out_unlock;
+		return -EBUSY;
 
 	error = loop_validate_file(file, bdev);
 	if (error)
-		goto out_unlock;
+		return error;
 
-	if ((config->info.lo_flags & ~LOOP_CONFIGURE_SETTABLE_FLAGS) != 0) {
-		error = -EINVAL;
-		goto out_unlock;
-	}
+	if ((config->info.lo_flags & ~LOOP_CONFIGURE_SETTABLE_FLAGS) != 0)
+		return -EINVAL;
 
 	error = loop_set_status_from_info(lo, &config->info);
 	if (error)
-		goto out_unlock;
+		return error;
 	lo->lo_flags = config->info.lo_flags;
 
 	if (!(file->f_mode & FMODE_WRITE) || !(mode & BLK_OPEN_WRITE) ||
@@ -1039,10 +1018,8 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 		lo->workqueue = alloc_workqueue("loop%d",
 						WQ_UNBOUND | WQ_FREEZABLE,
 						0, lo->lo_number);
-		if (!lo->workqueue) {
-			error = -ENOMEM;
-			goto out_unlock;
-		}
+		if (!lo->workqueue)
+			return -ENOMEM;
 	}
 
 	/* suppress uevents while reconfiguring the device */
@@ -1059,7 +1036,7 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 	/* No need to freeze the queue as the device isn't bound yet. */
 	error = queue_limits_commit_update(lo->lo_queue, &lim);
 	if (error)
-		goto out_unlock;
+		return error;
 
 	/*
 	 * We might switch to direct I/O mode for the loop device, write back
@@ -1080,14 +1057,66 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 	WRITE_ONCE(lo->lo_state, Lo_bound);
 	if (part_shift)
 		lo->lo_flags |= LO_FLAGS_PARTSCAN;
-	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
-	if (partscan)
+	*partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
+	if (*partscan)
 		clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
 
 	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
 	kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE);
 
-	loop_global_unlock(lo, is_loop);
+	return 0;
+}
+
+static int loop_configure(struct loop_device *lo, blk_mode_t mode,
+			  struct block_device *bdev,
+			  const struct loop_config *config)
+{
+	struct file *file = fget(config->fd);
+	int error;
+	bool partscan;
+	bool is_loop;
+
+	if (!file)
+		return -EBADF;
+
+	error = loop_check_backing_file(file);
+	if (error) {
+		fput(file);
+		return error;
+	}
+
+	is_loop = is_loop_device(file);
+
+	/* This is safe, since we have a reference from open(). */
+	__module_get(THIS_MODULE);
+
+	/*
+	 * If we don't hold exclusive handle for the device, upgrade to it
+	 * here to avoid changing device under exclusive owner.
+	 */
+	if (!(mode & BLK_OPEN_EXCL)) {
+		error = bd_prepare_to_claim(bdev, loop_configure, NULL);
+		if (error)
+			goto out_putf;
+	}
+
+	if (is_loop) {
+		error = loop_global_lock_killable(lo);
+		if (error)
+			goto out_bdev;
+		error = __loop_configure(lo, mode, bdev, config, file,
+					 &partscan);
+		loop_global_unlock(lo);
+	} else {
+		error = mutex_lock_killable(&lo->lo_mutex);
+		if (error)
+			goto out_bdev;
+		error = __loop_configure(lo, mode, bdev, config, file,
+					 &partscan);
+		mutex_unlock(&lo->lo_mutex);
+	}
+	if (error)
+		goto out_bdev;
 	if (partscan)
 		loop_reread_partitions(lo);
 
@@ -1096,8 +1125,6 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 
 	return 0;
 
-out_unlock:
-	loop_global_unlock(lo, is_loop);
 out_bdev:
 	if (!(mode & BLK_OPEN_EXCL))
 		bd_abort_claiming(bdev, loop_configure);
@@ -1194,11 +1221,11 @@ static int loop_clr_fd(struct loop_device *lo)
 	 * which loop_configure()/loop_change_fd() found via fget() was this
 	 * loop device.
 	 */
-	err = loop_global_lock_killable(lo, true);
+	err = loop_global_lock_killable(lo);
 	if (err)
 		return err;
 	if (lo->lo_state != Lo_bound) {
-		loop_global_unlock(lo, true);
+		loop_global_unlock(lo);
 		return -ENXIO;
 	}
 	/*
@@ -1210,7 +1237,7 @@ static int loop_clr_fd(struct loop_device *lo)
 	lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
 	if (disk_openers(lo->lo_disk) == 1)
 		WRITE_ONCE(lo->lo_state, Lo_rundown);
-	loop_global_unlock(lo, true);
+	loop_global_unlock(lo);
 
 	return 0;
 }