loop: support different logical block sizes

From: Omar Sandoval <[email protected]>

This is only used for setting the soft block size on the struct block_device once and then never used again.

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/loop.c | 10 ++--------
 drivers/block/loop.h |  1 -
 2 files changed, 2 insertions(+), 9 deletions(-)

From: Omar Sandoval <[email protected]>

The physical block size is "the lowest possible sector size that the hardware can operate on without reverting to read-modify-write operations" (from the comment on blk_queue_physical_block_size()). Since loop does buffered I/O on the backing file by default, the RMW unit is a page. This isn't the case for direct I/O mode, but let's keep it simple.

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/loop.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/block/loop.c +0 -0

index 54e091887199..1a5b4ecf54ec 100644

From: Omar Sandoval <[email protected]>

This is a different approach from the first attempt in f2c6df7dbf9a ("loop: support 4k physical blocksize"). Rather than extending LOOP_{GET,SET}_STATUS, add a separate ioctl just for setting the block size.

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/loop.c      | 24 ++++++++++++++++++++++++
 include/uapi/linux/loop.h |  1 +
 2 files changed, 25 insertions(+)

drivers/block/loop.c +1 -0

index 1a5b4ecf54ec..6b1d932028e3 100644
@@ -1047,6 +1047,7 @@ static int loop_clr_fd(struct loop_device *lo)
 	memset(lo->lo_encrypt_key, 0, LO_KEY_SIZE);
 	memset(lo->lo_crypt_name, 0, LO_NAME_SIZE);
 	memset(lo->lo_file_name, 0, LO_NAME_SIZE);
+	blk_queue_logical_block_size(lo->lo_queue, 512);
 	if (bdev) {
 		bdput(bdev);
 		invalidate_bdev(bdev);

drivers/block/loop.c +7 -0

+	loop_update_dio(lo);
+
+	blk_mq_unfreeze_queue(lo->lo_queue);
+
+	return 0;
+}
+
 static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 	unsigned int cmd, unsigned long arg)
 {

Quoted 29 lines · Omar Sandoval

From: Omar Sandoval <[email protected]>

This is a different approach from the first attempt in f2c6df7dbf9a ("loop: support 4k physical blocksize"). Rather than extending LOOP_{GET,SET}_STATUS, add a separate ioctl just for setting the block size.

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/loop.c      | 24 ++++++++++++++++++++++++
 include/uapi/linux/loop.h |  1 +
 2 files changed, 25 insertions(+)

drivers/block/loop.c +1 -0

index 1a5b4ecf54ec..6b1d932028e3 100644
@@ -1047,6 +1047,7 @@ static int loop_clr_fd(struct loop_device *lo)
 	memset(lo->lo_encrypt_key, 0, LO_KEY_SIZE);
 	memset(lo->lo_crypt_name, 0, LO_NAME_SIZE);
 	memset(lo->lo_file_name, 0, LO_NAME_SIZE);
+	blk_queue_logical_block_size(lo->lo_queue, 512);
 	if (bdev) {
 		bdput(bdev);
 		invalidate_bdev(bdev);

@@ -1330,6 +1331,24 @@ static int loop_set_dio(struct loop_device *lo, unsigned long arg)
 	return error;
 }
 

drivers/block/loop.c static int loop_set_dio(struct loop_device *lo, unsigned long arg) · line 1340

Quoted 7 lines

drivers/block/loop.c +7 -0

+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;

I'm not sure if the PAGE_SIZE is the right limit for the use-case where backing file is a block device ...but it's nothing important.

drivers/block/loop.c static int loop_set_dio(struct loop_device *lo, unsigned long arg) · line 1344

Quoted 3 lines

drivers/block/loop.c +3 -0

+	blk_mq_freeze_queue(lo->lo_queue);
+
+	blk_queue_logical_block_size(lo->lo_queue, arg);

Just note that this function also updates physical_block_size if the 'arg' is greater than the current setting. That's good thing :-)

Quoted 11 lines

drivers/block/loop.c +7 -0

+	loop_update_dio(lo);
+
+	blk_mq_unfreeze_queue(lo->lo_queue);
+
+	return 0;
+}
+
 static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 	unsigned int cmd, unsigned long arg)
 {

drivers/block/loop.c static int lo_ioctl(struct block_device *bdev, fmode_t mode, · line 1404

Quoted 8 lines

drivers/block/loop.c +5 -0

@@ -1378,6 +1397,11 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 		if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
 			err = loop_set_dio(lo, arg);
 		break;
+	case LOOP_SET_BLOCK_SIZE:
+		err = -EPERM;
+		if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
+			err = loop_set_block_size(lo, arg);
+		break;

I like it this ioctl idea. It makes loop based tests more comfortable as we can change sector size on the fly without loop device reset.

I did not test it yet, but this implementation seems the best. Thanks.

From: Omar Sandoval <[email protected]>

This is a different approach from the first attempt in f2c6df7dbf9a ("loop: support 4k physical blocksize"). Rather than extending LOOP_{GET,SET}_STATUS, add a separate ioctl just for setting the block size.

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/loop.c      | 24 ++++++++++++++++++++++++
 include/uapi/linux/loop.h |  1 +
 2 files changed, 25 insertions(+)

drivers/block/loop.c +24 -0

index 1a5b4ecf54ec..6b1d932028e3 100644
@@ -1047,6 +1047,7 @@ static int loop_clr_fd(struct loop_device *lo)
 	memset(lo->lo_encrypt_key, 0, LO_KEY_SIZE);
 	memset(lo->lo_crypt_name, 0, LO_NAME_SIZE);
 	memset(lo->lo_file_name, 0, LO_NAME_SIZE);
+	blk_queue_logical_block_size(lo->lo_queue, 512);
 	if (bdev) {
 		bdput(bdev);
 		invalidate_bdev(bdev);

@@ -1330,6 +1331,24 @@ static int loop_set_dio(struct loop_device *lo, unsigned long arg)
 	return error;
 }
 
+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;
+
+	blk_mq_freeze_queue(lo->lo_queue);
+
+	blk_queue_logical_block_size(lo->lo_queue, arg);
+	loop_update_dio(lo);
+
+	blk_mq_unfreeze_queue(lo->lo_queue);
+
+	return 0;
+}
+
 static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 	unsigned int cmd, unsigned long arg)
 {

@@ -1378,6 +1397,11 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 		if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
 			err = loop_set_dio(lo, arg);
 		break;
+	case LOOP_SET_BLOCK_SIZE:
+		err = -EPERM;
+		if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
+			err = loop_set_block_size(lo, arg);
+		break;
 	default:
 		err = lo->ioctl ? lo->ioctl(lo, cmd, arg) : -EINVAL;
 	}

