diff mbox series

generic/473: fix expectation properly in out file

Message ID 20210223134042.2212341-1-cgxu519@mykernel.net (mailing list archive)
State New, archived
Headers show
Series generic/473: fix expectation properly in out file | expand

Commit Message

Chengguang Xu Feb. 23, 2021, 1:40 p.m. UTC
It seems the expected result of testcase of "Hole + Data"
in generic/473 is not correct, so just fix it properly.

Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
---
 tests/generic/473.out | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Su Yue Feb. 24, 2021, 7:52 a.m. UTC | #1
Cc to the author and linux-xfs, since it's xfsprogs related.

On Tue 23 Feb 2021 at 21:40, Chengguang Xu <cgxu519@mykernel.net> 
wrote:

> It seems the expected result of testcase of "Hole + Data"
> in generic/473 is not correct, so just fix it properly.
>

But it's not proper...

> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>  tests/generic/473.out | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/generic/473.out b/tests/generic/473.out
> index 75816388..f1ee5805 100644
> --- a/tests/generic/473.out
> +++ b/tests/generic/473.out
> @@ -6,7 +6,7 @@ Data + Hole
>  1: [256..287]: hole
>  Hole + Data
>  0: [0..127]: hole
> -1: [128..255]: data
> +1: [128..135]: data
>
The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" $file | 
_filter_fiemap`.
0-64k is a hole and 64k-128k is a data extent.
fiemap ioctl always returns *complete* ranges of extents.

You may ask why the ending hole range is not aligned to 128 in 
473.out. Because
fiemap ioctl returns nothing of querying holes. xfs_io does the 
extra
print work for holes.

xfsprogs-dev/io/fiemap.c:
for holes:
 153     if (lstart > llast) {
 154         print_hole(0, 0, 0, cur_extent, lflag, true, llast, 
 lstart);
 155         cur_extent++;
 156         num_printed++;
 157     }

for the ending hole:
  381     if (cur_extent && last_logical < range_end)
  382         print_hole(foff_w, boff_w, tot_w, cur_extent, lflag, 
  !vflag,
  383                BTOBBT(last_logical), BTOBBT(range_end));

>  Hole + Data + Hole
>  0: [0..127]: hole
>  1: [128..255]: data
Su Yue Feb. 24, 2021, 8:10 a.m. UTC | #2
On Wed 24 Feb 2021 at 15:52, Su Yue <l@damenly.su> wrote:

> Cc to the author and linux-xfs, since it's xfsprogs related.
>
> On Tue 23 Feb 2021 at 21:40, Chengguang Xu 
> <cgxu519@mykernel.net> wrote:
>
>> It seems the expected result of testcase of "Hole + Data"
>> in generic/473 is not correct, so just fix it properly.
>>
>
> But it's not proper...
>
>> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
>> ---
>>  tests/generic/473.out | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/generic/473.out b/tests/generic/473.out
>> index 75816388..f1ee5805 100644
>> --- a/tests/generic/473.out
>> +++ b/tests/generic/473.out
>> @@ -6,7 +6,7 @@ Data + Hole
>>  1: [256..287]: hole
>>  Hole + Data
>>  0: [0..127]: hole
>> -1: [128..255]: data
>> +1: [128..135]: data
>>
> The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" $file 
> |
> _filter_fiemap`.
> 0-64k is a hole and 64k-128k is a data extent.
> fiemap ioctl always returns *complete* ranges of extents.
>
And what you want to change is only the filted output.
Without _filter_fiemap:

/mnt/test/fiemap.473: 
|
 EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS 
 |
   0: [0..127]:        hole               128 
   |
   1: [128..255]:      26792..26919       128   0x0

[128..255] corresponds to the BLOCK-RANGE of the extent 
26792..26919.

