* [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
[not found] <20200429140341.13294-1-maco@android.com>
@ 2020-04-29 14:03 ` Martijn Coenen
2020-05-06 6:09 ` Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Martijn Coenen @ 2020-04-29 14:03 UTC (permalink / raw)
To: axboe, hch, ming.lei
Cc: narayan, zezeozue, kernel-team, maco, bvanassche,
Chaitanya.Kulkarni, jaegeuk, linux-block, linux-kernel,
Martijn Coenen, Linux API
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-04-29 14:03 ` [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl Martijn Coenen
@ 2020-05-06 6:09 ` Christoph Hellwig
2020-05-06 9:55 ` Martijn Coenen
2020-05-06 9:44 ` Michael Kerrisk (man-pages)
2020-05-12 6:46 ` Martijn Coenen
2 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2020-05-06 6:09 UTC (permalink / raw)
To: Martijn Coenen
Cc: axboe, hch, ming.lei, narayan, zezeozue, kernel-team, maco,
bvanassche, Chaitanya.Kulkarni, jaegeuk, linux-block,
linux-kernel, Linux API
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?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-04-29 14:03 ` [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl Martijn Coenen
2020-05-06 6:09 ` Christoph Hellwig
@ 2020-05-06 9:44 ` Michael Kerrisk (man-pages)
2020-05-06 9:57 ` Martijn Coenen
2020-05-12 6:46 ` Martijn Coenen
2 siblings, 1 reply; 11+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-05-06 9:44 UTC (permalink / raw)
To: Martijn Coenen, axboe, hch, ming.lei
Cc: mtk.manpages, narayan, zezeozue, kernel-team, maco, bvanassche,
Chaitanya.Kulkarni, jaegeuk, linux-block, linux-kernel,
Linux API
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/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-05-06 6:09 ` Christoph Hellwig
@ 2020-05-06 9:55 ` Martijn Coenen
0 siblings, 0 replies; 11+ messages in thread
From: Martijn Coenen @ 2020-05-06 9:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Ming Lei, Narayan Kamath, Zimuzo Ezeozue,
kernel-team, Martijn Coenen, Bart Van Assche, Chaitanya Kulkarni,
Jaegeuk Kim, linux-block, LKML, Linux API
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-05-06 9:44 ` Michael Kerrisk (man-pages)
@ 2020-05-06 9:57 ` Martijn Coenen
0 siblings, 0 replies; 11+ messages in thread
From: Martijn Coenen @ 2020-05-06 9:57 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: Jens Axboe, Christoph Hellwig, Ming Lei, Narayan Kamath,
Zimuzo Ezeozue, kernel-team, Martijn Coenen, Bart Van Assche,
Chaitanya Kulkarni, Jaegeuk Kim, linux-block, LKML, Linux API
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/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-04-29 14:03 ` [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl Martijn Coenen
2020-05-06 6:09 ` Christoph Hellwig
2020-05-06 9:44 ` Michael Kerrisk (man-pages)
@ 2020-05-12 6:46 ` Martijn Coenen
2020-05-13 2:29 ` Jens Axboe
2 siblings, 1 reply; 11+ messages in thread
From: Martijn Coenen @ 2020-05-12 6:46 UTC (permalink / raw)
To: Jens Axboe
Cc: Narayan Kamath, Zimuzo Ezeozue, kernel-team, Martijn Coenen,
Bart Van Assche, Chaitanya Kulkarni, Jaegeuk Kim, linux-block,
LKML, Linux API, Christoph Hellwig, Ming Lei
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-05-12 6:46 ` Martijn Coenen
@ 2020-05-13 2:29 ` Jens Axboe
2020-05-13 2:30 ` Jens Axboe
0 siblings, 1 reply; 11+ messages in thread
From: Jens Axboe @ 2020-05-13 2:29 UTC (permalink / raw)
To: Martijn Coenen
Cc: Narayan Kamath, Zimuzo Ezeozue, kernel-team, Martijn Coenen,
Bart Van Assche, Chaitanya Kulkarni, Jaegeuk Kim, linux-block,
LKML, Linux API, Christoph Hellwig, Ming Lei
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-05-13 2:29 ` Jens Axboe
@ 2020-05-13 2:30 ` Jens Axboe
2020-05-13 7:07 ` Martijn Coenen
0 siblings, 1 reply; 11+ messages in thread
From: Jens Axboe @ 2020-05-13 2:30 UTC (permalink / raw)
To: Martijn Coenen
Cc: Narayan Kamath, Zimuzo Ezeozue, kernel-team, Martijn Coenen,
Bart Van Assche, Chaitanya Kulkarni, Jaegeuk Kim, linux-block,
LKML, Linux API, Christoph Hellwig, Ming Lei
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-05-13 2:30 ` Jens Axboe
@ 2020-05-13 7:07 ` Martijn Coenen
2020-05-13 10:22 ` Christoph Hellwig
0 siblings, 1 reply; 11+ messages in thread
From: Martijn Coenen @ 2020-05-13 7:07 UTC (permalink / raw)
To: Jens Axboe
Cc: Narayan Kamath, Zimuzo Ezeozue, kernel-team, Martijn Coenen,
Bart Van Assche, Chaitanya Kulkarni, Jaegeuk Kim, linux-block,
LKML, Linux API, Christoph Hellwig, Ming Lei
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-05-13 7:07 ` Martijn Coenen
@ 2020-05-13 10:22 ` Christoph Hellwig
2020-05-13 13:49 ` Martijn Coenen
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2020-05-13 10:22 UTC (permalink / raw)
To: Martijn Coenen
Cc: Jens Axboe, Narayan Kamath, Zimuzo Ezeozue, kernel-team,
Martijn Coenen, Bart Van Assche, Chaitanya Kulkarni, Jaegeuk Kim,
linux-block, LKML, Linux API, Christoph Hellwig, Ming Lei
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl
2020-05-13 10:22 ` Christoph Hellwig
@ 2020-05-13 13:49 ` Martijn Coenen
0 siblings, 0 replies; 11+ messages in thread
From: Martijn Coenen @ 2020-05-13 13:49 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Narayan Kamath, Zimuzo Ezeozue, kernel-team,
Martijn Coenen, Bart Van Assche, Chaitanya Kulkarni, Jaegeuk Kim,
linux-block, LKML, Linux API, Ming Lei
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-13 13:49 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200429140341.13294-1-maco@android.com>
2020-04-29 14:03 ` [PATCH v4 10/10] loop: Add LOOP_CONFIGURE ioctl Martijn Coenen
2020-05-06 6:09 ` Christoph Hellwig
2020-05-06 9:55 ` Martijn Coenen
2020-05-06 9:44 ` Michael Kerrisk (man-pages)
2020-05-06 9:57 ` Martijn Coenen
2020-05-12 6:46 ` Martijn Coenen
2020-05-13 2:29 ` Jens Axboe
2020-05-13 2:30 ` Jens Axboe
2020-05-13 7:07 ` Martijn Coenen
2020-05-13 10:22 ` Christoph Hellwig
2020-05-13 13:49 ` Martijn Coenen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).