Re: [PATCH v8 4/6] block: loop: prepare for supporing direct IO

On Thu, Jul 30, 2015 at 11:09 AM, Christoph Hellwig [off-list ref] wrote:

On Thu, Jul 30, 2015 at 07:36:22AM -0400, Ming Lei wrote:

quoted

This patches provides one interface for enabling direct IO
from user space:

      - userspace(such as losetup) can pass 'file' which is
      opened/fcntl as O_DIRECT

Also __loop_update_dio() is introduced to check if direct I/O
can be used on current loop setting.

The last big change is to introduce LO_FLAGS_DIRECT_IO flag
for userspace to know if direct IO is used to access backing
file.

Maybe i'm missing something because I was too busy to follow the
current discussion, but this still doesn't check that the loop
device sector size is aligned to the sector size of the underlying
filesystem.

E.g. with this you could create a loop device with a 512 byte
sector size on a filesystem with 4k sector size, and it would attempt
to use direct I/O and fail.

The I/O size & offset has to be checked in I/O path, see patch 6 please,
could you comment on that patch?

quoted

+     unsigned dio_align = inode->i_sb->s_bdev ?
+             (bdev_io_min(inode->i_sb->s_bdev) - 1) : 0;
+
+     /*
+      * We support direct I/O only if lo_offset is aligned
+      * with the min I/O size of backing device.
+      *
+      * Request's offset and size will be checked in I/O path.
+      */
+     if (dio) {
+             if (!dio_align || (lo->lo_offset & dio_align))
+                     use_dio = false;

Also this means you'll never use direct I/O on network filesystems,
which really would benefit from it.

OK, that can be done by removing !dio_align in above check.

Thanks,