Hi Eric, Thanks for your suggestion! From the kernel source ( https://github.com/torvalds/linux/blob/1f5abbd77e2c1787e74b7c2caffac97def78ba52/block/bdev.c#L1089 ) stat->dio_mem_align = bdev_dma_alignment(bdev) + 1; stat->dio_offset_align = bdev_logical_block_size(bdev); I think that stx_dio_mem_align=$(cat /sys/block/<dev>/queue/logical_block_size) stx_dio_offset_align=$(($(cat /sys/block/vda/queue/dma_alignment)+1)) The output on my system is like [root@localhost ~]# cat /sys/block/vda/queue/logical_block_size 512 [root@localhost ~]# echo $(($(cat /sys/block/<dev>/queue/dma_alignment)+1)) 512 I have hacked <kernel source>/samples/vfs/test-statx.c like [root@localhost ~]# git diff test-statx.c test-statx-mod.c diff --git a/test-statx.c b/test-statx-mod.c index 49c7a46..713d7b6 100644 --- a/test-statx.c +++ b/test-statx-mod.c @@ -107,6 +107,9 @@ static void dump_statx(struct statx *stx) printf("Device: %-15s", buffer); if (stx->stx_mask & STATX_INO) printf(" Inode: %-11llu", (unsigned long long) stx->stx_ino); + if (stx->stx_mask & STATX_DIOALIGN) + printf(" stx_dio_mem_align: %u", stx->stx_dio_mem_align); + printf(" stx_dio_offset_align: %u", stx->stx_dio_offset_align); if (stx->stx_mask & STATX_NLINK) printf(" Links: %-5u", stx->stx_nlink); if (stx->stx_mask & STATX_TYPE) { @@ -218,7 +221,7 @@ int main(int argc, char **argv) struct statx stx; int ret, raw = 0, atflag = AT_SYMLINK_NOFOLLOW; - unsigned int mask = STATX_BASIC_STATS | STATX_BTIME; + unsigned int mask = STATX_BASIC_STATS | STATX_BTIME | STATX_DIOALIGN; for (argv++; *argv; argv++) { if (strcmp(*argv, "-F") == 0) { and the output on my system is [root@localhost ~]# ./test-statx-mod testfile statx(testfile) = 0 results=3fff Size: 8388608 Blocks: 16384 IO Block: 4096 regular file Device: fc:02 Inode: 586 stx_dio_mem_align: 512 stx_dio_offset_align: 512 Links: 1 Access: (0644/-rw-r--r--) Uid: 0 Gid: 0 Access: 2023-01-06 04:59:46.967816162-0500 Modify: 2023-01-06 04:59:47.099816162-0500 Change: 2023-01-06 04:59:47.099816162-0500 Birth: 2023-01-06 04:59:46.967816162-0500 Attributes: 0000000000000000 (........ ........ ........ ........ ........ ..--.... ..---... .---.-..) Notice stx_dio_mem_align and stx_dio_offset_align both are 512, so I guess it shows STATX_DIOALIGN on my system works correctly. Unfortunately, I am still unable to get my patch working, even with your suggested fix, the stx_dio_mem_align and stx_dio_offset_align all printed as 0. I planned to test this functionality by hacking generic/423, which I think is a good framework for doing basic statx() validation, like ref_dio_mem_align=$(cat /sys/block/<dev>/queue/logical_block_size) ref_dio_offset_align=$(($(cat /sys/block/<dev>/queue/dma_alignment)+1)) ... check_stat $TEST_DIR/$seq-file \ stx_dio_mem_align=$ref_dio_mem_align \ stx_dio_offset_align=$ref_dio_offset_align I think this is adequate for a basic correctness test? Thanks, Boyang On Fri, Jan 6, 2023 at 4:01 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > Hi Boyang, > > On Wed, Jan 04, 2023 at 12:28:01PM +0800, bxue@xxxxxxxxxx wrote: > > From: Boyang Xue <bxue@xxxxxxxxxx> > > > > Signed-off-by: Boyang Xue <bxue@xxxxxxxxxx> > > --- > > Hi, > > > > The latest kernel has support for exposing direct I/O alignment > > information via statx() by > > > > 825cf206ed51 statx: add direct I/O alignment information > > > > I'm trying to enhance xfstests/src/stat_test.c to support this > > functionality, and the final goal is enhancing generic/423 to test it. > > > > I think I have made all the necessary change here, but it always prints > > stx_dio_mem_align and stx_dio_offset_align as 0 (should be 512) > > > > [root@localhost repo_xfstests-dev]# src/stat_test -v > > ../testfile stx_dio_offset_align=222 > > - call statx ../testfile > > - call stat ../testfile > > - compare statx and stat > > - begin time 0.000000000 > > - btime 1672804449.041990601 > > - atime 1672804449.041990601 > > - mtime 1672804449.127990601 > > - ctime 1672804449.127990601 > > - check stx_dio_offset_align=222 > > [!] stx_dio_offset_align differs, 0 != 222 > > Failed > > > > src/stat_test.c | 10 ++++++++++ > > src/statx.h | 10 ++++++++-- > > 2 files changed, 18 insertions(+), 2 deletions(-) > > > > The kernel version in test is kernel-6.2.0-0.rc1. > > > > Could you suggest how to fix it please? > > > > Thanks, > > Boyang > > Thanks for working on this! One of the challenges with testing STATX_DIOALIGN > is that it can only be usefully tested if DIO is supported, yet STATX_DIOALIGN > is itself the way to check whether DIO is supported. Another challenge is that > without something to compare the alignments against, it's hard to know whether > the correct values are being reported. (A test could try to validate the values > by attempting DIO, but the filesystem might fall back to buffered I/O for > unsupported or misaligned DIO, which would be hard to distinguish from true DIO. > Maybe something clever could be done with mincore() detect buffered I/O.) > > Is there a specific test that you're planning to add? A test that would be at > least somewhat useful would be to test that if STATX_DIOALIGN gives nonzero > stx_dio_mem_align and stx_dio_offset_align, then DIO aligned to *only* those > alignments doesn't return an error. > > Another possible test would to find a specific case where the DIO support and > alignments can be determined by another method and compared to what > STATX_DIOALIGN reports. For example, if testing XFS, the XFS_IOC_DIOINFO ioctl > can be used. Or if the test just creates a filesystem with the default options > and mounts it with the default options, it might just "know" that DIO is > supported with logical_block_size alignment. > > Anyway, as for why your patch to stat_test.c doesn't work, it's because there's > a bug in it. Try the following: > > diff --git a/src/stat_test.c b/src/stat_test.c > index cd38a54a..85d703a0 100644 > --- a/src/stat_test.c > +++ b/src/stat_test.c > @@ -570,6 +570,8 @@ static void check_field(const struct statx *stx, char *arg) > case stx_rdev_minor: > case stx_dev_major: > case stx_dev_minor: > + case stx_dio_mem_align: > + case stx_dio_offset_align: > ucheck = strtoull(val, &p, 0); > if (*p) > bad_arg("Field '%s' requires unsigned integer\n", key); > @@ -577,8 +579,6 @@ static void check_field(const struct statx *stx, char *arg) > "%s differs, %llu != %llu\n", key, uval, ucheck); > break; > > - case stx_dio_mem_align: > - case stx_dio_offset_align: > case stx_atime_tv_sec: > case stx_atime_tv_nsec: > case stx_btime_tv_sec: >
