[PATCHSET v2 0/3] fstests: random fixes

[PATCHSET v2 0/3] fstests: random fixes

[Darrick J. Wong]

Hi all,

Here's the usual batch of odd fixes for fstests.

v2: rebase against v2022.07.03, add another correction

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
---
 tests/xfs/018     |   52 +++++++++++++++++++++++++++++++++++++++++++++++-----
 tests/xfs/018.out |   16 ++++------------
 tests/xfs/144     |   14 +++++++++-----
 tests/xfs/547     |   14 ++++++++++----
 4 files changed, 70 insertions(+), 26 deletions(-)

[PATCH 1/3] xfs: fix test mkfs.xfs sizing of internal logs that overflow the AG

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Fix a few problems with this test -- one of the things we test require
mkfs to run in -N mode, so we need to have a certain amount of free
space, and fix that test not to use -N mode.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/144 |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)


diff --git a/tests/xfs/144 b/tests/xfs/144
index 2910eec9..706aff61 100755
--- a/tests/xfs/144
+++ b/tests/xfs/144
@@ -16,6 +16,10 @@ _begin_fstest auto mkfs
 # Modify as appropriate.
 _supported_fs xfs
 _require_test
+
+# The last testcase creates a (sparse) fs image with a 2GB log, so we need
+# 3GB to avoid failing the mkfs due to ENOSPC.
+_require_fs_space $TEST_DIR $((3 * 1048576))
 echo Silence is golden
 
 testfile=$TEST_DIR/a
@@ -26,7 +30,7 @@ test_format() {
 	shift
 
 	echo "$tag" >> $seqres.full
-	$MKFS_XFS_PROG $@ -d file,name=$testfile &>> $seqres.full
+	$MKFS_XFS_PROG -f $@ -d file,name=$testfile &>> $seqres.full
 	local res=$?
 	test $res -eq 0 || echo "$tag FAIL $res" | tee -a $seqres.full
 }
@@ -38,13 +42,13 @@ for M in `seq 298 302` `seq 490 520`; do
 	done
 done
 
+# log end rounded beyond EOAG due to stripe unit
+test_format "log end beyond eoag" -d agcount=3200,size=6366g -d su=256k,sw=4 -N
+
 # Log so large it pushes the root dir into AG 1.  We can't use -N for the mkfs
 # because this check only occurs after the root directory has been allocated,
 # which mkfs -N doesn't do.
-test_format "log pushes rootdir into AG 1" -d agcount=3200,size=6366g -lagnum=0 -N
-
-# log end rounded beyond EOAG due to stripe unit
-test_format "log end beyond eoag" -d agcount=3200,size=6366g -d su=256k,sw=4 -N
+test_format "log pushes rootdir into AG 1" -d agcount=3200,size=6366g -lagnum=0
 
 # success, all done
 status=0

[PATCH 2/3] xfs/018: fix LARP testing for small block sizes

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Fix this test to work properly when the filesystem block size is less
than 4k.  Tripping the error injection points on shape changes in the
xattr structure must be done dynamically.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/018     |   52 +++++++++++++++++++++++++++++++++++++++++++++++-----
 tests/xfs/018.out |   16 ++++------------
 2 files changed, 51 insertions(+), 17 deletions(-)


diff --git a/tests/xfs/018 b/tests/xfs/018
index 041a3b24..14a6f716 100755
--- a/tests/xfs/018
+++ b/tests/xfs/018
@@ -54,6 +54,45 @@ test_attr_replay()
 	echo ""
 }
 