> You may ask why the ending hole range is not aligned to 128 in 
> 473.out. Because
> fiemap ioctl returns nothing of querying holes. xfs_io does the 
> extra
> print work for holes.
>
> xfsprogs-dev/io/fiemap.c:
> for holes:
> 153     if (lstart > llast) {
> 154         print_hole(0, 0, 0, cur_extent, lflag, true, llast, 
> lstart);
> 155         cur_extent++;
> 156         num_printed++;
> 157     }
>
> for the ending hole:
>  381     if (cur_extent && last_logical < range_end)
>  382         print_hole(foff_w, boff_w, tot_w, cur_extent, 
>  lflag,   !vflag,
>  383                BTOBBT(last_logical), BTOBBT(range_end));
>
>>  Hole + Data + Hole
>>  0: [0..127]: hole
>>  1: [128..255]: data
Chengguang Xu Feb. 24, 2021, 8:51 a.m. UTC | #3
---- 在 星期三, 2021-02-24 15:52:17 Su Yue <l@damenly.su> 撰写 ----
 > 
 > Cc to the author and linux-xfs, since it's xfsprogs related.
 > 
 > On Tue 23 Feb 2021 at 21:40, Chengguang Xu <cgxu519@mykernel.net> 
 > wrote:
 > 
 > > It seems the expected result of testcase of "Hole + Data"
 > > in generic/473 is not correct, so just fix it properly.
 > >
 > 
 > But it's not proper...
 > 
 > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > > ---
 > >  tests/generic/473.out | 2 +-
 > >  1 file changed, 1 insertion(+), 1 deletion(-)
 > >
 > > diff --git a/tests/generic/473.out b/tests/generic/473.out
 > > index 75816388..f1ee5805 100644
 > > --- a/tests/generic/473.out
 > > +++ b/tests/generic/473.out
 > > @@ -6,7 +6,7 @@ Data + Hole
 > >  1: [256..287]: hole
 > >  Hole + Data
 > >  0: [0..127]: hole
 > > -1: [128..255]: data
 > > +1: [128..135]: data
 > >
 > The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" $file | 
 > _filter_fiemap`.
 > 0-64k is a hole and 64k-128k is a data extent.
 > fiemap ioctl always returns *complete* ranges of extents.

Manual testing result in latest kernel like below.

[root@centos test]# uname -a
Linux centos 5.11.0+ #5 SMP Tue Feb 23 21:02:27 CST 2021 x86_64 x86_64 x86_64 GNU/Linux

[root@centos test]# xfs_io -V
xfs_io version 5.0.0

[root@centos test]# stat a
  File: a
  Size: 4194304         Blocks: 0          IO Block: 4096   regular file
Device: fc01h/64513d    Inode: 140         Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2021-02-24 16:33:20.235654140 +0800
Modify: 2021-02-24 16:33:25.070641521 +0800
Change: 2021-02-24 16:33:25.070641521 +0800
 Birth: -
 
[root@centos test]# xfs_io -c "pwrite 64k 64k" a
wrote 65536/65536 bytes at offset 65536
64 KiB, 16 ops; 0.0000 sec (992.063 MiB/sec and 253968.2540 ops/sec)

[root@VM-8-4-centos test]# xfs_io -c "fiemap -v 0 65k" a
a:
 EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
   0: [0..127]:        hole               128
   1: [128..135]:      360..367             8   0x1
   
[root@centos test]# xfs_io -c "fiemap -v 0 128k" a
a:
 EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
   0: [0..127]:        hole               128
   1: [128..255]:      360..487           128   0x1


 > 
 > You may ask why the ending hole range is not aligned to 128 in 
 > 473.out. Because
 > fiemap ioctl returns nothing of querying holes. xfs_io does the 
 > extra
 > print work for holes.
 > 
 > xfsprogs-dev/io/fiemap.c:
 > for holes:
 >  153     if (lstart > llast) {
 >  154         print_hole(0, 0, 0, cur_extent, lflag, true, llast, 
 >  lstart);
 >  155         cur_extent++;
 >  156         num_printed++;
 >  157     }
 > 
 > for the ending hole:
 >   381     if (cur_extent && last_logical < range_end)
 >   382         print_hole(foff_w, boff_w, tot_w, cur_extent, lflag, 
 >   !vflag,
 >   383                BTOBBT(last_logical), BTOBBT(range_end));
 > 
 > >  Hole + Data + Hole
 > >  0: [0..127]: hole
 > >  1: [128..255]: data
 >
Chengguang Xu Feb. 24, 2021, 8:55 a.m. UTC | #4
---- 在 星期三, 2021-02-24 16:10:39 Su Yue <l@damenly.su> 撰写 ----
 > 
 > On Wed 24 Feb 2021 at 15:52, Su Yue <l@damenly.su> wrote:
 > 
 > > Cc to the author and linux-xfs, since it's xfsprogs related.
 > >
 > > On Tue 23 Feb 2021 at 21:40, Chengguang Xu 
 > > <cgxu519@mykernel.net> wrote:
 > >
 > >> It seems the expected result of testcase of "Hole + Data"
 > >> in generic/473 is not correct, so just fix it properly.
 > >>
 > >
 > > But it's not proper...
 > >
 > >> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > >> ---
 > >>  tests/generic/473.out | 2 +-
 > >>  1 file changed, 1 insertion(+), 1 deletion(-)
 > >>
 > >> diff --git a/tests/generic/473.out b/tests/generic/473.out
 > >> index 75816388..f1ee5805 100644
 > >> --- a/tests/generic/473.out
 > >> +++ b/tests/generic/473.out
 > >> @@ -6,7 +6,7 @@ Data + Hole
 > >>  1: [256..287]: hole
 > >>  Hole + Data
 > >>  0: [0..127]: hole
 > >> -1: [128..255]: data
 > >> +1: [128..135]: data
 > >>
 > > The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" $file 
 > > |
 > > _filter_fiemap`.
 > > 0-64k is a hole and 64k-128k is a data extent.
 > > fiemap ioctl always returns *complete* ranges of extents.
 > >
 > And what you want to change is only the filted output.
 > Without _filter_fiemap:

Without _filter_fiemap:

[root@centos xfstests-dev]# git diff
diff --git a/tests/generic/473 b/tests/generic/473
index 5c19703e..35f28677 100755
--- a/tests/generic/473
+++ b/tests/generic/473
@@ -60,7 +60,8 @@ echo "Data + Hole"
 $XFS_IO_PROG -c "fiemap -v 64k 80k" $file | _filter_fiemap
 
 echo "Hole + Data"
