[PATCH v4 0/3] xfs: testcases for shrinking free space in the last AG

[PATCH v4 0/3] xfs: testcases for shrinking free space in the last AG

[Gao Xiang]

Hi,

Sorry for little delay (yet since xfsprogs side isn't merged, and no
major changes compared with the previous version...)

This version matches
kernel: for-next
xfsprogs: https://lore.kernel.org/r/20210326024631.12921-1-hsiangkao@aol.com

and mainly addresses comments for the previous version (but note I don't
tend to dump shrink information but rather confirm the final state runtimely,
since blocksize needs to be fixed and output could change by time, so just
need to confirm xfs_repair can pass and final dblocks is what we want.)

Thanks,
Gao Xinag

changes since RFC v3 (Eryu):
 - [1/3] rename to _require_xfs_scratch_shrink;
 - [1/3] add growfs command check;
 - [1/3] try to shrink 1 dblock to check kernel support instead;
 - [2/3] use _check_scratch_fs instead;
 - [2/3] add comment on why agcount=3;
 - [2/3] add shrinkfs group;
 - [3/3] use _scratch_mount;
 - [3/3] Declare variables in stress_scratch() as local;
 - [3/3] run stress_scratch() in background;

Gao Xiang (3):
  common/xfs: add _require_xfs_scratch_shrink helper
  xfs: basic functionality test for shrinking free space in the last AG
  xfs: stress test for shrinking free space in the last AG

 common/xfs        |  14 ++++++
 tests/xfs/990     |  73 ++++++++++++++++++++++++++++
 tests/xfs/990.out |  12 +++++
 tests/xfs/991     | 118 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/991.out |   8 ++++
 tests/xfs/group   |   2 +
 6 files changed, 227 insertions(+)
 create mode 100755 tests/xfs/990
 create mode 100644 tests/xfs/990.out
 create mode 100755 tests/xfs/991
 create mode 100644 tests/xfs/991.out

-- 
2.27.0

[PATCH v4 1/3] common/xfs: add _require_xfs_scratch_shrink helper

[Gao Xiang]

In order to detect whether the current kernel supports XFS shrinking.

Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 common/xfs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/common/xfs b/common/xfs
index 69f76d6e..c6c2e3f5 100644
--- a/common/xfs
+++ b/common/xfs
@@ -766,6 +766,20 @@ _require_xfs_mkfs_without_validation()
 	fi
 }
 
+_require_xfs_scratch_shrink()
+{
+	_require_scratch
+	_require_command "$XFS_GROWFS_PROG" xfs_growfs
+
+	_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs >/dev/null
+	. $tmp.mkfs
+	_scratch_mount
+	# here just to check if kernel supports, no need do more extra work
+	$XFS_GROWFS_PROG -D$((dblocks-1)) "$SCRATCH_MNT" > /dev/null 2>&1 || \
+		_notrun "kernel does not support shrinking"
+	_scratch_unmount
+}
+
 # XFS ability to change UUIDs on V5/CRC filesystems
 #
 _require_meta_uuid()
-- 
2.27.0

[PATCH v4 2/3] xfs: basic functionality test for shrinking free space in the last AG

[Gao Xiang]

Add basic test to make sure the functionality works as expected.

Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 tests/xfs/990     | 73 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/990.out | 12 ++++++++
 tests/xfs/group   |  1 +
 3 files changed, 86 insertions(+)
 create mode 100755 tests/xfs/990
 create mode 100644 tests/xfs/990.out

diff --git a/tests/xfs/990 b/tests/xfs/990
new file mode 100755
index 00000000..322b09e1
--- /dev/null
+++ b/tests/xfs/990
@@ -0,0 +1,73 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Red Hat, Inc.  All Rights Reserved.
+#
+# FS QA Test 990
+#
+# XFS shrinkfs basic functionality test
+#
+# This test attempts to shrink with a small size (512K), half AG size and
+# an out-of-bound size (agsize + 1) to observe if it works as expected.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+test_shrink()
+{
+	$XFS_GROWFS_PROG -D"$1" $SCRATCH_MNT >> $seqres.full 2>&1
+	ret=$?
+
+	_scratch_unmount
+	_check_scratch_fs
+	_scratch_mount
+
+	$XFS_INFO_PROG $SCRATCH_MNT 2>&1 | _filter_mkfs 2>$tmp.growfs >/dev/null
+	. $tmp.growfs
+	[ $ret -eq 0 -a $1 -eq $dblocks ]
+}
+
+# real QA test starts here
+_supported_fs xfs
+_require_xfs_scratch_shrink
+
+rm -f $seqres.full
+echo "Format and mount"
+
+# agcount = 1 is forbidden on purpose, and need to ensure shrinking to
+# 2 AGs isn't fensible yet. So agcount = 3 is the minimum number now.
+_scratch_mkfs -dsize="$((512 * 1024 * 1024))" -dagcount=3 2>&1 | \
+	tee -a $seqres.full | _filter_mkfs 2>$tmp.mkfs
+. $tmp.mkfs
+t_dblocks=$dblocks
+_scratch_mount >> $seqres.full
+
+echo "Shrink fs (small size)"
+test_shrink $((t_dblocks-512*1024/dbsize)) || \
+	_fail "Shrink fs (small size) failure"
+
+echo "Shrink fs (half AG)"
+test_shrink $((t_dblocks-agsize/2)) || \
+	_fail "Shrink fs (half AG) failure"
+
+echo "Shrink fs (out-of-bound)"
+test_shrink $((t_dblocks-agsize-1)) && \
+	_fail "Shrink fs (out-of-bound) failure"
+[ $dblocks -ne $((t_dblocks-agsize/2)) ] && \
+	_fail "dblocks changed after shrinking failure"
+
+$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
+echo "*** done"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/990.out b/tests/xfs/990.out
new file mode 100644
index 00000000..812f89ef
--- /dev/null
+++ b/tests/xfs/990.out
@@ -0,0 +1,12 @@
+QA output created by 990
+Format and mount
+meta-data=DDEV isize=XXX agcount=N, agsize=XXX blks
+data     = bsize=XXX blocks=XXX, imaxpct=PCT
+         = sunit=XXX swidth=XXX, unwritten=X
+naming   =VERN bsize=XXX
+log      =LDEV bsize=XXX blocks=XXX
+realtime =RDEV extsz=XXX blocks=XXX, rtextents=XXX
+Shrink fs (small size)
+Shrink fs (half AG)
+Shrink fs (out-of-bound)
+*** done
diff --git a/tests/xfs/group b/tests/xfs/group
index fe83f82d..472c8f9a 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -520,3 +520,4 @@
 537 auto quick
 538 auto stress
 539 auto quick mount
