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. :)
--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
>
>