-$XFS_IO_PROG -c "fiemap -v 0 65k" $file | _filter_fiemap
+#$XFS_IO_PROG -c "fiemap -v 0 65k" $file | _filter_fiemap
+$XFS_IO_PROG -c "fiemap -v 0 65k" $file
 
 echo "Hole + Data + Hole"
 $XFS_IO_PROG -c "fiemap -v 0k 130k" $file | _filter_fiemap
 
[root@centos xfstests-dev]# ./check generic/473
FSTYP         -- ext4
PLATFORM      -- Linux/x86_64 centos 5.11.0+ #5 SMP Tue Feb 23 21:02:27 CST 2021
MKFS_OPTIONS  -- /dev/mapper/workvg-test2
MOUNT_OPTIONS -- -o acl,user_xattr /dev/mapper/workvg-test2 /mnt/scratch

generic/473 1s ... - output mismatch (see /git/xfstests-dev/results//generic/473.out.bad)
    --- tests/generic/473.out   2021-02-24 16:51:23.254845067 +0800
    +++ /git/xfstests-dev/results//generic/473.out.bad  2021-02-24 16:52:04.440737816 +0800
    @@ -5,8 +5,10 @@
     0: [128..255]: data
     1: [256..287]: hole
     Hole + Data
    -0: [0..127]: hole
    -1: [128..255]: data
    +/mnt/test/fiemap.473:
    + EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
    ...
    (Run 'diff -u /git/xfstests-dev/tests/generic/473.out /git/xfstests-dev/results//generic/473.out.bad'  to see the entire diff)
Ran: generic/473
Failures: generic/473
Failed 1 of 1 tests

[root@centos xfstests-dev]# diff -u /git/xfstests-dev/tests/generic/473.out /git/xfstests-dev/results//generic/473.out.bad
--- /git/xfstests-dev/tests/generic/473.out     2021-02-24 16:51:23.254845067 +0800
+++ /git/xfstests-dev/results//generic/473.out.bad      2021-02-24 16:52:04.440737816 +0800
@@ -5,8 +5,10 @@
 0: [128..255]: data
 1: [256..287]: hole
 Hole + Data
-0: [0..127]: hole
-1: [128..255]: data
+/mnt/test/fiemap.473:
+ EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
+   0: [0..127]:        hole               128
+   1: [128..135]:      274560..274567       8   0x1
 Hole + Data + Hole
 0: [0..127]: hole
 1: [128..255]: data



 > 
 > /mnt/test/fiemap.473: 
 > |
 >  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS 
 >  |
 >    0: [0..127]:        hole               128 
 >    |
 >    1: [128..255]:      26792..26919       128   0x0
 > 
 > [128..255] corresponds to the BLOCK-RANGE of the extent 
 > 26792..26919.
 > 
 > > You may ask why the ending hole range is not aligned to 128 in 
 > > 473.out. Because
 > > fiemap ioctl returns nothing of querying holes. xfs_io does the 
 > > extra
 > > print work for holes.
 > >
 > > xfsprogs-dev/io/fiemap.c:
 > > for holes:
 > > 153     if (lstart > llast) {
 > > 154         print_hole(0, 0, 0, cur_extent, lflag, true, llast, 
 > > lstart);
 > > 155         cur_extent++;
 > > 156         num_printed++;
 > > 157     }
 > >
 > > for the ending hole:
 > >  381     if (cur_extent && last_logical < range_end)
 > >  382         print_hole(foff_w, boff_w, tot_w, cur_extent, 
 > >  lflag,   !vflag,
 > >  383                BTOBBT(last_logical), BTOBBT(range_end));
 > >
 > >>  Hole + Data + Hole
 > >>  0: [0..127]: hole
 > >>  1: [128..255]: data
 > 
 >
Chengguang Xu Feb. 24, 2021, 9:16 a.m. UTC | #5
---- 在 星期三, 2021-02-24 16:51:03 Chengguang Xu <cgxu519@mykernel.net> 撰写 ----
 >  ---- 在 星期三, 2021-02-24 15:52:17 Su Yue <l@damenly.su> 撰写 ----
 >  > 
 >  > Cc to the author and linux-xfs, since it's xfsprogs related.
 >  > 
 >  > On Tue 23 Feb 2021 at 21:40, Chengguang Xu <cgxu519@mykernel.net> 
 >  > wrote:
 >  > 
 >  > > It seems the expected result of testcase of "Hole + Data"
 >  > > in generic/473 is not correct, so just fix it properly.
 >  > >
 >  > 
 >  > But it's not proper...
 >  > 
 >  > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 >  > > ---
 >  > >  tests/generic/473.out | 2 +-
 >  > >  1 file changed, 1 insertion(+), 1 deletion(-)
 >  > >
 >  > > diff --git a/tests/generic/473.out b/tests/generic/473.out
 >  > > index 75816388..f1ee5805 100644
 >  > > --- a/tests/generic/473.out
 >  > > +++ b/tests/generic/473.out
 >  > > @@ -6,7 +6,7 @@ Data + Hole
 >  > >  1: [256..287]: hole
 >  > >  Hole + Data
 >  > >  0: [0..127]: hole
 >  > > -1: [128..255]: data
 >  > > +1: [128..135]: data
 >  > >
 >  > The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" $file | 
 >  > _filter_fiemap`.
 >  > 0-64k is a hole and 64k-128k is a data extent.
 >  > fiemap ioctl always returns *complete* ranges of extents.

Finally, I found btrfs returns *complete* rangne of extents but xfs/ext4 does not. :-/


[root@VM-89-226-centos /test]# xfs_io -c "fiemap 0 65k" a
a:
        0: [0..127]: hole
        1: [128..255]: 24576..24703
[root@VM-89-226-centos /test]# xfs_io -c "fiemap 0 128k" a
a:
        0: [0..127]: hole
        1: [128..255]: 24576..24703



 > 
 > Manual testing result in latest kernel like below.
 > 
 > [root@centos test]# uname -a
 > Linux centos 5.11.0+ #5 SMP Tue Feb 23 21:02:27 CST 2021 x86_64 x86_64 x86_64 GNU/Linux
 > 
 > [root@centos test]# xfs_io -V
 > xfs_io version 5.0.0
 > 
 > [root@centos test]# stat a
 >   File: a
 >   Size: 4194304         Blocks: 0          IO Block: 4096   regular file
 > Device: fc01h/64513d    Inode: 140         Links: 1
 > Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
 > Access: 2021-02-24 16:33:20.235654140 +0800
 > Modify: 2021-02-24 16:33:25.070641521 +0800
 > Change: 2021-02-24 16:33:25.070641521 +0800
 >  Birth: -
 >  
 > [root@centos test]# xfs_io -c "pwrite 64k 64k" a
 > wrote 65536/65536 bytes at offset 65536
 > 64 KiB, 16 ops; 0.0000 sec (992.063 MiB/sec and 253968.2540 ops/sec)
 > 
 > [root@VM-8-4-centos test]# xfs_io -c "fiemap -v 0 65k" a
 > a:
 >  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
 >    0: [0..127]:        hole               128
 >    1: [128..135]:      360..367             8   0x1
 >    
 > [root@centos test]# xfs_io -c "fiemap -v 0 128k" a
 > a:
 >  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
 >    0: [0..127]:        hole               128
 >    1: [128..255]:      360..487           128   0x1
 > 
 > 
 >  > 
 >  > You may ask why the ending hole range is not aligned to 128 in 
 >  > 473.out. Because
 >  > fiemap ioctl returns nothing of querying holes. xfs_io does the 
 >  > extra
 >  > print work for holes.
 >  > 
 >  > xfsprogs-dev/io/fiemap.c:
 >  > for holes:
 >  >  153     if (lstart > llast) {
 >  >  154         print_hole(0, 0, 0, cur_extent, lflag, true, llast, 
 >  >  lstart);
 >  >  155         cur_extent++;
 >  >  156         num_printed++;
 >  >  157     }
 >  > 
 >  > for the ending hole:
 >  >   381     if (cur_extent && last_logical < range_end)
 >  >   382         print_hole(foff_w, boff_w, tot_w, cur_extent, lflag, 
 >  >   !vflag,
 >  >   383                BTOBBT(last_logical), BTOBBT(range_end));
 >  > 
 >  > >  Hole + Data + Hole
 >  > >  0: [0..127]: hole
 >  > >  1: [128..255]: data
 >  > 
 >
Su Yue Feb. 24, 2021, 9:22 a.m. UTC | #6
On Wed 24 Feb 2021 at 16:51, Chengguang Xu <cgxu519@mykernel.net> 
wrote:

>  ---- 在 星期三, 2021-02-24 15:52:17 Su Yue <l@damenly.su> 撰写 
>  ----
>  >
>  > Cc to the author and linux-xfs, since it's xfsprogs related.
>  >
>  > On Tue 23 Feb 2021 at 21:40, Chengguang Xu 
>  > <cgxu519@mykernel.net>
>  > wrote:
>  >
>  > > It seems the expected result of testcase of "Hole + Data"
>  > > in generic/473 is not correct, so just fix it properly.
>  > >
>  >
>  > But it's not proper...
>  >
>  > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
>  > > ---
>  > >  tests/generic/473.out | 2 +-
>  > >  1 file changed, 1 insertion(+), 1 deletion(-)
>  > >
>  > > diff --git a/tests/generic/473.out b/tests/generic/473.out
>  > > index 75816388..f1ee5805 100644
>  > > --- a/tests/generic/473.out
>  > > +++ b/tests/generic/473.out
>  > > @@ -6,7 +6,7 @@ Data + Hole
>  > >  1: [256..287]: hole
>  > >  Hole + Data
>  > >  0: [0..127]: hole
>  > > -1: [128..255]: data
>  > > +1: [128..135]: data
>  > >
>  > The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" 
>  > $file |
>  > _filter_fiemap`.
>  > 0-64k is a hole and 64k-128k is a data extent.
>  > fiemap ioctl always returns *complete* ranges of extents.
>
> Manual testing result in latest kernel like below.
>
> [root@centos test]# uname -a
> Linux centos 5.11.0+ #5 SMP Tue Feb 23 21:02:27 CST 2021 x86_64 
> x86_64 x86_64 GNU/Linux
>
> [root@centos test]# xfs_io -V
> xfs_io version 5.0.0
>
> [root@centos test]# stat a
>   File: a
>   Size: 4194304         Blocks: 0          IO Block: 4096 
>   regular file
> Device: fc01h/64513d    Inode: 140         Links: 1
> Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/ 
> root)
> Access: 2021-02-24 16:33:20.235654140 +0800
> Modify: 2021-02-24 16:33:25.070641521 +0800
> Change: 2021-02-24 16:33:25.070641521 +0800
>  Birth: -
>
> [root@centos test]# xfs_io -c "pwrite 64k 64k" a
> wrote 65536/65536 bytes at offset 65536
> 64 KiB, 16 ops; 0.0000 sec (992.063 MiB/sec and 253968.2540 
> ops/sec)
>
> [root@VM-8-4-centos test]# xfs_io -c "fiemap -v 0 65k" a
> a:
>  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>    0: [0..127]:        hole               128
>    1: [128..135]:      360..367             8   0x1
>