+990 auto quick growfs shrinkfs
-- 
2.27.0

[PATCH v4 3/3] xfs: stress test for shrinking free space in the last AG

[Gao Xiang]

This adds a stress testcase to shrink free space as much as
possible in the last AG with background fsstress workload.

The expectation is that no crash happens with expected output.

Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 tests/xfs/991     | 118 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/991.out |   8 ++++
 tests/xfs/group   |   1 +
 3 files changed, 127 insertions(+)
 create mode 100755 tests/xfs/991
 create mode 100644 tests/xfs/991.out

diff --git a/tests/xfs/991 b/tests/xfs/991
new file mode 100755
index 00000000..8ad0b8c7
--- /dev/null
+++ b/tests/xfs/991
@@ -0,0 +1,118 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2020-2021 Red Hat, Inc.  All Rights Reserved.
+#
+# FS QA Test 991
+#
+# XFS online shrinkfs stress test
+#
+# This test attempts to shrink unused space as much as possible with
+# background fsstress workload. It will decrease the shrink size if
+# larger size fails. And totally repeat 2 * TIME_FACTOR times.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+create_scratch()
+{
+	_scratch_mkfs_xfs $@ | tee -a $seqres.full | \
+		_filter_mkfs 2>$tmp.mkfs >/dev/null
+	. $tmp.mkfs
+
+	_scratch_mount
+	# fix the reserve block pool to a known size so that the enospc
+	# calculations work out correctly.
+	_scratch_resvblks 1024 > /dev/null 2>&1
+}
+
+fill_scratch()
+{
+	$XFS_IO_PROG -f -c "falloc 0 $1" $SCRATCH_MNT/resvfile
+}
+
+stress_scratch()
+{
+	local procs=3
+	local nops=$((1000 * LOAD_FACTOR))
+	# -w ensures that the only ops are ones which cause write I/O
+	local FSSTRESS_ARGS=`_scale_fsstress_args -d $SCRATCH_MNT -w \
+		-p $procs -n $nops $FSSTRESS_AVOID`
+	$FSSTRESS_PROG $FSSTRESS_ARGS >> $seqres.full 2>&1
+}
+
+# real QA test starts here
+_supported_fs xfs
+_require_xfs_scratch_shrink
+_require_xfs_io_command "falloc"
+
+rm -f $seqres.full
+_scratch_mkfs_xfs | tee -a $seqres.full | _filter_mkfs 2>$tmp.mkfs
+. $tmp.mkfs	# extract blocksize and data size for scratch device
+
+decsize=`expr  42 \* 1048576`	# shrink in chunks of this size at most
+endsize=`expr 125 \* 1048576`	# stop after shrinking this big
+[ `expr $endsize / $dbsize` -lt $dblocks ] || _notrun "Scratch device too small"
+
+nags=2
+totalcount=$((2 * TIME_FACTOR))
+
+while [ $totalcount -gt 0 ]; do
+	size=`expr 1010 \* 1048576`	# 1010 megabytes initially
+	logblks=$(_scratch_find_xfs_min_logblocks -dsize=${size} -dagcount=${nags})
+
+	create_scratch -lsize=${logblks}b -dsize=${size} -dagcount=${nags}
+
+	for i in `seq 125 -1 90`; do
+		fillsize=`expr $i \* 1048576`
+		out="$(fill_scratch $fillsize 2>&1)"
+		echo "$out" | grep -q 'No space left on device' && continue
+		test -n "${out}" && echo "$out"
+		break
+	done
+
+	while [ $size -gt $endsize ]; do
+		stress_scratch &
+		sleep 1
+
+		decb=`expr $decsize / $dbsize`    # in data blocks
+		while [ $decb -gt 0 ]; do
+			sizeb=`expr $size / $dbsize - $decb`
+
+			$XFS_GROWFS_PROG -D ${sizeb} $SCRATCH_MNT \
+				>> $seqres.full 2>&1 && break
+
+			[ $decb -gt 100 ] && decb=`expr $decb + $RANDOM % 10`
+			decb=`expr $decb / 2`
+		done
+
+		wait
+		[ $decb -eq 0 ] && break
+
+		# get latest dblocks
+		$XFS_INFO_PROG $SCRATCH_MNT 2>&1 | _filter_mkfs 2>$tmp.growfs >/dev/null
+		. $tmp.growfs
+
+		size=`expr $dblocks \* $dbsize`
+		_scratch_unmount
+		_repair_scratch_fs >> $seqres.full
+		_scratch_mount
+	done
+
+	_scratch_unmount
+	_repair_scratch_fs >> $seqres.full
+	totalcount=`expr $totalcount - 1`
+done
+
+echo "*** done"
+status=0
+exit
diff --git a/tests/xfs/991.out b/tests/xfs/991.out
new file mode 100644
index 00000000..e8209672
--- /dev/null
+++ b/tests/xfs/991.out
@@ -0,0 +1,8 @@
+QA output created by 991
+meta-data=DDEV isize=XXX agcount=N, agsize=XXX blks
+data     = bsize=XXX blocks=XXX, imaxpct=PCT
+         = sunit=XXX swidth=XXX, unwritten=X
+naming   =VERN bsize=XXX
+log      =LDEV bsize=XXX blocks=XXX
+realtime =RDEV extsz=XXX blocks=XXX, rtextents=XXX
+*** done
diff --git a/tests/xfs/group b/tests/xfs/group
index 472c8f9a..53e68bea 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -521,3 +521,4 @@
 538 auto stress
 539 auto quick mount
 990 auto quick growfs shrinkfs
