On Wed, Aug 28, 2024 at 06:27:16PM -0700, Darrick J. Wong wrote:
> On Wed, Aug 28, 2024 at 02:15:31PM -0400, Brian Foster wrote:
> > fsx supports the ability to skip through a certain number of
> > operations of a given command sequence before beginning full
> > operation. The way this works is by tracking the operation count,
> > simulating minimal side effects of skipped operations in-memory, and
> > then finally writing out the in-memory state to the target file when
> > full operation begins.
> >
> > Several fallocate() related operations don't correctly track
> > in-memory state when simulated, however. For example, consider an
> > ops file with the following two operations:
> >
> > zero_range 0x0 0x1000 0x0
> > read 0x0 0x1000 0x0
> >
> > ... and an fsx run like so:
> >
> > fsx -d -b 2 --replay-ops=<opsfile> <file>
> >
> > This simulates the zero_range operation, but fails to track the file
> > extension that occurs as a side effect such that the subsequent read
> > doesn't occur as expected:
> >
> > Will begin at operation 2
> > skipping zero size read
> >
> > The read is skipped in this case because the file size is zero. The
> > proper behavior, and what is consistent with other size changing
> > operations, is to make the appropriate in-core changes before
> > checking whether an operation is simulated so the end result of
> > those changes can be reflected on-disk for eventual non-simulated
> > operations. This results in expected behavior with the same ops file
> > and test command:
> >
> > Will begin at operation 2
> > 2 read 0x0 thru 0xfff (0x1000 bytes)
> >
> > Update zero, copy and clone range to do the file size and EOF change
> > related zeroing before checking against the simulated ops count.
> >
> > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
>
> Oh wow, I really got that wrong. :(
>
Eh, so did I. ;) That the code was mostly right was pretty much just
luck. Thanks for the thoughtful reviews.
Brian
> Well, thank you for uncovering that error;
> Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> --D
>
>
> > ---
> > ltp/fsx.c | 40 +++++++++++++++++++++-------------------
> > 1 file changed, 21 insertions(+), 19 deletions(-)
> >
> > diff --git a/ltp/fsx.c b/ltp/fsx.c
> > index 2dc59b06..c5727cff 100644
> > --- a/ltp/fsx.c
> > +++ b/ltp/fsx.c
> > @@ -1247,6 +1247,17 @@ do_zero_range(unsigned offset, unsigned length, int keep_size)
> > log4(OP_ZERO_RANGE, offset, length,
> > keep_size ? FL_KEEP_SIZE : FL_NONE);
> >
> > + if (!keep_size && end_offset > file_size) {
> > + /*
> > + * If there's a gap between the old file size and the offset of
> > + * the zero range operation, fill the gap with zeroes.
> > + */
> > + if (offset > file_size)
> > + memset(good_buf + file_size, '\0', offset - file_size);
> > +
> > + file_size = end_offset;
> > + }
> > +
> > if (testcalls <= simulatedopcount)
> > return;
> >
> > @@ -1263,17 +1274,6 @@ do_zero_range(unsigned offset, unsigned length, int keep_size)
> > }
> >
> > memset(good_buf + offset, '\0', length);
> > -
> > - if (!keep_size && end_offset > file_size) {
> > - /*
> > - * If there's a gap between the old file size and the offset of
> > - * the zero range operation, fill the gap with zeroes.
> > - */
> > - if (offset > file_size)
> > - memset(good_buf + file_size, '\0', offset - file_size);
> > -
> > - file_size = end_offset;
> > - }
> > }
> >
> > #else
> > @@ -1538,6 +1538,11 @@ do_clone_range(unsigned offset, unsigned length, unsigned dest)
> >
> > log5(OP_CLONE_RANGE, offset, length, dest, FL_NONE);
> >
> > + if (dest > file_size)
> > + memset(good_buf + file_size, '\0', dest - file_size);
> > + if (dest + length > file_size)
> > + file_size = dest + length;
> > +
> > if (testcalls <= simulatedopcount)
> > return;
> >
> > @@ -1556,10 +1561,6 @@ do_clone_range(unsigned offset, unsigned length, unsigned dest)
> > }
> >
> > memcpy(good_buf + dest, good_buf + offset, length);
> > - if (dest > file_size)
> > - memset(good_buf + file_size, '\0', dest - file_size);
> > - if (dest + length > file_size)
> > - file_size = dest + length;
> > }
> >
> > #else
> > @@ -1756,6 +1757,11 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
> >
> > log5(OP_COPY_RANGE, offset, length, dest, FL_NONE);
> >
> > + if (dest > file_size)
> > + memset(good_buf + file_size, '\0', dest - file_size);
> > + if (dest + length > file_size)
> > + file_size = dest + length;
> > +
> > if (testcalls <= simulatedopcount)
> > return;
> >
> > @@ -1792,10 +1798,6 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
> > }
> >
> > memcpy(good_buf + dest, good_buf + offset, length);
> > - if (dest > file_size)
> > - memset(good_buf + file_size, '\0', dest - file_size);
> > - if (dest + length > file_size)
> > - file_size = dest + length;
> > }
> >
> > #else
> > --
> > 2.45.0
> >
> >
>