+test_attr_replay_loop()
+{
+	testfile=$testdir/$1
+	attr_name=$2
+	attr_value=$3
+	flag=$4
+	error_tag=$5
+
+	# Inject error
+	_scratch_inject_error $error_tag
+
+	# Set attribute; hopefully 1000 of them is enough to cause whatever
+	# attr structure shape change that the caller wants to test.
+	for ((i = 0; i < 1024; i++)); do
+		echo "$attr_value" | \
+			${ATTR_PROG} -$flag "$attr_name$i" $testfile > $tmp.out 2> $tmp.err
+		cat $tmp.out $tmp.err >> $seqres.full
+		cat $tmp.err | _filter_scratch | sed -e 's/attr_name[0-9]*/attr_nameXXXX/g'
+		touch $testfile &>/dev/null || break
+	done
+
+	# FS should be shut down, touch will fail
+	touch $testfile 2>&1 | _filter_scratch
+
+	# Remount to replay log
+	_scratch_remount_dump_log >> $seqres.full
+
+	# FS should be online, touch should succeed
+	touch $testfile
+
+	# Verify attr recovery
+	$ATTR_PROG -l $testfile >> $seqres.full
+	echo "Checking contents of $attr_name$i" >> $seqres.full
+	echo -n "${attr_name}XXXX: "
+	$ATTR_PROG -q -g $attr_name$i $testfile 2> /dev/null | md5sum;
+
+	echo ""
+}
+
 create_test_file()
 {
 	filename=$testdir/$1
@@ -88,6 +127,7 @@ echo 1 > /sys/fs/xfs/debug/larp
 attr16="0123456789ABCDEF"
 attr64="$attr16$attr16$attr16$attr16"
 attr256="$attr64$attr64$attr64$attr64"
+attr512="$attr256$attr256"
 attr1k="$attr256$attr256$attr256$attr256"
 attr4k="$attr1k$attr1k$attr1k$attr1k"
 attr8k="$attr4k$attr4k"
@@ -140,12 +180,14 @@ test_attr_replay extent_file1 "attr_name2" $attr1k "s" "larp"
 test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
 
 # extent, inject error on split
-create_test_file extent_file2 3 $attr1k
-test_attr_replay extent_file2 "attr_name4" $attr1k "s" "da_leaf_split"
+create_test_file extent_file2 0 $attr1k
+test_attr_replay_loop extent_file2 "attr_name" $attr1k "s" "da_leaf_split"
 
-# extent, inject error on fork transition
-create_test_file extent_file3 3 $attr1k
-test_attr_replay extent_file3 "attr_name4" $attr1k "s" "attr_leaf_to_node"
+# extent, inject error on fork transition.  The attr value must be less than
+# a full filesystem block so that the attrs don't use remote xattr values,
+# which means we miss the leaf to node transition.
+create_test_file extent_file3 0 $attr1k
+test_attr_replay_loop extent_file3 "attr_name" $attr512 "s" "attr_leaf_to_node"
 
 # extent, remote
 create_test_file extent_file4 1 $attr1k
diff --git a/tests/xfs/018.out b/tests/xfs/018.out
index 022b0ca3..c3021ee3 100644
--- a/tests/xfs/018.out
+++ b/tests/xfs/018.out
@@ -87,22 +87,14 @@ Attribute "attr_name1" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file
 attr_name2: d41d8cd98f00b204e9800998ecf8427e  -
 
 attr_set: Input/output error
-Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file2
+Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file2
 touch: cannot touch 'SCRATCH_MNT/testdir/extent_file2': Input/output error
-Attribute "attr_name4" has a 1025 byte value for SCRATCH_MNT/testdir/extent_file2
-Attribute "attr_name2" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file2
-Attribute "attr_name3" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file2
-Attribute "attr_name1" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file2
-attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
+attr_nameXXXX: 9fd415c49d67afc4b78fad4055a3a376  -
 
 attr_set: Input/output error
-Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file3
+Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file3
 touch: cannot touch 'SCRATCH_MNT/testdir/extent_file3': Input/output error
-Attribute "attr_name4" has a 1025 byte value for SCRATCH_MNT/testdir/extent_file3
-Attribute "attr_name2" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file3
-Attribute "attr_name3" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file3
-Attribute "attr_name1" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file3
-attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
+attr_nameXXXX: a597dc41e4574873516420a7e4e5a3e0  -
 
 attr_set: Input/output error
 Could not set "attr_name2" for SCRATCH_MNT/testdir/extent_file4

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

[Darrick J. Wong]

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
+		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

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

[Zorro Lang]

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?

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
> 

Re: [PATCH 1/3] xfs: fix test mkfs.xfs sizing of internal logs that overflow the AG

[Zorro Lang]

On Tue, Jul 05, 2022 at 03:02:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Fix a few problems with this test -- one of the things we test require
> mkfs to run in -N mode, so we need to have a certain amount of free
> space, and fix that test not to use -N mode.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

This case has been merged, so it's fine to use "xfs/144: ..." as the subject
of this patch. Others looks good to me.

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

>  tests/xfs/144 |   14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> 
> diff --git a/tests/xfs/144 b/tests/xfs/144
> index 2910eec9..706aff61 100755
> --- a/tests/xfs/144
> +++ b/tests/xfs/144
> @@ -16,6 +16,10 @@ _begin_fstest auto mkfs
>  # Modify as appropriate.
>  _supported_fs xfs
>  _require_test
> +
> +# The last testcase creates a (sparse) fs image with a 2GB log, so we need
> +# 3GB to avoid failing the mkfs due to ENOSPC.
> +_require_fs_space $TEST_DIR $((3 * 1048576))
>  echo Silence is golden
>  
>  testfile=$TEST_DIR/a
> @@ -26,7 +30,7 @@ test_format() {
>  	shift
>  
>  	echo "$tag" >> $seqres.full
> -	$MKFS_XFS_PROG $@ -d file,name=$testfile &>> $seqres.full
> +	$MKFS_XFS_PROG -f $@ -d file,name=$testfile &>> $seqres.full
>  	local res=$?
>  	test $res -eq 0 || echo "$tag FAIL $res" | tee -a $seqres.full
>  }
> @@ -38,13 +42,13 @@ for M in `seq 298 302` `seq 490 520`; do
>  	done
>  done
>  
> +# log end rounded beyond EOAG due to stripe unit
> +test_format "log end beyond eoag" -d agcount=3200,size=6366g -d su=256k,sw=4 -N
> +
>  # Log so large it pushes the root dir into AG 1.  We can't use -N for the mkfs
>  # because this check only occurs after the root directory has been allocated,
>  # which mkfs -N doesn't do.
> -test_format "log pushes rootdir into AG 1" -d agcount=3200,size=6366g -lagnum=0 -N
> -
> -# log end rounded beyond EOAG due to stripe unit
> -test_format "log end beyond eoag" -d agcount=3200,size=6366g -d su=256k,sw=4 -N
> +test_format "log pushes rootdir into AG 1" -d agcount=3200,size=6366g -lagnum=0
>  
>  # success, all done
>  status=0
> 

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

[Darrick J. Wong]

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
> > 
> 

Re: [PATCH 2/3] xfs/018: fix LARP testing for small block sizes

[Darrick J. Wong]

I guess I should've cc'd Allison and Catherine on this one.

Could either of you review these test changes, please?

--D

On Tue, Jul 05, 2022 at 03:02:14PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Fix this test to work properly when the filesystem block size is less
> than 4k.  Tripping the error injection points on shape changes in the
> xattr structure must be done dynamically.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/018     |   52 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  tests/xfs/018.out |   16 ++++------------
>  2 files changed, 51 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/tests/xfs/018 b/tests/xfs/018
> index 041a3b24..14a6f716 100755
> --- a/tests/xfs/018
> +++ b/tests/xfs/018
> @@ -54,6 +54,45 @@ test_attr_replay()
>  	echo ""
>  }
>  
> +test_attr_replay_loop()
> +{
> +	testfile=$testdir/$1
> +	attr_name=$2
> +	attr_value=$3
> +	flag=$4
> +	error_tag=$5
> +
> +	# Inject error
> +	_scratch_inject_error $error_tag
> +
> +	# Set attribute; hopefully 1000 of them is enough to cause whatever
> +	# attr structure shape change that the caller wants to test.
> +	for ((i = 0; i < 1024; i++)); do
> +		echo "$attr_value" | \
> +			${ATTR_PROG} -$flag "$attr_name$i" $testfile > $tmp.out 2> $tmp.err
> +		cat $tmp.out $tmp.err >> $seqres.full
> +		cat $tmp.err | _filter_scratch | sed -e 's/attr_name[0-9]*/attr_nameXXXX/g'
> +		touch $testfile &>/dev/null || break
> +	done
> +
> +	# FS should be shut down, touch will fail
> +	touch $testfile 2>&1 | _filter_scratch
> +
> +	# Remount to replay log
> +	_scratch_remount_dump_log >> $seqres.full
> +
> +	# FS should be online, touch should succeed
> +	touch $testfile
> +
> +	# Verify attr recovery
> +	$ATTR_PROG -l $testfile >> $seqres.full
> +	echo "Checking contents of $attr_name$i" >> $seqres.full
> +	echo -n "${attr_name}XXXX: "
> +	$ATTR_PROG -q -g $attr_name$i $testfile 2> /dev/null | md5sum;
> +
> +	echo ""
> +}
> +
>  create_test_file()
>  {
>  	filename=$testdir/$1
> @@ -88,6 +127,7 @@ echo 1 > /sys/fs/xfs/debug/larp
>  attr16="0123456789ABCDEF"
>  attr64="$attr16$attr16$attr16$attr16"
>  attr256="$attr64$attr64$attr64$attr64"
> +attr512="$attr256$attr256"
>  attr1k="$attr256$attr256$attr256$attr256"
>  attr4k="$attr1k$attr1k$attr1k$attr1k"
>  attr8k="$attr4k$attr4k"
> @@ -140,12 +180,14 @@ test_attr_replay extent_file1 "attr_name2" $attr1k "s" "larp"
>  test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
>  
>  # extent, inject error on split
> -create_test_file extent_file2 3 $attr1k
> -test_attr_replay extent_file2 "attr_name4" $attr1k "s" "da_leaf_split"
> +create_test_file extent_file2 0 $attr1k
> +test_attr_replay_loop extent_file2 "attr_name" $attr1k "s" "da_leaf_split"
>  
> -# extent, inject error on fork transition
> -create_test_file extent_file3 3 $attr1k
> -test_attr_replay extent_file3 "attr_name4" $attr1k "s" "attr_leaf_to_node"
> +# extent, inject error on fork transition.  The attr value must be less than
> +# a full filesystem block so that the attrs don't use remote xattr values,
> +# which means we miss the leaf to node transition.
> +create_test_file extent_file3 0 $attr1k
> +test_attr_replay_loop extent_file3 "attr_name" $attr512 "s" "attr_leaf_to_node"
>  
>  # extent, remote
>  create_test_file extent_file4 1 $attr1k
> diff --git a/tests/xfs/018.out b/tests/xfs/018.out
> index 022b0ca3..c3021ee3 100644
> --- a/tests/xfs/018.out
> +++ b/tests/xfs/018.out
> @@ -87,22 +87,14 @@ Attribute "attr_name1" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file
>  attr_name2: d41d8cd98f00b204e9800998ecf8427e  -
>  
>  attr_set: Input/output error
> -Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file2
> +Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file2
>  touch: cannot touch 'SCRATCH_MNT/testdir/extent_file2': Input/output error
> -Attribute "attr_name4" has a 1025 byte value for SCRATCH_MNT/testdir/extent_file2
> -Attribute "attr_name2" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file2
> -Attribute "attr_name3" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file2
> -Attribute "attr_name1" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file2
> -attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
> +attr_nameXXXX: 9fd415c49d67afc4b78fad4055a3a376  -
>  
>  attr_set: Input/output error
> -Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file3
> +Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file3
>  touch: cannot touch 'SCRATCH_MNT/testdir/extent_file3': Input/output error
> -Attribute "attr_name4" has a 1025 byte value for SCRATCH_MNT/testdir/extent_file3
> -Attribute "attr_name2" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file3
> -Attribute "attr_name3" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file3
> -Attribute "attr_name1" has a 1024 byte value for SCRATCH_MNT/testdir/extent_file3
> -attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
> +attr_nameXXXX: a597dc41e4574873516420a7e4e5a3e0  -
>  
>  attr_set: Input/output error
>  Could not set "attr_name2" for SCRATCH_MNT/testdir/extent_file4
> 

Re: [PATCH 2/3] xfs/018: fix LARP testing for small block sizes

[Alli]

On Thu, 2022-07-07 at 11:06 -0700, Darrick J. Wong wrote:
> I guess I should've cc'd Allison and Catherine on this one.
> 
> Could either of you review these test changes, please?
> 
> --D
> 
> On Tue, Jul 05, 2022 at 03:02:14PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Fix this test to work properly when the filesystem block size is
> > less
> > than 4k.  Tripping the error injection points on shape changes in
> > the
> > xattr structure must be done dynamically.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/018     |   52
> > +++++++++++++++++++++++++++++++++++++++++++++++-----
> >  tests/xfs/018.out |   16 ++++------------
> >  2 files changed, 51 insertions(+), 17 deletions(-)
> > 
> > 
> > diff --git a/tests/xfs/018 b/tests/xfs/018
> > index 041a3b24..14a6f716 100755
> > --- a/tests/xfs/018
> > +++ b/tests/xfs/018
> > @@ -54,6 +54,45 @@ test_attr_replay()
> >  	echo ""
> >  }
> >  
> > +test_attr_replay_loop()
> > +{
> > +	testfile=$testdir/$1
> > +	attr_name=$2
> > +	attr_value=$3
> > +	flag=$4
> > +	error_tag=$5
> > +
> > +	# Inject error
> > +	_scratch_inject_error $error_tag
> > +
> > +	# Set attribute; hopefully 1000 of them is enough to cause
> > whatever
> > +	# attr structure shape change that the caller wants to test.
> > +	for ((i = 0; i < 1024; i++)); do
> > +		echo "$attr_value" | \
> > +			${ATTR_PROG} -$flag "$attr_name$i" $testfile >
> > $tmp.out 2> $tmp.err
> > +		cat $tmp.out $tmp.err >> $seqres.full
> > +		cat $tmp.err | _filter_scratch | sed -e 's/attr_name[0-
> > 9]*/attr_nameXXXX/g'
> > +		touch $testfile &>/dev/null || break
> > +	done
> > +
> > +	# FS should be shut down, touch will fail
> > +	touch $testfile 2>&1 | _filter_scratch
> > +
> > +	# Remount to replay log
> > +	_scratch_remount_dump_log >> $seqres.full
> > +
> > +	# FS should be online, touch should succeed
> > +	touch $testfile
> > +
> > +	# Verify attr recovery
> > +	$ATTR_PROG -l $testfile >> $seqres.full
> > +	echo "Checking contents of $attr_name$i" >> $seqres.full
> > +	echo -n "${attr_name}XXXX: "
> > +	$ATTR_PROG -q -g $attr_name$i $testfile 2> /dev/null | md5sum;
> > +
> > +	echo ""
> > +}
> > +


