[v2,2/2] file-posix: add statx(STATX_DIOALIGN) support

Commit Message

Stefan Hajnoczi Nov. 3, 2022, 6:36 p.m. UTC

Linux v6.1 commit 825cf206ed51 ("statx: add direct I/O alignment
information") added an interface to fetch O_DIRECT alignment values for
block devices and file systems.

Prefer STATX_DIOALIGN to older interfaces and probing, but keep them as
fallbacks in case STATX_DIOALIGN cannot provide the information.

Testing shows the status of STATX_DIOALIGN support in Linux 6.1-rc3
appears to be:
- btrfs: no
- ext4: yes
- XFS: yes
- NVMe block devices: yes
- dm-crypt: yes

Cc: Eric Biggers <[e-mail]>
Signed-off-by: Stefan Hajnoczi <[e-mail]>
---
v2:
- Make sure that XFS_IOC_DIOINFO takes priority over logical blocksize [Eric Biggers]
---
 block/file-posix.c | 60 ++++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 20 deletions(-)

Comments

Kevin Wolf Nov. 11, 2022, 11:22 a.m. UTC | #1

Am 03.11.2022 um 19:36 hat Stefan Hajnoczi geschrieben:
> Linux v6.1 commit 825cf206ed51 ("statx: add direct I/O alignment
> information") added an interface to fetch O_DIRECT alignment values for
> block devices and file systems.
> 
> Prefer STATX_DIOALIGN to older interfaces and probing, but keep them as
> fallbacks in case STATX_DIOALIGN cannot provide the information.
> 
> Testing shows the status of STATX_DIOALIGN support in Linux 6.1-rc3
> appears to be:
> - btrfs: no
> - ext4: yes
> - XFS: yes
> - NVMe block devices: yes
> - dm-crypt: yes
> 
> Cc: Eric Biggers <[e-mail]>
> Signed-off-by: Stefan Hajnoczi <[e-mail]>

I think it's worth noting in the commit message that this essentially
disables patch 1 again on Linux 6.1 because it doesn't even use the code
any more that patch 1 modified to work around the dm-crypt bug.

This is only okay because we think that the final 6.1 release is going
to have the bug fixed, and it's also the first version to support
STATX_DIOALIGN, so you won't have both STATX_DIOALIGN and the bug in a
stable kernel.

Kevin

Patch

diff --git a/block/file-posix.c b/block/file-posix.c
index b9d62f52fe..b7e5a08e41 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -372,29 +372,49 @@  static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 
     bs->bl.request_alignment = 0;
     s->buf_align = 0;
+
+#if defined(__linux__) && defined(STATX_DIOALIGN)
+    struct statx stx;
+
+    /*
+     * Linux 6.1 introduced an interface for both block devices and file
+     * systems. The system call returns with the STATX_DIOALIGN bit cleared
+     * when the information is unavailable.
+     */
+    if (statx(fd, "", AT_EMPTY_PATH, STATX_DIOALIGN, &stx) == 0 &&
+        (stx.stx_mask & STATX_DIOALIGN)) {
+        bs->bl.request_alignment = stx.stx_dio_offset_align;
+        s->buf_align = stx.stx_dio_mem_align;
+    }
+#endif /* defined(__linux__) && defined(STATX_DIOALIGN) */
+
+#ifdef __linux__
+    if (!bs->bl.request_alignment) {
+        /*
+         * The XFS ioctl definitions are shipped in extra packages that might
+         * not always be available. Since we just need the XFS_IOC_DIOINFO ioctl
+         * here, we simply use our own definition instead:
+         */
+        struct xfs_dioattr {
+            uint32_t d_mem;
+            uint32_t d_miniosz;
+            uint32_t d_maxiosz;
+        } da;
+        if (ioctl(fd, _IOR('X', 30, struct xfs_dioattr), &da) >= 0) {
+            bs->bl.request_alignment = da.d_miniosz;
+            /* The kernel returns wrong information for d_mem */
+            /* s->buf_align = da.d_mem; */
+        }
+    }
+#endif /* __linux__ */
+
     /* Let's try to use the logical blocksize for the alignment. */
-    if (probe_logical_blocksize(fd, &bs->bl.request_alignment) < 0) {
-        bs->bl.request_alignment = 0;
+    if (!bs->bl.request_alignment) {
+        if (probe_logical_blocksize(fd, &bs->bl.request_alignment) < 0) {
+            bs->bl.request_alignment = 0;
+        }
     }
 
-#ifdef __linux__
-    /*
-     * The XFS ioctl definitions are shipped in extra packages that might
-     * not always be available. Since we just need the XFS_IOC_DIOINFO ioctl
-     * here, we simply use our own definition instead:
-     */
-    struct xfs_dioattr {
-        uint32_t d_mem;
-        uint32_t d_miniosz;
-        uint32_t d_maxiosz;
-    } da;
-    if (ioctl(fd, _IOR('X', 30, struct xfs_dioattr), &da) >= 0) {
-        bs->bl.request_alignment = da.d_miniosz;
-        /* The kernel returns wrong information for d_mem */
-        /* s->buf_align = da.d_mem; */
-    }
-#endif
-
     /*
      * If we could not get the sizes so far, we can only guess them. First try
      * to detect request alignment, since it is more likely to succeed. Then