Re: [PATCH] generic/347: Fix sporadic test failures

Hi Zorro!

On Mon 03-08-26 16:27:15, Zorro Lang wrote:
> On Thu, Jul 30, 2026 at 05:18:17PM +0200, Jan Kara wrote:
> > generic/347 was occasionally failing on ext4 in our QA due to ext4
> > aborting its journal when the filesystem on thinp device was overfilled.
> > I have tracked the problem down to journal checkpointing failing to
> > write a metadata block to its final location due to ENOSPC failure from
> > the thinp device. Modify the test to first preallocate blocks for the
> > files and remount the filesystem which practically makes sure all
> > involved metadata blocks were written and so their further modifications
> > will not fail.
> > 
> > Signed-off-by: Jan Kara <[email protected]>
> > ---
> >  tests/generic/347 | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/generic/347 b/tests/generic/347
> > index 06df0cf9eddc..56538c160392 100755
> > --- a/tests/generic/347
> > +++ b/tests/generic/347
> > @@ -38,7 +38,17 @@ _setup_thin()
> >  
> >  _workout()
> >  {
> > -	# Overfill it by a bit
> > +	# Preallocate space to avoid failure for metadata writeback
> > +	for I in `seq 1 500`; do
> > +		$XFS_IO_PROG -f -c "falloc 0 1M" $SCRATCH_MNT/file$I &>/dev/null
> 
> Thanks for this fix! Adding fallocate introduces an extra dependency via
> _require_xfs_io_command "falloc", which will limit some filesystems can
> run this test.

I agree the fix has some downsides. OTOH when I was thinking about it I've
concluded that filesystems where you realistically care about behavior on
thinp storage also do support fallocate. So I don't think this it's a
serious test coverage limitation but it's up for discussion.

> Additionally, since the underlying storage is a thinp device, I doubt
> `falloc 0 1M` actually triggers physical block allocation on the thin
> pool (correct me if I'm wrong). If so, I suspect this 1M * 500 preallocation
> might exhaust the 500M BACKING_SIZE prematurely (along with file system
> metadata overhead), similar to the pwrite loop below.

We do *not* want fallocate to trigger the data block allocation. It will
however modify all the relevant metadata blocks and subsequent unmount will
writeout these modified metadata which forces the physical block allocation
for the metadata which is what we need. Physical space for data blocks is
not reserved from thinp during fallocate so we should not run out of
backing device space during this loop.

								Honza

-- 
Jan Kara <[email protected]>
SUSE Labs, CR