Re: [PATCH 1/4] fsx: add missing file size update on zero range operations

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Fri, Apr 17, 2020 at 6:48 PM Brian Foster <bfoster@xxxxxxxxxx> wrote:
>
> On Fri, Apr 17, 2020 at 06:32:03PM +0100, Filipe Manana wrote:
> > On Fri, Apr 17, 2020 at 6:26 PM Brian Foster <bfoster@xxxxxxxxxx> wrote:
> > >
> > > On Fri, Apr 17, 2020 at 06:20:24PM +0100, Filipe Manana wrote:
> > > > On Fri, Apr 17, 2020 at 6:10 PM Brian Foster <bfoster@xxxxxxxxxx> wrote:
> > > > >
> > > > > On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@xxxxxxxxxx wrote:
> > > > > > From: Filipe Manana <fdmanana@xxxxxxxx>
> > > > > >
> > > > > > When a zero range operation increases the size of the test file we were
> > > > > > not updating the global variable 'file_size' which tracks the current
> > > > > > size of the test file. This variable is used to for example compute the
> > > > > > offset for a source range of clone, dedupe and copy file range operations.
> > > > > >
> > > > > > So just fix it by updating the 'file_size' global variable whenever a zero
> > > > > > range operation does not use the keep size flag and its range goes beyond
> > > > > > the current file size.
> > > > > >
> > > > > > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> > > > > > ---
> > > > > >  ltp/fsx.c | 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/ltp/fsx.c b/ltp/fsx.c
> > > > > > index 9d598a4f..fa383c94 100644
> > > > > > --- a/ltp/fsx.c
> > > > > > +++ b/ltp/fsx.c
> > > > > > @@ -1212,6 +1212,8 @@ do_zero_range(unsigned offset, unsigned length, int keep_size)
> > > > > >       }
> > > > > >
> > > > > >       end_offset = keep_size ? 0 : offset + length;
> > > > > > +     if (!keep_size && end_offset > file_size)
> > > > > > +             file_size = end_offset;
> > > > >
> > > > > Should this ever happen if the caller uses TRIM_OFF_LEN() on the
> > > > > offset and length?
> > > >
> > > > TRIM_OFF_LEN only trims the range, not the file_size.
> > > > Or did I miss something?
> > > >
> > >
> > > Right, but TRIM_LEN() does:
> > >
> > >         if ((off) + (len) > (size))             \
> > >                 (len) = (size) - (off);         \
> > >
> > > ... where size is file_size. Hm?
> >
> > That only updates the range's length, not the file_size.
> >
>
> Yes, but it caps the range to within file_size.

Yes.

The problem I'm trying to solve is that because the file_size is not
updated by zero range operations,
a following clone/dedupe/copy_range call will not be able to use a
range that crosses the old file size and goes up to the new file size.

I.e. I'm not solving a problem where the range for those operations
(or any others) incorrectly crosses eof - that doesn't happen because
of the TRIM_* macros.

Does it make sense now?

Thanks.


>
> > Also, if you check the global style, you'll see that in the function
> > for every operation that can change file size we do update file_size
> > explicitly (e.g. do_preallocate(), and we call TRIM_OFF_LEN before
> > calling it as well).
> >
>
> do_preallocate() (fallocate) passes maxfilelen instead of file_size, as
> does write and mapwrite. Insert range uses TRIM_LEN() directly but also
> passes maxfilelen.
>
> Brian
>
> > Thanks.
> >
> > >
> > > Brian
> > >
> > > > Thanks.
> > > >
> > > > >
> > > > > Brian
> > > > >
> > > > > >
> > > > > >       if (end_offset > biggest) {
> > > > > >               biggest = end_offset;
> > > > > > --
> > > > > > 2.11.0
> > > > > >
> > > > >
> > > >
> > >
> >
>



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux