[PATCH v4 00/10] Add a new LOOP_CONFIGURE ioctl

[PATCH v4 00/10] Add a new LOOP_CONFIGURE ioctl

[Martijn Coenen]

This series introduces a new ioctl that makes it possible to atomically
configure a loop device. Previously, if you wanted to set parameters
such as the offset on a loop device, this required calling LOOP_SET_FD
to set the backing file, and then LOOP_SET_STATUS to set the offset.
However, in between these two calls, the loop device is available and
would accept requests, which is generally not desirable. Similar issues
exist around setting the block size (LOOP_SET_BLOCK_SIZE) and requesting
direct I/O mode (LOOP_SET_DIRECT_IO).

There are also performance benefits with combining these ioctls into
one, which are described in more detail in the last change in the
series.

Note that this series depends on
"loop: Call loop_config_discard() only after new config is applied."
[0], which I sent as a separate patch as it fixes an unrelated bug.

[0]: https://lkml.org/lkml/2020/3/31/755

---
v4:
  - Addressed review comments from Christoph Hellwig:
    -- Minor code cleanups
    -- Clarified what lo_flags LOOP_SET_STATUS can set and clear, and
       made that more explicit in the code (see [9/10])
    -- LOOP_CONFIGURE can now also be used to configure the block size
       and to explicitly request Direct I/O and read-only mode.
    -- Explicitly reject lo_flags we don't know about in LOOP_CONFIGURE
    -- Renamed LOOP_SET_FD_AND_STATUS to LOOP_CONFIGURE, since the ioctl
       can now do things LOOP_SET_STATUS couldn't do.
v3:
  - Addressed review comments from Christoph Hellwig:
    -- Factored out loop_validate_size()
    -- Split up the largish first patch in a few smaller ones
    -- Use set_capacity_revalidate_and_notify()
  - Fixed a variable wrongly using size_t instead of loff_t
v2:
  - Addressed review comments from Bart van Assche:
    -- Use SECTOR_SHIFT constant
    -- Renamed loop_set_from_status() to loop_set_status_from_info()
    -- Added kerneldoc for loop_set_status_from_info()
    -- Removed dots in patch subject lines
  - Addressed review comments from Christoph Hellwig:
    -- Added missing padding in struct loop_fd_and_status
    -- Cleaned up some __user pointer handling in lo_ioctl
    -- Pass in a stack-initialized loop_info64 for the legacy
       LOOP_SET_FD case

Martijn Coenen (10):
  loop: Factor out loop size validation
  loop: Factor out setting loop device size
  loop: Switch to set_capacity_revalidate_and_notify()
  loop: Refactor loop_set_status() size calculation
  loop: Remove figure_loop_size()
  loop: Factor out configuring loop from status
  loop: Move loop_set_status_from_info() and friends up
  loop: Rework lo_ioctl() __user argument casting
  loop: Clean up LOOP_SET_STATUS lo_flags handling.
  loop: Add LOOP_CONFIGURE ioctl

 drivers/block/loop.c      | 407 +++++++++++++++++++++++---------------
 include/uapi/linux/loop.h |  31 ++-
 2 files changed, 281 insertions(+), 157 deletions(-)

-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 01/10] loop: Factor out loop size validation

[Martijn Coenen]

Ensuring we don't truncate loff_t when casting to sector_t is done in
multiple places; factor it out.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index f1754262fc94..396b8bd4d75c 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -228,15 +228,30 @@ static void __loop_update_dio(struct loop_device *lo, bool dio)
 		blk_mq_unfreeze_queue(lo->lo_queue);
 }
 
+/**
+ * loop_validate_size() - validates that the passed in size fits in a sector_t
+ * @size: size to validate
+ */
+static int
+loop_validate_size(loff_t size)
+{
+	if ((loff_t)(sector_t)size != size)
+		return -EFBIG;
+
+	return 0;
+}
+
 static int
 figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
 {
+	int err;
 	loff_t size = get_size(offset, sizelimit, lo->lo_backing_file);
-	sector_t x = (sector_t)size;
 	struct block_device *bdev = lo->lo_device;
 
-	if (unlikely((loff_t)x != size))
-		return -EFBIG;
+	err = loop_validate_size(size);
+	if (err)
+		return err;
+
 	if (lo->lo_offset != offset)
 		lo->lo_offset = offset;
 	if (lo->lo_sizelimit != sizelimit)
@@ -1003,9 +1018,9 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	    !file->f_op->write_iter)
 		lo_flags |= LO_FLAGS_READ_ONLY;
 
-	error = -EFBIG;
 	size = get_loop_size(lo, file);
-	if ((loff_t)(sector_t)size != size)
+	error = loop_validate_size(size);
+	if (error)
 		goto out_unlock;
 	error = loop_prepare_queue(lo);
 	if (error)
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 02/10] loop: Factor out setting loop device size

[Martijn Coenen]

This code is used repeatedly.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 396b8bd4d75c..6643e48ad71c 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -241,12 +241,29 @@ loop_validate_size(loff_t size)
 	return 0;
 }
 
+/**
+ * loop_set_size() - sets device size and notifies userspace
+ * @lo: struct loop_device to set the size for
+ * @size: new size of the loop device
+ *
+ * Callers must validate that the size passed into this function fits into
+ * a sector_t, eg using loop_validate_size()
+ */
+static void loop_set_size(struct loop_device *lo, loff_t size)
+{
+	struct block_device *bdev = lo->lo_device;
+
+	set_capacity(lo->lo_disk, size);
+	bd_set_size(bdev, size << SECTOR_SHIFT);
+	/* let user-space know about the new size */
+	kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
+}
+
 static int
 figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
 {
 	int err;
 	loff_t size = get_size(offset, sizelimit, lo->lo_backing_file);
-	struct block_device *bdev = lo->lo_device;
 
 	err = loop_validate_size(size);
 	if (err)
@@ -256,10 +273,9 @@ figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
 		lo->lo_offset = offset;
 	if (lo->lo_sizelimit != sizelimit)
 		lo->lo_sizelimit = sizelimit;
-	set_capacity(lo->lo_disk, x);
-	bd_set_size(bdev, (loff_t)get_capacity(bdev->bd_disk) << 9);
-	/* let user-space know about the new size */
-	kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
+
+	loop_set_size(lo, size);
+
 	return 0;
 }
 
@@ -1055,11 +1071,8 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 
 	loop_update_rotational(lo);
 	loop_update_dio(lo);
-	set_capacity(lo->lo_disk, size);
-	bd_set_size(bdev, size << 9);
 	loop_sysfs_init(lo);
-	/* let user-space know about the new size */
-	kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
+	loop_set_size(lo, size);
 
 	set_blocksize(bdev, S_ISBLK(inode->i_mode) ?
 		      block_size(inode->i_bdev) : PAGE_SIZE);
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 03/10] loop: Switch to set_capacity_revalidate_and_notify()

[Martijn Coenen]

This was recently added to block/genhd.c, and takes care of both
updating the capacity and notifying userspace of the new size.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 6643e48ad71c..8f3194c2b8aa 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -253,10 +253,9 @@ static void loop_set_size(struct loop_device *lo, loff_t size)
 {
 	struct block_device *bdev = lo->lo_device;
 
-	set_capacity(lo->lo_disk, size);
 	bd_set_size(bdev, size << SECTOR_SHIFT);
-	/* let user-space know about the new size */
-	kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
+
+	set_capacity_revalidate_and_notify(lo->lo_disk, size, false);
 }
 
 static int
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 04/10] loop: Refactor loop_set_status() size calculation

[Martijn Coenen]

figure_loop_size() calculates the loop size based on the passed in
parameters, but at the same time it updates the offset and sizelimit
parameters in the loop device configuration. That is a somewhat
unexpected side effect of a function with this name, and it is only only
needed by one of the two callers of this function - loop_set_status().

Move the lo_offset and lo_sizelimit assignment back into loop_set_status(),
and use the newly factored out functions to validate and apply the newly
calculated size. This allows us to get rid of figure_loop_size() in a
follow-up commit.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 8f3194c2b8aa..9f5913879921 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -268,11 +268,6 @@ figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
 	if (err)
 		return err;
 
-	if (lo->lo_offset != offset)
-		lo->lo_offset = offset;
-	if (lo->lo_sizelimit != sizelimit)
-		lo->lo_sizelimit = sizelimit;
-
 	loop_set_size(lo, size);
 
 	return 0;