Ok, I think I see what you are trying to do, but I think we can do it
with less duplicated code and looping functions.  What about something
like this:

diff --git a/tests/xfs/018 b/tests/xfs/018
index 041a3b24..dc1324b1 100755
--- a/tests/xfs/018
+++ b/tests/xfs/018
@@ -95,6 +95,9 @@ attr16k="$attr8k$attr8k"
 attr32k="$attr16k$attr16k"
 attr64k="$attr32k$attr32k"
 
+blk_sz=$(_scratch_xfs_get_sb_field blocksize)
+multiplier=$(( $blk_sz / 256 ))
+
 echo "*** mkfs"
 _scratch_mkfs >/dev/null
 
@@ -140,7 +143,7 @@ test_attr_replay extent_file1 "attr_name2" $attr1k
"s" "larp"
 test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
 
 # extent, inject error on split
-create_test_file extent_file2 3 $attr1k
+create_test_file extent_file2 $(( $multiplier - 1 )) $attr256
 test_attr_replay extent_file2 "attr_name4" $attr1k "s" "da_leaf_split"
 
 # extent, inject error on fork transition



Same idea right?  We bring the attr fork right up and to the edge of
the block boundary and then pop it?  And then of course we apply the
same pattern to the rest of the tests.  I think that sort of reads
cleaner too.

