generic/365: Fix false failure when mapping ends with free space

[PATCH] generic/365: Fix false failure when mapping ends with free space

Posted by Ojaswin Mujoo 2 months ago

If we have a small FS where the first free space mapping is also the
last mapping of the FS, then the following sub-test fails:

  echo "test whatever came after freesp"
  $XFS_IO_PROG -c "fsmap -d $((freesp_end + 2)) $((freesp_end + 3))" $SCRATCH_MNT

since there is nothing after the freespace. Fix this by punching a 1M
hole in a 3M file to ensure that the first free space is always
surrounded by allocated blocks.

Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 tests/generic/365 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/generic/365 b/tests/generic/365
index 36cb2530..bbadae71 100755
--- a/tests/generic/365
+++ b/tests/generic/365
@@ -32,6 +32,10 @@ if ((blksz < 2048)); then
 	_notrun "test requires at least 4 bblocks per fsblock"
 fi
 
+# This makes sure there is free space surrounded by allocated blocks, which
+# is needed for some sub tests.
+$XFS_IO_PROG -fc 'falloc 0 3M' -c 'fpunch 1M 1M' -c 'fsync' $SCRATCH_MNT/f
+
 $XFS_IO_PROG -c 'fsmap' $SCRATCH_MNT >> $seqres.full
 
 find_freesp() {
-- 
2.49.0

Re: [PATCH] generic/365: Fix false failure when mapping ends with free space

Posted by Zorro Lang 1 month, 1 week ago

On Tue, Aug 05, 2025 at 02:55:56PM +0530, Ojaswin Mujoo wrote:
> If we have a small FS where the first free space mapping is also the
> last mapping of the FS, then the following sub-test fails:
> 
>   echo "test whatever came after freesp"
>   $XFS_IO_PROG -c "fsmap -d $((freesp_end + 2)) $((freesp_end + 3))" $SCRATCH_MNT
> 
> since there is nothing after the freespace. Fix this by punching a 1M
> hole in a 3M file to ensure that the first free space is always
> surrounded by allocated blocks.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> ---
>  tests/generic/365 | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/generic/365 b/tests/generic/365
> index 36cb2530..bbadae71 100755
> --- a/tests/generic/365
> +++ b/tests/generic/365
> @@ -32,6 +32,10 @@ if ((blksz < 2048)); then
>  	_notrun "test requires at least 4 bblocks per fsblock"
>  fi
>  
> +# This makes sure there is free space surrounded by allocated blocks, which
> +# is needed for some sub tests.
> +$XFS_IO_PROG -fc 'falloc 0 3M' -c 'fpunch 1M 1M' -c 'fsync' $SCRATCH_MNT/f

If you add "falloc" and "fpunch" operations, you need to:

_require_xfs_io_command "falloc"
_require_xfs_io_command "fpunch"

Due to not all fileystems support these two fs operations.

BTW, I'm wondering how small (and what kind of) the fs you use, cause there's only
one unused region, even this's a clean fs, there're some still many different
metadatas. I even tried a 100M ext2 fs (which doesn't has log space), there're
many free space regions. So I'm curious, how can you hit this issue? And if the
SCRATCH_DEV is too small (e.g. 10M), do we really need to test with that fs size:)

Thanks,
Zorro

> +
>  $XFS_IO_PROG -c 'fsmap' $SCRATCH_MNT >> $seqres.full
>  
>  find_freesp() {
> -- 
> 2.49.0
>

Re: [PATCH] generic/365: Fix false failure when mapping ends with free space

Posted by Ojaswin Mujoo 1 month, 1 week ago

On Mon, Aug 25, 2025 at 11:29:25PM +0800, Zorro Lang wrote:
> On Tue, Aug 05, 2025 at 02:55:56PM +0530, Ojaswin Mujoo wrote:
> > If we have a small FS where the first free space mapping is also the
> > last mapping of the FS, then the following sub-test fails:
> > 
> >   echo "test whatever came after freesp"
> >   $XFS_IO_PROG -c "fsmap -d $((freesp_end + 2)) $((freesp_end + 3))" $SCRATCH_MNT
> > 
> > since there is nothing after the freespace. Fix this by punching a 1M
> > hole in a 3M file to ensure that the first free space is always
> > surrounded by allocated blocks.
> > 
> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > ---
> >  tests/generic/365 | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tests/generic/365 b/tests/generic/365
> > index 36cb2530..bbadae71 100755
> > --- a/tests/generic/365
> > +++ b/tests/generic/365
> > @@ -32,6 +32,10 @@ if ((blksz < 2048)); then
> >  	_notrun "test requires at least 4 bblocks per fsblock"
> >  fi
> >  
> > +# This makes sure there is free space surrounded by allocated blocks, which
> > +# is needed for some sub tests.
> > +$XFS_IO_PROG -fc 'falloc 0 3M' -c 'fpunch 1M 1M' -c 'fsync' $SCRATCH_MNT/f
> 
> If you add "falloc" and "fpunch" operations, you need to:
> 
> _require_xfs_io_command "falloc"
> _require_xfs_io_command "fpunch"

Hey Zorro thanks for the review and sorry I keep missing this helper :/ 
I'll fix it.

> 
> Due to not all fileystems support these two fs operations.
> 
> BTW, I'm wondering how small (and what kind of) the fs you use, cause there's only
> one unused region, even this's a clean fs, there're some still many different
> metadatas. I even tried a 100M ext2 fs (which doesn't has log space), there're
> many free space regions. So I'm curious, how can you hit this issue? And if the
> SCRATCH_DEV is too small (e.g. 10M), do we really need to test with that fs size:)

Right so actually we used the standard 5G SCRATCH_DEV however we were
testing for 64kb blocksize as well as ext4 bigalloc:

 $ mkfs.ext4 -b 65536 -O bigalloc $SCRATCH_DEV

which can actually format an ext4 FS that can hold 65528 * 64KB = ~40G
in a single block group, so we end up with 1 block group where metadata
is at the top and free space is in the end.

Adding a small file like above seems like a easy and universal way of
getting around this issue.

Regards,
ojaswin

> 
> Thanks,
> Zorro

> 
> > +
> >  $XFS_IO_PROG -c 'fsmap' $SCRATCH_MNT >> $seqres.full
> >  
> >  find_freesp() {
> > -- 
> > 2.49.0
> > 
>

Re: [PATCH] generic/365: Fix false failure when mapping ends with free space

Posted by Zorro Lang 1 month ago

On Thu, Aug 28, 2025 at 08:30:59PM +0530, Ojaswin Mujoo wrote:
> On Mon, Aug 25, 2025 at 11:29:25PM +0800, Zorro Lang wrote:
> > On Tue, Aug 05, 2025 at 02:55:56PM +0530, Ojaswin Mujoo wrote:
> > > If we have a small FS where the first free space mapping is also the
> > > last mapping of the FS, then the following sub-test fails:
> > > 
> > >   echo "test whatever came after freesp"
> > >   $XFS_IO_PROG -c "fsmap -d $((freesp_end + 2)) $((freesp_end + 3))" $SCRATCH_MNT
> > > 
> > > since there is nothing after the freespace. Fix this by punching a 1M
> > > hole in a 3M file to ensure that the first free space is always
> > > surrounded by allocated blocks.
> > > 
> > > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > > ---
> > >  tests/generic/365 | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/tests/generic/365 b/tests/generic/365
> > > index 36cb2530..bbadae71 100755
> > > --- a/tests/generic/365
> > > +++ b/tests/generic/365
> > > @@ -32,6 +32,10 @@ if ((blksz < 2048)); then
> > >  	_notrun "test requires at least 4 bblocks per fsblock"
> > >  fi
> > >  
> > > +# This makes sure there is free space surrounded by allocated blocks, which
> > > +# is needed for some sub tests.
> > > +$XFS_IO_PROG -fc 'falloc 0 3M' -c 'fpunch 1M 1M' -c 'fsync' $SCRATCH_MNT/f
> > 
> > If you add "falloc" and "fpunch" operations, you need to:
> > 
> > _require_xfs_io_command "falloc"
> > _require_xfs_io_command "fpunch"
> 
> Hey Zorro thanks for the review and sorry I keep missing this helper :/ 
> I'll fix it.
> 
> > 
> > Due to not all fileystems support these two fs operations.
> > 
> > BTW, I'm wondering how small (and what kind of) the fs you use, cause there's only
> > one unused region, even this's a clean fs, there're some still many different
> > metadatas. I even tried a 100M ext2 fs (which doesn't has log space), there're
> > many free space regions. So I'm curious, how can you hit this issue? And if the
> > SCRATCH_DEV is too small (e.g. 10M), do we really need to test with that fs size:)
> 
> Right so actually we used the standard 5G SCRATCH_DEV however we were
> testing for 64kb blocksize as well as ext4 bigalloc:
> 
>  $ mkfs.ext4 -b 65536 -O bigalloc $SCRATCH_DEV
> 
> which can actually format an ext4 FS that can hold 65528 * 64KB = ~40G
> in a single block group, so we end up with 1 block group where metadata
> is at the top and free space is in the end.
> 
> Adding a small file like above seems like a easy and universal way of
> getting around this issue.

Thanks Ojaswin, if so I think we can accept this change :)

Reviewed-by: Zorro Lang <zlang@redhat.com>

> 
> Regards,
> ojaswin
> 
> > 
> > Thanks,
> > Zorro
> 
> > 
> > > +
> > >  $XFS_IO_PROG -c 'fsmap' $SCRATCH_MNT >> $seqres.full
> > >  
> > >  find_freesp() {
> > > -- 
> > > 2.49.0
> > > 
> > 
>

Re: [PATCH] generic/365: Fix false failure when mapping ends with free space

Posted by Disha Goel 1 month, 2 weeks ago

On 05/08/25 2:55 pm, Ojaswin Mujoo wrote:
> If we have a small FS where the first free space mapping is also the
> last mapping of the FS, then the following sub-test fails:
> 
>    echo "test whatever came after freesp"
>    $XFS_IO_PROG -c "fsmap -d $((freesp_end + 2)) $((freesp_end + 3))" $SCRATCH_MNT
> 
> since there is nothing after the freespace. Fix this by punching a 1M
> hole in a 3M file to ensure that the first free space is always
> surrounded by allocated blocks.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

I tested the patch on Power, and it resolves the test failures with a 
64k block size.

Tested-by: Disha Goel <disgoel@linux.ibm.com>

SECTION       -- ext4_64k
RECREATING    -- ext4 on /dev/loop0
FSTYP         -- ext4
PLATFORM      -- Linux/ppc64le localhost 6.17.0-rc2-00060-g068a56e56fa8 
#2 SMP Thu Aug 21 16:28:18 IST 2025
MKFS_OPTIONS  -- -F -b 65536 /dev/loop1
MOUNT_OPTIONS -- -o block_validity /dev/loop1 /mnt/scratch

generic/365 1s ...  1s
Ran: generic/365
Passed all 1 tests

> ---
>   tests/generic/365 | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/tests/generic/365 b/tests/generic/365
> index 36cb2530..bbadae71 100755
> --- a/tests/generic/365
> +++ b/tests/generic/365
> @@ -32,6 +32,10 @@ if ((blksz < 2048)); then
>   	_notrun "test requires at least 4 bblocks per fsblock"
>   fi
>   
> +# This makes sure there is free space surrounded by allocated blocks, which
> +# is needed for some sub tests.
> +$XFS_IO_PROG -fc 'falloc 0 3M' -c 'fpunch 1M 1M' -c 'fsync' $SCRATCH_MNT/f
> +
>   $XFS_IO_PROG -c 'fsmap' $SCRATCH_MNT >> $seqres.full
>   
>   find_freesp() {