@@ -1294,6 +1289,8 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 	kuid_t uid = current_uid();
 	struct block_device *bdev;
 	bool partscan = false;
+	bool size_changed = false;
+	loff_t validated_size;
 
 	err = mutex_lock_killable(&loop_ctl_mutex);
 	if (err)
@@ -1315,6 +1312,13 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 
 	if (lo->lo_offset != info->lo_offset ||
 	    lo->lo_sizelimit != info->lo_sizelimit) {
+		loff_t size = get_size(info->lo_offset, info->lo_sizelimit,
+				       lo->lo_backing_file);
+		err = loop_validate_size(size);
+		if (err)
+			goto out_unlock;
+		size_changed = true;
+		validated_size = size;
 		sync_blockdev(lo->lo_device);
 		kill_bdev(lo->lo_device);
 	}
@@ -1322,6 +1326,15 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 	/* I/O need to be drained during transfer transition */
 	blk_mq_freeze_queue(lo->lo_queue);
 
+	if (size_changed && lo->lo_device->bd_inode->i_mapping->nrpages) {
+		/* If any pages were dirtied after kill_bdev(), try again */
+		err = -EAGAIN;
+		pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
+			__func__, lo->lo_number, lo->lo_file_name,
+			lo->lo_device->bd_inode->i_mapping->nrpages);
+		goto out_unfreeze;
+	}
+
 	err = loop_release_xfer(lo);
 	if (err)
 		goto out_unfreeze;
@@ -1345,22 +1358,8 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 	if (err)
 		goto out_unfreeze;
 
-	if (lo->lo_offset != info->lo_offset ||
-	    lo->lo_sizelimit != info->lo_sizelimit) {
-		/* kill_bdev should have truncated all the pages */
-		if (lo->lo_device->bd_inode->i_mapping->nrpages) {
-			err = -EAGAIN;
-			pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
-				__func__, lo->lo_number, lo->lo_file_name,
-				lo->lo_device->bd_inode->i_mapping->nrpages);
-			goto out_unfreeze;
-		}
-		if (figure_loop_size(lo, info->lo_offset, info->lo_sizelimit)) {
-			err = -EFBIG;
-			goto out_unfreeze;
-		}
-	}
-
+	lo->lo_offset = info->lo_offset;
+	lo->lo_sizelimit = info->lo_sizelimit;
 	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
 	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
 	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
@@ -1384,6 +1383,9 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		lo->lo_key_owner = uid;
 	}
 
+	if (size_changed)
+		loop_set_size(lo, validated_size);
+
 	loop_config_discard(lo);
 
 	/* update dio if lo_offset or transfer is changed */
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 05/10] loop: Remove figure_loop_size()

[Martijn Coenen]

This function was now only used by loop_set_capacity(). Just open code
the remaining code in the caller instead.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 9f5913879921..32755e874326 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -258,21 +258,6 @@ static void loop_set_size(struct loop_device *lo, loff_t size)
 	set_capacity_revalidate_and_notify(lo->lo_disk, size, false);
 }
 
-static int
-figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
-{
-	int err;
-	loff_t size = get_size(offset, sizelimit, lo->lo_backing_file);
-
-	err = loop_validate_size(size);
-	if (err)
-		return err;
-
-	loop_set_size(lo, size);
-
-	return 0;
-}
-
 static inline int
 lo_do_transfer(struct loop_device *lo, int cmd,
 	       struct page *rpage, unsigned roffs,
@@ -1560,10 +1545,21 @@ loop_get_status64(struct loop_device *lo, struct loop_info64 __user *arg) {
 
 static int loop_set_capacity(struct loop_device *lo)
 {
+	int err;
+	loff_t size;
+
 	if (unlikely(lo->lo_state != Lo_bound))
 		return -ENXIO;
 
-	return figure_loop_size(lo, lo->lo_offset, lo->lo_sizelimit);
+	size = get_loop_size(lo, lo->lo_backing_file);
+
+	err = loop_validate_size(size);
+	if (err)
+		return err;
+
+	loop_set_size(lo, size);
+
+	return 0;
 }
 
 static int loop_set_dio(struct loop_device *lo, unsigned long arg)
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 06/10] loop: Factor out configuring loop from status

[Martijn Coenen]

Factor out this code into a separate function, so it can be reused by
other code more easily.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 117 +++++++++++++++++++++++++------------------
 1 file changed, 67 insertions(+), 50 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 32755e874326..4a36a3f47503 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1266,13 +1266,78 @@ static int loop_clr_fd(struct loop_device *lo)
 	return __loop_clr_fd(lo, false);
 }
 
+/**
+ * loop_set_status_from_info - configure device from loop_info
+ * @lo: struct loop_device to configure
+ * @info: struct loop_info64 to configure the device with
+ *
+ * Configures the loop device parameters according to the passed
+ * in loop_info64 configuration.
+ */
 static int
-loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
+loop_set_status_from_info(struct loop_device *lo,
+			  const struct loop_info64 *info)
 {
 	int err;
 	struct loop_func_table *xfer;
 	kuid_t uid = current_uid();
+
+	if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE)
+		return -EINVAL;
+
+	err = loop_release_xfer(lo);
+	if (err)
+		return err;
+
+	if (info->lo_encrypt_type) {
+		unsigned int type = info->lo_encrypt_type;
+
+		if (type >= MAX_LO_CRYPT)
+			return -EINVAL;
+		xfer = xfer_funcs[type];
+		if (xfer == NULL)
+			return -EINVAL;
+	} else
+		xfer = NULL;
+
+	err = loop_init_xfer(lo, xfer, info);
+	if (err)
+		return err;
+
+	lo->lo_offset = info->lo_offset;
+	lo->lo_sizelimit = info->lo_sizelimit;
+	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
+	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
+	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
+	lo->lo_crypt_name[LO_NAME_SIZE-1] = 0;
+
+	if (!xfer)
+		xfer = &none_funcs;
+	lo->transfer = xfer->transfer;
+	lo->ioctl = xfer->ioctl;
+
+	if ((lo->lo_flags & LO_FLAGS_AUTOCLEAR) !=
+	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
+		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
+
+	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
+	lo->lo_init[0] = info->lo_init[0];
+	lo->lo_init[1] = info->lo_init[1];
+	if (info->lo_encrypt_key_size) {
+		memcpy(lo->lo_encrypt_key, info->lo_encrypt_key,
+		       info->lo_encrypt_key_size);
+		lo->lo_key_owner = uid;
+	}
+
+	return 0;
+}
+
+static int
+loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
+{
+	int err;
 	struct block_device *bdev;
+	kuid_t uid = current_uid();
 	bool partscan = false;
 	bool size_changed = false;
 	loff_t validated_size;
@@ -1290,10 +1355,6 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		err = -ENXIO;
 		goto out_unlock;
 	}
-	if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE) {
-		err = -EINVAL;
-		goto out_unlock;
-	}
 
 	if (lo->lo_offset != info->lo_offset ||
 	    lo->lo_sizelimit != info->lo_sizelimit) {
@@ -1320,54 +1381,10 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		goto out_unfreeze;
 	}
 
-	err = loop_release_xfer(lo);
+	err = loop_set_status_from_info(lo, info);
 	if (err)
 		goto out_unfreeze;
 
-	if (info->lo_encrypt_type) {
-		unsigned int type = info->lo_encrypt_type;
-
-		if (type >= MAX_LO_CRYPT) {
-			err = -EINVAL;
-			goto out_unfreeze;
-		}
-		xfer = xfer_funcs[type];
-		if (xfer == NULL) {
-			err = -EINVAL;
-			goto out_unfreeze;
-		}
-	} else
-		xfer = NULL;
-
-	err = loop_init_xfer(lo, xfer, info);
-	if (err)
-		goto out_unfreeze;
-
-	lo->lo_offset = info->lo_offset;
-	lo->lo_sizelimit = info->lo_sizelimit;
-	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
-	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
-	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
-	lo->lo_crypt_name[LO_NAME_SIZE-1] = 0;
-
-	if (!xfer)
-		xfer = &none_funcs;
-	lo->transfer = xfer->transfer;
-	lo->ioctl = xfer->ioctl;
-
-	if ((lo->lo_flags & LO_FLAGS_AUTOCLEAR) !=
-	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
-		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
-
-	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
-	lo->lo_init[0] = info->lo_init[0];
-	lo->lo_init[1] = info->lo_init[1];
-	if (info->lo_encrypt_key_size) {
-		memcpy(lo->lo_encrypt_key, info->lo_encrypt_key,
-		       info->lo_encrypt_key_size);
-		lo->lo_key_owner = uid;
-	}
-
 	if (size_changed)
 		loop_set_size(lo, validated_size);
 
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 07/10] loop: Move loop_set_status_from_info() and friends up