include/uapi/linux/loop.h +0 -0

index c8125ec1f4f2..23158dbe2424 100644

From: Omar Sandoval <[email protected]>

The comments here are really outdated, and blk-mq made flushing much simpler, so just fold the two cases into the callers.

Signed-off-by Omar Sandoval <[email protected]>

---
 drivers/block/loop.c | 76 ++++++++--------------------------------------------
 1 file changed, 11 insertions(+), 65 deletions(-)

From: Omar Sandoval <[email protected]>

Hi, everyone,

Here's another attempt at supporting different logical block sizes for loop devices. First, some terminology:

* Logical block size: the lowest possible block size that the storage device can address.
* Physical block size: the lowest possible sector size that the hardware can operate on without reverting to read-modify-write operations. Always greater than or equal to the logical block size.

These are characteristics of the device itself tracked in the request_queue. For loop devices, both are currently always 512 bytes.

struct block_device also has another block size, basically a "soft" block size which is the preferred I/O size and can be changed from userspace. For loop devices, this is PAGE_SIZE if the backing file is a regular file or otherwise the soft block size of the backing block device.

Patch 1 simplifies how the soft block size is set. I'm tempted to not set it at all and use the default of PAGE_SIZE, but maybe someone is depending on inheriting the soft block size from the backing block device...

Patch 2 sets the physical block size of loop devices to a more meaningful value for loop, PAGE_SIZE.

Patch 3 allows us to change the logical block size. It adds a new ioctl instead of the previous approach (which extends LOOP_{GET,SET}_STATUS). Hannes, I assume you had a specific reason for doing it the way you did, care to explain?

Patch 4 is a cleanup.

This is based on my patch reverting the original block size support, targeting 4.14. Thanks!

Omar Sandoval (4):
loop: get rid of lo_blocksize
loop: set physical block size to PAGE_SIZE
loop: add ioctl for changing logical block size
loop: fold loop_switch() into callers

 drivers/block/loop.c      | 112 ++++++++++++++++------------------------------
 drivers/block/loop.h      |   1 -
 include/uapi/linux/loop.h |   1 +
 3 files changed, 40 insertions(+), 74 deletions(-)

From: Omar Sandoval <[email protected]>

Hi, everyone,

Here's another attempt at supporting different logical block sizes for loop devices. First, some terminology:

* Logical block size: the lowest possible block size that the storage device can address.
* Physical block size: the lowest possible sector size that the hardware can operate on without reverting to read-modify-write operations. Always greater than or equal to the logical block size.

These are characteristics of the device itself tracked in the request_queue. For loop devices, both are currently always 512 bytes.

struct block_device also has another block size, basically a "soft" block size which is the preferred I/O size and can be changed from userspace. For loop devices, this is PAGE_SIZE if the backing file is a regular file or otherwise the soft block size of the backing block device.

Patch 1 simplifies how the soft block size is set. I'm tempted to not set it at all and use the default of PAGE_SIZE, but maybe someone is depending on inheriting the soft block size from the backing block device...

Patch 2 sets the physical block size of loop devices to a more meaningful value for loop, PAGE_SIZE.

Patch 3 allows us to change the logical block size. It adds a new ioctl instead of the previous approach (which extends LOOP_{GET,SET}_STATUS). Hannes, I assume you had a specific reason for doing it the way you did, care to explain?

Patch 4 is a cleanup.

This is based on my patch reverting the original block size support, targeting 4.14. Thanks!