+991 auto growfs shrinkfs ioctl prealloc stress
-- 
2.27.0

Re: [PATCH v4 0/3] xfs: testcases for shrinking free space in the last AG

[Gao Xiang]

Hi Eryu,

On Fri, Apr 02, 2021 at 05:49:34PM +0800, Gao Xiang wrote:
> Hi,
> 
> Sorry for little delay (yet since xfsprogs side isn't merged, and no
> major changes compared with the previous version...)

ping. This fstests patchset is still valid (I've confirmed with new
kernels / old kernels and old xfsprogs) and
shrinking tail AG functionality has been merged in kernel and xfsprogs.

Thanks,
Gao Xiang

Re: [PATCH v4 1/3] common/xfs: add _require_xfs_scratch_shrink helper

[Darrick J. Wong]

On Fri, Apr 02, 2021 at 05:49:35PM +0800, Gao Xiang wrote:
> In order to detect whether the current kernel supports XFS shrinking.
> 
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
>  common/xfs | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 69f76d6e..c6c2e3f5 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -766,6 +766,20 @@ _require_xfs_mkfs_without_validation()
>  	fi
>  }
>  
> +_require_xfs_scratch_shrink()
> +{
> +	_require_scratch
> +	_require_command "$XFS_GROWFS_PROG" xfs_growfs
> +
> +	_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs >/dev/null
> +	. $tmp.mkfs
> +	_scratch_mount
> +	# here just to check if kernel supports, no need do more extra work
> +	$XFS_GROWFS_PROG -D$((dblocks-1)) "$SCRATCH_MNT" > /dev/null 2>&1 || \
> +		_notrun "kernel does not support shrinking"

I think isn't sufficiently precise -- if xfs_growfs (userspace) doesn't
support shrinking it'll error out with "data size XXX too small", and if
the kernel doesn't support shrink, it'll return EINVAL.

As written, this code attempts a single-block shrink and disables the
entire test if that fails for any reason, even if that reason is that
the last block in the filesystem isn't free, or we ran out of memory, or
something like that.

I think this needs to check the output of xfs_growfs to make the
decision to _notrun.

--D

> +	_scratch_unmount
> +}
> +
>  # XFS ability to change UUIDs on V5/CRC filesystems
>  #
>  _require_meta_uuid()
> -- 
> 2.27.0
> 

Re: [PATCH v4 2/3] xfs: basic functionality test for shrinking free space in the last AG

[Darrick J. Wong]

On Fri, Apr 02, 2021 at 05:49:36PM +0800, Gao Xiang wrote:
> Add basic test to make sure the functionality works as expected.
> 
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
>  tests/xfs/990     | 73 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/990.out | 12 ++++++++
>  tests/xfs/group   |  1 +
>  3 files changed, 86 insertions(+)
>  create mode 100755 tests/xfs/990
>  create mode 100644 tests/xfs/990.out
> 
> diff --git a/tests/xfs/990 b/tests/xfs/990
> new file mode 100755
> index 00000000..322b09e1
> --- /dev/null
> +++ b/tests/xfs/990
> @@ -0,0 +1,73 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Red Hat, Inc.  All Rights Reserved.
> +#
> +# FS QA Test 990
> +#
> +# XFS shrinkfs basic functionality test
> +#
> +# This test attempts to shrink with a small size (512K), half AG size and
> +# an out-of-bound size (agsize + 1) to observe if it works as expected.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1    # failure is the default!
> +trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +test_shrink()
> +{
> +	$XFS_GROWFS_PROG -D"$1" $SCRATCH_MNT >> $seqres.full 2>&1
> +	ret=$?
> +
> +	_scratch_unmount
> +	_check_scratch_fs
> +	_scratch_mount
> +
> +	$XFS_INFO_PROG $SCRATCH_MNT 2>&1 | _filter_mkfs 2>$tmp.growfs >/dev/null
> +	. $tmp.growfs
> +	[ $ret -eq 0 -a $1 -eq $dblocks ]
> +}
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_require_xfs_scratch_shrink
> +
> +rm -f $seqres.full
> +echo "Format and mount"
> +
> +# agcount = 1 is forbidden on purpose, and need to ensure shrinking to
> +# 2 AGs isn't fensible yet. So agcount = 3 is the minimum number now.

