improve minalign handling

improve minalign handling

[Christoph Hellwig]

Hi all,

this series improves how xfstests detects minimum direct I/O alignment
by adding a C utility that checks the kernel provided value in statx
before falling back to the existing strategy.

[PATCH 1/5] statx.h: update to latest kernel UAPI

[Christoph Hellwig]

Update the localy provided statx definition to the latest kernel UAPI,
and use it unconditionally instead only if no kernel version is provided.

This allows using more recent additions than provided in the system
headers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 src/statx.h | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/statx.h b/src/statx.h
index 3f239d791..ab29fe22d 100644
--- a/src/statx.h
+++ b/src/statx.h
@@ -28,8 +28,6 @@
 # endif
 #endif
 
-#ifndef STATX_TYPE
-
 /*
  * Timestamp structure for the timestamps in struct statx.
  *
@@ -44,6 +42,7 @@
  *
  * __reserved is held in case we need a yet finer resolution.
  */
+#define statx_timestamp statx_timestamp_fstests
 struct statx_timestamp {
 	__s64	tv_sec;
 	__s32	tv_nsec;
@@ -87,6 +86,7 @@ struct statx_timestamp {
  * will have values installed for compatibility purposes so that stat() and
  * co. can be emulated in userspace.
  */
+#define statx statx_fstests
 struct statx {
 	/* 0x00 */
 	__u32	stx_mask;	/* What results were written [uncond] */
@@ -102,7 +102,8 @@ struct statx {
 	__u64	stx_ino;	/* Inode number */
 	__u64	stx_size;	/* File size */
 	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
-	__u64	__spare1[1];
+	__u64	stx_attributes_mask; /* Mask to show what's supported in stx_attributes */
+
 	/* 0x40 */
 	struct statx_timestamp	stx_atime;	/* Last access time */
 	struct statx_timestamp	stx_btime;	/* File creation time */
@@ -114,7 +115,18 @@ struct statx {
 	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
 	__u32	stx_dev_minor;
 	/* 0x90 */
-	__u64	__spare2[14];	/* Spare space for future expansion */
+	__u64	stx_mnt_id;
+	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
+	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
+	/* 0xa0 */
+	__u64	stx_subvol;	/* Subvolume identifier */
+	__u32	stx_atomic_write_unit_min;	/* Min atomic write unit in bytes */
+	__u32	stx_atomic_write_unit_max;	/* Max atomic write unit in bytes */
+	/* 0xb0 */
+	__u32   stx_atomic_write_segments_max;	/* Max atomic write segment count */
+	__u32   __spare1[1];
+	/* 0xb8 */
+	__u64	__spare3[9];	/* Spare space for future expansion */
 	/* 0x100 */
 };
 
@@ -139,6 +151,12 @@ struct statx {
 #define STATX_BLOCKS		0x00000400U	/* Want/got stx_blocks */
 #define STATX_BASIC_STATS	0x000007ffU	/* The stuff in the normal stat struct */
 #define STATX_BTIME		0x00000800U	/* Want/got stx_btime */
+#define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
+#define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
+#define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
+#define STATX_SUBVOL		0x00008000U	/* Want/got stx_subvol */
+#define STATX_WRITE_ATOMIC	0x00010000U	/* Want/got atomic_write_* fields */
+
 #define STATX_ALL		0x00000fffU	/* All currently supported flags */
 
 /*
@@ -157,9 +175,11 @@ struct statx {
 #define STATX_ATTR_APPEND		0x00000020 /* [I] File is append-only */
 #define STATX_ATTR_NODUMP		0x00000040 /* [I] File is not to be dumped */
 #define STATX_ATTR_ENCRYPTED		0x00000800 /* [I] File requires key to decrypt in fs */
-
 #define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
-#endif /* STATX_TYPE */
+#define STATX_ATTR_MOUNT_ROOT		0x00002000 /* Root of a mount */
+#define STATX_ATTR_VERITY		0x00100000 /* [I] Verity protected file */
+#define STATX_ATTR_DAX			0x00200000 /* File is currently in DAX state */
+#define STATX_ATTR_WRITE_ATOMIC		0x00400000 /* File supports atomic write operations */
 
 static inline
 int xfstests_statx(int dfd, const char *filename, unsigned flags,
-- 
2.43.0

Re: [PATCH 1/5] statx.h: update to latest kernel UAPI

[Darrick J. Wong]

On Tue, Aug 13, 2024 at 09:35:00AM +0200, Christoph Hellwig wrote:
> Update the localy provided statx definition to the latest kernel UAPI,
> and use it unconditionally instead only if no kernel version is provided.
> 
> This allows using more recent additions than provided in the system
> headers.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  src/statx.h | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/src/statx.h b/src/statx.h
> index 3f239d791..ab29fe22d 100644
> --- a/src/statx.h
> +++ b/src/statx.h
> @@ -28,8 +28,6 @@
>  # endif
>  #endif
>  
> -#ifndef STATX_TYPE
> -
>  /*
>   * Timestamp structure for the timestamps in struct statx.
>   *
> @@ -44,6 +42,7 @@
>   *
>   * __reserved is held in case we need a yet finer resolution.
>   */
> +#define statx_timestamp statx_timestamp_fstests

Might want to put these #defines at the top with a comment so that
future people copy-pastaing too fast (i.e. me) don't obliterate them
accidentally.

/*
 * Use a fstests-specific name for these structures so we can always
 * find the latest version of the abi.
 */
#define statx_timestamp statx_timestamp_fstests
#define statx statx_fstests

[all the statx.h stuff here]

Otherwise looks fine to me.

--D

>  struct statx_timestamp {
>  	__s64	tv_sec;
>  	__s32	tv_nsec;
> @@ -87,6 +86,7 @@ struct statx_timestamp {
>   * will have values installed for compatibility purposes so that stat() and
>   * co. can be emulated in userspace.
>   */
> +#define statx statx_fstests
>  struct statx {
>  	/* 0x00 */
>  	__u32	stx_mask;	/* What results were written [uncond] */
> @@ -102,7 +102,8 @@ struct statx {
>  	__u64	stx_ino;	/* Inode number */
>  	__u64	stx_size;	/* File size */
>  	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
> -	__u64	__spare1[1];
> +	__u64	stx_attributes_mask; /* Mask to show what's supported in stx_attributes */
> +
>  	/* 0x40 */
>  	struct statx_timestamp	stx_atime;	/* Last access time */
>  	struct statx_timestamp	stx_btime;	/* File creation time */
> @@ -114,7 +115,18 @@ struct statx {
>  	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
>  	__u32	stx_dev_minor;
>  	/* 0x90 */
> -	__u64	__spare2[14];	/* Spare space for future expansion */
> +	__u64	stx_mnt_id;
> +	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
> +	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
> +	/* 0xa0 */
> +	__u64	stx_subvol;	/* Subvolume identifier */
> +	__u32	stx_atomic_write_unit_min;	/* Min atomic write unit in bytes */
> +	__u32	stx_atomic_write_unit_max;	/* Max atomic write unit in bytes */
> +	/* 0xb0 */
> +	__u32   stx_atomic_write_segments_max;	/* Max atomic write segment count */
> +	__u32   __spare1[1];
> +	/* 0xb8 */
> +	__u64	__spare3[9];	/* Spare space for future expansion */
>  	/* 0x100 */
>  };
>  
> @@ -139,6 +151,12 @@ struct statx {
>  #define STATX_BLOCKS		0x00000400U	/* Want/got stx_blocks */
>  #define STATX_BASIC_STATS	0x000007ffU	/* The stuff in the normal stat struct */
>  #define STATX_BTIME		0x00000800U	/* Want/got stx_btime */
> +#define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
> +#define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
> +#define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
> +#define STATX_SUBVOL		0x00008000U	/* Want/got stx_subvol */
> +#define STATX_WRITE_ATOMIC	0x00010000U	/* Want/got atomic_write_* fields */
> +
>  #define STATX_ALL		0x00000fffU	/* All currently supported flags */
>  
>  /*
> @@ -157,9 +175,11 @@ struct statx {
>  #define STATX_ATTR_APPEND		0x00000020 /* [I] File is append-only */
>  #define STATX_ATTR_NODUMP		0x00000040 /* [I] File is not to be dumped */
>  #define STATX_ATTR_ENCRYPTED		0x00000800 /* [I] File requires key to decrypt in fs */
> -
>  #define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
> -#endif /* STATX_TYPE */
> +#define STATX_ATTR_MOUNT_ROOT		0x00002000 /* Root of a mount */
> +#define STATX_ATTR_VERITY		0x00100000 /* [I] Verity protected file */
> +#define STATX_ATTR_DAX			0x00200000 /* File is currently in DAX state */
> +#define STATX_ATTR_WRITE_ATOMIC		0x00400000 /* File supports atomic write operations */
>  
>  static inline
>  int xfstests_statx(int dfd, const char *filename, unsigned flags,
> -- 
> 2.43.0
> 
> 

Re: [PATCH 1/5] statx.h: update to latest kernel UAPI

[Christoph Hellwig]

On Tue, Aug 13, 2024 at 07:37:15AM -0700, Darrick J. Wong wrote:
> Might want to put these #defines at the top with a comment so that
> future people copy-pastaing too fast (i.e. me) don't obliterate them
> accidentally.

Sure.

> /*
>  * Use a fstests-specific name for these structures so we can always
>  * find the latest version of the abi.
>  */
> #define statx_timestamp statx_timestamp_fstests
> #define statx statx_fstests

The comment might need a bit twiddling as we're not really using
different name we're just avoiding the conflict, but I'll see if
I can come up with a coherent enough explanation.

[PATCH 2/5] add a new min_dio_alignment helper

[Christoph Hellwig]

Add a new C program to find the minimum direct I/O alignment.  This
uses the statx stx_dio_offset_align field if provided, then falls
back to the BLKSSZGET ioctl for block backed file systems and finally
the page size.  It is intended as a more capable replacement for the
_min_dio_alignment bash helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 src/Makefile            |  2 +-
 src/min_dio_alignment.c | 66 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100644 src/min_dio_alignment.c

diff --git a/src/Makefile b/src/Makefile
index 559209be9..b3da59a0e 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -34,7 +34,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
 	fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
 	detached_mounts_propagation ext4_resize t_readdir_3 splice2pipe \
-	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault
+	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault min_dio_alignment
 
 EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
 	      btrfs_crc32c_forged_name.py popdir.pl popattr.py \
diff --git a/src/min_dio_alignment.c b/src/min_dio_alignment.c
new file mode 100644
index 000000000..c3345bfb2
--- /dev/null
+++ b/src/min_dio_alignment.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2024 Christoph Hellwig
+ */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/mount.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include "statx.h"
+
+static int min_dio_alignmenent(const char *mntpnt, const char *devname)
+{
+	struct statx stx = { };
+	struct stat st;
+	int fd;
+
+	/*
+	 * If the file system supports STATX_DIOALIGN, use the dio_offset_align
+	 * member, as that reports exactly the information that we are asking
+	 * for.
+	 *
+	 * STATX_DIOALIGN is only reported on regular files, so use O_TMPFILE
+	 * to create one without leaving a trace.
+	 */
+	fd = open(mntpnt, O_TMPFILE | O_RDWR | O_EXCL, 0600);
+	if (fd >= 0 &&
+	    xfstests_statx(fd, "", AT_EMPTY_PATH, STATX_DIOALIGN, &stx) == 0 &&
+	    (stx.stx_mask & STATX_DIOALIGN))
+		return stx.stx_dio_offset_align;
+
+	/*
+	 * If we are on a block device and no explicit aligned is reported, use
+	 * the logical block size as a guestimate.
+	 */
+	if (stat(devname, &st) == 0 && S_ISBLK(st.st_mode)) {
+		int dev_fd = open(devname, O_RDONLY);
+		int logical_block_size;
+
+		if (dev_fd > 0 &&
+		    fstat(dev_fd, &st) == 0 &&
+		    S_ISBLK(st.st_mode) &&
+		    ioctl(dev_fd, BLKSSZGET, &logical_block_size)) {
+			return logical_block_size;
+		}
+	}
+
+	/*
+	 * No support for STATX_DIOALIGN and not a block device:
+	 * default to PAGE_SIZE.
+	 */
+	return getpagesize();
+}
+
+int main(int argc, char **argv)
+{
+	if (argc != 3) {
+		fprintf(stderr, "usage: %s mountpoint devicename\n", argv[0]);
+		exit(1);
+	}
+
+	printf("%d\n", min_dio_alignmenent(argv[1], argv[2]));
+	exit(0);
+}
-- 
2.43.0

Re: [PATCH 2/5] add a new min_dio_alignment helper

[Darrick J. Wong]

On Tue, Aug 13, 2024 at 09:35:01AM +0200, Christoph Hellwig wrote:
> Add a new C program to find the minimum direct I/O alignment.  This
> uses the statx stx_dio_offset_align field if provided, then falls
> back to the BLKSSZGET ioctl for block backed file systems and finally
> the page size.  It is intended as a more capable replacement for the
> _min_dio_alignment bash helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  src/Makefile            |  2 +-
>  src/min_dio_alignment.c | 66 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+), 1 deletion(-)
>  create mode 100644 src/min_dio_alignment.c
> 
> diff --git a/src/Makefile b/src/Makefile
> index 559209be9..b3da59a0e 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -34,7 +34,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
>  	attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
>  	fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
>  	detached_mounts_propagation ext4_resize t_readdir_3 splice2pipe \
> -	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault
> +	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault min_dio_alignment
>  
>  EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
>  	      btrfs_crc32c_forged_name.py popdir.pl popattr.py \
> diff --git a/src/min_dio_alignment.c b/src/min_dio_alignment.c
> new file mode 100644
> index 000000000..c3345bfb2
> --- /dev/null
> +++ b/src/min_dio_alignment.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2024 Christoph Hellwig
> + */
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/mount.h>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include "statx.h"
> +
> +static int min_dio_alignmenent(const char *mntpnt, const char *devname)

              min_dio_alignment

> +{
> +	struct statx stx = { };
> +	struct stat st;
> +	int fd;
> +
> +	/*
> +	 * If the file system supports STATX_DIOALIGN, use the dio_offset_align
> +	 * member, as that reports exactly the information that we are asking
> +	 * for.
> +	 *
> +	 * STATX_DIOALIGN is only reported on regular files, so use O_TMPFILE
> +	 * to create one without leaving a trace.
> +	 */
> +	fd = open(mntpnt, O_TMPFILE | O_RDWR | O_EXCL, 0600);
> +	if (fd >= 0 &&
> +	    xfstests_statx(fd, "", AT_EMPTY_PATH, STATX_DIOALIGN, &stx) == 0 &&
> +	    (stx.stx_mask & STATX_DIOALIGN))
> +		return stx.stx_dio_offset_align;
> +
> +	/*
> +	 * If we are on a block device and no explicit aligned is reported, use
> +	 * the logical block size as a guestimate.
> +	 */
> +	if (stat(devname, &st) == 0 && S_ISBLK(st.st_mode)) {
> +		int dev_fd = open(devname, O_RDONLY);
> +		int logical_block_size;
> +
> +		if (dev_fd > 0 &&
> +		    fstat(dev_fd, &st) == 0 &&
> +		    S_ISBLK(st.st_mode) &&
> +		    ioctl(dev_fd, BLKSSZGET, &logical_block_size)) {
> +			return logical_block_size;
> +		}
> +	}
> +
> +	/*
> +	 * No support for STATX_DIOALIGN and not a block device:
> +	 * default to PAGE_SIZE.

Should we try DIOINFO here as a second to last gasp?

--D

> +	 */
> +	return getpagesize();
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	if (argc != 3) {
> +		fprintf(stderr, "usage: %s mountpoint devicename\n", argv[0]);
> +		exit(1);
> +	}
> +
> +	printf("%d\n", min_dio_alignmenent(argv[1], argv[2]));
> +	exit(0);
> +}
> -- 
> 2.43.0
> 
> 

Re: [PATCH 2/5] add a new min_dio_alignment helper

[Darrick J. Wong]

On Tue, Aug 13, 2024 at 07:40:04AM -0700, Darrick J. Wong wrote:
> On Tue, Aug 13, 2024 at 09:35:01AM +0200, Christoph Hellwig wrote:
> > Add a new C program to find the minimum direct I/O alignment.  This
> > uses the statx stx_dio_offset_align field if provided, then falls
> > back to the BLKSSZGET ioctl for block backed file systems and finally
> > the page size.  It is intended as a more capable replacement for the
> > _min_dio_alignment bash helper.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  src/Makefile            |  2 +-
> >  src/min_dio_alignment.c | 66 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 67 insertions(+), 1 deletion(-)
> >  create mode 100644 src/min_dio_alignment.c
> > 
> > diff --git a/src/Makefile b/src/Makefile
> > index 559209be9..b3da59a0e 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -34,7 +34,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> >  	attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
> >  	fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
> >  	detached_mounts_propagation ext4_resize t_readdir_3 splice2pipe \
> > -	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault
> > +	uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault min_dio_alignment

Also this program ought to be listed in gitignore.

--D

> >  
> >  EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
> >  	      btrfs_crc32c_forged_name.py popdir.pl popattr.py \
> > diff --git a/src/min_dio_alignment.c b/src/min_dio_alignment.c
> > new file mode 100644
> > index 000000000..c3345bfb2
> > --- /dev/null
> > +++ b/src/min_dio_alignment.c
> > @@ -0,0 +1,66 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2024 Christoph Hellwig
> > + */
> > +#include <fcntl.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <sys/mount.h>
> > +#include <sys/ioctl.h>
> > +#include <sys/stat.h>
> > +#include "statx.h"
> > +
> > +static int min_dio_alignmenent(const char *mntpnt, const char *devname)
> 
>               min_dio_alignment
> 
> > +{
> > +	struct statx stx = { };
> > +	struct stat st;
> > +	int fd;
> > +
> > +	/*
> > +	 * If the file system supports STATX_DIOALIGN, use the dio_offset_align
> > +	 * member, as that reports exactly the information that we are asking
> > +	 * for.
> > +	 *
> > +	 * STATX_DIOALIGN is only reported on regular files, so use O_TMPFILE
> > +	 * to create one without leaving a trace.
> > +	 */
> > +	fd = open(mntpnt, O_TMPFILE | O_RDWR | O_EXCL, 0600);
> > +	if (fd >= 0 &&
> > +	    xfstests_statx(fd, "", AT_EMPTY_PATH, STATX_DIOALIGN, &stx) == 0 &&
> > +	    (stx.stx_mask & STATX_DIOALIGN))
> > +		return stx.stx_dio_offset_align;
> > +
> > +	/*
> > +	 * If we are on a block device and no explicit aligned is reported, use
> > +	 * the logical block size as a guestimate.
> > +	 */
> > +	if (stat(devname, &st) == 0 && S_ISBLK(st.st_mode)) {
> > +		int dev_fd = open(devname, O_RDONLY);
> > +		int logical_block_size;
> > +
> > +		if (dev_fd > 0 &&
> > +		    fstat(dev_fd, &st) == 0 &&
> > +		    S_ISBLK(st.st_mode) &&
> > +		    ioctl(dev_fd, BLKSSZGET, &logical_block_size)) {
> > +			return logical_block_size;
> > +		}
> > +	}
> > +
> > +	/*
> > +	 * No support for STATX_DIOALIGN and not a block device:
> > +	 * default to PAGE_SIZE.
> 
> Should we try DIOINFO here as a second to last gasp?
> 
> --D
> 
> > +	 */
> > +	return getpagesize();
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +	if (argc != 3) {
> > +		fprintf(stderr, "usage: %s mountpoint devicename\n", argv[0]);
> > +		exit(1);
> > +	}
> > +
> > +	printf("%d\n", min_dio_alignmenent(argv[1], argv[2]));
> > +	exit(0);
> > +}
> > -- 
> > 2.43.0
> > 
> > 
> 

Re: [PATCH 2/5] add a new min_dio_alignment helper

[Christoph Hellwig]

On Tue, Aug 13, 2024 at 07:40:04AM -0700, Darrick J. Wong wrote:
> > +	/*
> > +	 * No support for STATX_DIOALIGN and not a block device:
> > +	 * default to PAGE_SIZE.
> 
> Should we try DIOINFO here as a second to last gasp?

Using it is a good idea, and it should probably be second choice.
But I'll do that as a separate patch at the end end.

Re: [PATCH 2/5] add a new min_dio_alignment helper

[Christoph Hellwig]

On Tue, Aug 13, 2024 at 07:40:04AM -0700, Darrick J. Wong wrote:
> > +	/*
> > +	 * No support for STATX_DIOALIGN and not a block device:
> > +	 * default to PAGE_SIZE.
> 
> Should we try DIOINFO here as a second to last gasp?

I implemented this last night, but have second thoughts now:

DIOINFO is only implemented for XFS, which both implemented
STATX_DIOALIGN from the start, and where the block size hack works
and is equivalent to the DIOINFO output.  So I don't think this
buys us anything but costs an extra syscall per invocation.

So I plan to drop this again before submission.

[PATCH 3/5] xfs/424: don't use _min_dio_alignment

[Christoph Hellwig]

xfs/424 tests xfs_db and not the kernel xfs code, and really wants
the device sector size and not the minimum direct I/O alignment.

Switch to a direct call of the blockdev utility.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 tests/xfs/424 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/xfs/424 b/tests/xfs/424
index 71d48bec1..6078d3489 100755
--- a/tests/xfs/424
+++ b/tests/xfs/424
@@ -50,7 +50,7 @@ echo "Silence is golden."
 # NOTE: skip attr3, bmapbta, bmapbtd, dir3, dqblk, inodata, symlink
 # rtbitmap, rtsummary, log
 #
-sec_sz=`_min_dio_alignment $SCRATCH_DEV`
+sec_sz=`blockdev --getss $SCRATCH_DEV`
 while [ $sec_sz -le 4096 ]; do
 	sector_sizes="$sector_sizes $sec_sz"
 	sec_sz=$((sec_sz * 2))
-- 
2.43.0

Re: [PATCH 3/5] xfs/424: don't use _min_dio_alignment

[Darrick J. Wong]

On Tue, Aug 13, 2024 at 09:35:02AM +0200, Christoph Hellwig wrote:
> xfs/424 tests xfs_db and not the kernel xfs code, and really wants
> the device sector size and not the minimum direct I/O alignment.
> 
> Switch to a direct call of the blockdev utility.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  tests/xfs/424 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/424 b/tests/xfs/424
> index 71d48bec1..6078d3489 100755
> --- a/tests/xfs/424
> +++ b/tests/xfs/424
> @@ -50,7 +50,7 @@ echo "Silence is golden."
>  # NOTE: skip attr3, bmapbta, bmapbtd, dir3, dqblk, inodata, symlink
>  # rtbitmap, rtsummary, log
>  #
> -sec_sz=`_min_dio_alignment $SCRATCH_DEV`
> +sec_sz=`blockdev --getss $SCRATCH_DEV`
>  while [ $sec_sz -le 4096 ]; do
>  	sector_sizes="$sector_sizes $sec_sz"
>  	sec_sz=$((sec_sz * 2))
> -- 
> 2.43.0
> 
> 

[PATCH 4/5] generic: don't use _min_dio_alignment without a device argument

[Christoph Hellwig]

Replace calls to _min_dio_alignment that do not provide a device to
check with calls to the feature utility to query the page size, as that
is what these calls actually do.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 tests/generic/521 | 2 +-
 tests/generic/617 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/generic/521 b/tests/generic/521
index 24eab8342..5f3aac570 100755
--- a/tests/generic/521
+++ b/tests/generic/521
@@ -22,7 +22,7 @@ nr_ops=$((1000000 * TIME_FACTOR))
 op_sz=$((128000 * LOAD_FACTOR))
 file_sz=$((600000 * LOAD_FACTOR))
 fsx_file=$TEST_DIR/fsx.$seq
-min_dio_sz=$(_min_dio_alignment)
+min_dio_sz=$($here/src/feature -s)
 
 fsx_args=(-q)
 fsx_args+=(-N $nr_ops)
diff --git a/tests/generic/617 b/tests/generic/617
index eb50a2da3..297d75538 100755
--- a/tests/generic/617
+++ b/tests/generic/617
@@ -24,7 +24,7 @@ nr_ops=$((20000 * TIME_FACTOR))
 op_sz=$((128000 * LOAD_FACTOR))
 file_sz=$((600000 * LOAD_FACTOR))
 fsx_file=$TEST_DIR/fsx.$seq
-min_dio_sz=$(_min_dio_alignment)
+min_dio_sz=$($here/src/feature -s)
 
 fsx_args=(-S 0)
 fsx_args+=(-U)
-- 
2.43.0

Re: [PATCH 4/5] generic: don't use _min_dio_alignment without a device argument

[Darrick J. Wong]

On Tue, Aug 13, 2024 at 09:35:03AM +0200, Christoph Hellwig wrote:
> Replace calls to _min_dio_alignment that do not provide a device to
> check with calls to the feature utility to query the page size, as that
> is what these calls actually do.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  tests/generic/521 | 2 +-
>  tests/generic/617 | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/521 b/tests/generic/521
> index 24eab8342..5f3aac570 100755
> --- a/tests/generic/521
> +++ b/tests/generic/521
> @@ -22,7 +22,7 @@ nr_ops=$((1000000 * TIME_FACTOR))
>  op_sz=$((128000 * LOAD_FACTOR))
>  file_sz=$((600000 * LOAD_FACTOR))
>  fsx_file=$TEST_DIR/fsx.$seq
> -min_dio_sz=$(_min_dio_alignment)
> +min_dio_sz=$($here/src/feature -s)

Or maybe _get_page_size() ?

Don't really care either way so:
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  
>  fsx_args=(-q)
>  fsx_args+=(-N $nr_ops)
> diff --git a/tests/generic/617 b/tests/generic/617
> index eb50a2da3..297d75538 100755
> --- a/tests/generic/617
> +++ b/tests/generic/617
> @@ -24,7 +24,7 @@ nr_ops=$((20000 * TIME_FACTOR))
>  op_sz=$((128000 * LOAD_FACTOR))
>  file_sz=$((600000 * LOAD_FACTOR))
>  fsx_file=$TEST_DIR/fsx.$seq
> -min_dio_sz=$(_min_dio_alignment)
> +min_dio_sz=$($here/src/feature -s)
>  
>  fsx_args=(-S 0)
>  fsx_args+=(-U)
> -- 
> 2.43.0
> 
> 

Re: [PATCH 4/5] generic: don't use _min_dio_alignment without a device argument

[Christoph Hellwig]

On Tue, Aug 13, 2024 at 07:43:31AM -0700, Darrick J. Wong wrote:
> > +min_dio_sz=$($here/src/feature -s)
> 
> Or maybe _get_page_size() ?

Sure.  Or maybe actually query the I/O size, but I need feedback
from the authors that they actually intended that..

[PATCH 5/5] replace _min_dio_alignment with calls to src/min_dio_alignment

[Christoph Hellwig]

Use the min_dio_alignment C tool to check the minimum alignment.
This allows using the values obtained from statx instead of just guessing
based on the sector size and page size.

For tests using the scratch device this sometimes required moving code
around a bit to ensure the scratch device is actually mounted before
querying the alignment.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 common/rc         | 15 ---------------
 tests/generic/091 |  2 +-
 tests/generic/095 |  7 ++++---
 tests/generic/114 |  2 +-
 tests/generic/240 |  2 +-
 tests/generic/252 |  2 +-
 tests/generic/263 |  2 +-
 tests/generic/329 |  2 +-
 tests/generic/330 |  2 +-
 tests/generic/450 |  2 +-
 tests/generic/465 |  2 +-
 tests/generic/538 |  2 +-
 tests/generic/551 |  2 +-
 tests/generic/591 |  2 +-
 tests/xfs/194     | 11 ++++++-----
 tests/xfs/201     | 47 ++++++++++++++++++++++++-----------------------
 tests/xfs/237     |  2 +-
 tests/xfs/239     |  2 +-
 tests/xfs/556     |  2 +-
 19 files changed, 49 insertions(+), 61 deletions(-)

diff --git a/common/rc b/common/rc
index afc33bbc2..449ac9fbf 100644
--- a/common/rc
+++ b/common/rc
@@ -4296,21 +4296,6 @@ _scale_fsstress_args()
     printf '%s\n' "$args"
 }
 
-#
-# Return the logical block size if running on a block device,
-# else substitute the page size.
-#
-_min_dio_alignment()
-{
-    local dev=$1
-
-    if [ -b "$dev" ]; then
-        blockdev --getss $dev
-    else
-        $here/src/feature -s
-    fi
-}
-
 run_check()
 {
 	echo "# $@" >> $seqres.full 2>&1
diff --git a/tests/generic/091 b/tests/generic/091
index 8f7c13da8..5cdf04890 100755
--- a/tests/generic/091
+++ b/tests/generic/091
@@ -16,7 +16,7 @@ _require_test
 _require_odirect
 
 psize=`$here/src/feature -s`
-bsize=`_min_dio_alignment $TEST_DEV`
+bsize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 # fsx usage:
 # 
diff --git a/tests/generic/095 b/tests/generic/095
index 7a0adf880..47e3b1e61 100755
--- a/tests/generic/095
+++ b/tests/generic/095
@@ -16,12 +16,15 @@ _require_scratch
 _require_odirect
 _require_aio
 
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+
 iodepth=$((16 * LOAD_FACTOR))
 iodepth_batch=$((8 * LOAD_FACTOR))
 numjobs=$((5 * LOAD_FACTOR))
 fio_config=$tmp.fio
 fio_out=$tmp.fio.out
-blksz=$(_min_dio_alignment $SCRATCH_DEV)
+blksz=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
 cat >$fio_config <<EOF
 [global]
 bs=8k
@@ -82,8 +85,6 @@ EOF
 echo 'bs=$pagesize' >> $fio_config
 
 _require_fio $fio_config
-_scratch_mkfs >>$seqres.full 2>&1
-_scratch_mount
 
 # There's a known EIO failure to report collisions between directio and buffered
 # writes to userspace, refer to upstream linux 5a9d929d6e13. So ignore EIO error
diff --git a/tests/generic/114 b/tests/generic/114
index 068ed9e26..e0696ad92 100755
--- a/tests/generic/114
+++ b/tests/generic/114
@@ -25,7 +25,7 @@ _require_sparse_files
 _require_aiodio aio-dio-eof-race
 
 # Test does 512 byte DIO, so make sure that'll work
-logical_block_size=`_min_dio_alignment $TEST_DEV`
+logical_block_size=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 if [ "$logical_block_size" -gt "512" ]; then
 	_notrun "device block size: $logical_block_size greater than 512"
diff --git a/tests/generic/240 b/tests/generic/240
index a333873ec..66a2ff74c 100755
--- a/tests/generic/240
+++ b/tests/generic/240
@@ -29,7 +29,7 @@ echo "Silence is golden."
 
 rm -f $TEST_DIR/aiodio_sparse
 
-logical_block_size=`_min_dio_alignment $TEST_DEV`
+logical_block_size=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 fs_block_size=`_get_block_size $TEST_DIR`
 file_size=$((8 * $fs_block_size))
 
diff --git a/tests/generic/252 b/tests/generic/252
index 39fa5531f..3ee2b0a67 100755
--- a/tests/generic/252
+++ b/tests/generic/252
@@ -49,7 +49,7 @@ nr=640
 bufnr=128
 filesize=$((blksz * nr))
 bufsize=$((blksz * bufnr))
-alignment=`_min_dio_alignment $TEST_DEV`
+alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 5 / 4))
 
diff --git a/tests/generic/263 b/tests/generic/263
index 62eaec1d7..91cfbe525 100755
--- a/tests/generic/263
+++ b/tests/generic/263
@@ -16,7 +16,7 @@ _require_test
 _require_odirect
 
 psize=`$here/src/feature -s`
-bsize=`_min_dio_alignment $TEST_DEV`
+bsize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 run_fsx -N 10000  -o 8192   -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
 run_fsx -N 10000  -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
diff --git a/tests/generic/329 b/tests/generic/329
index e29a8ca4c..ab37e047f 100755
--- a/tests/generic/329
+++ b/tests/generic/329
@@ -40,7 +40,7 @@ nr=640
 bufnr=128
 filesize=$((blksz * nr))
 bufsize=$((blksz * bufnr))
-alignment=`_min_dio_alignment $TEST_DEV`
+alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 3 * 5 / 4))
 
diff --git a/tests/generic/330 b/tests/generic/330
index 83e1459fa..4fa81f991 100755
--- a/tests/generic/330
+++ b/tests/generic/330
@@ -36,7 +36,7 @@ nr=640
 bufnr=128
 filesize=$((blksz * nr))
 bufsize=$((blksz * bufnr))
-alignment=`_min_dio_alignment $TEST_DEV`
+alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 3 * 5 / 4))
 
diff --git a/tests/generic/450 b/tests/generic/450
index 96e559da6..689f1051e 100755
--- a/tests/generic/450
+++ b/tests/generic/450
@@ -31,7 +31,7 @@ _require_test
 _require_odirect
 
 tfile=$TEST_DIR/testfile_${seq}
-ssize=`_min_dio_alignment $TEST_DEV`
+ssize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 bsize=`_get_block_size $TEST_DIR`
 
 # let's focus on the specific bug that only happens when $ssize <= $bsize
diff --git a/tests/generic/465 b/tests/generic/465
index eba3629ab..f8c4ea967 100755
--- a/tests/generic/465
+++ b/tests/generic/465
@@ -26,7 +26,7 @@ _require_aiodio aio-dio-append-write-read-race
 _require_test_program "feature"
 
 testfile=$TEST_DIR/$seq.$$
-min_dio_align=`_min_dio_alignment $TEST_DEV`
+min_dio_align=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 page_size=`$here/src/feature -s`
 
 echo "non-aio dio test"
diff --git a/tests/generic/538 b/tests/generic/538
index d6933cbb9..b9cf05de1 100755
--- a/tests/generic/538
+++ b/tests/generic/538
@@ -28,7 +28,7 @@ _require_test
 _require_aiodio aio-dio-write-verify
 
 localfile=$TEST_DIR/${seq}-aio-dio-write-verify-testfile
-diosize=`_min_dio_alignment $TEST_DEV`
+diosize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 blocksize=`_get_block_size $TEST_DIR`
 bufsize=$((blocksize * 2))
 truncsize=$((bufsize+diosize))
diff --git a/tests/generic/551 b/tests/generic/551
index f2907ac23..4a7f0a638 100755
--- a/tests/generic/551
+++ b/tests/generic/551
@@ -19,7 +19,7 @@ _scratch_mkfs > $seqres.full 2>&1
 _scratch_mount
 
 localfile=$SCRATCH_MNT/testfile
-diosize=`_min_dio_alignment $SCRATCH_DEV`
+diosize=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
 
 # The maximum write size and offset are both 32k diosize. So the maximum
 # file size will be (32 * 2)k
diff --git a/tests/generic/591 b/tests/generic/591
index c22dc701b..f2fcd6162 100755
--- a/tests/generic/591
+++ b/tests/generic/591
@@ -22,7 +22,7 @@ _require_test
 _require_odirect
 _require_test_program "splice-test"
 
-diosize=`_min_dio_alignment $TEST_DEV`
+diosize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 $here/src/splice-test -s $diosize -r $TEST_DIR/a
 $here/src/splice-test -rd $TEST_DIR/a
diff --git a/tests/xfs/194 b/tests/xfs/194
index 9abd2c321..1f83d534c 100755
--- a/tests/xfs/194
+++ b/tests/xfs/194
@@ -43,11 +43,6 @@ _scratch_mkfs_xfs >/dev/null 2>&1
 # For this test we use block size = 1/8 page size
 pgsize=`$here/src/feature -s`
 blksize=`expr $pgsize / 8`
-secsize=`_min_dio_alignment $SCRATCH_DEV`
-
-if [ $secsize -gt $blksize ];then
-	_notrun "sector size($secsize) too large for platform page size($pgsize)"
-fi
 
 # Filter out file mountpoint and physical location info
 # Input:
@@ -84,6 +79,12 @@ unset XFS_MKFS_OPTIONS
 # we need 512 byte block size, so crc's are turned off
 _scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1
 _scratch_mount
+
+secsize=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
+if [ $secsize -gt $blksize ];then
+	_notrun "sector size($secsize) too large for platform page size($pgsize)"
+fi
+
 test "$(_get_block_size $SCRATCH_MNT)" = $blksize || \
 	_notrun "Could not get $blksize-byte blocks"
 
diff --git a/tests/xfs/201 b/tests/xfs/201
index a0d2c9150..60cc84ed2 100755
--- a/tests/xfs/201
+++ b/tests/xfs/201
@@ -24,10 +24,9 @@ _cleanup()
 
 file=$SCRATCH_MNT/f
 
-min_align=`_min_dio_alignment $SCRATCH_DEV`
-
 do_pwrite()
 {
+	min_align=$3
 	offset=`expr $1 \* $min_align`
 	end=`expr $2 \* $min_align`
 	length=`expr $end - $offset`
@@ -40,28 +39,30 @@ _require_scratch
 _scratch_mkfs_xfs >/dev/null 2>&1
 _scratch_mount
 
+min_align=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
+
 # Create a fragmented file
-do_pwrite 30792 31039
-do_pwrite 30320 30791
-do_pwrite 29688 30319
-do_pwrite 29536 29687
-do_pwrite 27216 29535
-do_pwrite 24368 27215
-do_pwrite 21616 24367
-do_pwrite 20608 21615
-do_pwrite 19680 20607
-do_pwrite 19232 19679
-do_pwrite 17840 19231
-do_pwrite 16928 17839
-do_pwrite 15168 16927
-do_pwrite 14048 15167
-do_pwrite 12152 14047
-do_pwrite 11344 12151
-do_pwrite 8792 11343
-do_pwrite 6456 8791
-do_pwrite 5000 6455
-do_pwrite 1728 4999
-do_pwrite 0 1727
+do_pwrite 30792 31039 $min_align
+do_pwrite 30320 30791 $min_align
+do_pwrite 29688 30319 $min_align
+do_pwrite 29536 29687 $min_align
+do_pwrite 27216 29535 $min_align
+do_pwrite 24368 27215 $min_align
+do_pwrite 21616 24367 $min_align
+do_pwrite 20608 21615 $min_align
+do_pwrite 19680 20607 $min_align
+do_pwrite 19232 19679 $min_align
+do_pwrite 17840 19231 $min_align
+do_pwrite 16928 17839 $min_align
+do_pwrite 15168 16927 $min_align
+do_pwrite 14048 15167 $min_align
+do_pwrite 12152 14047 $min_align
+do_pwrite 11344 12151 $min_align
+do_pwrite 8792 11343 $min_align
+do_pwrite 6456 8791 $min_align
+do_pwrite 5000 6455 $min_align
+do_pwrite 1728 4999 $min_align
+do_pwrite 0 1727 $min_align
 
 sync
 sync
diff --git a/tests/xfs/237 b/tests/xfs/237
index 5f264ff44..194cd0459 100755
--- a/tests/xfs/237
+++ b/tests/xfs/237
@@ -41,7 +41,7 @@ nr=640
 bufnr=128
 filesize=$((blksz * nr))
 bufsize=$((blksz * bufnr))
-alignment=`_min_dio_alignment $TEST_DEV`
+alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 3 * 5 / 4))
 _require_congruent_file_oplen $SCRATCH_MNT $blksz
diff --git a/tests/xfs/239 b/tests/xfs/239
index 277bd4548..bfe722c0a 100755
--- a/tests/xfs/239
+++ b/tests/xfs/239
@@ -40,7 +40,7 @@ filesize=$((blksz * nr))
 bufsize=$((blksz * bufnr))
 filesize=$filesize
 bufsize=$bufsize
-alignment=`_min_dio_alignment $TEST_DEV`
+alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
 
 _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 3 * 5 / 4))
 
diff --git a/tests/xfs/556 b/tests/xfs/556
index 5a2e7fd6d..79e03caf4 100755
--- a/tests/xfs/556
+++ b/tests/xfs/556
@@ -82,7 +82,7 @@ ENDL
 
 # All sector numbers that we feed to the kernel must be in units of 512b, but
 # they also must be aligned to the device's logical block size.
-logical_block_size=$(_min_dio_alignment $SCRATCH_DEV)
+logical_block_size=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
 kernel_sectors_per_device_lba=$((logical_block_size / 512))
 
 # Mark as bad one of the device LBAs in the middle of the extent.  Target the
-- 
2.43.0

Re: [PATCH 5/5] replace _min_dio_alignment with calls to src/min_dio_alignment

[Darrick J. Wong]

On Tue, Aug 13, 2024 at 09:35:04AM +0200, Christoph Hellwig wrote:
> Use the min_dio_alignment C tool to check the minimum alignment.
> This allows using the values obtained from statx instead of just guessing
> based on the sector size and page size.
> 
> For tests using the scratch device this sometimes required moving code
> around a bit to ensure the scratch device is actually mounted before
> querying the alignment.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  common/rc         | 15 ---------------
>  tests/generic/091 |  2 +-
>  tests/generic/095 |  7 ++++---
>  tests/generic/114 |  2 +-
>  tests/generic/240 |  2 +-
>  tests/generic/252 |  2 +-
>  tests/generic/263 |  2 +-
>  tests/generic/329 |  2 +-
>  tests/generic/330 |  2 +-
>  tests/generic/450 |  2 +-
>  tests/generic/465 |  2 +-
>  tests/generic/538 |  2 +-
>  tests/generic/551 |  2 +-
>  tests/generic/591 |  2 +-
>  tests/xfs/194     | 11 ++++++-----
>  tests/xfs/201     | 47 ++++++++++++++++++++++++-----------------------
>  tests/xfs/237     |  2 +-
>  tests/xfs/239     |  2 +-
>  tests/xfs/556     |  2 +-
>  19 files changed, 49 insertions(+), 61 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index afc33bbc2..449ac9fbf 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4296,21 +4296,6 @@ _scale_fsstress_args()
>      printf '%s\n' "$args"
>  }
>  
> -#
> -# Return the logical block size if running on a block device,
> -# else substitute the page size.
> -#
> -_min_dio_alignment()
> -{
> -    local dev=$1
> -
> -    if [ -b "$dev" ]; then
> -        blockdev --getss $dev
> -    else
> -        $here/src/feature -s
> -    fi
> -}
> -
>  run_check()
>  {
>  	echo "# $@" >> $seqres.full 2>&1
> diff --git a/tests/generic/091 b/tests/generic/091
> index 8f7c13da8..5cdf04890 100755
> --- a/tests/generic/091
> +++ b/tests/generic/091
> @@ -16,7 +16,7 @@ _require_test
>  _require_odirect
>  
>  psize=`$here/src/feature -s`
> -bsize=`_min_dio_alignment $TEST_DEV`
> +bsize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  # fsx usage:
>  # 
> diff --git a/tests/generic/095 b/tests/generic/095
> index 7a0adf880..47e3b1e61 100755
> --- a/tests/generic/095
> +++ b/tests/generic/095
> @@ -16,12 +16,15 @@ _require_scratch
>  _require_odirect
>  _require_aio
>  
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
>  iodepth=$((16 * LOAD_FACTOR))
>  iodepth_batch=$((8 * LOAD_FACTOR))
>  numjobs=$((5 * LOAD_FACTOR))
>  fio_config=$tmp.fio
>  fio_out=$tmp.fio.out
> -blksz=$(_min_dio_alignment $SCRATCH_DEV)
> +blksz=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
>  cat >$fio_config <<EOF
>  [global]
>  bs=8k
> @@ -82,8 +85,6 @@ EOF
>  echo 'bs=$pagesize' >> $fio_config
>  
>  _require_fio $fio_config
> -_scratch_mkfs >>$seqres.full 2>&1
> -_scratch_mount
>  
>  # There's a known EIO failure to report collisions between directio and buffered
>  # writes to userspace, refer to upstream linux 5a9d929d6e13. So ignore EIO error
> diff --git a/tests/generic/114 b/tests/generic/114
> index 068ed9e26..e0696ad92 100755
> --- a/tests/generic/114
> +++ b/tests/generic/114
> @@ -25,7 +25,7 @@ _require_sparse_files
>  _require_aiodio aio-dio-eof-race
>  
>  # Test does 512 byte DIO, so make sure that'll work
> -logical_block_size=`_min_dio_alignment $TEST_DEV`
> +logical_block_size=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  if [ "$logical_block_size" -gt "512" ]; then
>  	_notrun "device block size: $logical_block_size greater than 512"
> diff --git a/tests/generic/240 b/tests/generic/240
> index a333873ec..66a2ff74c 100755
> --- a/tests/generic/240
> +++ b/tests/generic/240
> @@ -29,7 +29,7 @@ echo "Silence is golden."
>  
>  rm -f $TEST_DIR/aiodio_sparse
>  
> -logical_block_size=`_min_dio_alignment $TEST_DEV`
> +logical_block_size=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  fs_block_size=`_get_block_size $TEST_DIR`
>  file_size=$((8 * $fs_block_size))
>  
> diff --git a/tests/generic/252 b/tests/generic/252
> index 39fa5531f..3ee2b0a67 100755
> --- a/tests/generic/252
> +++ b/tests/generic/252
> @@ -49,7 +49,7 @@ nr=640
>  bufnr=128
>  filesize=$((blksz * nr))
>  bufsize=$((blksz * bufnr))
> -alignment=`_min_dio_alignment $TEST_DEV`
> +alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 5 / 4))
>  
> diff --git a/tests/generic/263 b/tests/generic/263
> index 62eaec1d7..91cfbe525 100755
> --- a/tests/generic/263
> +++ b/tests/generic/263
> @@ -16,7 +16,7 @@ _require_test
>  _require_odirect
>  
>  psize=`$here/src/feature -s`
> -bsize=`_min_dio_alignment $TEST_DEV`
> +bsize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  run_fsx -N 10000  -o 8192   -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
>  run_fsx -N 10000  -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
> diff --git a/tests/generic/329 b/tests/generic/329
> index e29a8ca4c..ab37e047f 100755
> --- a/tests/generic/329
> +++ b/tests/generic/329
> @@ -40,7 +40,7 @@ nr=640
>  bufnr=128
>  filesize=$((blksz * nr))
>  bufsize=$((blksz * bufnr))
> -alignment=`_min_dio_alignment $TEST_DEV`
> +alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 3 * 5 / 4))
>  
> diff --git a/tests/generic/330 b/tests/generic/330
> index 83e1459fa..4fa81f991 100755
> --- a/tests/generic/330
> +++ b/tests/generic/330
> @@ -36,7 +36,7 @@ nr=640
>  bufnr=128
>  filesize=$((blksz * nr))
>  bufsize=$((blksz * bufnr))
> -alignment=`_min_dio_alignment $TEST_DEV`
> +alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 3 * 5 / 4))
>  
> diff --git a/tests/generic/450 b/tests/generic/450
> index 96e559da6..689f1051e 100755
> --- a/tests/generic/450
> +++ b/tests/generic/450
> @@ -31,7 +31,7 @@ _require_test
>  _require_odirect
>  
>  tfile=$TEST_DIR/testfile_${seq}
> -ssize=`_min_dio_alignment $TEST_DEV`
> +ssize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  bsize=`_get_block_size $TEST_DIR`
>  
>  # let's focus on the specific bug that only happens when $ssize <= $bsize
> diff --git a/tests/generic/465 b/tests/generic/465
> index eba3629ab..f8c4ea967 100755
> --- a/tests/generic/465
> +++ b/tests/generic/465
> @@ -26,7 +26,7 @@ _require_aiodio aio-dio-append-write-read-race
>  _require_test_program "feature"
>  
>  testfile=$TEST_DIR/$seq.$$
> -min_dio_align=`_min_dio_alignment $TEST_DEV`
> +min_dio_align=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  page_size=`$here/src/feature -s`
>  
>  echo "non-aio dio test"
> diff --git a/tests/generic/538 b/tests/generic/538
> index d6933cbb9..b9cf05de1 100755
> --- a/tests/generic/538
> +++ b/tests/generic/538
> @@ -28,7 +28,7 @@ _require_test
>  _require_aiodio aio-dio-write-verify
>  
>  localfile=$TEST_DIR/${seq}-aio-dio-write-verify-testfile
> -diosize=`_min_dio_alignment $TEST_DEV`
> +diosize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  blocksize=`_get_block_size $TEST_DIR`
>  bufsize=$((blocksize * 2))
>  truncsize=$((bufsize+diosize))
> diff --git a/tests/generic/551 b/tests/generic/551
> index f2907ac23..4a7f0a638 100755
> --- a/tests/generic/551
> +++ b/tests/generic/551
> @@ -19,7 +19,7 @@ _scratch_mkfs > $seqres.full 2>&1
>  _scratch_mount
>  
>  localfile=$SCRATCH_MNT/testfile
> -diosize=`_min_dio_alignment $SCRATCH_DEV`
> +diosize=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
>  
>  # The maximum write size and offset are both 32k diosize. So the maximum
>  # file size will be (32 * 2)k
> diff --git a/tests/generic/591 b/tests/generic/591
> index c22dc701b..f2fcd6162 100755
> --- a/tests/generic/591
> +++ b/tests/generic/591
> @@ -22,7 +22,7 @@ _require_test
>  _require_odirect
>  _require_test_program "splice-test"
>  
> -diosize=`_min_dio_alignment $TEST_DEV`
> +diosize=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  $here/src/splice-test -s $diosize -r $TEST_DIR/a
>  $here/src/splice-test -rd $TEST_DIR/a
> diff --git a/tests/xfs/194 b/tests/xfs/194
> index 9abd2c321..1f83d534c 100755
> --- a/tests/xfs/194
> +++ b/tests/xfs/194
> @@ -43,11 +43,6 @@ _scratch_mkfs_xfs >/dev/null 2>&1
>  # For this test we use block size = 1/8 page size
>  pgsize=`$here/src/feature -s`
>  blksize=`expr $pgsize / 8`
> -secsize=`_min_dio_alignment $SCRATCH_DEV`
> -
> -if [ $secsize -gt $blksize ];then
> -	_notrun "sector size($secsize) too large for platform page size($pgsize)"
> -fi
>  
>  # Filter out file mountpoint and physical location info
>  # Input:
> @@ -84,6 +79,12 @@ unset XFS_MKFS_OPTIONS
>  # we need 512 byte block size, so crc's are turned off
>  _scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1
>  _scratch_mount
> +
> +secsize=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
> +if [ $secsize -gt $blksize ];then
> +	_notrun "sector size($secsize) too large for platform page size($pgsize)"
> +fi
> +
>  test "$(_get_block_size $SCRATCH_MNT)" = $blksize || \
>  	_notrun "Could not get $blksize-byte blocks"
>  
> diff --git a/tests/xfs/201 b/tests/xfs/201
> index a0d2c9150..60cc84ed2 100755
> --- a/tests/xfs/201
> +++ b/tests/xfs/201
> @@ -24,10 +24,9 @@ _cleanup()
>  
>  file=$SCRATCH_MNT/f
>  
> -min_align=`_min_dio_alignment $SCRATCH_DEV`
> -
>  do_pwrite()
>  {
> +	min_align=$3
>  	offset=`expr $1 \* $min_align`
>  	end=`expr $2 \* $min_align`
>  	length=`expr $end - $offset`
> @@ -40,28 +39,30 @@ _require_scratch
>  _scratch_mkfs_xfs >/dev/null 2>&1
>  _scratch_mount
>  
> +min_align=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
> +
>  # Create a fragmented file
> -do_pwrite 30792 31039
> -do_pwrite 30320 30791
> -do_pwrite 29688 30319
> -do_pwrite 29536 29687
> -do_pwrite 27216 29535
> -do_pwrite 24368 27215
> -do_pwrite 21616 24367
> -do_pwrite 20608 21615
> -do_pwrite 19680 20607
> -do_pwrite 19232 19679
> -do_pwrite 17840 19231
> -do_pwrite 16928 17839
> -do_pwrite 15168 16927
> -do_pwrite 14048 15167
> -do_pwrite 12152 14047
> -do_pwrite 11344 12151
> -do_pwrite 8792 11343
> -do_pwrite 6456 8791
> -do_pwrite 5000 6455
> -do_pwrite 1728 4999
> -do_pwrite 0 1727
> +do_pwrite 30792 31039 $min_align
> +do_pwrite 30320 30791 $min_align
> +do_pwrite 29688 30319 $min_align
> +do_pwrite 29536 29687 $min_align
> +do_pwrite 27216 29535 $min_align
> +do_pwrite 24368 27215 $min_align
> +do_pwrite 21616 24367 $min_align
> +do_pwrite 20608 21615 $min_align
> +do_pwrite 19680 20607 $min_align
> +do_pwrite 19232 19679 $min_align
> +do_pwrite 17840 19231 $min_align
> +do_pwrite 16928 17839 $min_align
> +do_pwrite 15168 16927 $min_align
> +do_pwrite 14048 15167 $min_align
> +do_pwrite 12152 14047 $min_align
> +do_pwrite 11344 12151 $min_align
> +do_pwrite 8792 11343 $min_align
> +do_pwrite 6456 8791 $min_align
> +do_pwrite 5000 6455 $min_align
> +do_pwrite 1728 4999 $min_align
> +do_pwrite 0 1727 $min_align
>  
>  sync
>  sync
> diff --git a/tests/xfs/237 b/tests/xfs/237
> index 5f264ff44..194cd0459 100755
> --- a/tests/xfs/237
> +++ b/tests/xfs/237
> @@ -41,7 +41,7 @@ nr=640
>  bufnr=128
>  filesize=$((blksz * nr))
>  bufsize=$((blksz * bufnr))
> -alignment=`_min_dio_alignment $TEST_DEV`
> +alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 3 * 5 / 4))
>  _require_congruent_file_oplen $SCRATCH_MNT $blksz
> diff --git a/tests/xfs/239 b/tests/xfs/239
> index 277bd4548..bfe722c0a 100755
> --- a/tests/xfs/239
> +++ b/tests/xfs/239
> @@ -40,7 +40,7 @@ filesize=$((blksz * nr))
>  bufsize=$((blksz * bufnr))
>  filesize=$filesize
>  bufsize=$bufsize
> -alignment=`_min_dio_alignment $TEST_DEV`
> +alignment=`$here/src/min_dio_alignment $TEST_DIR $TEST_DEV`
>  
>  _require_fs_space $SCRATCH_MNT $((filesize / 1024 * 3 * 5 / 4))
>  
> diff --git a/tests/xfs/556 b/tests/xfs/556
> index 5a2e7fd6d..79e03caf4 100755
> --- a/tests/xfs/556
> +++ b/tests/xfs/556
> @@ -82,7 +82,7 @@ ENDL
>  
>  # All sector numbers that we feed to the kernel must be in units of 512b, but
>  # they also must be aligned to the device's logical block size.
> -logical_block_size=$(_min_dio_alignment $SCRATCH_DEV)
> +logical_block_size=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`
>  kernel_sectors_per_device_lba=$((logical_block_size / 512))
>  
>  # Mark as bad one of the device LBAs in the middle of the extent.  Target the
> -- 
> 2.43.0
> 
> 

improve minalign handling v2

[Christoph Hellwig]

Hi all,

this series improves how xfstests detects minimum direct I/O alignment
by adding a C utility that checks the kernel provided value in statx
before falling back to the existing strategy.

Changes since v1:
 - add a comment about the namespace swizzling in statx.h
 - spell a function name correctly
 - update .gitignore

[PATCH 4/5] generic: don't use _min_dio_alignment without a device argument

[Christoph Hellwig]

Replace calls to _min_dio_alignment that do not provide a device to
check with calls to the feature utility to query the page size, as that
is what these calls actually do.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/generic/521 | 2 +-
 tests/generic/617 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/generic/521 b/tests/generic/521
index 24eab8342..5f3aac570 100755
--- a/tests/generic/521
+++ b/tests/generic/521
@@ -22,7 +22,7 @@ nr_ops=$((1000000 * TIME_FACTOR))
 op_sz=$((128000 * LOAD_FACTOR))
 file_sz=$((600000 * LOAD_FACTOR))
 fsx_file=$TEST_DIR/fsx.$seq
-min_dio_sz=$(_min_dio_alignment)
+min_dio_sz=$($here/src/feature -s)
 
 fsx_args=(-q)
 fsx_args+=(-N $nr_ops)
diff --git a/tests/generic/617 b/tests/generic/617
index eb50a2da3..297d75538 100755
--- a/tests/generic/617
+++ b/tests/generic/617
@@ -24,7 +24,7 @@ nr_ops=$((20000 * TIME_FACTOR))
 op_sz=$((128000 * LOAD_FACTOR))
 file_sz=$((600000 * LOAD_FACTOR))
 fsx_file=$TEST_DIR/fsx.$seq
-min_dio_sz=$(_min_dio_alignment)
+min_dio_sz=$($here/src/feature -s)
 
 fsx_args=(-S 0)
 fsx_args+=(-U)
-- 
2.43.0