[Martijn Coenen]

So we can use it without forward declaration. This is a separate commit
to make it easier to verify that this is just a move, without functional
modifications.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 206 +++++++++++++++++++++----------------------
 1 file changed, 103 insertions(+), 103 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 4a36a3f47503..60ba1ed95d77 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -962,6 +962,109 @@ static void loop_update_rotational(struct loop_device *lo)
 		blk_queue_flag_clear(QUEUE_FLAG_NONROT, q);
 }
 
+static int
+loop_release_xfer(struct loop_device *lo)
+{
+	int err = 0;
+	struct loop_func_table *xfer = lo->lo_encryption;
+
+	if (xfer) {
+		if (xfer->release)
+			err = xfer->release(lo);
+		lo->transfer = NULL;
+		lo->lo_encryption = NULL;
+		module_put(xfer->owner);
+	}
+	return err;
+}
+
+static int
+loop_init_xfer(struct loop_device *lo, struct loop_func_table *xfer,
+	       const struct loop_info64 *i)
+{
+	int err = 0;
+
+	if (xfer) {
+		struct module *owner = xfer->owner;
+
+		if (!try_module_get(owner))
+			return -EINVAL;
+		if (xfer->init)
+			err = xfer->init(lo, i);
+		if (err)
+			module_put(owner);
+		else
+			lo->lo_encryption = xfer;
+	}
+	return err;
+}
+
+/**
+ * loop_set_status_from_info - configure device from loop_info
+ * @lo: struct loop_device to configure
+ * @info: struct loop_info64 to configure the device with
+ *
+ * Configures the loop device parameters according to the passed
+ * in loop_info64 configuration.
+ */
+static int
+loop_set_status_from_info(struct loop_device *lo,
+			  const struct loop_info64 *info)
+{
+	int err;
+	struct loop_func_table *xfer;
+	kuid_t uid = current_uid();
+
+	if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE)
+		return -EINVAL;
+
+	err = loop_release_xfer(lo);
+	if (err)
+		return err;
+
+	if (info->lo_encrypt_type) {
+		unsigned int type = info->lo_encrypt_type;
+
+		if (type >= MAX_LO_CRYPT)
+			return -EINVAL;
+		xfer = xfer_funcs[type];
+		if (xfer == NULL)
+			return -EINVAL;
+	} else
+		xfer = NULL;
+
+	err = loop_init_xfer(lo, xfer, info);
+	if (err)
+		return err;
+
+	lo->lo_offset = info->lo_offset;
+	lo->lo_sizelimit = info->lo_sizelimit;
+	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
+	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
+	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
+	lo->lo_crypt_name[LO_NAME_SIZE-1] = 0;
+
+	if (!xfer)
+		xfer = &none_funcs;
+	lo->transfer = xfer->transfer;
+	lo->ioctl = xfer->ioctl;
+
+	if ((lo->lo_flags & LO_FLAGS_AUTOCLEAR) !=
+	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
+		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
+
+	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
+	lo->lo_init[0] = info->lo_init[0];
+	lo->lo_init[1] = info->lo_init[1];
+	if (info->lo_encrypt_key_size) {
+		memcpy(lo->lo_encrypt_key, info->lo_encrypt_key,
+		       info->lo_encrypt_key_size);
+		lo->lo_key_owner = uid;
+	}
+
+	return 0;
+}
+
 static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 		       struct block_device *bdev, unsigned int arg)
 {
@@ -1085,43 +1188,6 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	return error;
 }
 
-static int
-loop_release_xfer(struct loop_device *lo)
-{
-	int err = 0;
-	struct loop_func_table *xfer = lo->lo_encryption;
-
-	if (xfer) {
-		if (xfer->release)
-			err = xfer->release(lo);
-		lo->transfer = NULL;
-		lo->lo_encryption = NULL;
-		module_put(xfer->owner);
-	}
-	return err;
-}
-
-static int
-loop_init_xfer(struct loop_device *lo, struct loop_func_table *xfer,
-	       const struct loop_info64 *i)
-{
-	int err = 0;
-
-	if (xfer) {
-		struct module *owner = xfer->owner;
-
-		if (!try_module_get(owner))
-			return -EINVAL;
-		if (xfer->init)
-			err = xfer->init(lo, i);
-		if (err)
-			module_put(owner);
-		else
-			lo->lo_encryption = xfer;
-	}
-	return err;
-}
-
 static int __loop_clr_fd(struct loop_device *lo, bool release)
 {
 	struct file *filp = NULL;
@@ -1266,72 +1332,6 @@ static int loop_clr_fd(struct loop_device *lo)
 	return __loop_clr_fd(lo, false);
 }
 
-/**
- * loop_set_status_from_info - configure device from loop_info
- * @lo: struct loop_device to configure
- * @info: struct loop_info64 to configure the device with
- *
- * Configures the loop device parameters according to the passed
- * in loop_info64 configuration.
- */
-static int
-loop_set_status_from_info(struct loop_device *lo,
-			  const struct loop_info64 *info)
-{
-	int err;
-	struct loop_func_table *xfer;
-	kuid_t uid = current_uid();
-
-	if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE)
-		return -EINVAL;
-
-	err = loop_release_xfer(lo);
-	if (err)
-		return err;
-
-	if (info->lo_encrypt_type) {
-		unsigned int type = info->lo_encrypt_type;
-
-		if (type >= MAX_LO_CRYPT)
-			return -EINVAL;
-		xfer = xfer_funcs[type];
-		if (xfer == NULL)
-			return -EINVAL;
-	} else
-		xfer = NULL;
-
-	err = loop_init_xfer(lo, xfer, info);
-	if (err)
-		return err;
-
-	lo->lo_offset = info->lo_offset;
-	lo->lo_sizelimit = info->lo_sizelimit;
-	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
-	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
-	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
-	lo->lo_crypt_name[LO_NAME_SIZE-1] = 0;
-
-	if (!xfer)
-		xfer = &none_funcs;
-	lo->transfer = xfer->transfer;
-	lo->ioctl = xfer->ioctl;
-
-	if ((lo->lo_flags & LO_FLAGS_AUTOCLEAR) !=
-	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
-		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
-
-	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
-	lo->lo_init[0] = info->lo_init[0];
-	lo->lo_init[1] = info->lo_init[1];
-	if (info->lo_encrypt_key_size) {
-		memcpy(lo->lo_encrypt_key, info->lo_encrypt_key,
-		       info->lo_encrypt_key_size);
-		lo->lo_key_owner = uid;
-	}
-
-	return 0;
-}
-
 static int
 loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 {
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 08/10] loop: Rework lo_ioctl() __user argument casting

[Martijn Coenen]

In preparation for a new ioctl that needs to copy_from_user(); makes the
code easier to read as well.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 60ba1ed95d77..f172a64333aa 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1659,6 +1659,7 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 	unsigned int cmd, unsigned long arg)
 {
 	struct loop_device *lo = bdev->bd_disk->private_data;
+	void __user *argp = (void __user *) arg;
 	int err;
 
 	switch (cmd) {
@@ -1671,21 +1672,19 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 	case LOOP_SET_STATUS:
 		err = -EPERM;
 		if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN)) {
-			err = loop_set_status_old(lo,
-					(struct loop_info __user *)arg);
+			err = loop_set_status_old(lo, argp);
 		}
 		break;
 	case LOOP_GET_STATUS:
-		return loop_get_status_old(lo, (struct loop_info __user *) arg);
+		return loop_get_status_old(lo, argp);
 	case LOOP_SET_STATUS64:
 		err = -EPERM;
 		if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN)) {
-			err = loop_set_status64(lo,
-					(struct loop_info64 __user *) arg);
+			err = loop_set_status64(lo, argp);
 		}
 		break;
 	case LOOP_GET_STATUS64:
