Re: [PATCH 3/3] xfs/547: fix problems with realtime

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jul 07, 2022 at 09:15:27PM +0800, Zorro Lang wrote:
> On Tue, Jul 05, 2022 at 03:02:19PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > This test needs to fragment the free space on the data device so that
> > each block added to the attr fork gets its own mapping.  If the test
> > configuration sets up a rt device and rtinherit=1 on the root dir, the
> > test will erroneously fragment space on the *realtime* volume.  When
> > this happens, attr fork allocations are contiguous and get merged into
> > fewer than 10 extents and the test fails.
> > 
> > Fix this test to force all allocations to be on the data device, and fix
> > incorrect variable usage in the error messages.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/547 |   14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > 
> > diff --git a/tests/xfs/547 b/tests/xfs/547
> > index 9d4216ca..60121eb9 100755
> > --- a/tests/xfs/547
> > +++ b/tests/xfs/547
> > @@ -33,6 +33,10 @@ for nrext64 in 0 1; do
> >  		      >> $seqres.full
> >  	_scratch_mount >> $seqres.full
> >  
> > +	# Force data device extents so that we can fragment the free space
> > +	# and force attr fork allocations to be non-contiguous
> > +	_xfs_force_bdev data $SCRATCH_MNT
> > +
> >  	bsize=$(_get_file_block_size $SCRATCH_MNT)
> >  
> >  	testfile=$SCRATCH_MNT/testfile
> > @@ -76,13 +80,15 @@ for nrext64 in 0 1; do
> >  	acnt=$(_scratch_xfs_get_metadata_field core.naextents \
> >  					       "path /$(basename $testfile)")
> >  
> > -	if (( $dcnt != 10 )); then
> > -		echo "Invalid data fork extent count: $dextcnt"
> > +	echo "nrext64: $nrext64 dcnt: $dcnt acnt: $acnt" >> $seqres.full
> > +
> > +	if [ -z "$dcnt" ] || (( $dcnt != 10 )); then
> 
> I'm wondering why we need to use bash ((...)) operator at here, is $dcnt
> an expression? Can [ "$dcnt" != "10" ] help that?

dcnt should be a decimal number, or the empty string if the xfs_db
totally failed.  The fancy comparison protects against xfs_db someday
returning results in hexadecimal or a non-number string, but I don't
think it'll ever do that.  I think your suggestion would work for this
case.

I don't think it works so well for the second case, since the fancy
comparison "(( $acnt < 10 ))" apparently returns false even if acnt is
non-numeric, whereas "[ $acnt -lt 10 ]" would error out.

--D

> Thanks,
> Zorro
> 
> > +		echo "Invalid data fork extent count: $dcnt"
> >  		exit 1
> >  	fi
> >  
> > -	if (( $acnt < 10 )); then
> > -		echo "Invalid attr fork extent count: $aextcnt"
> > +	if [ -z "$acnt" ] || (( $acnt < 10 )); then
> > +		echo "Invalid attr fork extent count: $acnt"
> >  		exit 1
> >  	fi
> >  done
> > 
>