s/fensible/feasible/

> +_scratch_mkfs -dsize="$((512 * 1024 * 1024))" -dagcount=3 2>&1 | \
> +	tee -a $seqres.full | _filter_mkfs 2>$tmp.mkfs
> +. $tmp.mkfs
> +t_dblocks=$dblocks
> +_scratch_mount >> $seqres.full
> +
> +echo "Shrink fs (small size)"
> +test_shrink $((t_dblocks-512*1024/dbsize)) || \
> +	_fail "Shrink fs (small size) failure"
> +
> +echo "Shrink fs (half AG)"
> +test_shrink $((t_dblocks-agsize/2)) || \
> +	_fail "Shrink fs (half AG) failure"
> +
> +echo "Shrink fs (out-of-bound)"
> +test_shrink $((t_dblocks-agsize-1)) && \
> +	_fail "Shrink fs (out-of-bound) failure"
> +[ $dblocks -ne $((t_dblocks-agsize/2)) ] && \
> +	_fail "dblocks changed after shrinking failure"

Can these be echo statements, since the diff in golden output will trip
anyway?

--D

> +
> +$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
> +echo "*** done"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/990.out b/tests/xfs/990.out
> new file mode 100644
> index 00000000..812f89ef
> --- /dev/null
> +++ b/tests/xfs/990.out
> @@ -0,0 +1,12 @@
> +QA output created by 990
> +Format and mount
> +meta-data=DDEV isize=XXX agcount=N, agsize=XXX blks
> +data     = bsize=XXX blocks=XXX, imaxpct=PCT
> +         = sunit=XXX swidth=XXX, unwritten=X
> +naming   =VERN bsize=XXX
> +log      =LDEV bsize=XXX blocks=XXX
> +realtime =RDEV extsz=XXX blocks=XXX, rtextents=XXX
> +Shrink fs (small size)
> +Shrink fs (half AG)
> +Shrink fs (out-of-bound)
> +*** done
> diff --git a/tests/xfs/group b/tests/xfs/group
> index fe83f82d..472c8f9a 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -520,3 +520,4 @@
>  537 auto quick
>  538 auto stress
>  539 auto quick mount
> +990 auto quick growfs shrinkfs
> -- 
> 2.27.0
> 

Re: [PATCH v4 3/3] xfs: stress test for shrinking free space in the last AG

[Darrick J. Wong]

On Fri, Apr 02, 2021 at 05:49:37PM +0800, Gao Xiang wrote:
> This adds a stress testcase to shrink free space as much as
> possible in the last AG with background fsstress workload.
> 
> The expectation is that no crash happens with expected output.
> 
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
>  tests/xfs/991     | 118 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/991.out |   8 ++++
>  tests/xfs/group   |   1 +
>  3 files changed, 127 insertions(+)
>  create mode 100755 tests/xfs/991
>  create mode 100644 tests/xfs/991.out
> 
> diff --git a/tests/xfs/991 b/tests/xfs/991
> new file mode 100755
> index 00000000..8ad0b8c7
> --- /dev/null
> +++ b/tests/xfs/991
> @@ -0,0 +1,118 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2020-2021 Red Hat, Inc.  All Rights Reserved.
> +#
> +# FS QA Test 991
> +#
> +# XFS online shrinkfs stress test
> +#
> +# This test attempts to shrink unused space as much as possible with
> +# background fsstress workload. It will decrease the shrink size if
> +# larger size fails. And totally repeat 2 * TIME_FACTOR times.
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +create_scratch()
> +{
> +	_scratch_mkfs_xfs $@ | tee -a $seqres.full | \
> +		_filter_mkfs 2>$tmp.mkfs >/dev/null
> +	. $tmp.mkfs
> +
> +	_scratch_mount
> +	# fix the reserve block pool to a known size so that the enospc
> +	# calculations work out correctly.
> +	_scratch_resvblks 1024 > /dev/null 2>&1
> +}
> +
> +fill_scratch()
> +{
> +	$XFS_IO_PROG -f -c "falloc 0 $1" $SCRATCH_MNT/resvfile
> +}
> +
> +stress_scratch()
> +{
> +	local procs=3
> +	local nops=$((1000 * LOAD_FACTOR))

Um... _scale_fsstress_args already scales the -p and -n arguments, why
is it necessary to scale nops by time /and/ load?

> +	# -w ensures that the only ops are ones which cause write I/O
> +	local FSSTRESS_ARGS=`_scale_fsstress_args -d $SCRATCH_MNT -w \
> +		-p $procs -n $nops $FSSTRESS_AVOID`
> +	$FSSTRESS_PROG $FSSTRESS_ARGS >> $seqres.full 2>&1
> +}
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_require_xfs_scratch_shrink
> +_require_xfs_io_command "falloc"
> +
> +rm -f $seqres.full
> +_scratch_mkfs_xfs | tee -a $seqres.full | _filter_mkfs 2>$tmp.mkfs
> +. $tmp.mkfs	# extract blocksize and data size for scratch device
> +
> +decsize=`expr  42 \* 1048576`	# shrink in chunks of this size at most