Allison

> >  create_test_file()
> >  {
> >  	filename=$testdir/$1
> > @@ -88,6 +127,7 @@ echo 1 > /sys/fs/xfs/debug/larp
> >  attr16="0123456789ABCDEF"
> >  attr64="$attr16$attr16$attr16$attr16"
> >  attr256="$attr64$attr64$attr64$attr64"
> > +attr512="$attr256$attr256"
> >  attr1k="$attr256$attr256$attr256$attr256"
> >  attr4k="$attr1k$attr1k$attr1k$attr1k"
> >  attr8k="$attr4k$attr4k"
> > @@ -140,12 +180,14 @@ test_attr_replay extent_file1 "attr_name2"
> > $attr1k "s" "larp"
> >  test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
> >  
> >  # extent, inject error on split
> > -create_test_file extent_file2 3 $attr1k
> > -test_attr_replay extent_file2 "attr_name4" $attr1k "s"
> > "da_leaf_split"
> > +create_test_file extent_file2 0 $attr1k
> > +test_attr_replay_loop extent_file2 "attr_name" $attr1k "s"
> > "da_leaf_split"
> >  
> > -# extent, inject error on fork transition
> > -create_test_file extent_file3 3 $attr1k
> > -test_attr_replay extent_file3 "attr_name4" $attr1k "s"
> > "attr_leaf_to_node"
> > +# extent, inject error on fork transition.  The attr value must be
> > less than
> > +# a full filesystem block so that the attrs don't use remote xattr
> > values,
> > +# which means we miss the leaf to node transition.
> > +create_test_file extent_file3 0 $attr1k
> > +test_attr_replay_loop extent_file3 "attr_name" $attr512 "s"
> > "attr_leaf_to_node"
> >  
> >  # extent, remote
> >  create_test_file extent_file4 1 $attr1k
> > diff --git a/tests/xfs/018.out b/tests/xfs/018.out
> > index 022b0ca3..c3021ee3 100644
> > --- a/tests/xfs/018.out
> > +++ b/tests/xfs/018.out
> > @@ -87,22 +87,14 @@ Attribute "attr_name1" has a 1024 byte value
> > for SCRATCH_MNT/testdir/extent_file
> >  attr_name2: d41d8cd98f00b204e9800998ecf8427e  -
> >  
> >  attr_set: Input/output error
> > -Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file2
> > +Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file2
> >  touch: cannot touch 'SCRATCH_MNT/testdir/extent_file2':
> > Input/output error
> > -Attribute "attr_name4" has a 1025 byte value for
> > SCRATCH_MNT/testdir/extent_file2
> > -Attribute "attr_name2" has a 1024 byte value for
> > SCRATCH_MNT/testdir/extent_file2
> > -Attribute "attr_name3" has a 1024 byte value for
> > SCRATCH_MNT/testdir/extent_file2
> > -Attribute "attr_name1" has a 1024 byte value for
> > SCRATCH_MNT/testdir/extent_file2
> > -attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
> > +attr_nameXXXX: 9fd415c49d67afc4b78fad4055a3a376  -
> >  
> >  attr_set: Input/output error
> > -Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file3
> > +Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file3
> >  touch: cannot touch 'SCRATCH_MNT/testdir/extent_file3':
> > Input/output error
> > -Attribute "attr_name4" has a 1025 byte value for
> > SCRATCH_MNT/testdir/extent_file3
> > -Attribute "attr_name2" has a 1024 byte value for
> > SCRATCH_MNT/testdir/extent_file3
> > -Attribute "attr_name3" has a 1024 byte value for
> > SCRATCH_MNT/testdir/extent_file3
> > -Attribute "attr_name1" has a 1024 byte value for
> > SCRATCH_MNT/testdir/extent_file3
> > -attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
> > +attr_nameXXXX: a597dc41e4574873516420a7e4e5a3e0  -
> >  
> >  attr_set: Input/output error
> >  Could not set "attr_name2" for SCRATCH_MNT/testdir/extent_file4
> > 

Re: [PATCH 2/3] xfs/018: fix LARP testing for small block sizes

[Darrick J. Wong]

On Thu, Jul 07, 2022 at 02:55:07PM -0700, Alli wrote:
> On Thu, 2022-07-07 at 11:06 -0700, Darrick J. Wong wrote:
> > I guess I should've cc'd Allison and Catherine on this one.
> > 
> > Could either of you review these test changes, please?
> > 
> > --D
> > 
> > On Tue, Jul 05, 2022 at 03:02:14PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Fix this test to work properly when the filesystem block size is
> > > less
> > > than 4k.  Tripping the error injection points on shape changes in
> > > the
> > > xattr structure must be done dynamically.
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > >  tests/xfs/018     |   52
> > > +++++++++++++++++++++++++++++++++++++++++++++++-----
> > >  tests/xfs/018.out |   16 ++++------------
> > >  2 files changed, 51 insertions(+), 17 deletions(-)
> > > 
> > > 
> > > diff --git a/tests/xfs/018 b/tests/xfs/018
> > > index 041a3b24..14a6f716 100755
> > > --- a/tests/xfs/018
> > > +++ b/tests/xfs/018
> > > @@ -54,6 +54,45 @@ test_attr_replay()
> > >  	echo ""
> > >  }
> > >  
> > > +test_attr_replay_loop()
> > > +{
> > > +	testfile=$testdir/$1
> > > +	attr_name=$2
> > > +	attr_value=$3
> > > +	flag=$4
> > > +	error_tag=$5
> > > +
> > > +	# Inject error
> > > +	_scratch_inject_error $error_tag
> > > +
> > > +	# Set attribute; hopefully 1000 of them is enough to cause
> > > whatever
> > > +	# attr structure shape change that the caller wants to test.
> > > +	for ((i = 0; i < 1024; i++)); do
> > > +		echo "$attr_value" | \
> > > +			${ATTR_PROG} -$flag "$attr_name$i" $testfile >
> > > $tmp.out 2> $tmp.err
> > > +		cat $tmp.out $tmp.err >> $seqres.full
> > > +		cat $tmp.err | _filter_scratch | sed -e 's/attr_name[0-
> > > 9]*/attr_nameXXXX/g'
> > > +		touch $testfile &>/dev/null || break
> > > +	done
> > > +
> > > +	# FS should be shut down, touch will fail
> > > +	touch $testfile 2>&1 | _filter_scratch
> > > +
> > > +	# Remount to replay log
> > > +	_scratch_remount_dump_log >> $seqres.full
> > > +
> > > +	# FS should be online, touch should succeed
> > > +	touch $testfile
> > > +
> > > +	# Verify attr recovery
> > > +	$ATTR_PROG -l $testfile >> $seqres.full
> > > +	echo "Checking contents of $attr_name$i" >> $seqres.full
> > > +	echo -n "${attr_name}XXXX: "
> > > +	$ATTR_PROG -q -g $attr_name$i $testfile 2> /dev/null | md5sum;
> > > +
> > > +	echo ""
> > > +}
> > > +
> 
> 
> Ok, I think I see what you are trying to do, but I think we can do it
> with less duplicated code and looping functions.  What about something
> like this:
> 
> diff --git a/tests/xfs/018 b/tests/xfs/018
> index 041a3b24..dc1324b1 100755
> --- a/tests/xfs/018
> +++ b/tests/xfs/018
> @@ -95,6 +95,9 @@ attr16k="$attr8k$attr8k"
>  attr32k="$attr16k$attr16k"
>  attr64k="$attr32k$attr32k"
>  
> +blk_sz=$(_scratch_xfs_get_sb_field blocksize)
> +multiplier=$(( $blk_sz / 256 ))

