On Thu, Aug 22, 2024 at 01:54:30PM -0700, Darrick J. Wong wrote: > On Thu, Aug 22, 2024 at 10:44:22AM -0400, Brian Foster wrote: > > Filesystem regressions related to partial page zeroing can go > > unnoticed for a decent amount of time. A recent example is the issue > > of iomap zero range not handling dirty pagecache over unwritten > > extents, which leads to wrong behavior on certain file extending > > operations (i.e. truncate, write extension, etc.). > > > > fsx does occasionally uncover these sorts of problems, but failures > > can be rare and/or require longer running tests outside what is > > typically run via full fstests regression runs. fsx now supports a > > mode that injects post-eof data in order to explicitly test partial > > eof zeroing behavior. This uncovers certain problems more quickly > > and applies coverage more broadly across size changing operations. > > > > Add a new test that runs an fsx instance (modeled after generic/127) > > with eof pollution mode enabled. While the test is generic, it is > > currently limited to XFS as that is currently the only known major > > fs that does enough zeroing to satisfy the strict semantics expected > > by fsx. The long term goal is to uncover and fix issues so more > > filesystems can enable this test. > > > > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx> > > --- > > tests/generic/362 | 27 +++++++++++++++++++++++++++ > > tests/generic/362.out | 2 ++ > > 2 files changed, 29 insertions(+) > > create mode 100755 tests/generic/362 > > create mode 100644 tests/generic/362.out > > > > diff --git a/tests/generic/362 b/tests/generic/362 > > new file mode 100755 > > index 00000000..30870cd0 > > --- /dev/null > > +++ b/tests/generic/362 > > @@ -0,0 +1,27 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2024 Red Hat, Inc. All Rights Reserved. > > +# > > +# FSQA Test No. 362 > > +# > > +# Run fsx with EOF pollution enabled. This provides test coverage for partial > > +# EOF page/block zeroing for operations that change file size. > > +# > > + > > +. ./common/preamble > > +_begin_fstest rw auto > > + > > +FSX_FILE_SIZE=262144 > > +# on failure, replace -q with -d to see post-eof writes in the dump output > > +FSX_ARGS="-q -l $FSX_FILE_SIZE -e 1 -N 100000" > > + > > +_require_test > > + > > +# currently only xfs performs enough zeroing to satisfy fsx > > +_supported_fs xfs > > Should get rid of this. ;) > Agreed. But just to be clear, I was planning to leave this in for now so we don't have to go and fix the various other filesystem issues (assuming various maintainers agree) as a gate to fixing iomap zeroing and having decent test coverage. I'm planning to look into the other failures once I get through the iomap revalidation thing to avoid the flush on XFS. > > +ltp/fsx $FSX_ARGS $FSX_AVOID $TEST_DIR/fsx.$seq > $tmp.output 2>&1 > > I wonder, is there a reason not to use run_fsx from common/rc? > At first glance the only thing that stands out to me is run_fsx hardcodes the file to $TEST_DIR/junk instead of using and leaving around a test-specific file. I find that ever so slightly annoying, but not enough to hardcode the fsx command in the test, so I'll make that change. My initial hope was to be able to just turn on the eof pollution thing by default and rely on existing fstests to (which I've confirmed do) detect the iomap zeroing problems, but the other non-iomap fs failures mean we can't do that without being disruptive and hence the need for the custom fstest. If we can get to the point of completely removing the _supported_fs check above, then perhaps we could also revisit changing the fsx default and removing this test. > Otherwise this looks ok to me. > Thanks for the comments. Brian > --D > > > +cat $tmp.output > > + > > +status=0 > > +exit > > diff --git a/tests/generic/362.out b/tests/generic/362.out > > new file mode 100644 > > index 00000000..7af6b96a > > --- /dev/null > > +++ b/tests/generic/362.out > > @@ -0,0 +1,2 @@ > > +QA output created by 362 > > +All 100000 operations completed A-OK! > > -- > > 2.45.0 > > > > >