-		return loop_get_status64(lo, (struct loop_info64 __user *) arg);
+		return loop_get_status64(lo, argp);
 	case LOOP_SET_CAPACITY:
 	case LOOP_SET_DIRECT_IO:
 	case LOOP_SET_BLOCK_SIZE:
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 09/10] loop: Clean up LOOP_SET_STATUS lo_flags handling.

[Martijn Coenen]

LOOP_SET_STATUS(64) will actually allow some lo_flags to be modified; in
particular, LO_FLAGS_AUTOCLEAR can be set and cleared, whereas
LO_FLAGS_PARTSCAN can be set to request a partition scan. Make this
explicit by updating the UAPI to include the flags that can be
set/cleared using this ioctl.

The implementation can then blindly take over the passed in flags,
and use the previous flags for those flags that can't be set / cleared
using LOOP_SET_STATUS.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c      | 19 +++++++++++++------
 include/uapi/linux/loop.h | 10 ++++++++--
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index f172a64333aa..cfbdd99fdb1a 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1049,9 +1049,7 @@ loop_set_status_from_info(struct loop_device *lo,
 	lo->transfer = xfer->transfer;
 	lo->ioctl = xfer->ioctl;
 
-	if ((lo->lo_flags & LO_FLAGS_AUTOCLEAR) !=
-	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
-		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
+	lo->lo_flags = info->lo_flags;
 
 	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
 	lo->lo_init[0] = info->lo_init[0];
@@ -1338,6 +1336,7 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 	int err;
 	struct block_device *bdev;
 	kuid_t uid = current_uid();
+	int prev_lo_flags;
 	bool partscan = false;
 	bool size_changed = false;
 	loff_t validated_size;
@@ -1381,10 +1380,19 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		goto out_unfreeze;
 	}
 
+	prev_lo_flags = lo->lo_flags;
+
 	err = loop_set_status_from_info(lo, info);
 	if (err)
 		goto out_unfreeze;
 
+	/* Mask out flags that can't be set using LOOP_SET_STATUS. */
+	lo->lo_flags &= ~LOOP_SET_STATUS_SETTABLE_FLAGS;
+	/* For those flags, use the previous values instead */
+	lo->lo_flags |= prev_lo_flags & ~LOOP_SET_STATUS_SETTABLE_FLAGS;
+	/* For flags that can't be cleared, use previous values too */
+	lo->lo_flags |= prev_lo_flags & ~LOOP_SET_STATUS_CLEARABLE_FLAGS;
+
 	if (size_changed)
 		loop_set_size(lo, validated_size);
 
@@ -1396,9 +1404,8 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 out_unfreeze:
 	blk_mq_unfreeze_queue(lo->lo_queue);
 
-	if (!err && (info->lo_flags & LO_FLAGS_PARTSCAN) &&
-	     !(lo->lo_flags & LO_FLAGS_PARTSCAN)) {
-		lo->lo_flags |= LO_FLAGS_PARTSCAN;
+	if (!err && (lo->lo_flags & LO_FLAGS_PARTSCAN) &&
+	     !(prev_lo_flags & LO_FLAGS_PARTSCAN)) {
 		lo->lo_disk->flags &= ~GENHD_FL_NO_PART_SCAN;
 		bdev = lo->lo_device;
 		partscan = true;
diff --git a/include/uapi/linux/loop.h b/include/uapi/linux/loop.h
index 080a8df134ef..6b32fee80ce0 100644
--- a/include/uapi/linux/loop.h
+++ b/include/uapi/linux/loop.h
@@ -25,6 +25,12 @@ enum {
 	LO_FLAGS_DIRECT_IO	= 16,
 };
 
+/* LO_FLAGS that can be set using LOOP_SET_STATUS(64) */
+#define LOOP_SET_STATUS_SETTABLE_FLAGS (LO_FLAGS_AUTOCLEAR | LO_FLAGS_PARTSCAN)
+
+/* LO_FLAGS that can be cleared using LOOP_SET_STATUS(64) */
+#define LOOP_SET_STATUS_CLEARABLE_FLAGS (LO_FLAGS_AUTOCLEAR)
+
 #include <asm/posix_types.h>	/* for __kernel_old_dev_t */
 #include <linux/types.h>	/* for __u64 */
 
@@ -37,7 +43,7 @@ struct loop_info {
 	int		   lo_offset;
 	int		   lo_encrypt_type;
 	int		   lo_encrypt_key_size; 	/* ioctl w/o */
-	int		   lo_flags;			/* ioctl r/o */
+	int		   lo_flags;
 	char		   lo_name[LO_NAME_SIZE];
 	unsigned char	   lo_encrypt_key[LO_KEY_SIZE]; /* ioctl w/o */
 	unsigned long	   lo_init[2];
@@ -53,7 +59,7 @@ struct loop_info64 {
 	__u32		   lo_number;			/* ioctl r/o */
 	__u32		   lo_encrypt_type;
 	__u32		   lo_encrypt_key_size;		/* ioctl w/o */
-	__u32		   lo_flags;			/* ioctl r/o */
+	__u32		   lo_flags;
 	__u8		   lo_file_name[LO_NAME_SIZE];
 	__u8		   lo_crypt_name[LO_NAME_SIZE];
 	__u8		   lo_encrypt_key[LO_KEY_SIZE]; /* ioctl w/o */
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Martijn Coenen]

This allows userspace to completely setup a loop device with a single
ioctl, removing the in-between state where the device can be partially
configured - eg the loop device has a backing file associated with it,
but is reading from the wrong offset.

Besides removing the intermediate state, another big benefit of this
ioctl is that LOOP_SET_STATUS can be slow; the main reason for this
slowness is that LOOP_SET_STATUS(64) calls blk_mq_freeze_queue() to
freeze the associated queue; this requires waiting for RCU
synchronization, which I've measured can take about 15-20ms on this
device on average.

In addition to doing what LOOP_SET_STATUS can do, LOOP_CONFIGURE can
also be used to:
- Set the correct block size immediately by setting
  loop_config.block_size (avoids LOOP_SET_BLOCK_SIZE)
- Explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
  in loop_config.info.lo_flags (avoids LOOP_SET_DIRECT_IO)
- Explicitly request read-only mode by setting LO_FLAGS_READ_ONLY
  in loop_config.info.lo_flags

Here's setting up ~70 regular loop devices with an offset on an x86
Android device, using LOOP_SET_FD and LOOP_SET_STATUS:

vsoc_x86:/system/apex # time for i in `seq 30 100`;
do losetup -r -o 4096 /dev/block/loop$i com.android.adbd.apex; done
    0m03.40s real     0m00.02s user     0m00.03s system

Here's configuring ~70 devices in the same way, but using a modified
losetup that uses the new LOOP_CONFIGURE ioctl:

vsoc_x86:/system/apex # time for i in `seq 30 100`;
do losetup -r -o 4096 /dev/block/loop$i com.android.adbd.apex; done
    0m01.94s real     0m00.01s user     0m00.01s system

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c      | 107 +++++++++++++++++++++++++++-----------
 include/uapi/linux/loop.h |  21 ++++++++
 2 files changed, 99 insertions(+), 29 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index cfbdd99fdb1a..a353ce55fd18 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -241,6 +241,19 @@ loop_validate_size(loff_t size)
 	return 0;
 }
 
+/**
+ * loop_validate_block_size() - validates the passed in block size
+ * @bsize: size to validate
+ */
+static int
+loop_validate_block_size(unsigned short bsize)
+{
+	if (bsize < 512 || bsize > PAGE_SIZE || !is_power_of_2(bsize))
+		return -EINVAL;
+
+	return 0;
+}
+
 /**
  * loop_set_size() - sets device size and notifies userspace
  * @lo: struct loop_device to set the size for
@@ -1063,23 +1076,24 @@ loop_set_status_from_info(struct loop_device *lo,
 	return 0;
 }
 
-static int loop_set_fd(struct loop_device *lo, fmode_t mode,
-		       struct block_device *bdev, unsigned int arg)
+static int loop_configure(struct loop_device *lo, fmode_t mode,
+			  struct block_device *bdev,
+			  const struct loop_config *config)
 {
 	struct file	*file;
 	struct inode	*inode;
 	struct address_space *mapping;
 	struct block_device *claimed_bdev = NULL;
-	int		lo_flags = 0;
 	int		error;
 	loff_t		size;
 	bool		partscan;
+	unsigned short  bsize;
 
 	/* This is safe, since we have a reference from open(). */
 	__module_get(THIS_MODULE);
 
 	error = -EBADF;
-	file = fget(arg);
+	file = fget(config->fd);
 	if (!file)
 		goto out;
 
@@ -1088,7 +1102,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	 * here to avoid changing device under exclusive owner.
 	 */
 	if (!(mode & FMODE_EXCL)) {
-		claimed_bdev = bd_start_claiming(bdev, loop_set_fd);
+		claimed_bdev = bd_start_claiming(bdev, loop_configure);
 		if (IS_ERR(claimed_bdev)) {
 			error = PTR_ERR(claimed_bdev);
 			goto out_putf;
@@ -1110,44 +1124,58 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	mapping = file->f_mapping;
 	inode = mapping->host;
 
-	if (!(file->f_mode & FMODE_WRITE) || !(mode & FMODE_WRITE) ||
-	    !file->f_op->write_iter)
-		lo_flags |= LO_FLAGS_READ_ONLY;
-
 	size = get_loop_size(lo, file);
 	error = loop_validate_size(size);
 	if (error)
 		goto out_unlock;
+
+	if ((config->info.lo_flags & ~LOOP_CONFIGURE_SETTABLE_FLAGS) != 0) {
+		error = -EINVAL;
+		goto out_unlock;
+	}
+
+	if (config->block_size) {
+		error = loop_validate_block_size(config->block_size);
+		if (error)
+			goto out_unlock;
+	}
+
+	error = loop_set_status_from_info(lo, &config->info);
+	if (error)
+		goto out_unlock;
+
+	if (!(file->f_mode & FMODE_WRITE) || !(mode & FMODE_WRITE) ||
+	    !file->f_op->write_iter)
+		lo->lo_flags |= LO_FLAGS_READ_ONLY;
+
 	error = loop_prepare_queue(lo);
 	if (error)
 		goto out_unlock;
 
 	error = 0;
 
-	set_device_ro(bdev, (lo_flags & LO_FLAGS_READ_ONLY) != 0);
+	set_device_ro(bdev, (lo->lo_flags & LO_FLAGS_READ_ONLY) != 0);
 
-	lo->use_dio = false;
+	lo->use_dio = lo->lo_flags & LO_FLAGS_DIRECT_IO;
 	lo->lo_device = bdev;
-	lo->lo_flags = lo_flags;
 	lo->lo_backing_file = file;
-	lo->transfer = NULL;
-	lo->ioctl = NULL;
-	lo->lo_sizelimit = 0;
 	lo->old_gfp_mask = mapping_gfp_mask(mapping);
 	mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
 
-	if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
+	if (!(lo->lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
 		blk_queue_write_cache(lo->lo_queue, true, false);
 
-	if (io_is_direct(lo->lo_backing_file) && inode->i_sb->s_bdev) {
+	if (config->block_size)
+		bsize = config->block_size;
+	else if (io_is_direct(lo->lo_backing_file) && inode->i_sb->s_bdev)
 		/* In case of direct I/O, match underlying block size */
-		unsigned short bsize = bdev_logical_block_size(
-			inode->i_sb->s_bdev);
+		bsize = bdev_logical_block_size(inode->i_sb->s_bdev);
+	else
+		bsize = 512;
 
-		blk_queue_logical_block_size(lo->lo_queue, bsize);
-		blk_queue_physical_block_size(lo->lo_queue, bsize);
-		blk_queue_io_min(lo->lo_queue, bsize);
-	}
+	blk_queue_logical_block_size(lo->lo_queue, bsize);
+	blk_queue_physical_block_size(lo->lo_queue, bsize);
+	blk_queue_io_min(lo->lo_queue, bsize);
 
 	loop_update_rotational(lo);
 	loop_update_dio(lo);
@@ -1170,14 +1198,14 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	if (partscan)
 		loop_reread_partitions(lo, bdev);
 	if (claimed_bdev)
-		bd_abort_claiming(bdev, claimed_bdev, loop_set_fd);
+		bd_abort_claiming(bdev, claimed_bdev, loop_configure);
 	return 0;
 
 out_unlock:
 	mutex_unlock(&loop_ctl_mutex);
 out_bdev:
 	if (claimed_bdev)
-		bd_abort_claiming(bdev, claimed_bdev, loop_set_fd);
+		bd_abort_claiming(bdev, claimed_bdev, loop_configure);
 out_putf:
 	fput(file);
 out:
@@ -1607,8 +1635,9 @@ static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
 	if (lo->lo_state != Lo_bound)
 		return -ENXIO;
 
-	if (arg < 512 || arg > PAGE_SIZE || !is_power_of_2(arg))
-		return -EINVAL;
+	err = loop_validate_block_size(arg);
+	if (err)
+		return err;
 
 	if (lo->lo_queue->limits.logical_block_size == arg)
 		return 0;
@@ -1670,8 +1699,27 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 	int err;
 
 	switch (cmd) {
-	case LOOP_SET_FD:
-		return loop_set_fd(lo, mode, bdev, arg);
+	case LOOP_SET_FD: {
+		/*
+		 * Legacy case - pass in a zeroed out struct loop_config with
+		 * only the file descriptor set , which corresponds with the
+		 * default parameters we'd have used otherwise.
+		 */
+		struct loop_config config;
+
+		memset(&config, 0, sizeof(config));
+		config.fd = arg;
+
+		return loop_configure(lo, mode, bdev, &config);
+	}
+	case LOOP_CONFIGURE: {
+		struct loop_config config;
+
+		if (copy_from_user(&config, argp, sizeof(config)))
+			return -EFAULT;
+
+		return loop_configure(lo, mode, bdev, &config);
+	}
 	case LOOP_CHANGE_FD:
 		return loop_change_fd(lo, bdev, arg);
 	case LOOP_CLR_FD:
@@ -1843,6 +1891,7 @@ static int lo_compat_ioctl(struct block_device *bdev, fmode_t mode,
 	case LOOP_CLR_FD:
 	case LOOP_GET_STATUS64:
 	case LOOP_SET_STATUS64:
+	case LOOP_CONFIGURE:
 		arg = (unsigned long) compat_ptr(arg);
 		/* fall through */
 	case LOOP_SET_FD:
diff --git a/include/uapi/linux/loop.h b/include/uapi/linux/loop.h
index 6b32fee80ce0..24a1c45bd1ae 100644
--- a/include/uapi/linux/loop.h
+++ b/include/uapi/linux/loop.h
@@ -31,6 +31,10 @@ enum {
 /* LO_FLAGS that can be cleared using LOOP_SET_STATUS(64) */
 #define LOOP_SET_STATUS_CLEARABLE_FLAGS (LO_FLAGS_AUTOCLEAR)
 
+/* LO_FLAGS that can be set using LOOP_CONFIGURE */
+#define LOOP_CONFIGURE_SETTABLE_FLAGS (LO_FLAGS_READ_ONLY | LO_FLAGS_AUTOCLEAR \
+				       | LO_FLAGS_PARTSCAN | LO_FLAGS_DIRECT_IO)
+
 #include <asm/posix_types.h>	/* for __kernel_old_dev_t */
 #include <linux/types.h>	/* for __u64 */
 
@@ -66,6 +70,22 @@ struct loop_info64 {
 	__u64		   lo_init[2];
 };
 
+/**
+ * struct loop_config - Complete configuration for a loop device.
+ * @fd: fd of the file to be used as a backing file for the loop device.
+ * @block_size: block size to use; ignored if 0.
+ * @info: struct loop_info64 to configure the loop device with.
+ *
+ * This structure is used with the LOOP_CONFIGURE ioctl, and can be used to
+ * atomically setup and configure all loop device parameters at once.
+ */
+struct loop_config {
+	__u32			fd;
+	__u32                   block_size;
+	struct loop_info64	info;
+	__u64			__reserved[8];
+};
+
 /*
  * Loop filter types
  */
@@ -96,6 +116,7 @@ struct loop_info64 {
 #define LOOP_SET_CAPACITY	0x4C07
 #define LOOP_SET_DIRECT_IO	0x4C08
 #define LOOP_SET_BLOCK_SIZE	0x4C09
+#define LOOP_CONFIGURE		0x4C0A
 
 /* /dev/loop-control interface */
 #define LOOP_CTL_ADD		0x4C80
-- 
2.26.2.303.gf8c07b1a785-goog

Re: [PATCH v4 01/10] loop: Factor out loop size validation

[Ming Lei]

On Wed, Apr 29, 2020 at 04:03:32PM +0200, Martijn Coenen wrote:
> Ensuring we don't truncate loff_t when casting to sector_t is done in
> multiple places; factor it out.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>
> ---
>  drivers/block/loop.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index f1754262fc94..396b8bd4d75c 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -228,15 +228,30 @@ static void __loop_update_dio(struct loop_device *lo, bool dio)
>  		blk_mq_unfreeze_queue(lo->lo_queue);
>  }
>  
> +/**
> + * loop_validate_size() - validates that the passed in size fits in a sector_t
> + * @size: size to validate
> + */
> +static int
> +loop_validate_size(loff_t size)
> +{
> +	if ((loff_t)(sector_t)size != size)
> +		return -EFBIG;
> +
> +	return 0;
> +}
> +

Now sector_t has been switched to u64 unconditionally, do we still need such
validation?


Thanks, 
Ming

Re: [PATCH v4 01/10] loop: Factor out loop size validation

[Martijn Coenen]

Hi Ming,

On Wed, Apr 29, 2020 at 4:12 PM Ming Lei <ming.lei@redhat.com> wrote:
> Now sector_t has been switched to u64 unconditionally, do we still need such
> validation?

I think you're right; I hadn't seen that change, but truncating
because of sector_t shouldn't be an issue anymore. I wondered if we
could actually have a smaller loff_t, but looks like that is 'long
long', which should always be 8 bytes as well. I might send this as a
separate patch, I don't want to drag this series on for too long.

Thanks,
Martijn

>
>
> Thanks,
> Ming
>
>

Re: [PATCH v4 01/10] loop: Factor out loop size validation

[Christoph Hellwig]

On Wed, Apr 29, 2020 at 10:12:29PM +0800, Ming Lei wrote:
> > +static int
> > +loop_validate_size(loff_t size)
> > +{
> > +	if ((loff_t)(sector_t)size != size)
> > +		return -EFBIG;
> > +
> > +	return 0;
> > +}
> > +
> 
> Now sector_t has been switched to u64 unconditionally, do we still need such
> validation?

Oops, completely forgot about that.  Yes, we can just kill the
checks.

Re: [PATCH v4 02/10] loop: Factor out setting loop device size

[Christoph Hellwig]

On Wed, Apr 29, 2020 at 04:03:33PM +0200, Martijn Coenen wrote:
> This code is used repeatedly.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>
> ---
>  drivers/block/loop.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v4 03/10] loop: Switch to set_capacity_revalidate_and_notify()

[Christoph Hellwig]

On Wed, Apr 29, 2020 at 04:03:34PM +0200, Martijn Coenen wrote:
> This was recently added to block/genhd.c, and takes care of both
> updating the capacity and notifying userspace of the new size.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v4 04/10] loop: Refactor loop_set_status() size calculation

[Christoph Hellwig]

For some reason this fails to apply for me against both
Jens' for-5.8/block and Linus' current tree.

What is the baseline for this series?

Re: [PATCH v4 04/10] loop: Refactor loop_set_status() size calculation

[Martijn Coenen]

On Fri, May 1, 2020 at 7:30 PM Christoph Hellwig <hch@lst.de> wrote:
>
> For some reason this fails to apply for me against both
> Jens' for-5.8/block and Linus' current tree.
>
> What is the baseline for this series?

This was based on Linus' tree from a week or two ago or so, but I
think you're most likely missing this one?

https://lkml.org/lkml/2020/3/31/755

(I mentioned it in the cover letter, but can make it a part of this
series if you prefer).

Re: [PATCH v4 04/10] loop: Refactor loop_set_status() size calculation

[Christoph Hellwig]

On Fri, May 01, 2020 at 09:33:23PM +0200, Martijn Coenen wrote:
> On Fri, May 1, 2020 at 7:30 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > For some reason this fails to apply for me against both
> > Jens' for-5.8/block and Linus' current tree.
> >
> > What is the baseline for this series?
> 
> This was based on Linus' tree from a week or two ago or so, but I
> think you're most likely missing this one?
> 
> https://lkml.org/lkml/2020/3/31/755
> 
> (I mentioned it in the cover letter, but can make it a part of this
> series if you prefer).

Yes, I missed that one.

Re: [PATCH v4 04/10] loop: Refactor loop_set_status() size calculation

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v4 05/10] loop: Remove figure_loop_size()

[Christoph Hellwig]

On Wed, Apr 29, 2020 at 04:03:36PM +0200, Martijn Coenen wrote:
> This function was now only used by loop_set_capacity(). Just open code
> the remaining code in the caller instead.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v4 06/10] loop: Factor out configuring loop from status

[Christoph Hellwig]

On Wed, Apr 29, 2020 at 04:03:37PM +0200, Martijn Coenen wrote:
> Factor out this code into a separate function, so it can be reused by
> other code more easily.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v4 07/10] loop: Move loop_set_status_from_info() and friends up

[Christoph Hellwig]

On Wed, Apr 29, 2020 at 04:03:38PM +0200, Martijn Coenen wrote:
> So we can use it without forward declaration. This is a separate commit
> to make it easier to verify that this is just a move, without functional
> modifications.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v4 08/10] loop: Rework lo_ioctl() __user argument casting

[Christoph Hellwig]

On Wed, Apr 29, 2020 at 04:03:39PM +0200, Martijn Coenen wrote:
> In preparation for a new ioctl that needs to copy_from_user(); makes the
> code easier to read as well.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Btw, it seems like you have dropped all the reviews from the previous
versions, even if nothing substantial changed.  Please keep them unless
a patch actually is reworked.

Re: [PATCH v4 09/10] loop: Clean up LOOP_SET_STATUS lo_flags handling.

[Christoph Hellwig]

On Wed, Apr 29, 2020 at 04:03:40PM +0200, Martijn Coenen wrote:
> LOOP_SET_STATUS(64) will actually allow some lo_flags to be modified; in
> particular, LO_FLAGS_AUTOCLEAR can be set and cleared, whereas
> LO_FLAGS_PARTSCAN can be set to request a partition scan. Make this
> explicit by updating the UAPI to include the flags that can be
> set/cleared using this ioctl.
> 
> The implementation can then blindly take over the passed in flags,
> and use the previous flags for those flags that can't be set / cleared
> using LOOP_SET_STATUS.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Christoph Hellwig]

Thanks, this looks very nice!

Reviewed-by: Christoph Hellwig <hch@lst.de>

Are you also going to submit a patch to util-linux to use the new
ioctl?

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Michael Kerrisk (man-pages)]

Hi Martijn,

On 4/29/20 4:03 PM, Martijn Coenen wrote:
> This allows userspace to completely setup a loop device with a single
> ioctl, removing the in-between state where the device can be partially
> configured - eg the loop device has a backing file associated with it,
> but is reading from the wrong offset.
> 
> Besides removing the intermediate state, another big benefit of this
> ioctl is that LOOP_SET_STATUS can be slow; the main reason for this
> slowness is that LOOP_SET_STATUS(64) calls blk_mq_freeze_queue() to
> freeze the associated queue; this requires waiting for RCU
> synchronization, which I've measured can take about 15-20ms on this
> device on average.
> 
> In addition to doing what LOOP_SET_STATUS can do, LOOP_CONFIGURE can
> also be used to:
> - Set the correct block size immediately by setting
>   loop_config.block_size (avoids LOOP_SET_BLOCK_SIZE)
> - Explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
>   in loop_config.info.lo_flags (avoids LOOP_SET_DIRECT_IO)
> - Explicitly request read-only mode by setting LO_FLAGS_READ_ONLY
>   in loop_config.info.lo_flags
> 
> Here's setting up ~70 regular loop devices with an offset on an x86
> Android device, using LOOP_SET_FD and LOOP_SET_STATUS:
> 
> vsoc_x86:/system/apex # time for i in `seq 30 100`;
> do losetup -r -o 4096 /dev/block/loop$i com.android.adbd.apex; done
>     0m03.40s real     0m00.02s user     0m00.03s system
> 
> Here's configuring ~70 devices in the same way, but using a modified
> losetup that uses the new LOOP_CONFIGURE ioctl:
> 
> vsoc_x86:/system/apex # time for i in `seq 30 100`;
> do losetup -r -o 4096 /dev/block/loop$i com.android.adbd.apex; done
>     0m01.94s real     0m00.01s user     0m00.01s system
> 
> Signed-off-by: Martijn Coenen <maco@android.com>


Can we have also a patch for the loop.4 manual page please?

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Martijn Coenen]