Sorry, my carelessness. I only checked btrfs implementation but 
xfs
and ext4 do return the change you made.


> [root@centos test]# xfs_io -c "fiemap -v 0 128k" a
> a:
>  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>    0: [0..127]:        hole               128
>    1: [128..255]:      360..487           128   0x1
>
>
>  >
>  > You may ask why the ending hole range is not aligned to 128 
>  > in
>  > 473.out. Because
>  > fiemap ioctl returns nothing of querying holes. xfs_io does 
>  > the
>  > extra
>  > print work for holes.
>  >
>  > xfsprogs-dev/io/fiemap.c:
>  > for holes:
>  >  153     if (lstart > llast) {
>  >  154         print_hole(0, 0, 0, cur_extent, lflag, true, 
>  >  llast,
>  >  lstart);
>  >  155         cur_extent++;
>  >  156         num_printed++;
>  >  157     }
>  >
>  > for the ending hole:
>  >   381     if (cur_extent && last_logical < range_end)
>  >   382         print_hole(foff_w, boff_w, tot_w, cur_extent, 
>  >   lflag,
>  >   !vflag,
>  >   383                BTOBBT(last_logical), 
>  >   BTOBBT(range_end));
>  >
>  > >  Hole + Data + Hole
>  > >  0: [0..127]: hole
>  > >  1: [128..255]: data
>  >
Chengguang Xu Feb. 24, 2021, 9:37 a.m. UTC | #7
---- 在 星期三, 2021-02-24 17:22:35 Su Yue <l@damenly.su> 撰写 ----
 > 
 > On Wed 24 Feb 2021 at 16:51, Chengguang Xu <cgxu519@mykernel.net> 
 > wrote:
 > 
 > >  ---- 在 星期三, 2021-02-24 15:52:17 Su Yue <l@damenly.su> 撰写 
 > >  ----
 > >  >
 > >  > Cc to the author and linux-xfs, since it's xfsprogs related.
 > >  >
 > >  > On Tue 23 Feb 2021 at 21:40, Chengguang Xu 
 > >  > <cgxu519@mykernel.net>
 > >  > wrote:
 > >  >
 > >  > > It seems the expected result of testcase of "Hole + Data"
 > >  > > in generic/473 is not correct, so just fix it properly.
 > >  > >
 > >  >
 > >  > But it's not proper...
 > >  >
 > >  > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > >  > > ---
 > >  > >  tests/generic/473.out | 2 +-
 > >  > >  1 file changed, 1 insertion(+), 1 deletion(-)
 > >  > >
 > >  > > diff --git a/tests/generic/473.out b/tests/generic/473.out
 > >  > > index 75816388..f1ee5805 100644
 > >  > > --- a/tests/generic/473.out
 > >  > > +++ b/tests/generic/473.out
 > >  > > @@ -6,7 +6,7 @@ Data + Hole
 > >  > >  1: [256..287]: hole
 > >  > >  Hole + Data
 > >  > >  0: [0..127]: hole
 > >  > > -1: [128..255]: data
 > >  > > +1: [128..135]: data
 > >  > >
 > >  > The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" 
 > >  > $file |
 > >  > _filter_fiemap`.
 > >  > 0-64k is a hole and 64k-128k is a data extent.
 > >  > fiemap ioctl always returns *complete* ranges of extents.
 > >
 > > Manual testing result in latest kernel like below.
 > >
 > > [root@centos test]# uname -a
 > > Linux centos 5.11.0+ #5 SMP Tue Feb 23 21:02:27 CST 2021 x86_64 
 > > x86_64 x86_64 GNU/Linux
 > >
 > > [root@centos test]# xfs_io -V
 > > xfs_io version 5.0.0
 > >
 > > [root@centos test]# stat a
 > >   File: a
 > >   Size: 4194304         Blocks: 0          IO Block: 4096 
 > >   regular file
 > > Device: fc01h/64513d    Inode: 140         Links: 1
 > > Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/ 
 > > root)
 > > Access: 2021-02-24 16:33:20.235654140 +0800
 > > Modify: 2021-02-24 16:33:25.070641521 +0800
 > > Change: 2021-02-24 16:33:25.070641521 +0800
 > >  Birth: -
 > >
 > > [root@centos test]# xfs_io -c "pwrite 64k 64k" a
 > > wrote 65536/65536 bytes at offset 65536
 > > 64 KiB, 16 ops; 0.0000 sec (992.063 MiB/sec and 253968.2540 
 > > ops/sec)
 > >
 > > [root@VM-8-4-centos test]# xfs_io -c "fiemap -v 0 65k" a
 > > a:
 > >  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
 > >    0: [0..127]:        hole               128
 > >    1: [128..135]:      360..367             8   0x1
 > >
 > 
 > Sorry, my carelessness. I only checked btrfs implementation but 
 > xfs
 > and ext4 do return the change you made.
 > 

Yeah, it seems there is no bad side effect to show  only specified range of extents
and keep all the same behavior is also good for testing. I can post a fix patch for
this but before that let us to wait some feedback from maintainers and experts.

Thanks,
Chengguang


 > 
 > > [root@centos test]# xfs_io -c "fiemap -v 0 128k" a
 > > a:
 > >  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
 > >    0: [0..127]:        hole               128
 > >    1: [128..255]:      360..487           128   0x1
 > >
 > >
 > >  >
 > >  > You may ask why the ending hole range is not aligned to 128 
 > >  > in
 > >  > 473.out. Because
 > >  > fiemap ioctl returns nothing of querying holes. xfs_io does 
 > >  > the
 > >  > extra
 > >  > print work for holes.
 > >  >
 > >  > xfsprogs-dev/io/fiemap.c:
 > >  > for holes:
 > >  >  153     if (lstart > llast) {
 > >  >  154         print_hole(0, 0, 0, cur_extent, lflag, true, 
 > >  >  llast,
 > >  >  lstart);
 > >  >  155         cur_extent++;
 > >  >  156         num_printed++;
 > >  >  157     }
 > >  >
 > >  > for the ending hole:
 > >  >   381     if (cur_extent && last_logical < range_end)
 > >  >   382         print_hole(foff_w, boff_w, tot_w, cur_extent, 
 > >  >   lflag,
 > >  >   !vflag,
 > >  >   383                BTOBBT(last_logical), 
 > >  >   BTOBBT(range_end));
 > >  >
 > >  > >  Hole + Data + Hole
 > >  > >  0: [0..127]: hole
 > >  > >  1: [128..255]: data
 > >  >
 > 
 >
Eryu Guan Feb. 24, 2021, 1:31 p.m. UTC | #8
On Wed, Feb 24, 2021 at 05:37:20PM +0800, Chengguang Xu wrote:
>  ---- 在 星期三, 2021-02-24 17:22:35 Su Yue <l@damenly.su> 撰写 ----
>  > 
>  > On Wed 24 Feb 2021 at 16:51, Chengguang Xu <cgxu519@mykernel.net> 
>  > wrote:
>  > 
>  > >  ---- 在 星期三, 2021-02-24 15:52:17 Su Yue <l@damenly.su> 撰写 
>  > >  ----
>  > >  >
>  > >  > Cc to the author and linux-xfs, since it's xfsprogs related.
>  > >  >
>  > >  > On Tue 23 Feb 2021 at 21:40, Chengguang Xu 
>  > >  > <cgxu519@mykernel.net>
>  > >  > wrote:
>  > >  >
>  > >  > > It seems the expected result of testcase of "Hole + Data"
>  > >  > > in generic/473 is not correct, so just fix it properly.
>  > >  > >
>  > >  >
>  > >  > But it's not proper...
>  > >  >
>  > >  > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
>  > >  > > ---
>  > >  > >  tests/generic/473.out | 2 +-
>  > >  > >  1 file changed, 1 insertion(+), 1 deletion(-)
>  > >  > >
>  > >  > > diff --git a/tests/generic/473.out b/tests/generic/473.out
>  > >  > > index 75816388..f1ee5805 100644
>  > >  > > --- a/tests/generic/473.out
>  > >  > > +++ b/tests/generic/473.out
>  > >  > > @@ -6,7 +6,7 @@ Data + Hole
>  > >  > >  1: [256..287]: hole
>  > >  > >  Hole + Data
>  > >  > >  0: [0..127]: hole
>  > >  > > -1: [128..255]: data
>  > >  > > +1: [128..135]: data
>  > >  > >
>  > >  > The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" 
>  > >  > $file |
>  > >  > _filter_fiemap`.
>  > >  > 0-64k is a hole and 64k-128k is a data extent.
>  > >  > fiemap ioctl always returns *complete* ranges of extents.
>  > >
>  > > Manual testing result in latest kernel like below.
>  > >
>  > > [root@centos test]# uname -a
>  > > Linux centos 5.11.0+ #5 SMP Tue Feb 23 21:02:27 CST 2021 x86_64 
>  > > x86_64 x86_64 GNU/Linux
>  > >
>  > > [root@centos test]# xfs_io -V
>  > > xfs_io version 5.0.0
>  > >
>  > > [root@centos test]# stat a
>  > >   File: a
>  > >   Size: 4194304         Blocks: 0          IO Block: 4096 
>  > >   regular file
>  > > Device: fc01h/64513d    Inode: 140         Links: 1
>  > > Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/ 
>  > > root)
>  > > Access: 2021-02-24 16:33:20.235654140 +0800
>  > > Modify: 2021-02-24 16:33:25.070641521 +0800
>  > > Change: 2021-02-24 16:33:25.070641521 +0800
>  > >  Birth: -
>  > >
>  > > [root@centos test]# xfs_io -c "pwrite 64k 64k" a
>  > > wrote 65536/65536 bytes at offset 65536
>  > > 64 KiB, 16 ops; 0.0000 sec (992.063 MiB/sec and 253968.2540 
>  > > ops/sec)
>  > >
>  > > [root@VM-8-4-centos test]# xfs_io -c "fiemap -v 0 65k" a
>  > > a:
>  > >  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
>  > >    0: [0..127]:        hole               128
>  > >    1: [128..135]:      360..367             8   0x1
>  > >
>  > 
>  > Sorry, my carelessness. I only checked btrfs implementation but 
>  > xfs
>  > and ext4 do return the change you made.
>  > 
> 
> Yeah, it seems there is no bad side effect to show  only specified range of extents
> and keep all the same behavior is also good for testing. I can post a fix patch for
> this but before that let us to wait some feedback from maintainers and experts.