Might it be nice to inject a little bit of randomness here?

> +endsize=`expr 125 \* 1048576`	# stop after shrinking this big
> +[ `expr $endsize / $dbsize` -lt $dblocks ] || _notrun "Scratch device too small"
> +
> +nags=2
> +totalcount=$((2 * TIME_FACTOR))
> +
> +while [ $totalcount -gt 0 ]; do
> +	size=`expr 1010 \* 1048576`	# 1010 megabytes initially
> +	logblks=$(_scratch_find_xfs_min_logblocks -dsize=${size} -dagcount=${nags})

(Does all this logic still work if an external log device is present?)

> +
> +	create_scratch -lsize=${logblks}b -dsize=${size} -dagcount=${nags}
> +
> +	for i in `seq 125 -1 90`; do
> +		fillsize=`expr $i \* 1048576`
> +		out="$(fill_scratch $fillsize 2>&1)"
> +		echo "$out" | grep -q 'No space left on device' && continue
> +		test -n "${out}" && echo "$out"
> +		break
> +	done
> +
> +	while [ $size -gt $endsize ]; do
> +		stress_scratch &
> +		sleep 1
> +
> +		decb=`expr $decsize / $dbsize`    # in data blocks
> +		while [ $decb -gt 0 ]; do
> +			sizeb=`expr $size / $dbsize - $decb`
> +
> +			$XFS_GROWFS_PROG -D ${sizeb} $SCRATCH_MNT \
> +				>> $seqres.full 2>&1 && break
> +
> +			[ $decb -gt 100 ] && decb=`expr $decb + $RANDOM % 10`
> +			decb=`expr $decb / 2`
> +		done
> +
> +		wait
> +		[ $decb -eq 0 ] && break
> +
> +		# get latest dblocks
> +		$XFS_INFO_PROG $SCRATCH_MNT 2>&1 | _filter_mkfs 2>$tmp.growfs >/dev/null
> +		. $tmp.growfs
> +
> +		size=`expr $dblocks \* $dbsize`
> +		_scratch_unmount
> +		_repair_scratch_fs >> $seqres.full

Why isn't "_scratch_xfs_repair -n" here sufficient?

> +		_scratch_mount
> +	done
> +
> +	_scratch_unmount
> +	_repair_scratch_fs >> $seqres.full

...and here?

> +	totalcount=`expr $totalcount - 1`
> +done
> +
> +echo "*** done"
> +status=0
> +exit
> diff --git a/tests/xfs/991.out b/tests/xfs/991.out
> new file mode 100644
> index 00000000..e8209672
> --- /dev/null
> +++ b/tests/xfs/991.out
> @@ -0,0 +1,8 @@
> +QA output created by 991
> +meta-data=DDEV isize=XXX agcount=N, agsize=XXX blks
> +data     = bsize=XXX blocks=XXX, imaxpct=PCT
> +         = sunit=XXX swidth=XXX, unwritten=X
> +naming   =VERN bsize=XXX
> +log      =LDEV bsize=XXX blocks=XXX
> +realtime =RDEV extsz=XXX blocks=XXX, rtextents=XXX
> +*** done
> diff --git a/tests/xfs/group b/tests/xfs/group
> index 472c8f9a..53e68bea 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -521,3 +521,4 @@
>  538 auto stress
>  539 auto quick mount
>  990 auto quick growfs shrinkfs
> +991 auto growfs shrinkfs ioctl prealloc stress
> -- 
> 2.27.0
> 

Re: [PATCH v4 1/3] common/xfs: add _require_xfs_scratch_shrink helper

[Gao Xiang]

