| Message ID | 20190828103229.191853-1-maco@android.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | loop: change queue block size to match when using DIO. | expand |
On Wed, Aug 28, 2019 at 12:32:29PM +0200, Martijn Coenen wrote: > The loop driver assumes that if the passed in fd is opened with > O_DIRECT, the caller wants to use direct I/O on the loop device. > However, if the underlying filesystem has a different block size than > the loop block queue, direct I/O can't be enabled. Instead of requiring > userspace to manually change the blocksize and re-enable direct I/O, > just change the queue block size to match. Why can't we enable the block device in that case? All the usual block filesystems support 512 byte aligned direct I/O with a 4k file system block size (as long as the underlying block device sector size is also 512 bytes).
On Fri, Aug 30, 2019 at 5:50 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Wed, Aug 28, 2019 at 12:32:29PM +0200, Martijn Coenen wrote: > > The loop driver assumes that if the passed in fd is opened with > > O_DIRECT, the caller wants to use direct I/O on the loop device. > > However, if the underlying filesystem has a different block size than > > the loop block queue, direct I/O can't be enabled. Instead of requiring > > userspace to manually change the blocksize and re-enable direct I/O, > > just change the queue block size to match. > > Why can't we enable the block device in that case? All the usual > block filesystems support 512 byte aligned direct I/O with a 4k > file system block size (as long as the underlying block device > sector size is also 512 bytes). Sorry, I didn't word that correctly: it's not the logical block size of the filesystem, but the logical block size of the underlying block device that loop's queue must match (or exceed). With the current loop code, if the backing file is opened with O_DIRECT and resides on a block device with a 512 bytes logical block size, the loop device will correctly use direct I/O. If instead the backing file happened to reside on a block device with a 4k logical block size, the loop device would silently fall back to cached mode. I think there's a benefit in the behavior being consistent independent of the block size of the backing device, and I don't see a good reason for not automatically switching loop's logical/physical queue sizes to match the backing device in this specific case. Will send a v2. Thanks, Martijn
diff --git a/drivers/block/loop.c b/drivers/block/loop.c index ab7ca5989097a..1138162ff6c4d 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -994,6 +994,12 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode, if (!(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) { + /* In case of direct I/O, match underlying block size */ + blk_queue_logical_block_size(lo->lo_queue, + bdev_logical_block_size(inode->i_sb->s_bdev)); + } + loop_update_rotational(lo); loop_update_dio(lo); set_capacity(lo->lo_disk, size);
The loop driver assumes that if the passed in fd is opened with O_DIRECT, the caller wants to use direct I/O on the loop device. However, if the underlying filesystem has a different block size than the loop block queue, direct I/O can't be enabled. Instead of requiring userspace to manually change the blocksize and re-enable direct I/O, just change the queue block size to match. Signed-off-by: Martijn Coenen <maco@android.com> --- drivers/block/loop.c | 6 ++++++ 1 file changed, 6 insertions(+)