The scratch fs hasn't been formatted yet, but if you use _get_block_size
after it's mounted, then, yes, I'm with you so far.

> +
>  echo "*** mkfs"
>  _scratch_mkfs >/dev/null
>  
> @@ -140,7 +143,7 @@ test_attr_replay extent_file1 "attr_name2" $attr1k
> "s" "larp"
>  test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
>  
>  # extent, inject error on split
> -create_test_file extent_file2 3 $attr1k
> +create_test_file extent_file2 $(( $multiplier - 1 )) $attr256

Hm.  The calculations seem slightly off here -- name is ~8 bytes long,
the value is 256 bytes, which means there's at least 264 bytes per attr.
I guess you do only create multiplier-1 attrs, though, so that probably
works for all the blocksizes I can think of...

>  test_attr_replay extent_file2 "attr_name4" $attr1k "s" "da_leaf_split"

If we keep the 1k attr value here, do we still trip the leaf split even
if that 1k value ends up in a remote block?

>  # extent, inject error on fork transition
> 
> 
> 
> Same idea right?  We bring the attr fork right up and to the edge of
> the block boundary and then pop it?  And then of course we apply the
> same pattern to the rest of the tests.  I think that sort of reads
> cleaner too.

Right, I think that would work in principle.  Does the same sort of fix
apply to the "extent, inject error on fork transition" case too?