On Mon, May 10, 2021 at 10:59:52AM -0700, Darrick J. Wong wrote:
> On Fri, Apr 02, 2021 at 05:49:35PM +0800, Gao Xiang wrote:
> > In order to detect whether the current kernel supports XFS shrinking.
> > 
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > ---
> >  common/xfs | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/common/xfs b/common/xfs
> > index 69f76d6e..c6c2e3f5 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -766,6 +766,20 @@ _require_xfs_mkfs_without_validation()
> >  	fi
> >  }
> >  
> > +_require_xfs_scratch_shrink()
> > +{
> > +	_require_scratch
> > +	_require_command "$XFS_GROWFS_PROG" xfs_growfs
> > +
> > +	_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs >/dev/null
> > +	. $tmp.mkfs
> > +	_scratch_mount
> > +	# here just to check if kernel supports, no need do more extra work
> > +	$XFS_GROWFS_PROG -D$((dblocks-1)) "$SCRATCH_MNT" > /dev/null 2>&1 || \
> > +		_notrun "kernel does not support shrinking"
> 
> I think isn't sufficiently precise -- if xfs_growfs (userspace) doesn't
> support shrinking it'll error out with "data size XXX too small", and if
> the kernel doesn't support shrink, it'll return EINVAL.

I'm not sure if we need to identify such 2 cases (xfsprogs doesn't support
and/or kernel doesn't support), but if it's really needed I think I could
update it. But I've confirmed with testing that both two cases can be
handled with the statements above properly.

> 
> As written, this code attempts a single-block shrink and disables the
> entire test if that fails for any reason, even if that reason is that
> the last block in the filesystem isn't free, or we ran out of memory, or
> something like that.

hmm... the filesystem here is brandly new, I think at least it'd be
considered as "the last block in the new filesystem is free". If we're
worried that such promise could be broken, I think some other golden
output is unstable as well (although unrelated to this.) By that time,
I think the test script should be updated then instead. Or am I missing
something?

If we're worried about runing out of memory, I think the whole xfstests
could not be predictable. I'm not sure if we need to handle such case.

> 
> I think this needs to check the output of xfs_growfs to make the
> decision to _notrun.

I could check some golden output such as "data size XXX too small", yet
I still don't think we should check some cases e.g. run out of memory..

Thanks,
Gao Xiang

> 
> --D
> 
> > +	_scratch_unmount
> > +}
> > +
> >  # XFS ability to change UUIDs on V5/CRC filesystems
> >  #
> >  _require_meta_uuid()
> > -- 
> > 2.27.0
> > 
> 

Re: [PATCH v4 2/3] xfs: basic functionality test for shrinking free space in the last AG

[Gao Xiang]

On Mon, May 10, 2021 at 11:01:47AM -0700, Darrick J. Wong wrote:
> On Fri, Apr 02, 2021 at 05:49:36PM +0800, Gao Xiang wrote:
> > Add basic test to make sure the functionality works as expected.
> > 
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > ---
> >  tests/xfs/990     | 73 +++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/990.out | 12 ++++++++
> >  tests/xfs/group   |  1 +
> >  3 files changed, 86 insertions(+)
> >  create mode 100755 tests/xfs/990
> >  create mode 100644 tests/xfs/990.out
> > 
> > diff --git a/tests/xfs/990 b/tests/xfs/990
> > new file mode 100755
> > index 00000000..322b09e1
> > --- /dev/null
> > +++ b/tests/xfs/990
> > @@ -0,0 +1,73 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2021 Red Hat, Inc.  All Rights Reserved.
> > +#
> > +# FS QA Test 990
> > +#
> > +# XFS shrinkfs basic functionality test
> > +#
> > +# This test attempts to shrink with a small size (512K), half AG size and
> > +# an out-of-bound size (agsize + 1) to observe if it works as expected.
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1    # failure is the default!
> > +trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +test_shrink()
> > +{
> > +	$XFS_GROWFS_PROG -D"$1" $SCRATCH_MNT >> $seqres.full 2>&1
> > +	ret=$?
> > +
> > +	_scratch_unmount
> > +	_check_scratch_fs
> > +	_scratch_mount
> > +
> > +	$XFS_INFO_PROG $SCRATCH_MNT 2>&1 | _filter_mkfs 2>$tmp.growfs >/dev/null
> > +	. $tmp.growfs
> > +	[ $ret -eq 0 -a $1 -eq $dblocks ]
> > +}
> > +
> > +# real QA test starts here
> > +_supported_fs xfs
> > +_require_xfs_scratch_shrink
> > +
> > +rm -f $seqres.full
> > +echo "Format and mount"
> > +
> > +# agcount = 1 is forbidden on purpose, and need to ensure shrinking to
> > +# 2 AGs isn't fensible yet. So agcount = 3 is the minimum number now.
> 
> s/fensible/feasible/
> 
> > +_scratch_mkfs -dsize="$((512 * 1024 * 1024))" -dagcount=3 2>&1 | \
> > +	tee -a $seqres.full | _filter_mkfs 2>$tmp.mkfs
> > +. $tmp.mkfs
> > +t_dblocks=$dblocks
> > +_scratch_mount >> $seqres.full
> > +
> > +echo "Shrink fs (small size)"
> > +test_shrink $((t_dblocks-512*1024/dbsize)) || \
> > +	_fail "Shrink fs (small size) failure"
> > +
> > +echo "Shrink fs (half AG)"
> > +test_shrink $((t_dblocks-agsize/2)) || \
> > +	_fail "Shrink fs (half AG) failure"
> > +
> > +echo "Shrink fs (out-of-bound)"
> > +test_shrink $((t_dblocks-agsize-1)) && \
> > +	_fail "Shrink fs (out-of-bound) failure"
> > +[ $dblocks -ne $((t_dblocks-agsize/2)) ] && \
> > +	_fail "dblocks changed after shrinking failure"
> 
> Can these be echo statements, since the diff in golden output will trip
> anyway?

Ok, will update.

Thanks,
Gao Xiang

> 
> --D

Re: [PATCH v4 3/3] xfs: stress test for shrinking free space in the last AG

[Gao Xiang]