generic/473 is marked as broken by commit 715eac1a9e66 ("generic/47[23]:
remove from auto/quick groups").

Thanks,
Eryu
Chengguang Xu Feb. 24, 2021, 1:48 p.m. UTC | #9
---- 在 星期三, 2021-02-24 21:31:46 Eryu Guan <eguan@linux.alibaba.com> 撰写 ----
 > On Wed, Feb 24, 2021 at 05:37:20PM +0800, Chengguang Xu wrote:
 > >  ---- 在 星期三, 2021-02-24 17:22:35 Su Yue <l@damenly.su> 撰写 ----
 > >  > 
 > >  > On Wed 24 Feb 2021 at 16:51, Chengguang Xu <cgxu519@mykernel.net> 
 > >  > wrote:
 > >  > 
 > >  > >  ---- 在 星期三, 2021-02-24 15:52:17 Su Yue <l@damenly.su> 撰写 
 > >  > >  ----
 > >  > >  >
 > >  > >  > Cc to the author and linux-xfs, since it's xfsprogs related.
 > >  > >  >
 > >  > >  > On Tue 23 Feb 2021 at 21:40, Chengguang Xu 
 > >  > >  > <cgxu519@mykernel.net>
 > >  > >  > wrote:
 > >  > >  >
 > >  > >  > > It seems the expected result of testcase of "Hole + Data"
 > >  > >  > > in generic/473 is not correct, so just fix it properly.
 > >  > >  > >
 > >  > >  >
 > >  > >  > But it's not proper...
 > >  > >  >
 > >  > >  > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > >  > >  > > ---
 > >  > >  > >  tests/generic/473.out | 2 +-
 > >  > >  > >  1 file changed, 1 insertion(+), 1 deletion(-)
 > >  > >  > >
 > >  > >  > > diff --git a/tests/generic/473.out b/tests/generic/473.out
 > >  > >  > > index 75816388..f1ee5805 100644
 > >  > >  > > --- a/tests/generic/473.out
 > >  > >  > > +++ b/tests/generic/473.out
 > >  > >  > > @@ -6,7 +6,7 @@ Data + Hole
 > >  > >  > >  1: [256..287]: hole
 > >  > >  > >  Hole + Data
 > >  > >  > >  0: [0..127]: hole
 > >  > >  > > -1: [128..255]: data
 > >  > >  > > +1: [128..135]: data
 > >  > >  > >
 > >  > >  > The line is produced by `$XFS_IO_PROG -c "fiemap -v 0 65k" 
 > >  > >  > $file |
 > >  > >  > _filter_fiemap`.
 > >  > >  > 0-64k is a hole and 64k-128k is a data extent.
 > >  > >  > fiemap ioctl always returns *complete* ranges of extents.
 > >  > >
 > >  > > Manual testing result in latest kernel like below.
 > >  > >
 > >  > > [root@centos test]# uname -a
 > >  > > Linux centos 5.11.0+ #5 SMP Tue Feb 23 21:02:27 CST 2021 x86_64 
 > >  > > x86_64 x86_64 GNU/Linux
 > >  > >
 > >  > > [root@centos test]# xfs_io -V
 > >  > > xfs_io version 5.0.0
 > >  > >
 > >  > > [root@centos test]# stat a
 > >  > >   File: a
 > >  > >   Size: 4194304         Blocks: 0          IO Block: 4096 
 > >  > >   regular file
 > >  > > Device: fc01h/64513d    Inode: 140         Links: 1
 > >  > > Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/ 
 > >  > > root)
 > >  > > Access: 2021-02-24 16:33:20.235654140 +0800
 > >  > > Modify: 2021-02-24 16:33:25.070641521 +0800
 > >  > > Change: 2021-02-24 16:33:25.070641521 +0800
 > >  > >  Birth: -
 > >  > >
 > >  > > [root@centos test]# xfs_io -c "pwrite 64k 64k" a
 > >  > > wrote 65536/65536 bytes at offset 65536
 > >  > > 64 KiB, 16 ops; 0.0000 sec (992.063 MiB/sec and 253968.2540 
 > >  > > ops/sec)
 > >  > >
 > >  > > [root@VM-8-4-centos test]# xfs_io -c "fiemap -v 0 65k" a
 > >  > > a:
 > >  > >  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
 > >  > >    0: [0..127]:        hole               128
 > >  > >    1: [128..135]:      360..367             8   0x1
 > >  > >
 > >  > 
 > >  > Sorry, my carelessness. I only checked btrfs implementation but 
 > >  > xfs
 > >  > and ext4 do return the change you made.
 > >  > 
 > > 
 > > Yeah, it seems there is no bad side effect to show  only specified range of extents
 > > and keep all the same behavior is also good for testing. I can post a fix patch for
 > > this but before that let us to wait some feedback from maintainers and experts.
 > 
 > generic/473 is marked as broken by commit 715eac1a9e66 ("generic/47[23]:
 > remove from auto/quick groups").
 > 

I got it, thanks Eryu!
Darrick J. Wong Feb. 24, 2021, 4:50 p.m. UTC | #10
On Tue, Feb 23, 2021 at 09:40:42PM +0800, Chengguang Xu wrote:
> It seems the expected result of testcase of "Hole + Data"
> in generic/473 is not correct, so just fix it properly.
> 
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
>  tests/generic/473.out | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/473.out b/tests/generic/473.out
> index 75816388..f1ee5805 100644
> --- a/tests/generic/473.out
> +++ b/tests/generic/473.out
> @@ -6,7 +6,7 @@ Data + Hole
>  1: [256..287]: hole
>  Hole + Data
>  0: [0..127]: hole
> -1: [128..255]: data
> +1: [128..135]: data

This again.  While the FIEMAP documentation allows the call to return
file mapping data outside the requested range, it doesn't require it,
and neither XFS nor ext4 have ever done so.

This test *enforces* that the FIEMAP implementation provide data outside
the requested range, which means it has never passed on xfs/ext4.  This
is no surprise, since it's enforcing one behavior where the spec allows
for two behaviors.  The only fs I know of where it passes is btrfs.

Delete this test or move it to tests/btrfs/, because it should not have
been added in the first place.

--D

>  Hole + Data + Hole
>  0: [0..127]: hole
>  1: [128..255]: data
> -- 
> 2.27.0
> 
>
Chengguang Xu Feb. 25, 2021, 2:16 a.m. UTC | #11
---- 在 星期四, 2021-02-25 00:50:57 Darrick J. Wong <djwong@kernel.org> 撰写 ----
 > On Tue, Feb 23, 2021 at 09:40:42PM +0800, Chengguang Xu wrote:
 > > It seems the expected result of testcase of "Hole + Data"
 > > in generic/473 is not correct, so just fix it properly.
 > > 
 > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > > ---
 > >  tests/generic/473.out | 2 +-
 > >  1 file changed, 1 insertion(+), 1 deletion(-)
 > > 
 > > diff --git a/tests/generic/473.out b/tests/generic/473.out
 > > index 75816388..f1ee5805 100644
 > > --- a/tests/generic/473.out
 > > +++ b/tests/generic/473.out
 > > @@ -6,7 +6,7 @@ Data + Hole
 > >  1: [256..287]: hole
 > >  Hole + Data
 > >  0: [0..127]: hole
 > > -1: [128..255]: data
 > > +1: [128..135]: data
 > 
 > This again.  While the FIEMAP documentation allows the call to return
 > file mapping data outside the requested range, it doesn't require it,
 > and neither XFS nor ext4 have ever done so.

I did more tests on ext4 and found in old 4.14 kernel ext4 returned whole extent range.


 > 
 > This test *enforces* that the FIEMAP implementation provide data outside
 > the requested range, which means it has never passed on xfs/ext4.  This
 > is no surprise, since it's enforcing one behavior where the spec allows
 > for two behaviors.  The only fs I know of where it passes is btrfs.

plus f2fs.

Thanks,
Chengguang

 > 
 > Delete this test or move it to tests/btrfs/, because it should not have
 > been added in the first place.
 > 
 > --D
 > 
 > >  Hole + Data + Hole
 > >  0: [0..127]: hole
 > >  1: [128..255]: data
 > > -- 
 > > 2.27.0
 > > 
 > > 
 >
diff mbox series

Patch

diff --git a/tests/generic/473.out b/tests/generic/473.out
index 75816388..f1ee5805 100644
--- a/tests/generic/473.out
+++ b/tests/generic/473.out
@@ -6,7 +6,7 @@  Data + Hole
 1: [256..287]: hole
 Hole + Data
 0: [0..127]: hole
-1: [128..255]: data
+1: [128..135]: data
 Hole + Data + Hole
 0: [0..127]: hole
 1: [128..255]: data