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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.