On Mon, May 10, 2021 at 11:08:36AM -0700, Darrick J. Wong wrote:
> On Fri, Apr 02, 2021 at 05:49:37PM +0800, Gao Xiang wrote:
> > This adds a stress testcase to shrink free space as much as
> > possible in the last AG with background fsstress workload.
> > 
> > The expectation is that no crash happens with expected output.
> > 
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > ---
> >  tests/xfs/991     | 118 ++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/991.out |   8 ++++
> >  tests/xfs/group   |   1 +
> >  3 files changed, 127 insertions(+)
> >  create mode 100755 tests/xfs/991
> >  create mode 100644 tests/xfs/991.out
> > 
> > diff --git a/tests/xfs/991 b/tests/xfs/991
> > new file mode 100755
> > index 00000000..8ad0b8c7
> > --- /dev/null
> > +++ b/tests/xfs/991
> > @@ -0,0 +1,118 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2020-2021 Red Hat, Inc.  All Rights Reserved.
> > +#
> > +# FS QA Test 991
> > +#
> > +# XFS online shrinkfs stress test
> > +#
> > +# This test attempts to shrink unused space as much as possible with
> > +# background fsstress workload. It will decrease the shrink size if
> > +# larger size fails. And totally repeat 2 * TIME_FACTOR times.
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +create_scratch()
> > +{
> > +	_scratch_mkfs_xfs $@ | tee -a $seqres.full | \
> > +		_filter_mkfs 2>$tmp.mkfs >/dev/null
> > +	. $tmp.mkfs
> > +
> > +	_scratch_mount
> > +	# fix the reserve block pool to a known size so that the enospc
> > +	# calculations work out correctly.
> > +	_scratch_resvblks 1024 > /dev/null 2>&1
> > +}
> > +
> > +fill_scratch()
> > +{
> > +	$XFS_IO_PROG -f -c "falloc 0 $1" $SCRATCH_MNT/resvfile
> > +}
> > +
> > +stress_scratch()
> > +{
> > +	local procs=3
> > +	local nops=$((1000 * LOAD_FACTOR))
> 
> Um... _scale_fsstress_args already scales the -p and -n arguments, why
> is it necessary to scale nops by time /and/ load?

Yeah, I forgot to check what _scale_fsstress_args's implemented.
Yes, it's unnecessary. will fix.

> 
> > +	# -w ensures that the only ops are ones which cause write I/O
> > +	local FSSTRESS_ARGS=`_scale_fsstress_args -d $SCRATCH_MNT -w \
> > +		-p $procs -n $nops $FSSTRESS_AVOID`
> > +	$FSSTRESS_PROG $FSSTRESS_ARGS >> $seqres.full 2>&1
> > +}
> > +
> > +# real QA test starts here
> > +_supported_fs xfs
> > +_require_xfs_scratch_shrink
> > +_require_xfs_io_command "falloc"
> > +
> > +rm -f $seqres.full
> > +_scratch_mkfs_xfs | tee -a $seqres.full | _filter_mkfs 2>$tmp.mkfs
> > +. $tmp.mkfs	# extract blocksize and data size for scratch device
> > +
> > +decsize=`expr  42 \* 1048576`	# shrink in chunks of this size at most
> 
> Might it be nice to inject a little bit of randomness here?

okay, will update.

> 
> > +endsize=`expr 125 \* 1048576`	# stop after shrinking this big
> > +[ `expr $endsize / $dbsize` -lt $dblocks ] || _notrun "Scratch device too small"
> > +
> > +nags=2
> > +totalcount=$((2 * TIME_FACTOR))
> > +
> > +while [ $totalcount -gt 0 ]; do
> > +	size=`expr 1010 \* 1048576`	# 1010 megabytes initially
> > +	logblks=$(_scratch_find_xfs_min_logblocks -dsize=${size} -dagcount=${nags})
> 
> (Does all this logic still work if an external log device is present?)

I didn't check the external log device case, but it seems
_scratch_find_xfs_min_logblocks was used in many cases before?
(e.g. xfs/104, how such cases work with an external log device?)

> 
> > +
> > +	create_scratch -lsize=${logblks}b -dsize=${size} -dagcount=${nags}
> > +
> > +	for i in `seq 125 -1 90`; do
> > +		fillsize=`expr $i \* 1048576`
> > +		out="$(fill_scratch $fillsize 2>&1)"
> > +		echo "$out" | grep -q 'No space left on device' && continue
> > +		test -n "${out}" && echo "$out"
> > +		break
> > +	done
> > +
> > +	while [ $size -gt $endsize ]; do
> > +		stress_scratch &
> > +		sleep 1
> > +
> > +		decb=`expr $decsize / $dbsize`    # in data blocks
> > +		while [ $decb -gt 0 ]; do
> > +			sizeb=`expr $size / $dbsize - $decb`
> > +
> > +			$XFS_GROWFS_PROG -D ${sizeb} $SCRATCH_MNT \
> > +				>> $seqres.full 2>&1 && break
> > +
> > +			[ $decb -gt 100 ] && decb=`expr $decb + $RANDOM % 10`
> > +			decb=`expr $decb / 2`
> > +		done
> > +
> > +		wait
> > +		[ $decb -eq 0 ] && break
> > +
> > +		# get latest dblocks
> > +		$XFS_INFO_PROG $SCRATCH_MNT 2>&1 | _filter_mkfs 2>$tmp.growfs >/dev/null
> > +		. $tmp.growfs
> > +
> > +		size=`expr $dblocks \* $dbsize`
> > +		_scratch_unmount
> > +		_repair_scratch_fs >> $seqres.full
> 
> Why isn't "_scratch_xfs_repair -n" here sufficient?

