All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martijn Coenen <maco@android.com>
To: axboe@kernel.dk, hch@lst.de
Cc: ming.lei@redhat.com, bvanassche@acm.org,
	Chaitanya.Kulkarni@wdc.com, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-team@android.com,
	Martijn Coenen <maco@android.com>
Subject: [PATCH] loop: Add LOOP_SET_FD_WITH_OFFSET ioctl.
Date: Tue, 31 Mar 2020 13:40:30 +0200	[thread overview]
Message-ID: <20200331114030.21262-1-maco@android.com> (raw)

Configuring a loop device for a filesystem that is located at an offset
currently requires calling LOOP_SET_FD and LOOP_SET_STATUS(64)
consecutively. This has some downsides.

The most important downside is that it can be slow. Here's setting
up ~70 regular loop devices on an x86 Android device:

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

Here's configuring ~70 devices in the same way, but with an offset:

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

This is almost twice as 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.

A more minor downside of having to do two ioctls is that on devices with
max_part > 0, the kernel will initiate a partition scan, which is
needless work if the image is at an offset.

This change introduces a new ioctl to combine setting the backing file
together with the offset, which avoids the above problems. Adding more
parameters could be a consideration, but offset appears to be the only
commonly used parameter that is required for accessing the device
safely.

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

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index a42c49e04954..517031e1d10c 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -932,8 +932,8 @@ static void loop_update_rotational(struct loop_device *lo)
 		blk_queue_flag_clear(QUEUE_FLAG_NONROT, q);
 }
 
-static int loop_set_fd(struct loop_device *lo, fmode_t mode,
-		       struct block_device *bdev, unsigned int arg)
+static int loop_set_fd_with_offset(struct loop_device *lo, fmode_t mode,
+		struct block_device *bdev, unsigned int arg, loff_t offset)
 {
 	struct file	*file;
 	struct inode	*inode;
@@ -957,7 +957,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_set_fd_with_offset);
 		if (IS_ERR(claimed_bdev)) {
 			error = PTR_ERR(claimed_bdev);
 			goto out_putf;
@@ -1002,6 +1002,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	lo->transfer = NULL;
 	lo->ioctl = NULL;
 	lo->lo_sizelimit = 0;
+	lo->lo_offset = offset;
 	lo->old_gfp_mask = mapping_gfp_mask(mapping);
 	mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
 
@@ -1042,14 +1043,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_set_fd_with_offset);
 	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_set_fd_with_offset);
 out_putf:
 	fput(file);
 out:
@@ -1601,7 +1602,7 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 
 	switch (cmd) {
 	case LOOP_SET_FD:
-		return loop_set_fd(lo, mode, bdev, arg);
+		return loop_set_fd_with_offset(lo, mode, bdev, arg, 0);
 	case LOOP_CHANGE_FD:
 		return loop_change_fd(lo, bdev, arg);
 	case LOOP_CLR_FD:
@@ -1624,6 +1625,17 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 		break;
 	case LOOP_GET_STATUS64:
 		return loop_get_status64(lo, (struct loop_info64 __user *) arg);
+	case LOOP_SET_FD_WITH_OFFSET: {
+		struct loop_fd_with_offset fdwo;
+
+		if (copy_from_user(&fdwo,
+				(struct loop_fd_with_offset __user *) arg,
+				sizeof(struct loop_fd_with_offset)))
+			return -EFAULT;
+
+		return loop_set_fd_with_offset(lo, mode, bdev, fdwo.fd,
+				fdwo.lo_offset);
+	}
 	case LOOP_SET_CAPACITY:
 	case LOOP_SET_DIRECT_IO:
 	case LOOP_SET_BLOCK_SIZE:
@@ -1774,6 +1786,7 @@ static int lo_compat_ioctl(struct block_device *bdev, fmode_t mode,
 	case LOOP_SET_CAPACITY:
 	case LOOP_CLR_FD:
 	case LOOP_GET_STATUS64:
+	case LOOP_SET_FD_WITH_OFFSET:
 	case LOOP_SET_STATUS64:
 		arg = (unsigned long) compat_ptr(arg);
 		/* fall through */
diff --git a/include/uapi/linux/loop.h b/include/uapi/linux/loop.h
index 080a8df134ef..289829bc5abd 100644
--- a/include/uapi/linux/loop.h
+++ b/include/uapi/linux/loop.h
@@ -60,6 +60,11 @@ struct loop_info64 {
 	__u64		   lo_init[2];
 };
 
+struct loop_fd_with_offset {
+	__u64          lo_offset;
+	__u32          fd;
+};
+
 /*
  * Loop filter types
  */
@@ -90,6 +95,7 @@ struct loop_info64 {
 #define LOOP_SET_CAPACITY	0x4C07
 #define LOOP_SET_DIRECT_IO	0x4C08
 #define LOOP_SET_BLOCK_SIZE	0x4C09
+#define LOOP_SET_FD_WITH_OFFSET	0x4C0A
 
 /* /dev/loop-control interface */
 #define LOOP_CTL_ADD		0x4C80
-- 
2.26.0.rc2.310.g2932bb562d-goog


             reply	other threads:[~2020-03-31 11:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-31 11:40 Martijn Coenen [this message]
2020-03-31 11:42 ` [PATCH] loop: Add LOOP_SET_FD_WITH_OFFSET ioctl Martijn Coenen
  -- strict thread matches above, loose matches on Subject: below --
2020-03-29 14:04 Martijn Coenen
2020-03-29 16:48 ` Chaitanya Kulkarni
2020-03-29 20:19 ` Bart Van Assche
2020-03-30  1:00 ` Ming Lei
2020-03-30  8:06   ` Martijn Coenen
2020-03-31  7:48     ` Christoph Hellwig
2020-03-31 11:25       ` Martijn Coenen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200331114030.21262-1-maco@android.com \
    --to=maco@android.com \
    --cc=Chaitanya.Kulkarni@wdc.com \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=hch@lst.de \
    --cc=kernel-team@android.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.