On Wed, May 6, 2020 at 8:09 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Thanks, this looks very nice!

Thanks! And thanks for all the feedback, it has made this series a lot cleaner.

>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Are you also going to submit a patch to util-linux to use the new
> ioctl?

Yeah, I'm happy to - will do it as soon as the series lands.

Best,
Martijn

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Martijn Coenen]

On Wed, May 6, 2020 at 11:44 AM Michael Kerrisk (man-pages) >
> Can we have also a patch for the loop.4 manual page please?

Ack, will do when the series lands.

Best,
Martijn

>
> Thanks,
>
> Michael
>
>
> --
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Martijn Coenen]

Hi Jens,

What do you think of this series?

Thanks,
Martijn

On Wed, Apr 29, 2020 at 4:03 PM Martijn Coenen <maco@android.com> wrote:
>
> This allows userspace to completely setup a loop device with a single
> ioctl, removing the in-between state where the device can be partially
> configured - eg the loop device has a backing file associated with it,
> but is reading from the wrong offset.
>
> Besides removing the intermediate state, another big benefit of this
> ioctl is that LOOP_SET_STATUS can be slow; the main reason for this
> slowness is that LOOP_SET_STATUS(64) calls blk_mq_freeze_queue() to
> freeze the associated queue; this requires waiting for RCU
> synchronization, which I've measured can take about 15-20ms on this
> device on average.
>
> In addition to doing what LOOP_SET_STATUS can do, LOOP_CONFIGURE can
> also be used to:
> - Set the correct block size immediately by setting
>   loop_config.block_size (avoids LOOP_SET_BLOCK_SIZE)
> - Explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
>   in loop_config.info.lo_flags (avoids LOOP_SET_DIRECT_IO)
> - Explicitly request read-only mode by setting LO_FLAGS_READ_ONLY
>   in loop_config.info.lo_flags
>
> Here's setting up ~70 regular loop devices with an offset on an x86
> Android device, using LOOP_SET_FD and LOOP_SET_STATUS:
>
> vsoc_x86:/system/apex # time for i in `seq 30 100`;
> do losetup -r -o 4096 /dev/block/loop$i com.android.adbd.apex; done
>     0m03.40s real     0m00.02s user     0m00.03s system
>
> Here's configuring ~70 devices in the same way, but using a modified
> losetup that uses the new LOOP_CONFIGURE ioctl:
>
> vsoc_x86:/system/apex # time for i in `seq 30 100`;
> do losetup -r -o 4096 /dev/block/loop$i com.android.adbd.apex; done
>     0m01.94s real     0m00.01s user     0m00.01s system
>
> Signed-off-by: Martijn Coenen <maco@android.com>
> ---
>  drivers/block/loop.c      | 107 +++++++++++++++++++++++++++-----------
>  include/uapi/linux/loop.h |  21 ++++++++
>  2 files changed, 99 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index cfbdd99fdb1a..a353ce55fd18 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -241,6 +241,19 @@ loop_validate_size(loff_t size)
>         return 0;
>  }
>
> +/**
> + * loop_validate_block_size() - validates the passed in block size
> + * @bsize: size to validate
> + */
> +static int
> +loop_validate_block_size(unsigned short bsize)
> +{
> +       if (bsize < 512 || bsize > PAGE_SIZE || !is_power_of_2(bsize))
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
>  /**
>   * loop_set_size() - sets device size and notifies userspace
>   * @lo: struct loop_device to set the size for
> @@ -1063,23 +1076,24 @@ loop_set_status_from_info(struct loop_device *lo,
>         return 0;
>  }
>
> -static int loop_set_fd(struct loop_device *lo, fmode_t mode,
> -                      struct block_device *bdev, unsigned int arg)
> +static int loop_configure(struct loop_device *lo, fmode_t mode,
> +                         struct block_device *bdev,
> +                         const struct loop_config *config)
>  {
>         struct file     *file;
>         struct inode    *inode;
>         struct address_space *mapping;
>         struct block_device *claimed_bdev = NULL;
> -       int             lo_flags = 0;
>         int             error;
>         loff_t          size;
>         bool            partscan;
> +       unsigned short  bsize;
>
>         /* This is safe, since we have a reference from open(). */
>         __module_get(THIS_MODULE);
>
>         error = -EBADF;
> -       file = fget(arg);
> +       file = fget(config->fd);
>         if (!file)
>                 goto out;
>
> @@ -1088,7 +1102,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>          * here to avoid changing device under exclusive owner.
>          */
>         if (!(mode & FMODE_EXCL)) {
> -               claimed_bdev = bd_start_claiming(bdev, loop_set_fd);
> +               claimed_bdev = bd_start_claiming(bdev, loop_configure);
>                 if (IS_ERR(claimed_bdev)) {
>                         error = PTR_ERR(claimed_bdev);
>                         goto out_putf;
> @@ -1110,44 +1124,58 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>         mapping = file->f_mapping;
>         inode = mapping->host;
>
> -       if (!(file->f_mode & FMODE_WRITE) || !(mode & FMODE_WRITE) ||
> -           !file->f_op->write_iter)
> -               lo_flags |= LO_FLAGS_READ_ONLY;
> -
>         size = get_loop_size(lo, file);
>         error = loop_validate_size(size);
>         if (error)
>                 goto out_unlock;
> +
> +       if ((config->info.lo_flags & ~LOOP_CONFIGURE_SETTABLE_FLAGS) != 0) {
> +               error = -EINVAL;
> +               goto out_unlock;
> +       }
> +
> +       if (config->block_size) {
> +               error = loop_validate_block_size(config->block_size);
> +               if (error)
> +                       goto out_unlock;
> +       }
> +
> +       error = loop_set_status_from_info(lo, &config->info);
> +       if (error)
> +               goto out_unlock;
> +
> +       if (!(file->f_mode & FMODE_WRITE) || !(mode & FMODE_WRITE) ||
> +           !file->f_op->write_iter)
> +               lo->lo_flags |= LO_FLAGS_READ_ONLY;
> +
>         error = loop_prepare_queue(lo);
>         if (error)
>                 goto out_unlock;
>
>         error = 0;
>
> -       set_device_ro(bdev, (lo_flags & LO_FLAGS_READ_ONLY) != 0);
> +       set_device_ro(bdev, (lo->lo_flags & LO_FLAGS_READ_ONLY) != 0);
>
> -       lo->use_dio = false;
> +       lo->use_dio = lo->lo_flags & LO_FLAGS_DIRECT_IO;
>         lo->lo_device = bdev;
> -       lo->lo_flags = lo_flags;
>         lo->lo_backing_file = file;
> -       lo->transfer = NULL;
> -       lo->ioctl = NULL;
> -       lo->lo_sizelimit = 0;
>         lo->old_gfp_mask = mapping_gfp_mask(mapping);
>         mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
>
> -       if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
> +       if (!(lo->lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
>                 blk_queue_write_cache(lo->lo_queue, true, false);
>
> -       if (io_is_direct(lo->lo_backing_file) && inode->i_sb->s_bdev) {
> +       if (config->block_size)
> +               bsize = config->block_size;
> +       else if (io_is_direct(lo->lo_backing_file) && inode->i_sb->s_bdev)
>                 /* In case of direct I/O, match underlying block size */
> -               unsigned short bsize = bdev_logical_block_size(
> -                       inode->i_sb->s_bdev);
> +               bsize = bdev_logical_block_size(inode->i_sb->s_bdev);
> +       else
> +               bsize = 512;
>
> -               blk_queue_logical_block_size(lo->lo_queue, bsize);
> -               blk_queue_physical_block_size(lo->lo_queue, bsize);
> -               blk_queue_io_min(lo->lo_queue, bsize);
> -       }
> +       blk_queue_logical_block_size(lo->lo_queue, bsize);
> +       blk_queue_physical_block_size(lo->lo_queue, bsize);
> +       blk_queue_io_min(lo->lo_queue, bsize);
>
>         loop_update_rotational(lo);
>         loop_update_dio(lo);
> @@ -1170,14 +1198,14 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>         if (partscan)
>                 loop_reread_partitions(lo, bdev);
>         if (claimed_bdev)
> -               bd_abort_claiming(bdev, claimed_bdev, loop_set_fd);
> +               bd_abort_claiming(bdev, claimed_bdev, loop_configure);
>         return 0;
>
>  out_unlock:
>         mutex_unlock(&loop_ctl_mutex);
>  out_bdev:
>         if (claimed_bdev)
> -               bd_abort_claiming(bdev, claimed_bdev, loop_set_fd);
> +               bd_abort_claiming(bdev, claimed_bdev, loop_configure);
>  out_putf:
>         fput(file);
>  out:
> @@ -1607,8 +1635,9 @@ static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
>         if (lo->lo_state != Lo_bound)
>                 return -ENXIO;
>
> -       if (arg < 512 || arg > PAGE_SIZE || !is_power_of_2(arg))
> -               return -EINVAL;
> +       err = loop_validate_block_size(arg);
> +       if (err)
> +               return err;
>
>         if (lo->lo_queue->limits.logical_block_size == arg)
>                 return 0;
> @@ -1670,8 +1699,27 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
>         int err;
>
>         switch (cmd) {
> -       case LOOP_SET_FD:
> -               return loop_set_fd(lo, mode, bdev, arg);
> +       case LOOP_SET_FD: {
> +               /*
> +                * Legacy case - pass in a zeroed out struct loop_config with
> +                * only the file descriptor set , which corresponds with the
> +                * default parameters we'd have used otherwise.
> +                */
> +               struct loop_config config;
> +
> +               memset(&config, 0, sizeof(config));
> +               config.fd = arg;
> +
> +               return loop_configure(lo, mode, bdev, &config);
> +       }
> +       case LOOP_CONFIGURE: {
> +               struct loop_config config;
> +
> +               if (copy_from_user(&config, argp, sizeof(config)))
> +                       return -EFAULT;
> +
> +               return loop_configure(lo, mode, bdev, &config);
> +       }
>         case LOOP_CHANGE_FD:
>                 return loop_change_fd(lo, bdev, arg);
>         case LOOP_CLR_FD:
> @@ -1843,6 +1891,7 @@ static int lo_compat_ioctl(struct block_device *bdev, fmode_t mode,
>         case LOOP_CLR_FD:
>         case LOOP_GET_STATUS64:
>         case LOOP_SET_STATUS64:
> +       case LOOP_CONFIGURE:
>                 arg = (unsigned long) compat_ptr(arg);
>                 /* fall through */
>         case LOOP_SET_FD:
> diff --git a/include/uapi/linux/loop.h b/include/uapi/linux/loop.h
> index 6b32fee80ce0..24a1c45bd1ae 100644
> --- a/include/uapi/linux/loop.h
> +++ b/include/uapi/linux/loop.h
> @@ -31,6 +31,10 @@ enum {
>  /* LO_FLAGS that can be cleared using LOOP_SET_STATUS(64) */
>  #define LOOP_SET_STATUS_CLEARABLE_FLAGS (LO_FLAGS_AUTOCLEAR)
>
> +/* LO_FLAGS that can be set using LOOP_CONFIGURE */
> +#define LOOP_CONFIGURE_SETTABLE_FLAGS (LO_FLAGS_READ_ONLY | LO_FLAGS_AUTOCLEAR \
> +                                      | LO_FLAGS_PARTSCAN | LO_FLAGS_DIRECT_IO)
> +
>  #include <asm/posix_types.h>   /* for __kernel_old_dev_t */
>  #include <linux/types.h>       /* for __u64 */
>
> @@ -66,6 +70,22 @@ struct loop_info64 {
>         __u64              lo_init[2];
>  };
>
> +/**
> + * struct loop_config - Complete configuration for a loop device.
> + * @fd: fd of the file to be used as a backing file for the loop device.
> + * @block_size: block size to use; ignored if 0.
> + * @info: struct loop_info64 to configure the loop device with.
> + *
> + * This structure is used with the LOOP_CONFIGURE ioctl, and can be used to
> + * atomically setup and configure all loop device parameters at once.
> + */
> +struct loop_config {
> +       __u32                   fd;
> +       __u32                   block_size;
> +       struct loop_info64      info;
> +       __u64                   __reserved[8];
> +};
> +
>  /*
>   * Loop filter types
>   */
> @@ -96,6 +116,7 @@ struct loop_info64 {
>  #define LOOP_SET_CAPACITY      0x4C07
>  #define LOOP_SET_DIRECT_IO     0x4C08
>  #define LOOP_SET_BLOCK_SIZE    0x4C09
> +#define LOOP_CONFIGURE         0x4C0A
>
>  /* /dev/loop-control interface */
>  #define LOOP_CTL_ADD           0x4C80
> --
> 2.26.2.303.gf8c07b1a785-goog
>

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Jens Axboe]

On 5/12/20 12:46 AM, Martijn Coenen wrote:
> Hi Jens,
> 
> What do you think of this series?

Looks acceptable to me, but I'm getting a failure applying it to
for-5.8/drivers on this patch:

Applying: loop: Refactor loop_set_status() size calculation

So you'll probably want to respin on the right branch.

-- 
Jens Axboe

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Jens Axboe]

On 5/12/20 8:29 PM, Jens Axboe wrote:
> On 5/12/20 12:46 AM, Martijn Coenen wrote:
>> Hi Jens,
>>
>> What do you think of this series?
> 
> Looks acceptable to me, but I'm getting a failure applying it to
> for-5.8/drivers on this patch:
> 
> Applying: loop: Refactor loop_set_status() size calculation
> 
> So you'll probably want to respin on the right branch.

Then you can also drop patch #1.

-- 
Jens Axboe

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Martijn Coenen]

On Wed, May 13, 2020 at 4:30 AM Jens Axboe <axboe@kernel.dk> wrote:
> > Looks acceptable to me, but I'm getting a failure applying it to
> > for-5.8/drivers on this patch:
> >
> > Applying: loop: Refactor loop_set_status() size calculation
> >
> > So you'll probably want to respin on the right branch.

This series depends on a separate bugfix I sent to LKML earlier - see
https://lkml.org/lkml/2020/3/31/755 . I mentioned it in [00/10] of
this series, but perhaps I should have just included that patch.

I just verified that patch + this series still applies cleanly on your
for-5.8/drivers tree, but if you prefer I send a v5 with that patch
going first let me know.

Thanks,
Martijn



>
> Then you can also drop patch #1.
>
> --
> Jens Axboe
>

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Christoph Hellwig]

On Wed, May 13, 2020 at 09:07:43AM +0200, Martijn Coenen wrote:
> On Wed, May 13, 2020 at 4:30 AM Jens Axboe <axboe@kernel.dk> wrote:
> > > Looks acceptable to me, but I'm getting a failure applying it to
> > > for-5.8/drivers on this patch:
> > >
> > > Applying: loop: Refactor loop_set_status() size calculation
> > >
> > > So you'll probably want to respin on the right branch.
> 
> This series depends on a separate bugfix I sent to LKML earlier - see
> https://lkml.org/lkml/2020/3/31/755 . I mentioned it in [00/10] of
> this series, but perhaps I should have just included that patch.
> 
> I just verified that patch + this series still applies cleanly on your
> for-5.8/drivers tree, but if you prefer I send a v5 with that patch
> going first let me know.

You probably want to resend with the fix includes as the first patch.
And drop the truncation check now that we figured out that we don't
actually need it.

Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl

[Martijn Coenen]

On Wed, May 13, 2020 at 12:22 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, May 13, 2020 at 09:07:43AM +0200, Martijn Coenen wrote:
> > On Wed, May 13, 2020 at 4:30 AM Jens Axboe <axboe@kernel.dk> wrote:
> > > > Looks acceptable to me, but I'm getting a failure applying it to
> > > > for-5.8/drivers on this patch:
> > > >
> > > > Applying: loop: Refactor loop_set_status() size calculation
> > > >
> > > > So you'll probably want to respin on the right branch.
> >
> > This series depends on a separate bugfix I sent to LKML earlier - see
> > https://lkml.org/lkml/2020/3/31/755 . I mentioned it in [00/10] of
> > this series, but perhaps I should have just included that patch.
> >
> > I just verified that patch + this series still applies cleanly on your
> > for-5.8/drivers tree, but if you prefer I send a v5 with that patch
> > going first let me know.
>
> You probably want to resend with the fix includes as the first patch.
> And drop the truncation check now that we figured out that we don't
> actually need it.

Just sent v5, thanks!

Martijn