That sounds better if '-n' can detect all cases without '-n' (since I've
heard previously some buggy fsckxxx doesn't produce the same result with
or without '-n'...)

Will update.

> 
> > +		_scratch_mount
> > +	done
> > +
> > +	_scratch_unmount
> > +	_repair_scratch_fs >> $seqres.full
> 
> ...and here?

Will update too.

Thanks,
Gao Xiang


> 
> > +	totalcount=`expr $totalcount - 1`
> > +done
> > +
> > +echo "*** done"
> > +status=0
> > +exit
> > diff --git a/tests/xfs/991.out b/tests/xfs/991.out
> > new file mode 100644
> > index 00000000..e8209672
> > --- /dev/null
> > +++ b/tests/xfs/991.out
> > @@ -0,0 +1,8 @@
> > +QA output created by 991
> > +meta-data=DDEV isize=XXX agcount=N, agsize=XXX blks
> > +data     = bsize=XXX blocks=XXX, imaxpct=PCT
> > +         = sunit=XXX swidth=XXX, unwritten=X
> > +naming   =VERN bsize=XXX
> > +log      =LDEV bsize=XXX blocks=XXX
> > +realtime =RDEV extsz=XXX blocks=XXX, rtextents=XXX
> > +*** done
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index 472c8f9a..53e68bea 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -521,3 +521,4 @@
> >  538 auto stress
> >  539 auto quick mount
> >  990 auto quick growfs shrinkfs
> > +991 auto growfs shrinkfs ioctl prealloc stress
> > -- 
> > 2.27.0
> > 
> 

Re: [PATCH v4 1/3] common/xfs: add _require_xfs_scratch_shrink helper

[Darrick J. Wong]

On Tue, May 11, 2021 at 10:02:48AM +0800, Gao Xiang wrote:
> On Mon, May 10, 2021 at 10:59:52AM -0700, Darrick J. Wong wrote:
> > On Fri, Apr 02, 2021 at 05:49:35PM +0800, Gao Xiang wrote:
> > > In order to detect whether the current kernel supports XFS shrinking.
> > > 
> > > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > > ---
> > >  common/xfs | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/common/xfs b/common/xfs
> > > index 69f76d6e..c6c2e3f5 100644
> > > --- a/common/xfs
> > > +++ b/common/xfs
> > > @@ -766,6 +766,20 @@ _require_xfs_mkfs_without_validation()
> > >  	fi
> > >  }
> > >  
> > > +_require_xfs_scratch_shrink()
> > > +{
> > > +	_require_scratch
> > > +	_require_command "$XFS_GROWFS_PROG" xfs_growfs
> > > +
> > > +	_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs >/dev/null
> > > +	. $tmp.mkfs
> > > +	_scratch_mount
> > > +	# here just to check if kernel supports, no need do more extra work
> > > +	$XFS_GROWFS_PROG -D$((dblocks-1)) "$SCRATCH_MNT" > /dev/null 2>&1 || \
> > > +		_notrun "kernel does not support shrinking"
> > 
> > I think isn't sufficiently precise -- if xfs_growfs (userspace) doesn't
> > support shrinking it'll error out with "data size XXX too small", and if
> > the kernel doesn't support shrink, it'll return EINVAL.
> 
> I'm not sure if we need to identify such 2 cases (xfsprogs doesn't support
> and/or kernel doesn't support), but if it's really needed I think I could
> update it. But I've confirmed with testing that both two cases can be
> handled with the statements above properly.
> 
> > 
> > As written, this code attempts a single-block shrink and disables the
> > entire test if that fails for any reason, even if that reason is that
> > the last block in the filesystem isn't free, or we ran out of memory, or
> > something like that.
> 
> hmm... the filesystem here is brandly new, I think at least it'd be
> considered as "the last block in the new filesystem is free". If we're
> worried that such promise could be broken, I think some other golden
> output is unstable as well (although unrelated to this.) By that time,
> I think the test script should be updated then instead. Or am I missing
> something?
> 
> If we're worried about runing out of memory, I think the whole xfstests
> could not be predictable. I'm not sure if we need to handle such case.

I'm not specifically worried about running out of memory, I'm mostly
worried that some /other/ implementation bug (or disk format variation)
will show up and triggers the _notrun, and nobody will notice that the
shrink tests quietly stop running.

--D

> > 
> > I think this needs to check the output of xfs_growfs to make the
> > decision to _notrun.
> 
> I could check some golden output such as "data size XXX too small", yet
> I still don't think we should check some cases e.g. run out of memory..
> 
> Thanks,
> Gao Xiang
> 
> > 
> > --D
> > 
> > > +	_scratch_unmount
> > > +}
> > > +
> > >  # XFS ability to change UUIDs on V5/CRC filesystems
> > >  #
> > >  _require_meta_uuid()
> > > -- 
> > > 2.27.0
> > > 
> > 
>