--D

> Allison
> 
> > >  create_test_file()
> > >  {
> > >  	filename=$testdir/$1
> > > @@ -88,6 +127,7 @@ echo 1 > /sys/fs/xfs/debug/larp
> > >  attr16="0123456789ABCDEF"
> > >  attr64="$attr16$attr16$attr16$attr16"
> > >  attr256="$attr64$attr64$attr64$attr64"
> > > +attr512="$attr256$attr256"
> > >  attr1k="$attr256$attr256$attr256$attr256"
> > >  attr4k="$attr1k$attr1k$attr1k$attr1k"
> > >  attr8k="$attr4k$attr4k"
> > > @@ -140,12 +180,14 @@ test_attr_replay extent_file1 "attr_name2"
> > > $attr1k "s" "larp"
> > >  test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
> > >  
> > >  # extent, inject error on split
> > > -create_test_file extent_file2 3 $attr1k
> > > -test_attr_replay extent_file2 "attr_name4" $attr1k "s"
> > > "da_leaf_split"
> > > +create_test_file extent_file2 0 $attr1k
> > > +test_attr_replay_loop extent_file2 "attr_name" $attr1k "s"
> > > "da_leaf_split"
> > >  
> > > -# extent, inject error on fork transition
> > > -create_test_file extent_file3 3 $attr1k
> > > -test_attr_replay extent_file3 "attr_name4" $attr1k "s"
> > > "attr_leaf_to_node"
> > > +# extent, inject error on fork transition.  The attr value must be
> > > less than
> > > +# a full filesystem block so that the attrs don't use remote xattr
> > > values,
> > > +# which means we miss the leaf to node transition.
> > > +create_test_file extent_file3 0 $attr1k
> > > +test_attr_replay_loop extent_file3 "attr_name" $attr512 "s"
> > > "attr_leaf_to_node"
> > >  
> > >  # extent, remote
> > >  create_test_file extent_file4 1 $attr1k
> > > diff --git a/tests/xfs/018.out b/tests/xfs/018.out
> > > index 022b0ca3..c3021ee3 100644
> > > --- a/tests/xfs/018.out
> > > +++ b/tests/xfs/018.out
> > > @@ -87,22 +87,14 @@ Attribute "attr_name1" has a 1024 byte value
> > > for SCRATCH_MNT/testdir/extent_file
> > >  attr_name2: d41d8cd98f00b204e9800998ecf8427e  -
> > >  
> > >  attr_set: Input/output error
> > > -Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file2
> > > +Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file2
> > >  touch: cannot touch 'SCRATCH_MNT/testdir/extent_file2':
> > > Input/output error
> > > -Attribute "attr_name4" has a 1025 byte value for
> > > SCRATCH_MNT/testdir/extent_file2
> > > -Attribute "attr_name2" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file2
> > > -Attribute "attr_name3" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file2
> > > -Attribute "attr_name1" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file2
> > > -attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
> > > +attr_nameXXXX: 9fd415c49d67afc4b78fad4055a3a376  -
> > >  
> > >  attr_set: Input/output error
> > > -Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file3
> > > +Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file3
> > >  touch: cannot touch 'SCRATCH_MNT/testdir/extent_file3':
> > > Input/output error
> > > -Attribute "attr_name4" has a 1025 byte value for
> > > SCRATCH_MNT/testdir/extent_file3
> > > -Attribute "attr_name2" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file3
> > > -Attribute "attr_name3" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file3
> > > -Attribute "attr_name1" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file3
> > > -attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
> > > +attr_nameXXXX: a597dc41e4574873516420a7e4e5a3e0  -
> > >  
> > >  attr_set: Input/output error
> > >  Could not set "attr_name2" for SCRATCH_MNT/testdir/extent_file4
> > > 
> 

Re: [PATCH 2/3] xfs/018: fix LARP testing for small block sizes

[Alli]

On Thu, 2022-07-07 at 15:10 -0700, Darrick J. Wong wrote:
> On Thu, Jul 07, 2022 at 02:55:07PM -0700, Alli wrote:
> > On Thu, 2022-07-07 at 11:06 -0700, Darrick J. Wong wrote:
> > > I guess I should've cc'd Allison and Catherine on this one.
> > > 
> > > Could either of you review these test changes, please?
> > > 
> > > --D
> > > 
> > > On Tue, Jul 05, 2022 at 03:02:14PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > Fix this test to work properly when the filesystem block size
> > > > is
> > > > less
> > > > than 4k.  Tripping the error injection points on shape changes
> > > > in
> > > > the
> > > > xattr structure must be done dynamically.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > ---
> > > >  tests/xfs/018     |   52
> > > > +++++++++++++++++++++++++++++++++++++++++++++++-----
> > > >  tests/xfs/018.out |   16 ++++------------
> > > >  2 files changed, 51 insertions(+), 17 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/tests/xfs/018 b/tests/xfs/018
> > > > index 041a3b24..14a6f716 100755
> > > > --- a/tests/xfs/018
> > > > +++ b/tests/xfs/018
> > > > @@ -54,6 +54,45 @@ test_attr_replay()
> > > >  	echo ""
> > > >  }
> > > >  
> > > > +test_attr_replay_loop()
> > > > +{
> > > > +	testfile=$testdir/$1
> > > > +	attr_name=$2
> > > > +	attr_value=$3
> > > > +	flag=$4
> > > > +	error_tag=$5
> > > > +
> > > > +	# Inject error
> > > > +	_scratch_inject_error $error_tag
> > > > +
> > > > +	# Set attribute; hopefully 1000 of them is enough to
> > > > cause
> > > > whatever
> > > > +	# attr structure shape change that the caller wants to
> > > > test.
> > > > +	for ((i = 0; i < 1024; i++)); do
> > > > +		echo "$attr_value" | \
> > > > +			${ATTR_PROG} -$flag "$attr_name$i"
> > > > $testfile >
> > > > $tmp.out 2> $tmp.err
> > > > +		cat $tmp.out $tmp.err >> $seqres.full
> > > > +		cat $tmp.err | _filter_scratch | sed -e
> > > > 's/attr_name[0-
> > > > 9]*/attr_nameXXXX/g'
> > > > +		touch $testfile &>/dev/null || break
> > > > +	done
> > > > +
> > > > +	# FS should be shut down, touch will fail
> > > > +	touch $testfile 2>&1 | _filter_scratch
> > > > +
> > > > +	# Remount to replay log
> > > > +	_scratch_remount_dump_log >> $seqres.full
> > > > +
> > > > +	# FS should be online, touch should succeed
> > > > +	touch $testfile
> > > > +
> > > > +	# Verify attr recovery
> > > > +	$ATTR_PROG -l $testfile >> $seqres.full
> > > > +	echo "Checking contents of $attr_name$i" >>
> > > > $seqres.full
> > > > +	echo -n "${attr_name}XXXX: "
> > > > +	$ATTR_PROG -q -g $attr_name$i $testfile 2> /dev/null |
> > > > md5sum;
> > > > +
> > > > +	echo ""
> > > > +}
> > > > +
> > 
> > Ok, I think I see what you are trying to do, but I think we can do
> > it
> > with less duplicated code and looping functions.  What about
> > something
> > like this:
> > 
> > diff --git a/tests/xfs/018 b/tests/xfs/018
> > index 041a3b24..dc1324b1 100755
> > --- a/tests/xfs/018
> > +++ b/tests/xfs/018
> > @@ -95,6 +95,9 @@ attr16k="$attr8k$attr8k"
> >  attr32k="$attr16k$attr16k"
> >  attr64k="$attr32k$attr32k"
> >  
> > +blk_sz=$(_scratch_xfs_get_sb_field blocksize)
> > +multiplier=$(( $blk_sz / 256 ))
> 
> The scratch fs hasn't been formatted yet, but if you use
> _get_block_size
> after it's mounted, then, yes, I'm with you so far.
> 
> > +
> >  echo "*** mkfs"
> >  _scratch_mkfs >/dev/null
> >  
> > @@ -140,7 +143,7 @@ test_attr_replay extent_file1 "attr_name2"
> > $attr1k
> > "s" "larp"
> >  test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
> >  
> >  # extent, inject error on split
> > -create_test_file extent_file2 3 $attr1k
> > +create_test_file extent_file2 $(( $multiplier - 1 )) $attr256
> 
> Hm.  The calculations seem slightly off here -- name is ~8 bytes
> long,
> the value is 256 bytes, which means there's at least 264 bytes per
> attr.
> I guess you do only create multiplier-1 attrs, though, so that
> probably
> works for all the blocksizes I can think of...
> 
> >  test_attr_replay extent_file2 "attr_name4" $attr1k "s"
> > "da_leaf_split"
> 
> If we keep the 1k attr value here, do we still trip the leaf split
> even
> if that 1k value ends up in a remote block?
I think so.  If I'm following the code correctly, it moves through the
states in order.  As long as we run across the node state in the state
machine, da_leaf_split should trip.

