[PATCH v2 0/4] test for XFS umount hang caused by the pending dquota log item in AIL

[PATCH v2 0/4] test for XFS umount hang caused by the pending dquota log item in AIL

[Hou Tao]

Hi,

This is the v2 patchset for testing the XFS umount hang problem caused by
the pending dquota log item in AIL. The problem has not been fixed in
XFS yet, and Carlos Maiolino is working on it [1]. I had tested
it on 4.14-rc7, and the problem can be reproduced reliably.

Changes since v1:
* dmflakey: support multiple dm targets for a dm-flakey device
	* no update

* xfs: test for umount hang caused by the pending dquota log item in AIL
	* a bunch of suggestions from Eryu Guan
	* update the dquota buffer after the error injection to better
	  reproduce the problem

* common/rc: support checking the version of dm-target in
_require_dm_target()
	* new patch
	* introduce a way to check the availability of error_write feature
	in dm-flakey target, and accomplish it by checking the version of
	dm-target.

* common/rc: factor out _get|set_xfs_scratch_sb_field()
	* new patch
	* the modified test cases had been tested after applying the patch:
	(1) xfs/098 failed, but the failure has nothing to do with the patch
	(2) xfs/339 and xfs/340 were not runnable, because rtdev + rmap is not
	  supported now.
	(3) the remaing test cases passed

v1:
	* https://www.spinics.net/lists/fstests/msg07622.html

Comments and questions are welcome.

Regards,
Tao

[1]: https://www.spinics.net/lists/linux-xfs/msg11614.html
---
Hou Tao (4):
  dmflakey: support multiple dm targets for a dm-flakey device
  common/rc: support checking the version of dm-target in
    _require_dm_target()
  xfs: test for umount hang caused by the pending dquota log item in AIL
  common/rc: factor out _get|set_xfs_scratch_sb_field()

 common/dmflakey   |   2 +-
 common/rc         |  39 +++++++++++--
 common/xfs        |  20 +++++++
 tests/xfs/007     |   6 +-
 tests/xfs/098     |   4 +-
 tests/xfs/186     |   3 +-
 tests/xfs/199     |  13 ++---
 tests/xfs/307     |  11 +---
 tests/xfs/308     |  11 +---
 tests/xfs/339     |   6 +-
 tests/xfs/340     |   2 +-
 tests/xfs/999     | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/999.out |   2 +
 tests/xfs/group   |   1 +
 14 files changed, 238 insertions(+), 45 deletions(-)
 create mode 100755 tests/xfs/999
 create mode 100644 tests/xfs/999.out

-- 
2.9.5

[PATCH v2 1/4] dmflakey: support multiple dm targets for a dm-flakey device

[Hou Tao]

dm-flakey can be used to emulate IO write error, however, when
we also need to prevent the IO error for a specific range of the
block device (eg., the log region of a XFS), we need to specify
multiple dm targets for the dm device.

Option --table can not accommodate the multiple dm targets case,
so let dmsetup get the possible-multiple-targets table from
standard input.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 common/dmflakey | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/dmflakey b/common/dmflakey
index 4434307..16b82d2 100644
--- a/common/dmflakey
+++ b/common/dmflakey
@@ -77,7 +77,7 @@ _load_flakey_table()
 	$DMSETUP_PROG suspend $suspend_opt flakey-test
 	[ $? -ne 0 ] && _fatal "failed to suspend flakey-test"
 
-	$DMSETUP_PROG load flakey-test --table "$table"
+	echo -e "$table" | $DMSETUP_PROG load flakey-test
 	[ $? -ne 0 ] && _fatal "failed to load table into flakey-test"
 
 	$DMSETUP_PROG resume flakey-test
-- 
2.9.5

[PATCH v2 2/4] common/rc: support checking the version of dm-target in _require_dm_target()

[Hou Tao]

Some features of dm-target are not available on the old linux kernel,
and we can use the version of dm-target to check the availability.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 common/rc | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/common/rc b/common/rc
index e2a8229..9ea84ba 100644
--- a/common/rc
+++ b/common/rc
@@ -1779,10 +1779,12 @@ _require_sane_bdev_flush()
 	fi
 }
 
-# this test requires a specific device mapper target
+# this test requires a specific device mapper target and
+# an optional version number (e.g., 1.4.0)
 _require_dm_target()
 {
-	_target=$1
+	local target=$1
+	local version=$2
 
 	# require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF
 	# behaviour
@@ -1790,11 +1792,38 @@ _require_dm_target()
 	_require_sane_bdev_flush $SCRATCH_DEV
 	_require_command "$DMSETUP_PROG" dmsetup
 
-	modprobe dm-$_target >/dev/null 2>&1
+	modprobe dm-$target >/dev/null 2>&1
 
-	$DMSETUP_PROG targets 2>&1 | grep -q ^$_target
+	$DMSETUP_PROG targets 2>&1 | grep -q ^$target
 	if [ $? -ne 0 ]; then
-		_notrun "This test requires dm $_target support"
+		_notrun "This test requires dm $target support"
+	fi
+
+	if [ -n "$version" ]; then
+		local got
+
+		got=$($DMSETUP_PROG targets 2>&1 | \
+				awk -v tgt=$target '$0 ~ tgt {sub("v", "", $2); print $2}')
+		if [ -z "$got" ]; then
+			_notrun "This test requires dm $target $version at least (got unknown)"
+		fi
+
+		awk -v min=$version -v got=$got '
+		BEGIN \
+		{
+			cmin = split(min, amin, ".");
+			cgot = split(got, agot, ".");
+			for (i = 1; i <= cgot && i <= cmin; i++) {
+				if (agot[i] != amin[i]) {
+					exit(agot[i] < amin[i]);
+				}
+			}
+			exit(cgot < cmin);
+		}
+		'
+		if [ $? -ne 0 ]; then
+			_notrun "This test requires dm $target $version at least (got $got)"
+		fi
 	fi
 }
 
-- 
2.9.5

Re: [PATCH v2 2/4] common/rc: support checking the version of dm-target in _require_dm_target()

[Eryu Guan]

On Wed, Nov 08, 2017 at 04:02:48PM +0800, Hou Tao wrote:
> Some features of dm-target are not available on the old linux kernel,
> and we can use the version of dm-target to check the availability.

We don't check version numbers in fstests, but actually try what is
requested and _notrun if the test run failed.

I think we can keep _require_dm_target unchanged and add a new
_require_dmflakey_error_writes in common/dmflakey, e.g.

_require_dmflakey_error_writes()
{
	_require_dm_targets flakey

	<try creating flakey table with error_writes option here and
	_notrun if failed>
}

And call this new _require rule in the test.

Thanks,
Eryu

Re: [PATCH v2 2/4] common/rc: support checking the version of dm-target in _require_dm_target()

[Hou Tao]

Hi,

On 2017/11/8 17:49, Eryu Guan wrote:
> On Wed, Nov 08, 2017 at 04:02:48PM +0800, Hou Tao wrote:
>> Some features of dm-target are not available on the old linux kernel,
>> and we can use the version of dm-target to check the availability.
> 
> We don't check version numbers in fstests, but actually try what is
> requested and _notrun if the test run failed.

OK.

> I think we can keep _require_dm_target unchanged and add a new
> _require_dmflakey_error_writes in common/dmflakey, e.g.
> 
> _require_dmflakey_error_writes()
> {
> 	_require_dm_targets flakey
> 
> 	<try creating flakey table with error_writes option here and
> 	_notrun if failed>
> }
> 
> And call this new _require rule in the test.

Will do it.

Thanks.
Tao

> Thanks,
> Eryu
> 
> .
> 

[PATCH v2 3/4] xfs: test for umount hang caused by the pending dquota log item in AIL

[Hou Tao]

When the first writeback and the retried writeback of dquota buffer get
the same IO error, XFS will let xfsaild to restart the writeback and
xfs_qm_dqflush_done() will not be invoked. xfsaild will try to re-push
the quota log item in AIL, the push will return early everytime after
checking xfs_dqflock_nowait(), and xfsaild will try to push it again.

IOWs, AIL will never be empty, and the umount process will wait for the
drain of AIL, so the umount process hangs.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 tests/xfs/999     | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/999.out |   2 +
 tests/xfs/group   |   1 +
 3 files changed, 174 insertions(+)
 create mode 100755 tests/xfs/999
 create mode 100644 tests/xfs/999.out

diff --git a/tests/xfs/999 b/tests/xfs/999
new file mode 100755
index 0000000..cea2f85
--- /dev/null
+++ b/tests/xfs/999
@@ -0,0 +1,171 @@
+#! /bin/bash
+# FS QA Test No. 999
+#
+# Test for XFS umount hang problem caused by the unceasing push
+# of dquot log item in AIL. Because xfs_qm_dqflush_done() will
+# not be invoked, so each time xfsaild initiates the push,
+# the push will return early after checking xfs_dqflock_nowait().
+#
+# xfs_qm_dqflush_done() should be invoked by xfs_buf_do_callbacks().
+# However after the first write and the retried write of dquota buffer
+# get the same IO error, XFS will let xfsaild to restart the write and
+# xfs_buf_do_callbacks() will not be inovked.
+#
+# This test emulates the write error by using dm-flakey. The log
+# area of the XFS filesystem is excluded from the range covered by
+# dm-flakey, so the XFS will not be shutdown prematurely.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Huawei Technologies Co., Ltd. All Rights Reserved.
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	[ -z "${interval}" ] || \
+		sysctl -w fs.xfs.xfssyncd_centisecs=${interval} >/dev/null 2>&1
+	cd /
+	rm -f $tmp.*
+	_unmount_flakey >/dev/null 2>&1
+	_cleanup_flakey > /dev/null 2>&1
+}
+
+_get_xfs_scratch_sb_field()
+{
+	local field=$1
+
+	_scratch_xfs_db -r -c "sb 0" -c "print $field" | \
+	awk -v field=$field '$0 ~ field {print $3}'
+}
+
+# inject IO write error for the XFS filesystem except its log section
+make_xfs_scratch_flakey_table()
+{
+	local tgt=flakey
+	local opt="0 1 1 error_writes"
+	local dev=${SCRATCH_DEV}
+	local dev_sz=$(blockdev --getsz $dev)
+
+	if [ "${USE_EXTERNAL}" = "yes" -a ! -z "$SCRATCH_LOGDEV" ]; then
+		echo "0 ${dev_sz} $tgt $dev 0 $opt"
+		return
+	fi
+
+	local blk_sz=$(_get_xfs_scratch_sb_field blocksize)
+	local log_ofs=$(_get_xfs_scratch_sb_field logstart)
+	local log_sz=$(_get_xfs_scratch_sb_field logblocks)
+	local table=""
+	local ofs=0
+	local sz
+
+	let "log_ofs *= blk_sz / 512"
+	let "log_sz *= blk_sz / 512"
+
+	if [ "$ofs" -lt "${log_ofs}" ]; then
+		let "sz = log_ofs - ofs"
+		table="$ofs $sz $tgt $dev $ofs $opt"
+	fi
+
+	table="$table\n${log_ofs} ${log_sz} linear $dev ${log_ofs}"
+
+	let "ofs = log_ofs + log_sz"
+	if [ "$ofs" -lt "${dev_sz}" ]; then
+		let "sz = dev_sz - ofs"
+		table="$table\n$ofs $sz $tgt $dev $ofs $opt"
+	fi
+
+	echo -e $table
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/dmflakey
+. ./common/quota
+
+_supported_fs xfs
+_supported_os Linux
+
+# due to the injection of write IO error, the fs will be inconsistent
+_require_scratch_nocheck
+# error_writes feature is introduced in dm-flakey v1.4.0, so check for that
+_require_dm_target flakey "1.4.0"
+_require_user
+_require_xfs_quota
+_require_freeze
+
+rm -f $seqres.full
+
+echo "Silence is golden"
+
+_scratch_mkfs_xfs > $seqres.full 2>&1
+
+# no error will be injected
+_init_flakey
+$DMSETUP_PROG info >> $seqres.full
+$DMSETUP_PROG table >> $seqres.full
+
+# save the old value for _cleanup()
+interval=$(sysctl -n fs.xfs.xfssyncd_centisecs 2>/dev/null)
+# shorten the time waiting for the push of ail items
+sysctl -w fs.xfs.xfssyncd_centisecs=100 >> $seqres.full 2>&1
+
+_qmount_option "usrquota"
+_mount_flakey
+
+# We need to set the quota limitation twice, and inject the write error
+# after the second setting. If we try to inject the write error after
+# the first setting, the initialization of the dquota buffer will get
+# IO error and also be retried, and during the umount process the
+# write will be ended, and xfs_qm_dqflush_done() will be inovked, and
+# the umount will exit normally.
+$XFS_QUOTA_PROG -x -c "limit -u isoft=500 fsgqa" $SCRATCH_MNT
+$XFS_QUOTA_PROG -x -c "report -ih" $SCRATCH_MNT >> $seqres.full
+
+# ensure the initialization of the dquota buffer is done
+xfs_freeze -f $SCRATCH_MNT
+xfs_freeze -u $SCRATCH_MNT
+
+# inject write IO error
+FLAKEY_TABLE_DROP=$(make_xfs_scratch_flakey_table)
+_load_flakey_table ${FLAKEY_DROP_WRITES}
+$DMSETUP_PROG info >> $seqres.full
+$DMSETUP_PROG table >> $seqres.full
+
+# update the dquota buffer
+$XFS_QUOTA_PROG -x -c "limit -u isoft=400 fsgqa" $SCRATCH_MNT
+$XFS_QUOTA_PROG -x -c "report -ih" $SCRATCH_MNT >> $seqres.full
+
+sync
+
+# wait for the push of the dquota log item in AIL and
+# the completion of the retried write of dquota buffer
+sleep 2
+
+_unmount_flakey
+
+_cleanup_flakey
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/999.out b/tests/xfs/999.out
new file mode 100644
index 0000000..3b276ca
--- /dev/null
+++ b/tests/xfs/999.out
@@ -0,0 +1,2 @@
+QA output created by 999
+Silence is golden
diff --git a/tests/xfs/group b/tests/xfs/group
index b439842..127019a 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -431,3 +431,4 @@
 431 auto quick dangerous
 432 auto quick dir metadata
 433 auto quick attr
+999 auto quick quota dangerous
-- 
2.9.5

Re: [PATCH v2 3/4] xfs: test for umount hang caused by the pending dquota log item in AIL

[Eryu Guan]

On Wed, Nov 08, 2017 at 04:02:49PM +0800, Hou Tao wrote:
> When the first writeback and the retried writeback of dquota buffer get
> the same IO error, XFS will let xfsaild to restart the writeback and
> xfs_qm_dqflush_done() will not be invoked. xfsaild will try to re-push
> the quota log item in AIL, the push will return early everytime after
> checking xfs_dqflock_nowait(), and xfsaild will try to push it again.
> 
> IOWs, AIL will never be empty, and the umount process will wait for the
> drain of AIL, so the umount process hangs.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>

I still couldn't reproduce the hang either on my test vms nor a real
hardware. I've confirmed /sys/fs/xfs/<dev>/error/fail_at_unmount is the
default value (1), and all max_retries and retry_timeout_seconds are -1
in /sys/fs/xfs/<dev>/error/metadata/*

999.full
meta-data=/dev/sda6              isize=512    agcount=4, agsize=983040 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=0, rmapbt=0, reflink=0
data     =                       bsize=4096   blocks=3932160, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
log      =internal log           bsize=4096   blocks=2560, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
Name:              flakey-test
State:             ACTIVE
Read Ahead:        256
Tables present:    LIVE
Open count:        0
Event number:      0
Major, minor:      253, 0
Number of targets: 1

flakey-test: 0 31457280 flakey 8:6 0 180 0 0 
fs.xfs.xfssyncd_centisecs = 100
MOUNT_OPTIONS =  -o usrquota
User quota on /mnt/testarea/scratch (/dev/mapper/flakey-test)
                        Inodes              
User ID      Used   Soft   Hard Warn/Grace  
---------- --------------------------------- 
root            3      0      0  00 [------]
fsgqa           0    500      0  00 [------]

Name:              flakey-test
State:             ACTIVE
Read Ahead:        256
Tables present:    LIVE
Open count:        1
Event number:      0
Major, minor:      253, 0
Number of targets: 3

flakey-test: 0 16777256 flakey 8:6 0 0 1 1 error_writes 
flakey-test: 16777256 20480 linear 8:6 16777256
flakey-test: 16797736 14659544 flakey 8:6 16797736 0 1 1 error_writes 
User quota on /mnt/testarea/scratch (/dev/mapper/flakey-test)
                        Inodes              
User ID      Used   Soft   Hard Warn/Grace  
---------- --------------------------------- 
root            3      0      0  00 [------]
fsgqa           0    400      0  00 [------]

dmesg log
[696341.359293] run fstests xfs/999 at 2017-11-08 17:50:12
[696341.670317] XFS (sda6): Unmounting Filesystem
[696341.950979] XFS (dm-0): Mounting V5 Filesystem
[696341.968803] XFS (dm-0): Ending clean mount
[696341.971460] XFS (dm-0): Quotacheck needed: Please wait.
[696341.981099] XFS (dm-0): Quotacheck: Done.
[696342.201576] XFS (dm-0): metadata I/O error: block 0xf00049 ("xlog_iodone") error 5 numblks 64
[696342.203589] XFS (dm-0): xfs_do_force_shutdown(0x2) called from line 1260 of file fs/xfs/xfs_log.c.  Return address = 0xffffffffa0158ee6
[696342.206708] XFS (dm-0): Log I/O Error Detected.  Shutting down filesystem
[696342.208160] XFS (dm-0): Please umount the filesystem and rectify the problem(s)
[696344.222212] XFS (dm-0): Unmounting Filesystem
[696344.223587] XFS (dm-0): xfs_qm_dqpurge: dquot ffff8801b75270e0 flush failed
[696344.413415] XFS (sda5): Unmounting Filesystem

> ---
>  tests/xfs/999     | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |   2 +
>  tests/xfs/group   |   1 +
>  3 files changed, 174 insertions(+)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> diff --git a/tests/xfs/999 b/tests/xfs/999
> new file mode 100755
> index 0000000..cea2f85
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,171 @@
> +#! /bin/bash
> +# FS QA Test No. 999
> +#
> +# Test for XFS umount hang problem caused by the unceasing push
> +# of dquot log item in AIL. Because xfs_qm_dqflush_done() will
> +# not be invoked, so each time xfsaild initiates the push,
> +# the push will return early after checking xfs_dqflock_nowait().
> +#
> +# xfs_qm_dqflush_done() should be invoked by xfs_buf_do_callbacks().
> +# However after the first write and the retried write of dquota buffer
> +# get the same IO error, XFS will let xfsaild to restart the write and
> +# xfs_buf_do_callbacks() will not be inovked.
> +#
> +# This test emulates the write error by using dm-flakey. The log
> +# area of the XFS filesystem is excluded from the range covered by
> +# dm-flakey, so the XFS will not be shutdown prematurely.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Huawei Technologies Co., Ltd. All Rights Reserved.
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	[ -z "${interval}" ] || \
> +		sysctl -w fs.xfs.xfssyncd_centisecs=${interval} >/dev/null 2>&1
> +	cd /
> +	rm -f $tmp.*
> +	_unmount_flakey >/dev/null 2>&1
> +	_cleanup_flakey > /dev/null 2>&1
> +}
> +
> +_get_xfs_scratch_sb_field()
> +{
> +	local field=$1
> +
> +	_scratch_xfs_db -r -c "sb 0" -c "print $field" | \
> +	awk -v field=$field '$0 ~ field {print $3}'
        ^^^ $AWK_PROG
> +}

Put this function in common/xfs in the first place, no need to move it
there in patch 4.

> +
> +# inject IO write error for the XFS filesystem except its log section
> +make_xfs_scratch_flakey_table()
> +{
> +	local tgt=flakey
> +	local opt="0 1 1 error_writes"
> +	local dev=${SCRATCH_DEV}
> +	local dev_sz=$(blockdev --getsz $dev)
> +
> +	if [ "${USE_EXTERNAL}" = "yes" -a ! -z "$SCRATCH_LOGDEV" ]; then
> +		echo "0 ${dev_sz} $tgt $dev 0 $opt"
> +		return
> +	fi
> +
> +	local blk_sz=$(_get_xfs_scratch_sb_field blocksize)
> +	local log_ofs=$(_get_xfs_scratch_sb_field logstart)
> +	local log_sz=$(_get_xfs_scratch_sb_field logblocks)
> +	local table=""
> +	local ofs=0
> +	local sz
> +
> +	let "log_ofs *= blk_sz / 512"
> +	let "log_sz *= blk_sz / 512"
> +
> +	if [ "$ofs" -lt "${log_ofs}" ]; then
> +		let "sz = log_ofs - ofs"
> +		table="$ofs $sz $tgt $dev $ofs $opt"
> +	fi
> +
> +	table="$table\n${log_ofs} ${log_sz} linear $dev ${log_ofs}"
> +
> +	let "ofs = log_ofs + log_sz"
> +	if [ "$ofs" -lt "${dev_sz}" ]; then
> +		let "sz = dev_sz - ofs"
> +		table="$table\n$ofs $sz $tgt $dev $ofs $opt"
> +	fi
> +
> +	echo -e $table
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/dmflakey
> +. ./common/quota
> +
> +_supported_fs xfs
> +_supported_os Linux
> +
> +# due to the injection of write IO error, the fs will be inconsistent
> +_require_scratch_nocheck
> +# error_writes feature is introduced in dm-flakey v1.4.0, so check for that
> +_require_dm_target flakey "1.4.0"
> +_require_user
> +_require_xfs_quota
> +_require_freeze
> +
> +rm -f $seqres.full
> +
> +echo "Silence is golden"
> +
> +_scratch_mkfs_xfs > $seqres.full 2>&1
> +
> +# no error will be injected
> +_init_flakey
> +$DMSETUP_PROG info >> $seqres.full
> +$DMSETUP_PROG table >> $seqres.full
> +
> +# save the old value for _cleanup()
> +interval=$(sysctl -n fs.xfs.xfssyncd_centisecs 2>/dev/null)
> +# shorten the time waiting for the push of ail items
> +sysctl -w fs.xfs.xfssyncd_centisecs=100 >> $seqres.full 2>&1
> +
> +_qmount_option "usrquota"
> +_mount_flakey
> +
> +# We need to set the quota limitation twice, and inject the write error
> +# after the second setting. If we try to inject the write error after
> +# the first setting, the initialization of the dquota buffer will get
> +# IO error and also be retried, and during the umount process the
> +# write will be ended, and xfs_qm_dqflush_done() will be inovked, and
> +# the umount will exit normally.
> +$XFS_QUOTA_PROG -x -c "limit -u isoft=500 fsgqa" $SCRATCH_MNT

Use $qa_user instead of the hardcoded "fsgqa".

Thanks,
Eryu

> +$XFS_QUOTA_PROG -x -c "report -ih" $SCRATCH_MNT >> $seqres.full
> +
> +# ensure the initialization of the dquota buffer is done
> +xfs_freeze -f $SCRATCH_MNT
> +xfs_freeze -u $SCRATCH_MNT
> +
> +# inject write IO error
> +FLAKEY_TABLE_DROP=$(make_xfs_scratch_flakey_table)
> +_load_flakey_table ${FLAKEY_DROP_WRITES}
> +$DMSETUP_PROG info >> $seqres.full
> +$DMSETUP_PROG table >> $seqres.full
> +
> +# update the dquota buffer
> +$XFS_QUOTA_PROG -x -c "limit -u isoft=400 fsgqa" $SCRATCH_MNT
> +$XFS_QUOTA_PROG -x -c "report -ih" $SCRATCH_MNT >> $seqres.full
> +
> +sync
> +
> +# wait for the push of the dquota log item in AIL and
> +# the completion of the retried write of dquota buffer
> +sleep 2
> +
> +_unmount_flakey
> +
> +_cleanup_flakey
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> new file mode 100644
> index 0000000..3b276ca
> --- /dev/null
> +++ b/tests/xfs/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> diff --git a/tests/xfs/group b/tests/xfs/group
> index b439842..127019a 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -431,3 +431,4 @@
>  431 auto quick dangerous
>  432 auto quick dir metadata
>  433 auto quick attr
> +999 auto quick quota dangerous
> -- 
> 2.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/4] xfs: test for umount hang caused by the pending dquota log item in AIL

[Hou Tao]

Hi,

On 2017/11/8 17:56, Eryu Guan wrote:
> On Wed, Nov 08, 2017 at 04:02:49PM +0800, Hou Tao wrote:
>> When the first writeback and the retried writeback of dquota buffer get
>> the same IO error, XFS will let xfsaild to restart the writeback and
>> xfs_qm_dqflush_done() will not be invoked. xfsaild will try to re-push
>> the quota log item in AIL, the push will return early everytime after
>> checking xfs_dqflock_nowait(), and xfsaild will try to push it again.
>>
>> IOWs, AIL will never be empty, and the umount process will wait for the
>> drain of AIL, so the umount process hangs.
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
> 
> I still couldn't reproduce the hang either on my test vms nor a real
> hardware. I've confirmed /sys/fs/xfs/<dev>/error/fail_at_unmount is the
> default value (1), and all max_retries and retry_timeout_seconds are -1
> in /sys/fs/xfs/<dev>/error/metadata/*
> 
> 999.full
> meta-data=/dev/sda6              isize=512    agcount=4, agsize=983040 blks
>          =                       sectsz=512   attr=2, projid32bit=1
>          =                       crc=1        finobt=1, sparse=0, rmapbt=0, reflink=0
> data     =                       bsize=4096   blocks=3932160, imaxpct=25
>          =                       sunit=0      swidth=0 blks
> naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
> log      =internal log           bsize=4096   blocks=2560, version=2
>          =                       sectsz=512   sunit=0 blks, lazy-count=1
> realtime =none                   extsz=4096   blocks=0, rtextents=0
> Name:              flakey-test
> State:             ACTIVE
> Read Ahead:        256
> Tables present:    LIVE
> Open count:        0
> Event number:      0
> Major, minor:      253, 0
> Number of targets: 1
> 
> flakey-test: 0 31457280 flakey 8:6 0 180 0 0 
> fs.xfs.xfssyncd_centisecs = 100
> MOUNT_OPTIONS =  -o usrquota
> User quota on /mnt/testarea/scratch (/dev/mapper/flakey-test)
>                         Inodes              
> User ID      Used   Soft   Hard Warn/Grace  
> ---------- --------------------------------- 
> root            3      0      0  00 [------]
> fsgqa           0    500      0  00 [------]
> 
> Name:              flakey-test
> State:             ACTIVE
> Read Ahead:        256
> Tables present:    LIVE
> Open count:        1
> Event number:      0
> Major, minor:      253, 0
> Number of targets: 3
> 
> flakey-test: 0 16777256 flakey 8:6 0 0 1 1 error_writes 
> flakey-test: 16777256 20480 linear 8:6 16777256
> flakey-test: 16797736 14659544 flakey 8:6 16797736 0 1 1 error_writes 
> User quota on /mnt/testarea/scratch (/dev/mapper/flakey-test)
>                         Inodes              
> User ID      Used   Soft   Hard Warn/Grace  
> ---------- --------------------------------- 
> root            3      0      0  00 [------]
> fsgqa           0    400      0  00 [------]
> 
> dmesg log
> [696341.359293] run fstests xfs/999 at 2017-11-08 17:50:12
> [696341.670317] XFS (sda6): Unmounting Filesystem
> [696341.950979] XFS (dm-0): Mounting V5 Filesystem
> [696341.968803] XFS (dm-0): Ending clean mount
> [696341.971460] XFS (dm-0): Quotacheck needed: Please wait.
> [696341.981099] XFS (dm-0): Quotacheck: Done.
> [696342.201576] XFS (dm-0): metadata I/O error: block 0xf00049 ("xlog_iodone") error 5 numblks 64
> [696342.203589] XFS (dm-0): xfs_do_force_shutdown(0x2) called from line 1260 of file fs/xfs/xfs_log.c.  Return address = 0xffffffffa0158ee6
> [696342.206708] XFS (dm-0): Log I/O Error Detected.  Shutting down filesystem
> [696342.208160] XFS (dm-0): Please umount the filesystem and rectify the problem(s)

Oops, the calculation of log_ofs in make_xfs_scratch_flakey_table() is incorrect,
so the block 15728713 (0xf00049) is included in the first target of flakey-test,
but it should locate in the second target of flakey-test.

The first target is flakey target instead of linear target, so the write of the log block
got IO errors, and XFS was shutdown prematurely, and the write of dquota buffer was aborted
instead of retried, so no hang.

In make_xfs_scratch_flakey_table(), i calculate log_ofs by using the following commands:
    let "log_ofs *= blk_sz / 512",
I should use convert cmd in xfs_db instead, and i will fix it in v3:
    log_ofs=$(_scratch_xfs_db -r -c "convert fsb ${log_ofs} bb" | \
            awk '{gsub("[()]", "", $2); print $2}')

Thanks,
Tao

> [696344.222212] XFS (dm-0): Unmounting Filesystem
> [696344.223587] XFS (dm-0): xfs_qm_dqpurge: dquot ffff8801b75270e0 flush failed
> [696344.413415] XFS (sda5): Unmounting Filesystem
> 
>> ---
>>  tests/xfs/999     | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/xfs/999.out |   2 +
>>  tests/xfs/group   |   1 +
>>  3 files changed, 174 insertions(+)
>>  create mode 100755 tests/xfs/999
>>  create mode 100644 tests/xfs/999.out
>>
>> diff --git a/tests/xfs/999 b/tests/xfs/999
>> new file mode 100755
>> index 0000000..cea2f85
>> --- /dev/null
>> +++ b/tests/xfs/999
>> @@ -0,0 +1,171 @@
>> +#! /bin/bash
>> +# FS QA Test No. 999
>> +#
>> +# Test for XFS umount hang problem caused by the unceasing push
>> +# of dquot log item in AIL. Because xfs_qm_dqflush_done() will
>> +# not be invoked, so each time xfsaild initiates the push,
>> +# the push will return early after checking xfs_dqflock_nowait().
>> +#
>> +# xfs_qm_dqflush_done() should be invoked by xfs_buf_do_callbacks().
>> +# However after the first write and the retried write of dquota buffer
>> +# get the same IO error, XFS will let xfsaild to restart the write and
>> +# xfs_buf_do_callbacks() will not be inovked.
>> +#
>> +# This test emulates the write error by using dm-flakey. The log
>> +# area of the XFS filesystem is excluded from the range covered by
>> +# dm-flakey, so the XFS will not be shutdown prematurely.
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2017 Huawei Technologies Co., Ltd. All Rights Reserved.
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write the Free Software Foundation,
>> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> +#-----------------------------------------------------------------------
>> +#
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1	# failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +	[ -z "${interval}" ] || \
>> +		sysctl -w fs.xfs.xfssyncd_centisecs=${interval} >/dev/null 2>&1
>> +	cd /
>> +	rm -f $tmp.*
>> +	_unmount_flakey >/dev/null 2>&1
>> +	_cleanup_flakey > /dev/null 2>&1
>> +}
>> +
>> +_get_xfs_scratch_sb_field()
>> +{
>> +	local field=$1
>> +
>> +	_scratch_xfs_db -r -c "sb 0" -c "print $field" | \
>> +	awk -v field=$field '$0 ~ field {print $3}'
>         ^^^ $AWK_PROG
>> +}
> 
> Put this function in common/xfs in the first place, no need to move it
> there in patch 4.
> 
>> +
>> +# inject IO write error for the XFS filesystem except its log section
>> +make_xfs_scratch_flakey_table()
>> +{
>> +	local tgt=flakey
>> +	local opt="0 1 1 error_writes"
>> +	local dev=${SCRATCH_DEV}
>> +	local dev_sz=$(blockdev --getsz $dev)
>> +
>> +	if [ "${USE_EXTERNAL}" = "yes" -a ! -z "$SCRATCH_LOGDEV" ]; then
>> +		echo "0 ${dev_sz} $tgt $dev 0 $opt"
>> +		return
>> +	fi
>> +
>> +	local blk_sz=$(_get_xfs_scratch_sb_field blocksize)
>> +	local log_ofs=$(_get_xfs_scratch_sb_field logstart)
>> +	local log_sz=$(_get_xfs_scratch_sb_field logblocks)
>> +	local table=""
>> +	local ofs=0
>> +	local sz
>> +
>> +	let "log_ofs *= blk_sz / 512"
>> +	let "log_sz *= blk_sz / 512"
>> +
>> +	if [ "$ofs" -lt "${log_ofs}" ]; then
>> +		let "sz = log_ofs - ofs"
>> +		table="$ofs $sz $tgt $dev $ofs $opt"
>> +	fi
>> +
>> +	table="$table\n${log_ofs} ${log_sz} linear $dev ${log_ofs}"
>> +
>> +	let "ofs = log_ofs + log_sz"
>> +	if [ "$ofs" -lt "${dev_sz}" ]; then
>> +		let "sz = dev_sz - ofs"
>> +		table="$table\n$ofs $sz $tgt $dev $ofs $opt"
>> +	fi
>> +
>> +	echo -e $table
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/dmflakey
>> +. ./common/quota
>> +
>> +_supported_fs xfs
>> +_supported_os Linux
>> +
>> +# due to the injection of write IO error, the fs will be inconsistent
>> +_require_scratch_nocheck
>> +# error_writes feature is introduced in dm-flakey v1.4.0, so check for that
>> +_require_dm_target flakey "1.4.0"
>> +_require_user
>> +_require_xfs_quota
>> +_require_freeze
>> +
>> +rm -f $seqres.full
>> +
>> +echo "Silence is golden"
>> +
>> +_scratch_mkfs_xfs > $seqres.full 2>&1
>> +
>> +# no error will be injected
>> +_init_flakey
>> +$DMSETUP_PROG info >> $seqres.full
>> +$DMSETUP_PROG table >> $seqres.full
>> +
>> +# save the old value for _cleanup()
>> +interval=$(sysctl -n fs.xfs.xfssyncd_centisecs 2>/dev/null)
>> +# shorten the time waiting for the push of ail items
>> +sysctl -w fs.xfs.xfssyncd_centisecs=100 >> $seqres.full 2>&1
>> +
>> +_qmount_option "usrquota"
>> +_mount_flakey
>> +
>> +# We need to set the quota limitation twice, and inject the write error
>> +# after the second setting. If we try to inject the write error after
>> +# the first setting, the initialization of the dquota buffer will get
>> +# IO error and also be retried, and during the umount process the
>> +# write will be ended, and xfs_qm_dqflush_done() will be inovked, and
>> +# the umount will exit normally.
>> +$XFS_QUOTA_PROG -x -c "limit -u isoft=500 fsgqa" $SCRATCH_MNT
> 
> Use $qa_user instead of the hardcoded "fsgqa".
> 
> Thanks,
> Eryu
> 
>> +$XFS_QUOTA_PROG -x -c "report -ih" $SCRATCH_MNT >> $seqres.full
>> +
>> +# ensure the initialization of the dquota buffer is done
>> +xfs_freeze -f $SCRATCH_MNT
>> +xfs_freeze -u $SCRATCH_MNT
>> +
>> +# inject write IO error
>> +FLAKEY_TABLE_DROP=$(make_xfs_scratch_flakey_table)
>> +_load_flakey_table ${FLAKEY_DROP_WRITES}
>> +$DMSETUP_PROG info >> $seqres.full
>> +$DMSETUP_PROG table >> $seqres.full
>> +
>> +# update the dquota buffer
>> +$XFS_QUOTA_PROG -x -c "limit -u isoft=400 fsgqa" $SCRATCH_MNT
>> +$XFS_QUOTA_PROG -x -c "report -ih" $SCRATCH_MNT >> $seqres.full
>> +
>> +sync
>> +
>> +# wait for the push of the dquota log item in AIL and
>> +# the completion of the retried write of dquota buffer
>> +sleep 2
>> +
>> +_unmount_flakey
>> +
>> +_cleanup_flakey
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
>> new file mode 100644
>> index 0000000..3b276ca
>> --- /dev/null
>> +++ b/tests/xfs/999.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 999
>> +Silence is golden
>> diff --git a/tests/xfs/group b/tests/xfs/group
>> index b439842..127019a 100644
>> --- a/tests/xfs/group
>> +++ b/tests/xfs/group
>> @@ -431,3 +431,4 @@
>>  431 auto quick dangerous
>>  432 auto quick dir metadata
>>  433 auto quick attr
>> +999 auto quick quota dangerous
>> -- 
>> 2.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

[PATCH v2 4/4] common/rc: factor out _get|set_xfs_scratch_sb_field()

[Hou Tao]

It's common to get and set the values of fields in XFS super block,
so factor them out as _get|set_xfs_scratch_sb_field() and update the
related test cases accordingly.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 common/xfs    | 20 ++++++++++++++++++++
 tests/xfs/007 |  6 ++----
 tests/xfs/098 |  4 ++--
 tests/xfs/186 |  3 +--
 tests/xfs/199 | 13 ++++---------
 tests/xfs/307 | 11 ++---------
 tests/xfs/308 | 11 ++---------
 tests/xfs/339 |  6 +++---
 tests/xfs/340 |  2 +-
 tests/xfs/999 |  8 --------
 10 files changed, 37 insertions(+), 47 deletions(-)

diff --git a/common/xfs b/common/xfs
index d4fef94..82ddc24 100644
--- a/common/xfs
+++ b/common/xfs
@@ -599,3 +599,23 @@ _require_no_xfs_debug()
 		_notrun "Require XFS built without CONFIG_XFS_DEBUG"
 	fi
 }
+
+_get_xfs_scratch_sb_field()
+{
+	local field=$1
+
+	shift
+
+	_scratch_xfs_db -r -c 'sb 0' "$@" -c "print $field" | \
+	awk -v field=$field '$0 ~ field {print $3}'
+}
+
+_set_xfs_scratch_sb_field()
+{
+	local field=$1
+	local value=$2
+
+	shift 2
+
+	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"
+}
diff --git a/tests/xfs/007 b/tests/xfs/007
index d80d380..c477db7 100755
--- a/tests/xfs/007
+++ b/tests/xfs/007
@@ -62,10 +62,8 @@ do_test()
 	echo "*** umount"
 	_scratch_unmount
 
-	QINO_1=`xfs_db -c "sb 0" -c "p" $SCRATCH_DEV | \
-			grep $qino_1 | awk '{print $NF}'`
-	QINO_2=`xfs_db -c "sb 0" -c "p" $SCRATCH_DEV | \
-			grep $qino_2 | awk '{print $NF}'`
+	QINO_1=`_get_xfs_scratch_sb_field $qino_1`
+	QINO_2=`_get_xfs_scratch_sb_field $qino_2`
 
 	echo "*** Usage before quotarm ***"
 	_scratch_xfs_db -c "inode $QINO_1" -c "p core.nblocks"
diff --git a/tests/xfs/098 b/tests/xfs/098
index 7873f32..6edac63 100755
--- a/tests/xfs/098
+++ b/tests/xfs/098
@@ -96,9 +96,9 @@ echo "+ check fs"
 _scratch_xfs_repair -n >> $seqres.full 2>&1 || _fail "xfs_repair should not fail"
 
 echo "+ corrupt image"
-logstart="$(_scratch_xfs_db -c 'sb 0' -c 'p' | grep '^logstart =' | cut -d ' ' -f 3)"
+logstart="$(_get_xfs_scratch_sb_field logstart)"
 logstart="$(_scratch_xfs_db -c "convert fsblock ${logstart} byte" | sed -e 's/^.*(\([0-9]*\).*$/\1/g')"
-logblocks="$(xfs_db -c 'sb 0' -c 'p' "${SCRATCH_DEV}" | grep '^logblocks =' | cut -d ' ' -f 3)"
+logblocks="$(_get_xfs_scratch_sb_field logblocks)"
 $XFS_IO_PROG -f -c "pwrite -S 0x62 ${logstart} $((logblocks * blksz))" "${SCRATCH_DEV}" >> $seqres.full
 
 echo "+ mount image"
diff --git a/tests/xfs/186 b/tests/xfs/186
index 4b36ae6..aaae202 100755
--- a/tests/xfs/186
+++ b/tests/xfs/186
@@ -173,8 +173,7 @@ fi
 
 # set inum to root dir ino
 # we'll add in dirents and EAs into the root directory
-eval `_scratch_xfs_db -r -c 'sb 0' -c 'p rootino' | $SED_PROG 's/ //g'`
-inum=$rootino
+inum=`_get_xfs_scratch_sb_field rootino`
 fork_dir=$SCRATCH_MNT
 _print_inode
 
diff --git a/tests/xfs/199 b/tests/xfs/199
index ee26439..8f79cbf 100755
--- a/tests/xfs/199
+++ b/tests/xfs/199
@@ -49,11 +49,6 @@ _supported_os Linux
 
 _require_scratch
 
-get_features()
-{
-	_scratch_xfs_db -x  -c "sb" -c "print $1" | awk '// {print $3}'
-}
-
 # clear any mkfs options so that we can directly specify the options we need to
 # be able to test the features bitmask behaviour correctly.
 MKFS_OPTIONS=
@@ -63,8 +58,8 @@ _scratch_mkfs_xfs -m crc=0 -l lazy-count=1 >/dev/null 2>&1
 # gives us the values the resultant tests should be the same as for testing at
 # the end of the test. We don't hard code feature values, because that makes the
 # test break every time we change mkfs feature defaults.
-f2=`get_features features2`
-bf2=`get_features bad_features2`
+f2=`_get_xfs_scratch_sb_field features2`
+bf2=`_get_xfs_scratch_sb_field bad_features2`
 
 #
 # Now clear the normal flags
@@ -74,7 +69,7 @@ _scratch_xfs_db -x  -c 'sb' -c 'write features2 0'
 
 _scratch_mount
 _scratch_unmount
-rwf2=`get_features features2`
+rwf2=`_get_xfs_scratch_sb_field features2`
 
 #
 # Clear the normal flags again for the second rount.
@@ -88,7 +83,7 @@ _scratch_xfs_db -x  -c 'sb' -c 'write features2 0'
 _scratch_mount -o ro
 _scratch_mount -o remount,rw
 _scratch_unmount
-rof2=`get_features features2`
+rof2=`_get_xfs_scratch_sb_field features2`
 
 [ "$f2" != "$bf2" ] && echo "mkfs: features2 $f2 != bad_features2 $bf2"
 [ "$f2" != "$rwf2" ] && echo "mount rw: old features2 $f2 != new features2 $rwf2"
diff --git a/tests/xfs/307 b/tests/xfs/307
index 4ce3e66..1e6cd80 100755
--- a/tests/xfs/307
+++ b/tests/xfs/307
@@ -71,18 +71,11 @@ _set_agf_data() {
 }
 
 _get_sb_data() {
-	field="$1"
-	shift
-
-	_scratch_xfs_db -c 'sb 0' "$@" -c "p $field"  | awk '{print $3}'
+	_get_xfs_scratch_sb_field "$@"
 }
 
 _set_sb_data() {
-	field="$1"
-	value="$2"
-	shift; shift
-
-	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"  >> $seqres.full
+	_set_xfs_scratch_sb_field "$@" >> $seqres.full
 }
 
 _filter_leftover() {
diff --git a/tests/xfs/308 b/tests/xfs/308
index e9d7f76..21bde02 100755
--- a/tests/xfs/308
+++ b/tests/xfs/308
@@ -71,18 +71,11 @@ _set_agf_data() {
 }
 
 _get_sb_data() {
-	field="$1"
-	shift
-
-	_scratch_xfs_db -c 'sb 0' "$@" -c "p $field"  | awk '{print $3}'
+	_get_xfs_scratch_sb_field "$@"
 }
 
 _set_sb_data() {
-	field="$1"
-	value="$2"
-	shift; shift
-
-	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"  >> $seqres.full
+	_set_xfs_scratch_sb_field "$@" >> $seqres.full
 }
 
 _filter_leftover() {
diff --git a/tests/xfs/339 b/tests/xfs/339
index 734d47b..2acd63f 100755
--- a/tests/xfs/339
+++ b/tests/xfs/339
@@ -59,9 +59,9 @@ ln $SCRATCH_MNT/f3 $SCRATCH_MNT/f4
 _scratch_unmount
 
 echo "Corrupt fs"
-rrmapino=$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' | awk '{print $3}')
-_scratch_xfs_db -x -c 'sb 0' -c 'addr rootino' \
-	-c "write u3.sfdir3.list[3].inumber.i4 $rrmapino" >> $seqres.full
+rrmapino=`_get_xfs_scratch_sb_field rrmapino`
+_set_xfs_scratch_sb_field "u3.sfdir3.list[3].inumber.i4" $rrmapino \
+	-c 'addr rootino' >> $seqres.full
 _scratch_mount
 
 echo "Check files"
diff --git a/tests/xfs/340 b/tests/xfs/340
index 8cc50a2..2df8819 100755
--- a/tests/xfs/340
+++ b/tests/xfs/340
@@ -59,7 +59,7 @@ ino=$(stat -c '%i' $SCRATCH_MNT/f3)
 _scratch_unmount
 
 echo "Corrupt fs"
-rrmapino=$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' | awk '{print $3}')
+rrmapino=$(_get_xfs_scratch_sb_field rrmapino)
 _scratch_xfs_db -x -c "inode $rrmapino" \
 	-c 'write core.format 2' -c 'write core.size 0' \
 	-c 'write core.nblocks 0' -c 'sb 0' -c 'addr rootino' \
diff --git a/tests/xfs/999 b/tests/xfs/999
index cea2f85..73be58a 100755
--- a/tests/xfs/999
+++ b/tests/xfs/999
@@ -51,14 +51,6 @@ _cleanup()
 	_cleanup_flakey > /dev/null 2>&1
 }
 
-_get_xfs_scratch_sb_field()
-{
-	local field=$1
-
-	_scratch_xfs_db -r -c "sb 0" -c "print $field" | \
-	awk -v field=$field '$0 ~ field {print $3}'
-}
-
 # inject IO write error for the XFS filesystem except its log section
 make_xfs_scratch_flakey_table()
 {
-- 
2.9.5

Re: [PATCH v2 4/4] common/rc: factor out _get|set_xfs_scratch_sb_field()

[Darrick J. Wong]

On Wed, Nov 08, 2017 at 04:02:50PM +0800, Hou Tao wrote:
> It's common to get and set the values of fields in XFS super block,
> so factor them out as _get|set_xfs_scratch_sb_field() and update the
> related test cases accordingly.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  common/xfs    | 20 ++++++++++++++++++++
>  tests/xfs/007 |  6 ++----
>  tests/xfs/098 |  4 ++--
>  tests/xfs/186 |  3 +--
>  tests/xfs/199 | 13 ++++---------
>  tests/xfs/307 | 11 ++---------
>  tests/xfs/308 | 11 ++---------
>  tests/xfs/339 |  6 +++---
>  tests/xfs/340 |  2 +-
>  tests/xfs/999 |  8 --------
>  10 files changed, 37 insertions(+), 47 deletions(-)
> 
> diff --git a/common/xfs b/common/xfs
> index d4fef94..82ddc24 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -599,3 +599,23 @@ _require_no_xfs_debug()
>  		_notrun "Require XFS built without CONFIG_XFS_DEBUG"
>  	fi
>  }
> +
> +_get_xfs_scratch_sb_field()
> +{
> +	local field=$1
> +
> +	shift
> +
> +	_scratch_xfs_db -r -c 'sb 0' "$@" -c "print $field" | \
> +	awk -v field=$field '$0 ~ field {print $3}'
> +}
> +
> +_set_xfs_scratch_sb_field()
> +{
> +	local field=$1
> +	local value=$2
> +
> +	shift 2
> +
> +	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"
> +}

FWIW we already have _scratch_xfs_[gs]et_metadata_field in common/fuzzy.

--D

> diff --git a/tests/xfs/007 b/tests/xfs/007
> index d80d380..c477db7 100755
> --- a/tests/xfs/007
> +++ b/tests/xfs/007
> @@ -62,10 +62,8 @@ do_test()
>  	echo "*** umount"
>  	_scratch_unmount
>  
> -	QINO_1=`xfs_db -c "sb 0" -c "p" $SCRATCH_DEV | \
> -			grep $qino_1 | awk '{print $NF}'`
> -	QINO_2=`xfs_db -c "sb 0" -c "p" $SCRATCH_DEV | \
> -			grep $qino_2 | awk '{print $NF}'`
> +	QINO_1=`_get_xfs_scratch_sb_field $qino_1`
> +	QINO_2=`_get_xfs_scratch_sb_field $qino_2`
>  
>  	echo "*** Usage before quotarm ***"
>  	_scratch_xfs_db -c "inode $QINO_1" -c "p core.nblocks"
> diff --git a/tests/xfs/098 b/tests/xfs/098
> index 7873f32..6edac63 100755
> --- a/tests/xfs/098
> +++ b/tests/xfs/098
> @@ -96,9 +96,9 @@ echo "+ check fs"
>  _scratch_xfs_repair -n >> $seqres.full 2>&1 || _fail "xfs_repair should not fail"
>  
>  echo "+ corrupt image"
> -logstart="$(_scratch_xfs_db -c 'sb 0' -c 'p' | grep '^logstart =' | cut -d ' ' -f 3)"
> +logstart="$(_get_xfs_scratch_sb_field logstart)"
>  logstart="$(_scratch_xfs_db -c "convert fsblock ${logstart} byte" | sed -e 's/^.*(\([0-9]*\).*$/\1/g')"
> -logblocks="$(xfs_db -c 'sb 0' -c 'p' "${SCRATCH_DEV}" | grep '^logblocks =' | cut -d ' ' -f 3)"
> +logblocks="$(_get_xfs_scratch_sb_field logblocks)"
>  $XFS_IO_PROG -f -c "pwrite -S 0x62 ${logstart} $((logblocks * blksz))" "${SCRATCH_DEV}" >> $seqres.full
>  
>  echo "+ mount image"
> diff --git a/tests/xfs/186 b/tests/xfs/186
> index 4b36ae6..aaae202 100755
> --- a/tests/xfs/186
> +++ b/tests/xfs/186
> @@ -173,8 +173,7 @@ fi
>  
>  # set inum to root dir ino
>  # we'll add in dirents and EAs into the root directory
> -eval `_scratch_xfs_db -r -c 'sb 0' -c 'p rootino' | $SED_PROG 's/ //g'`
> -inum=$rootino
> +inum=`_get_xfs_scratch_sb_field rootino`
>  fork_dir=$SCRATCH_MNT
>  _print_inode
>  
> diff --git a/tests/xfs/199 b/tests/xfs/199
> index ee26439..8f79cbf 100755
> --- a/tests/xfs/199
> +++ b/tests/xfs/199
> @@ -49,11 +49,6 @@ _supported_os Linux
>  
>  _require_scratch
>  
> -get_features()
> -{
> -	_scratch_xfs_db -x  -c "sb" -c "print $1" | awk '// {print $3}'
> -}
> -
>  # clear any mkfs options so that we can directly specify the options we need to
>  # be able to test the features bitmask behaviour correctly.
>  MKFS_OPTIONS=
> @@ -63,8 +58,8 @@ _scratch_mkfs_xfs -m crc=0 -l lazy-count=1 >/dev/null 2>&1
>  # gives us the values the resultant tests should be the same as for testing at
>  # the end of the test. We don't hard code feature values, because that makes the
>  # test break every time we change mkfs feature defaults.
> -f2=`get_features features2`
> -bf2=`get_features bad_features2`
> +f2=`_get_xfs_scratch_sb_field features2`
> +bf2=`_get_xfs_scratch_sb_field bad_features2`
>  
>  #
>  # Now clear the normal flags
> @@ -74,7 +69,7 @@ _scratch_xfs_db -x  -c 'sb' -c 'write features2 0'
>  
>  _scratch_mount
>  _scratch_unmount
> -rwf2=`get_features features2`
> +rwf2=`_get_xfs_scratch_sb_field features2`
>  
>  #
>  # Clear the normal flags again for the second rount.
> @@ -88,7 +83,7 @@ _scratch_xfs_db -x  -c 'sb' -c 'write features2 0'
>  _scratch_mount -o ro
>  _scratch_mount -o remount,rw
>  _scratch_unmount
> -rof2=`get_features features2`
> +rof2=`_get_xfs_scratch_sb_field features2`
>  
>  [ "$f2" != "$bf2" ] && echo "mkfs: features2 $f2 != bad_features2 $bf2"
>  [ "$f2" != "$rwf2" ] && echo "mount rw: old features2 $f2 != new features2 $rwf2"
> diff --git a/tests/xfs/307 b/tests/xfs/307
> index 4ce3e66..1e6cd80 100755
> --- a/tests/xfs/307
> +++ b/tests/xfs/307
> @@ -71,18 +71,11 @@ _set_agf_data() {
>  }
>  
>  _get_sb_data() {
> -	field="$1"
> -	shift
> -
> -	_scratch_xfs_db -c 'sb 0' "$@" -c "p $field"  | awk '{print $3}'
> +	_get_xfs_scratch_sb_field "$@"
>  }
>  
>  _set_sb_data() {
> -	field="$1"
> -	value="$2"
> -	shift; shift
> -
> -	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"  >> $seqres.full
> +	_set_xfs_scratch_sb_field "$@" >> $seqres.full
>  }
>  
>  _filter_leftover() {
> diff --git a/tests/xfs/308 b/tests/xfs/308
> index e9d7f76..21bde02 100755
> --- a/tests/xfs/308
> +++ b/tests/xfs/308
> @@ -71,18 +71,11 @@ _set_agf_data() {
>  }
>  
>  _get_sb_data() {
> -	field="$1"
> -	shift
> -
> -	_scratch_xfs_db -c 'sb 0' "$@" -c "p $field"  | awk '{print $3}'
> +	_get_xfs_scratch_sb_field "$@"
>  }
>  
>  _set_sb_data() {
> -	field="$1"
> -	value="$2"
> -	shift; shift
> -
> -	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"  >> $seqres.full
> +	_set_xfs_scratch_sb_field "$@" >> $seqres.full
>  }
>  
>  _filter_leftover() {
> diff --git a/tests/xfs/339 b/tests/xfs/339
> index 734d47b..2acd63f 100755
> --- a/tests/xfs/339
> +++ b/tests/xfs/339
> @@ -59,9 +59,9 @@ ln $SCRATCH_MNT/f3 $SCRATCH_MNT/f4
>  _scratch_unmount
>  
>  echo "Corrupt fs"
> -rrmapino=$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' | awk '{print $3}')
> -_scratch_xfs_db -x -c 'sb 0' -c 'addr rootino' \
> -	-c "write u3.sfdir3.list[3].inumber.i4 $rrmapino" >> $seqres.full
> +rrmapino=`_get_xfs_scratch_sb_field rrmapino`
> +_set_xfs_scratch_sb_field "u3.sfdir3.list[3].inumber.i4" $rrmapino \
> +	-c 'addr rootino' >> $seqres.full
>  _scratch_mount
>  
>  echo "Check files"
> diff --git a/tests/xfs/340 b/tests/xfs/340
> index 8cc50a2..2df8819 100755
> --- a/tests/xfs/340
> +++ b/tests/xfs/340
> @@ -59,7 +59,7 @@ ino=$(stat -c '%i' $SCRATCH_MNT/f3)
>  _scratch_unmount
>  
>  echo "Corrupt fs"
> -rrmapino=$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' | awk '{print $3}')
> +rrmapino=$(_get_xfs_scratch_sb_field rrmapino)
>  _scratch_xfs_db -x -c "inode $rrmapino" \
>  	-c 'write core.format 2' -c 'write core.size 0' \
>  	-c 'write core.nblocks 0' -c 'sb 0' -c 'addr rootino' \
> diff --git a/tests/xfs/999 b/tests/xfs/999
> index cea2f85..73be58a 100755
> --- a/tests/xfs/999
> +++ b/tests/xfs/999
> @@ -51,14 +51,6 @@ _cleanup()
>  	_cleanup_flakey > /dev/null 2>&1
>  }
>  
> -_get_xfs_scratch_sb_field()
> -{
> -	local field=$1
> -
> -	_scratch_xfs_db -r -c "sb 0" -c "print $field" | \
> -	awk -v field=$field '$0 ~ field {print $3}'
> -}
> -
>  # inject IO write error for the XFS filesystem except its log section
>  make_xfs_scratch_flakey_table()
>  {
> -- 
> 2.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/4] common/rc: factor out _get|set_xfs_scratch_sb_field()

[Eryu Guan]

On Wed, Nov 08, 2017 at 08:49:08AM -0800, Darrick J. Wong wrote:
> On Wed, Nov 08, 2017 at 04:02:50PM +0800, Hou Tao wrote:
> > It's common to get and set the values of fields in XFS super block,
> > so factor them out as _get|set_xfs_scratch_sb_field() and update the
> > related test cases accordingly.
> > 
> > Signed-off-by: Hou Tao <houtao1@huawei.com>
> > ---
> >  common/xfs    | 20 ++++++++++++++++++++
> >  tests/xfs/007 |  6 ++----
> >  tests/xfs/098 |  4 ++--
> >  tests/xfs/186 |  3 +--
> >  tests/xfs/199 | 13 ++++---------
> >  tests/xfs/307 | 11 ++---------
> >  tests/xfs/308 | 11 ++---------
> >  tests/xfs/339 |  6 +++---
> >  tests/xfs/340 |  2 +-
> >  tests/xfs/999 |  8 --------
> >  10 files changed, 37 insertions(+), 47 deletions(-)
> > 
> > diff --git a/common/xfs b/common/xfs
> > index d4fef94..82ddc24 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -599,3 +599,23 @@ _require_no_xfs_debug()
> >  		_notrun "Require XFS built without CONFIG_XFS_DEBUG"
> >  	fi
> >  }
> > +
> > +_get_xfs_scratch_sb_field()
> > +{
> > +	local field=$1
> > +
> > +	shift
> > +
> > +	_scratch_xfs_db -r -c 'sb 0' "$@" -c "print $field" | \
> > +	awk -v field=$field '$0 ~ field {print $3}'
> > +}
> > +
> > +_set_xfs_scratch_sb_field()
> > +{
> > +	local field=$1
> > +	local value=$2
> > +
> > +	shift 2
> > +
> > +	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"
> > +}
> 
> FWIW we already have _scratch_xfs_[gs]et_metadata_field in common/fuzzy.

Ah, thanks for the reminder! I forgot about them.. Tao, are the existing
helpers in common/fuzzy something you can use? And perhaps we should
move them to common/xfs then.

Thanks,
Eryu

Re: [PATCH v2 4/4] common/rc: factor out _get|set_xfs_scratch_sb_field()

[Hou Tao]

Hi,

On 2017/11/9 11:53, Eryu Guan wrote:
> On Wed, Nov 08, 2017 at 08:49:08AM -0800, Darrick J. Wong wrote:
>> On Wed, Nov 08, 2017 at 04:02:50PM +0800, Hou Tao wrote:
>>> It's common to get and set the values of fields in XFS super block,
>>> so factor them out as _get|set_xfs_scratch_sb_field() and update the
>>> related test cases accordingly.
>>>
>>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>>> ---
>>>  common/xfs    | 20 ++++++++++++++++++++
>>>  tests/xfs/007 |  6 ++----
>>>  tests/xfs/098 |  4 ++--
>>>  tests/xfs/186 |  3 +--
>>>  tests/xfs/199 | 13 ++++---------
>>>  tests/xfs/307 | 11 ++---------
>>>  tests/xfs/308 | 11 ++---------
>>>  tests/xfs/339 |  6 +++---
>>>  tests/xfs/340 |  2 +-
>>>  tests/xfs/999 |  8 --------
>>>  10 files changed, 37 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/common/xfs b/common/xfs
>>> index d4fef94..82ddc24 100644
>>> --- a/common/xfs
>>> +++ b/common/xfs
>>> @@ -599,3 +599,23 @@ _require_no_xfs_debug()
>>>  		_notrun "Require XFS built without CONFIG_XFS_DEBUG"
>>>  	fi
>>>  }
>>> +
>>> +_get_xfs_scratch_sb_field()
>>> +{
>>> +	local field=$1
>>> +
>>> +	shift
>>> +
>>> +	_scratch_xfs_db -r -c 'sb 0' "$@" -c "print $field" | \
>>> +	awk -v field=$field '$0 ~ field {print $3}'
>>> +}
>>> +
>>> +_set_xfs_scratch_sb_field()
>>> +{
>>> +	local field=$1
>>> +	local value=$2
>>> +
>>> +	shift 2
>>> +
>>> +	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"
>>> +}
>>
>> FWIW we already have _scratch_xfs_[gs]et_metadata_field in common/fuzzy.
> 
> Ah, thanks for the reminder! I forgot about them.. Tao, are the existing
> helpers in common/fuzzy something you can use? And perhaps we should
> move them to common/xfs then.

Yes, _scratch_xfs_[gs]et_metadata_field can be used to implement _get|set_xfs_scratch_sb_field().
So we can move _scratch_xfs_[gs]et_metadata_field to common/xfs and implement wrappers for getting
and setting fields of XFS super block.

Regards,
Tao

> Thanks,
> Eryu
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

Re: [PATCH v2 0/4] test for XFS umount hang caused by the pending dquota log item in AIL

[Eryu Guan]

On Wed, Nov 08, 2017 at 04:02:46PM +0800, Hou Tao wrote:
> Hi,
> 
> This is the v2 patchset for testing the XFS umount hang problem caused by
> the pending dquota log item in AIL. The problem has not been fixed in
> XFS yet, and Carlos Maiolino is working on it [1]. I had tested
> it on 4.14-rc7, and the problem can be reproduced reliably.

Thanks a lot for the updated version! But unfortunately, I still
couldn't reproduce the hang, I'll provide more information in comments
to each specific patch.

Thanks,
Eryu

> 
> Changes since v1:
> * dmflakey: support multiple dm targets for a dm-flakey device
> 	* no update
> 
> * xfs: test for umount hang caused by the pending dquota log item in AIL
> 	* a bunch of suggestions from Eryu Guan
> 	* update the dquota buffer after the error injection to better
> 	  reproduce the problem
> 
> * common/rc: support checking the version of dm-target in
> _require_dm_target()
> 	* new patch
> 	* introduce a way to check the availability of error_write feature
> 	in dm-flakey target, and accomplish it by checking the version of
> 	dm-target.
> 
> * common/rc: factor out _get|set_xfs_scratch_sb_field()
> 	* new patch
> 	* the modified test cases had been tested after applying the patch:
> 	(1) xfs/098 failed, but the failure has nothing to do with the patch
> 	(2) xfs/339 and xfs/340 were not runnable, because rtdev + rmap is not
> 	  supported now.
> 	(3) the remaing test cases passed
> 
> v1:
> 	* https://www.spinics.net/lists/fstests/msg07622.html
> 
> Comments and questions are welcome.
> 
> Regards,
> Tao
> 
> [1]: https://www.spinics.net/lists/linux-xfs/msg11614.html
> ---
> Hou Tao (4):
>   dmflakey: support multiple dm targets for a dm-flakey device
>   common/rc: support checking the version of dm-target in
>     _require_dm_target()
>   xfs: test for umount hang caused by the pending dquota log item in AIL
>   common/rc: factor out _get|set_xfs_scratch_sb_field()
> 
>  common/dmflakey   |   2 +-
>  common/rc         |  39 +++++++++++--
>  common/xfs        |  20 +++++++
>  tests/xfs/007     |   6 +-
>  tests/xfs/098     |   4 +-
>  tests/xfs/186     |   3 +-
>  tests/xfs/199     |  13 ++---
>  tests/xfs/307     |  11 +---
>  tests/xfs/308     |  11 +---
>  tests/xfs/339     |   6 +-
>  tests/xfs/340     |   2 +-
>  tests/xfs/999     | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |   2 +
>  tests/xfs/group   |   1 +
>  14 files changed, 238 insertions(+), 45 deletions(-)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> -- 
> 2.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html