From: Eric Biggers <ebiggers@kernel.org>
To: Boyang Xue <bxue@redhat.com>
Cc: fstests@vger.kernel.org, lczerner@redhat.com
Subject: Re: [PATCH v1] src/stat_test.c: add STATX_DIOALIGN support
Date: Fri, 6 Jan 2023 19:02:04 +0000 [thread overview]
Message-ID: <Y7hwLDl+ocQ1Xm/l@gmail.com> (raw)
In-Reply-To: <CAHLe9YamOY1UJ5Bq0w1ykmG47cztGP1U5CPPD-kiEWtEcFUKtg@mail.gmail.com>
On Fri, Jan 06, 2023 at 07:10:04PM +0800, Boyang Xue wrote:
> 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?
Not in general. The logical_block_size and dma_alignment+1 of the block device
(assuming the filesystem has a block device, and only one of them...) are only
the *typical* values for stx_dio_mem_align and stx_dio_offset_align. They are
*not* the guaranteed values, since the DIO support and alignment restrictions
are filesystem-specific. They depend on the filesystem type, mkfs options,
mount options, kernel version, block device, and other things. So if you add
the above, it will make generic/423 fail in various cases.
This problem is the whole reason that STATX_DIOALIGN was added: it provides a
way to query DIO support and alignment restrictions. If there was already
another way to *reliably* query DIO support and alignment restrictions, then
there would have been no need to add STATX_DIOALIGN.
Nonetheless, see my previous email for some ideas about tests of STATX_DIOALIGN
that might be possible.
Another idea is to test STATX_DIOALIGN on a block device node, not a regular
file. The results from block devices are more predictable; currently
STATX_DIOALIGN on a block device always reports logical_block_size and
dma_alignment+1. Though, that could still change in future kernel versions.
- Eric
prev parent reply other threads:[~2023-01-06 19:03 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-04 4:28 [PATCH v1] src/stat_test.c: add STATX_DIOALIGN support bxue
2023-01-06 7:59 ` Eric Biggers
2023-01-06 11:10 ` Boyang Xue
2023-01-06 19:02 ` Eric Biggers [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y7hwLDl+ocQ1Xm/l@gmail.com \
--to=ebiggers@kernel.org \
--cc=bxue@redhat.com \
--cc=fstests@vger.kernel.org \
--cc=lczerner@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox