* [PATCH 1/4] fsx: add missing file size update on zero range operations
@ 2020-04-08 10:35 fdmanana
2020-04-17 17:10 ` Brian Foster
2020-04-19 14:55 ` Eryu Guan
0 siblings, 2 replies; 10+ messages in thread
From: fdmanana @ 2020-04-08 10:35 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, Filipe Manana
From: Filipe Manana <fdmanana@suse.com>
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@suse.com>
---
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;
if (end_offset > biggest) {
biggest = end_offset;
--
2.11.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-08 10:35 [PATCH 1/4] fsx: add missing file size update on zero range operations fdmanana
@ 2020-04-17 17:10 ` Brian Foster
2020-04-17 17:20 ` Filipe Manana
2020-04-19 14:55 ` Eryu Guan
1 sibling, 1 reply; 10+ messages in thread
From: Brian Foster @ 2020-04-17 17:10 UTC (permalink / raw)
To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana
On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> 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@suse.com>
> ---
> 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?
Brian
>
> if (end_offset > biggest) {
> biggest = end_offset;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-17 17:10 ` Brian Foster
@ 2020-04-17 17:20 ` Filipe Manana
2020-04-17 17:26 ` Brian Foster
0 siblings, 1 reply; 10+ messages in thread
From: Filipe Manana @ 2020-04-17 17:20 UTC (permalink / raw)
To: Brian Foster; +Cc: fstests, linux-btrfs, Filipe Manana
On Fri, Apr 17, 2020 at 6:10 PM Brian Foster <bfoster@redhat.com> wrote:
>
> On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > 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@suse.com>
> > ---
> > 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?
Thanks.
>
> Brian
>
> >
> > if (end_offset > biggest) {
> > biggest = end_offset;
> > --
> > 2.11.0
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-17 17:20 ` Filipe Manana
@ 2020-04-17 17:26 ` Brian Foster
2020-04-17 17:32 ` Filipe Manana
0 siblings, 1 reply; 10+ messages in thread
From: Brian Foster @ 2020-04-17 17:26 UTC (permalink / raw)
To: Filipe Manana; +Cc: fstests, linux-btrfs, Filipe Manana
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@redhat.com> wrote:
> >
> > On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > 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@suse.com>
> > > ---
> > > 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?
Brian
> Thanks.
>
> >
> > Brian
> >
> > >
> > > if (end_offset > biggest) {
> > > biggest = end_offset;
> > > --
> > > 2.11.0
> > >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-17 17:26 ` Brian Foster
@ 2020-04-17 17:32 ` Filipe Manana
2020-04-17 17:47 ` Brian Foster
0 siblings, 1 reply; 10+ messages in thread
From: Filipe Manana @ 2020-04-17 17:32 UTC (permalink / raw)
To: Brian Foster; +Cc: fstests, linux-btrfs, Filipe Manana
On Fri, Apr 17, 2020 at 6:26 PM Brian Foster <bfoster@redhat.com> 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@redhat.com> wrote:
> > >
> > > On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> > > > From: Filipe Manana <fdmanana@suse.com>
> > > >
> > > > 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@suse.com>
> > > > ---
> > > > 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.
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).
Thanks.
>
> Brian
>
> > Thanks.
> >
> > >
> > > Brian
> > >
> > > >
> > > > if (end_offset > biggest) {
> > > > biggest = end_offset;
> > > > --
> > > > 2.11.0
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-17 17:32 ` Filipe Manana
@ 2020-04-17 17:47 ` Brian Foster
2020-04-17 17:53 ` Filipe Manana
0 siblings, 1 reply; 10+ messages in thread
From: Brian Foster @ 2020-04-17 17:47 UTC (permalink / raw)
To: Filipe Manana; +Cc: fstests, linux-btrfs, Filipe Manana
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@redhat.com> 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@redhat.com> wrote:
> > > >
> > > > On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> > > > > From: Filipe Manana <fdmanana@suse.com>
> > > > >
> > > > > 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@suse.com>
> > > > > ---
> > > > > 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.
> 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
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-17 17:47 ` Brian Foster
@ 2020-04-17 17:53 ` Filipe Manana
2020-04-17 18:25 ` Brian Foster
0 siblings, 1 reply; 10+ messages in thread
From: Filipe Manana @ 2020-04-17 17:53 UTC (permalink / raw)
To: Brian Foster; +Cc: fstests, linux-btrfs, Filipe Manana
On Fri, Apr 17, 2020 at 6:48 PM Brian Foster <bfoster@redhat.com> 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@redhat.com> 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@redhat.com> wrote:
> > > > >
> > > > > On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> > > > > > From: Filipe Manana <fdmanana@suse.com>
> > > > > >
> > > > > > 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@suse.com>
> > > > > > ---
> > > > > > 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
> > > > > >
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-17 17:53 ` Filipe Manana
@ 2020-04-17 18:25 ` Brian Foster
2020-04-20 17:07 ` Filipe Manana
0 siblings, 1 reply; 10+ messages in thread
From: Brian Foster @ 2020-04-17 18:25 UTC (permalink / raw)
To: Filipe Manana; +Cc: fstests, linux-btrfs, Filipe Manana
On Fri, Apr 17, 2020 at 06:53:27PM +0100, Filipe Manana wrote:
> On Fri, Apr 17, 2020 at 6:48 PM Brian Foster <bfoster@redhat.com> 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@redhat.com> 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@redhat.com> wrote:
> > > > > >
> > > > > > On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> > > > > > > From: Filipe Manana <fdmanana@suse.com>
> > > > > > >
> > > > > > > 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@suse.com>
> > > > > > > ---
> > > > > > > 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?
>
Not really. When is end_offset > file_size ever true in do_zero_range()?
Brian
> 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
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-08 10:35 [PATCH 1/4] fsx: add missing file size update on zero range operations fdmanana
2020-04-17 17:10 ` Brian Foster
@ 2020-04-19 14:55 ` Eryu Guan
1 sibling, 0 replies; 10+ messages in thread
From: Eryu Guan @ 2020-04-19 14:55 UTC (permalink / raw)
To: fdmanana; +Cc: fstests, linux-btrfs, Filipe Manana
On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> 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@suse.com>
> ---
> 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;
I think this should be done after we really excute fallocate(2),
otherwise we may return early below:
if (testcalls <= simulatedopcount)
return;
Thanks,
Eryu
>
> if (end_offset > biggest) {
> biggest = end_offset;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] fsx: add missing file size update on zero range operations
2020-04-17 18:25 ` Brian Foster
@ 2020-04-20 17:07 ` Filipe Manana
0 siblings, 0 replies; 10+ messages in thread
From: Filipe Manana @ 2020-04-20 17:07 UTC (permalink / raw)
To: Brian Foster; +Cc: fstests, linux-btrfs, Filipe Manana
On Fri, Apr 17, 2020 at 7:25 PM Brian Foster <bfoster@redhat.com> wrote:
>
> On Fri, Apr 17, 2020 at 06:53:27PM +0100, Filipe Manana wrote:
> > On Fri, Apr 17, 2020 at 6:48 PM Brian Foster <bfoster@redhat.com> 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@redhat.com> 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@redhat.com> wrote:
> > > > > > >
> > > > > > > On Wed, Apr 08, 2020 at 11:35:52AM +0100, fdmanana@kernel.org wrote:
> > > > > > > > From: Filipe Manana <fdmanana@suse.com>
> > > > > > > >
> > > > > > > > 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@suse.com>
> > > > > > > > ---
> > > > > > > > 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?
> >
>
> Not really. When is end_offset > file_size ever true in do_zero_range()?
Ok, I see what you mean now. It's the trimming before calling the zero
range function.
So we have a different type of problem - we never exercise zero range
operations that cross the current eof.
Patch updated in the next verson.
Thanks.
>
> Brian
>
> > 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
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-20 17:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-08 10:35 [PATCH 1/4] fsx: add missing file size update on zero range operations fdmanana
2020-04-17 17:10 ` Brian Foster
2020-04-17 17:20 ` Filipe Manana
2020-04-17 17:26 ` Brian Foster
2020-04-17 17:32 ` Filipe Manana
2020-04-17 17:47 ` Brian Foster
2020-04-17 17:53 ` Filipe Manana
2020-04-17 18:25 ` Brian Foster
2020-04-20 17:07 ` Filipe Manana
2020-04-19 14:55 ` Eryu Guan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.