> 
> >  # extent, inject error on fork transition
> > 
> > 
> > 
> > Same idea right?  We bring the attr fork right up and to the edge
> > of
> > the block boundary and then pop it?  And then of course we apply
> > the
> > same pattern to the rest of the tests.  I think that sort of reads
> > cleaner too.
> 
> Right, I think that would work in principle.  Does the same sort of
> fix
> apply to the "extent, inject error on fork transition" case too?
> 
I think it should.  These two tests have the same set up really, the
only difference is where the error tag is set.  So as long as the next
attr is enough to catapult us through node, we should hit that state in
the state machine.

Allison

> --D
> 
> > Allison
> > 
> > > >  create_test_file()
> > > >  {
> > > >  	filename=$testdir/$1
> > > > @@ -88,6 +127,7 @@ echo 1 > /sys/fs/xfs/debug/larp
> > > >  attr16="0123456789ABCDEF"
> > > >  attr64="$attr16$attr16$attr16$attr16"
> > > >  attr256="$attr64$attr64$attr64$attr64"
> > > > +attr512="$attr256$attr256"
> > > >  attr1k="$attr256$attr256$attr256$attr256"
> > > >  attr4k="$attr1k$attr1k$attr1k$attr1k"
> > > >  attr8k="$attr4k$attr4k"
> > > > @@ -140,12 +180,14 @@ test_attr_replay extent_file1
> > > > "attr_name2"
> > > > $attr1k "s" "larp"
> > > >  test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
> > > >  
> > > >  # extent, inject error on split
> > > > -create_test_file extent_file2 3 $attr1k
> > > > -test_attr_replay extent_file2 "attr_name4" $attr1k "s"
> > > > "da_leaf_split"
> > > > +create_test_file extent_file2 0 $attr1k
> > > > +test_attr_replay_loop extent_file2 "attr_name" $attr1k "s"
> > > > "da_leaf_split"
> > > >  
> > > > -# extent, inject error on fork transition
> > > > -create_test_file extent_file3 3 $attr1k
> > > > -test_attr_replay extent_file3 "attr_name4" $attr1k "s"
> > > > "attr_leaf_to_node"
> > > > +# extent, inject error on fork transition.  The attr value
> > > > must be
> > > > less than
> > > > +# a full filesystem block so that the attrs don't use remote
> > > > xattr
> > > > values,
> > > > +# which means we miss the leaf to node transition.
> > > > +create_test_file extent_file3 0 $attr1k
> > > > +test_attr_replay_loop extent_file3 "attr_name" $attr512 "s"
> > > > "attr_leaf_to_node"
> > > >  
> > > >  # extent, remote
> > > >  create_test_file extent_file4 1 $attr1k
> > > > diff --git a/tests/xfs/018.out b/tests/xfs/018.out
> > > > index 022b0ca3..c3021ee3 100644
> > > > --- a/tests/xfs/018.out
> > > > +++ b/tests/xfs/018.out
> > > > @@ -87,22 +87,14 @@ Attribute "attr_name1" has a 1024 byte
> > > > value
> > > > for SCRATCH_MNT/testdir/extent_file
> > > >  attr_name2: d41d8cd98f00b204e9800998ecf8427e  -
> > > >  
> > > >  attr_set: Input/output error
> > > > -Could not set "attr_name4" for
> > > > SCRATCH_MNT/testdir/extent_file2
> > > > +Could not set "attr_nameXXXX" for
> > > > SCRATCH_MNT/testdir/extent_file2
> > > >  touch: cannot touch 'SCRATCH_MNT/testdir/extent_file2':
> > > > Input/output error
> > > > -Attribute "attr_name4" has a 1025 byte value for
> > > > SCRATCH_MNT/testdir/extent_file2
> > > > -Attribute "attr_name2" has a 1024 byte value for
> > > > SCRATCH_MNT/testdir/extent_file2
> > > > -Attribute "attr_name3" has a 1024 byte value for
> > > > SCRATCH_MNT/testdir/extent_file2
> > > > -Attribute "attr_name1" has a 1024 byte value for
> > > > SCRATCH_MNT/testdir/extent_file2
> > > > -attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
> > > > +attr_nameXXXX: 9fd415c49d67afc4b78fad4055a3a376  -
> > > >  
> > > >  attr_set: Input/output error
> > > > -Could not set "attr_name4" for
> > > > SCRATCH_MNT/testdir/extent_file3
> > > > +Could not set "attr_nameXXXX" for
> > > > SCRATCH_MNT/testdir/extent_file3
> > > >  touch: cannot touch 'SCRATCH_MNT/testdir/extent_file3':
> > > > Input/output error
> > > > -Attribute "attr_name4" has a 1025 byte value for
> > > > SCRATCH_MNT/testdir/extent_file3
> > > > -Attribute "attr_name2" has a 1024 byte value for
> > > > SCRATCH_MNT/testdir/extent_file3
> > > > -Attribute "attr_name3" has a 1024 byte value for
> > > > SCRATCH_MNT/testdir/extent_file3
> > > > -Attribute "attr_name1" has a 1024 byte value for
> > > > SCRATCH_MNT/testdir/extent_file3
> > > > -attr_name4: 9fd415c49d67afc4b78fad4055a3a376  -
> > > > +attr_nameXXXX: a597dc41e4574873516420a7e4e5a3e0  -
> > > >  
> > > >  attr_set: Input/output error
> > > >  Could not set "attr_name2" for
> > > > SCRATCH_MNT/testdir/extent_file4
> > > > 

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

[Zorro Lang]

On Thu, Jul 07, 2022 at 11:05:25AM -0700, Darrick J. Wong wrote:
> 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

Oh, it might be hexadecimal. OK, I'd like to respect the history reason.
So this looks good to me.

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

> 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
> > > 
> > 
> 

Re: [PATCHSET v2 0/3] fstests: random fixes

[Zorro Lang]

On Tue, Jul 05, 2022 at 03:02:02PM -0700, Darrick J. Wong wrote:
> Hi all,
> 
> Here's the usual batch of odd fixes for fstests.
> 
> v2: rebase against v2022.07.03, add another correction
> 
> If you're going to start using this mess, you probably ought to just
> pull from my git trees, which are linked below.
> 
> This is an extraordinary way to destroy everything.  Enjoy!
> Comments and questions are, as always, welcome.
> 
> --D

Hi Darrick,

JFYI, Patch [1/3] and [3/3] have been reviewed and merged. Patch [2/3] is still
under reviewing, and had review points, so feel free to update it with your
other patches together.

Thanks,
Zorro

> 
> kernel git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes
> 
> xfsprogs git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes
> 
> fstests git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
> ---
>  tests/xfs/018     |   52 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  tests/xfs/018.out |   16 ++++------------
>  tests/xfs/144     |   14 +++++++++-----
>  tests/xfs/547     |   14 ++++++++++----
>  4 files changed, 70 insertions(+), 26 deletions(-)
>