From: "Pankaj Raghav (Samsung)" <kernel@pankajraghav.com>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: fstests@vger.kernel.org, linux-fsdevel@vger.kernel.org,
willy@infradead.org, p.raghav@samsung.com, da.gomez@samsung.com,
hare@suse.de, john.g.garry@oracle.com,
linux-xfs@vger.kernel.org, patches@lists.linux.dev
Subject: Re: [RFC] fstests: add mmap page boundary tests
Date: Mon, 22 Apr 2024 16:12:56 +0200 [thread overview]
Message-ID: <5xt5ultarn7rkidw66ii2kceobb3t2d4pqtfvzogmuk23zcdqh@dzvknx352vsg> (raw)
In-Reply-To: <20240415081054.1782715-1-mcgrof@kernel.org>
On Mon, Apr 15, 2024 at 01:10:54AM -0700, Luis Chamberlain wrote:
> mmap() POSIX compliance says we should zero fill data beyond a file
> size up to page boundary, and issue a SIGBUS if we go beyond. While fsx
> helps us test zero-fill sometimes, fsstress also let's us sometimes test
> for SIGBUS however that is based on a random value and its not likley we
> always test it. Dedicate a specic test for this to make testing for
> this specific situation and to easily expand on other corner cases.
>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>
> Does enough to get us to use to test this, however I'm aware of a bit
> more polishing up to do:
>
> * maybe moving mread to common as generic/574 did it first
> * sharing round_up_to_page_boundary() as well
>
> generic/574 is special, it was just testing for correctness of
> integrity if we muck with mmap() however if you don't have verity
> stuff available obviously you won't end up testing it.
>
> This generalizes mmap() zero-fill and SIGBUS corner case tests.
>
> I've tested so far only 4k and it works well there. For 16k bs on LBS
> just the SIGBUS issue exists, I'll test smaller block sizes later like
> 512, 1k, 2k as well. We'll fix triggering the SIBGUS when LBS is used,
> we'll address that in the next iteration.
>
> Is this a worthy test as a generic test?
>
> common/filter | 6 ++
> tests/generic/740 | 231 ++++++++++++++++++++++++++++++++++++++++++
> tests/generic/740.out | 2 +
> 3 files changed, 239 insertions(+)
> create mode 100755 tests/generic/740
> create mode 100644 tests/generic/740.out
>
We could also extend the test to include mmap writes.
I took the lazy approach here but I think we can make the code look
prettier by adding some more functions that can do both mmap read and writes.
diff --git a/tests/generic/740 b/tests/generic/740
index cbb82301..93663964 100755
--- a/tests/generic/740
+++ b/tests/generic/740
@@ -53,6 +53,25 @@ round_up_to_page_boundary()
echo $(( (n + page_size - 1) & ~(page_size - 1) ))
}
+mwrite()
+{
+ local file=$1
+ local map_len=$2
+ local offset=$3
+ local length=$4
+
+ # Some callers expect xfs_io to crash with SIGBUS due to the mread,
+ # causing the shell to print "Bus error" to stderr. To allow this
+ # message to be redirected, execute xfs_io in a new shell instance.
+ # However, for this to work reliably, we also need to prevent the new
+ # shell instance from optimizing out the fork and directly exec'ing
+ # xfs_io. The easiest way to do that is to append 'true' to the
+ # commands, so that xfs_io is no longer the last command the shell sees.
+ bash -c "trap '' SIGBUS; $XFS_IO_PROG $file \
+ -c 'mmap -w 0 $map_len' \
+ -c 'mwrite $offset $length'; true"
+}
+
mread()
{
local file=$1
@@ -180,6 +199,12 @@ do_mmap_tests()
_fail
fi
+ # This should just work
+ mwrite $test_file $map_len 0 $map_len >> $seqres.full 2>$tmp.err
+ if [[ $? -ne 0 ]]; then
+ _fail
+ fi
+
# If we mmap() on the boundary but try to read beyond it just
# fails, we don't get a SIGBUS
$XFS_IO_PROG -r $test_file \
@@ -192,12 +217,29 @@ do_mmap_tests()
_fail
fi
+ $XFS_IO_PROG -w $test_file \
+ -c "mmap -w 0 $map_len" \
+ -c "mwrite 0 $((map_len + 10))" >> $seqres.full 2>$tmp.err
+ local mwrite_err=$?
+ if [[ $mwrite_err -eq 0 ]]; then
+ echo "mmap() to page boundary works as expected but writing beyond should fail"
+ echo "err: $?"
+ _fail
+ fi
+
# Now let's go beyond the allowed mmap() page boundary
mread $test_file $((map_len + 10)) 0 $((map_len + 10)) >> $seqres.full 2>$tmp.err
if ! grep -q 'Bus error' $tmp.err; then
echo "Expected SIGBUS when mmap() reading beyond page boundary"
_fail
fi
+
+ mwrite $test_file $((map_len + 10)) 0 $((map_len + 10)) >> $seqres.full 2>$tmp.err
+ if ! grep -q 'Bus error' $tmp.err; then
+ echo "Expected SIGBUS when mmap() writing beyond page boundary"
+ _fail
+ fi
+
local filelen_test=$(_get_filesize $test_file)
if [[ "$filelen_test" != "$new_filelen" ]]; then
echo "Expected file length: $new_filelen"
--
Pankaj
prev parent reply other threads:[~2024-04-22 14:23 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-15 8:10 [RFC] fstests: add mmap page boundary tests Luis Chamberlain
2024-04-15 15:58 ` Darrick J. Wong
2024-04-22 14:09 ` Pankaj Raghav (Samsung)
2024-05-28 22:18 ` Luis Chamberlain
2024-04-15 20:05 ` Zorro Lang
2024-05-28 22:23 ` Luis Chamberlain
2024-04-22 14:12 ` Pankaj Raghav (Samsung) [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=5xt5ultarn7rkidw66ii2kceobb3t2d4pqtfvzogmuk23zcdqh@dzvknx352vsg \
--to=kernel@pankajraghav.com \
--cc=da.gomez@samsung.com \
--cc=fstests@vger.kernel.org \
--cc=hare@suse.de \
--cc=john.g.garry@oracle.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=p.raghav@samsung.com \
--cc=patches@lists.linux.dev \
--cc=willy@infradead.org \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).