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 Signed-off-by: Bart Van Assche --- 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; }