On Thu, Aug 22, 2024 at 01:52:57PM -0700, Darrick J. Wong wrote:
> On Thu, Aug 22, 2024 at 10:44:21AM -0400, Brian Foster wrote:
> > File ranges that are newly exposed via size changing operations are
> > expected to return zeroes until written to. This behavior tends to
> > be difficult to regression test as failures can be racy and
> > transient. fsx is probably the best tool for this type of test
> > coverage, but uncovering issues can require running for a
> > significantly longer period of time than is typically invoked
> > through fstests tests. As a result, these types of regressions tend
> > to go unnoticed for an unfortunate amount of time.
> >
> > To facilitate uncovering these problems more quickly, implement an
> > eof pollution mode in fsx that opportunistically injects post-eof
> > data prior to operations that change file size. Since data injection
> > occurs immediately before the size changing operation, it can be
> > used to detect problems in partial eof page/block zeroing associated
> > with each relevant operation.
> >
> > The implementation takes advantage of the fact that mapped writes
> > can place data beyond eof so long as the page starts within eof. The
> > main reason for the isolated per-operation approach (vs. something
> > like allowing mapped writes to write beyond eof, for example) is to
> > accommodate the fact that writeback zeroes post-eof data on the eof
> > page. The current approach is therefore not necessarily guaranteed
> > to detect all problems, but provides more generic and broad test
> > coverage than the alternative of testing explicit command sequences
> > and doesn't require significant changes to how fsx works. If this
> > proves useful long term, further enhancements can be considered that
> > might facilitate the presence of post-eof data across operations.
> >
> > Enable the feature with the -e command line option. It is disabled
> > by default because zeroing behavior is inconsistent across
> > filesystems. This can also be revisited in the future if zeroing
> > behavior is refined for the major filesystems that rely on fstests
> > for regression testing.
> >
> > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> > ---
> > ltp/fsx.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 72 insertions(+), 2 deletions(-)
> >
> > diff --git a/ltp/fsx.c b/ltp/fsx.c
> > index 1389c51d..20b8cd9f 100644
> > --- a/ltp/fsx.c
> > +++ b/ltp/fsx.c
> > @@ -178,6 +178,7 @@ int dedupe_range_calls = 1; /* -B flag disables */
> > int copy_range_calls = 1; /* -E flag disables */
> > int exchange_range_calls = 1; /* -0 flag disables */
> > int integrity = 0; /* -i flag */
> > +int pollute_eof = 0; /* -e flag */
> > int fsxgoodfd = 0;
> > int o_direct; /* -Z */
> > int aio = 0;
> > @@ -983,6 +984,58 @@ gendata(char *original_buf, char *good_buf, unsigned offset, unsigned size)
> > }
> > }
> >
> > +/*
> > + * Pollute the EOF page with data beyond EOF prior to size change operations.
> > + * This provides additional test coverage for partial EOF block/page zeroing.
> > + * If the upcoming operation does not correctly zero, incorrect file data will
> > + * be detected.
> > + */
> > +void
> > +pollute_eofpage(unsigned int maxoff)
> > +{
> > + unsigned offset = file_size;
> > + unsigned pg_offset;
> > + unsigned write_size;
> > + char *p;
> > +
> > + if (!pollute_eof)
> > + return;
> > +
> > + /* write up to specified max or the end of the eof page */
> > + pg_offset = offset & mmap_mask;
> > + write_size = MIN(PAGE_SIZE - pg_offset, maxoff - offset);
> > +
> > + if (!pg_offset)
> > + return;
> > +
> > + if (!quiet &&
> > + ((progressinterval && testcalls % progressinterval == 0) ||
> > + (debug &&
> > + (monitorstart == -1 ||
> > + (offset + write_size > monitorstart &&
> > + (monitorend == -1 || offset <= monitorend)))))) {
> > + prt("%lld pollute_eof\t0x%x thru\t0x%x\t(0x%x bytes)\n",
> > + testcalls, offset, offset + write_size - 1, write_size);
> > + }
> > +
> > + if ((p = (char *)mmap(0, PAGE_SIZE, PROT_READ | PROT_WRITE,
> > + MAP_FILE | MAP_SHARED, fd,
> > + (off_t)(offset - pg_offset))) == (char *)-1) {
>
> Nit:
>
> if (mmap(...) == MAP_FAILED)?
>
> Otherwise I like the concept here. :)
>
Yep, copy paste fail. Will fix.
Brian
> --D
>
> > + prterr("pollute_eofpage: mmap");
> > + return;
> > + }
> > +
> > + /*
> > + * Write to a range just past EOF of the test file. Do not update the
> > + * good buffer because the upcoming operation is expected to zero this
> > + * range of the file.
> > + */
> > + gendata(original_buf, p, pg_offset, write_size);
> > +
> > + if (munmap(p, PAGE_SIZE) != 0)
> > + prterr("pollute_eofpage: munmap");
> > +}
> > +
> > /*
> > * Helper to update the tracked file size. If the offset begins beyond current
> > * EOF, zero the range from EOF to offset in the good buffer.
> > @@ -990,8 +1043,10 @@ gendata(char *original_buf, char *good_buf, unsigned offset, unsigned size)
> > void
> > update_file_size(unsigned offset, unsigned size)
> > {
> > - if (offset > file_size)
> > + if (offset > file_size) {
> > + pollute_eofpage(offset + size);
> > memset(good_buf + file_size, '\0', offset - file_size);
> > + }
> > file_size = offset + size;
> > }
> >
> > @@ -1143,6 +1198,9 @@ dotruncate(unsigned size)
> >
> > log4(OP_TRUNCATE, 0, size, FL_NONE);
> >
> > + /* pollute the current EOF before a truncate down */
> > + if (size < file_size)
> > + pollute_eofpage(maxfilelen);
> > update_file_size(size, 0);
> >
> > if (testcalls <= simulatedopcount)
> > @@ -1305,6 +1363,9 @@ do_collapse_range(unsigned offset, unsigned length)
> >
> > log4(OP_COLLAPSE_RANGE, offset, length, FL_NONE);
> >
> > + /* pollute current eof before collapse truncates down */
> > + pollute_eofpage(maxfilelen);
> > +
> > if (testcalls <= simulatedopcount)
> > return;
> >
> > @@ -1356,6 +1417,9 @@ do_insert_range(unsigned offset, unsigned length)
> >
> > log4(OP_INSERT_RANGE, offset, length, FL_NONE);
> >
> > + /* pollute current eof before insert truncates up */
> > + pollute_eofpage(maxfilelen);
> > +
> > if (testcalls <= simulatedopcount)
> > return;
> >
> > @@ -2385,6 +2449,7 @@ usage(void)
> > -b opnum: beginning operation number (default 1)\n\
> > -c P: 1 in P chance of file close+open at each op (default infinity)\n\
> > -d: debug output for all operations\n\
> > + -e: pollute post-eof on size changes (default 0)\n\
> > -f: flush and invalidate cache after I/O\n\
> > -g X: write character X instead of random generated data\n\
> > -i logdev: do integrity testing, logdev is the dm log writes device\n\
> > @@ -2783,7 +2848,7 @@ main(int argc, char **argv)
> > setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */
> >
> > while ((ch = getopt_long(argc, argv,
> > - "0b:c:dfg:i:j:kl:m:no:p:qr:s:t:w:xyABD:EFJKHzCILN:OP:RS:UWXZ",
> > + "0b:c:de:fg:i:j:kl:m:no:p:qr:s:t:w:xyABD:EFJKHzCILN:OP:RS:UWXZ",
> > longopts, NULL)) != EOF)
> > switch (ch) {
> > case 'b':
> > @@ -2805,6 +2870,11 @@ main(int argc, char **argv)
> > case 'd':
> > debug = 1;
> > break;
> > + case 'e':
> > + pollute_eof = getnum(optarg, &endp);
> > + if (pollute_eof < 0 || pollute_eof > 1)
> > + usage();
> > + break;
> > case 'f':
> > flush = 1;
> > break;
> > --
> > 2.45.0
> >
> >
>
