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

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

[Hou Tao]

Hi,

This is the v3 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].

The bug that the test case can not reproduced on a 15GB scratch device
has been fixed, and many thanks to Eryu Guan. I had tested it on 4.14-rc8,
and the problem can be reproduced reliably now.

Comments and questions are welcome.

Regards,
Tao

---
Changes since v2:

v3:
* dmflakey: support error_writes feature for dm-flakey
	* check the availability of error_writes by trial instead of version number

* xfs: test for umount hang caused by the pending dquota log item in AIL
	* calculate log_ofs correctly by using convert cmd in xfs_db

* common/rc: factor out _scratch_xfs_[get|set]_sb_field
	* reimplement _scratch_xfs_[get|set]_sb_field based on
	  _scratch_xfs_[get|set]_metadata_field.

v2:
	* http://www.spinics.net/lists/fstests/msg07792.html

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

[1]: https://www.spinics.net/lists/linux-xfs/msg11614.html

Hou Tao (4):
  dmflakey: support multiple dm targets for a dm-flakey device
  dmflakey: support error_writes feature for dm-flakey
  xfs: test for umount hang caused by the pending dquota log item in AIL
  common/rc: factor out _scratch_xfs_[get|set]_sb_field

 common/dmflakey   |  29 +++++++++-
 common/fuzzy      |  33 -----------
 common/xfs        |  47 ++++++++++++++++
 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, 257 insertions(+), 74 deletions(-)
 create mode 100755 tests/xfs/999
 create mode 100644 tests/xfs/999.out

-- 
2.9.5

[PATCH v3 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 v3 2/4] dmflakey: support error_writes feature for dm-flakey

[Hou Tao]

Add _require_flakey_with_error_writes() to check the availability of
dm-flakey target and its error_writes feature, and support for enabling
FLAKEY_TABLE_ERROR table by passing FLAKEY_ERROR_WRITES to
_load_flakey_table().

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

diff --git a/common/dmflakey b/common/dmflakey
index 16b82d2..845b466 100644
--- a/common/dmflakey
+++ b/common/dmflakey
@@ -20,18 +20,43 @@
 
 FLAKEY_ALLOW_WRITES=0
 FLAKEY_DROP_WRITES=1
+FLAKEY_ERROR_WRITES=2
 
 echo $MOUNT_OPTIONS | grep -q dax
 if [ $? -eq 0 ]; then
 	_notrun "Cannot run tests with DAX on dmflakey devices"
 fi
 
+_require_flakey_with_error_writes()
+{
+	local SIZE
+	local TABLE
+	local NAME=flakey-test
+
+	_require_dm_target flakey
+
+	SIZE=`blockdev --getsz $SCRATCH_DEV`
+	TABLE="0 $SIZE flakey $SCRATCH_DEV 0 0 180 1 error_writes"
+
+	$DMSETUP_PROG create $NAME --table "$TABLE" >/dev/null 2>&1
+	if [ $? -ne 0 ]; then
+		_notrun "This test requires error_writes feature in dm-flakey"
+	fi
+	$DMSETUP_PROG mknodes >/dev/null 2>&1
+
+	# copy from _cleanup_flakey()
+	$UDEV_SETTLE_PROG >/dev/null 2>&1
+	$DMSETUP_PROG remove $NAME >/dev/null 2>&1
+	$DMSETUP_PROG mknodes >/dev/null 2>&1
+}
+
 _init_flakey()
 {
 	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
 	FLAKEY_DEV=/dev/mapper/flakey-test
 	FLAKEY_TABLE="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 180 0"
 	FLAKEY_TABLE_DROP="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 0 180 1 drop_writes"
+	FLAKEY_TABLE_ERROR="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 0 180 1 error_writes"
 	$DMSETUP_PROG create flakey-test --table "$FLAKEY_TABLE" || \
 		_fatal "failed to create flakey device"
 	$DMSETUP_PROG mknodes > /dev/null 2>&1
@@ -67,9 +92,9 @@ _cleanup_flakey()
 # table, so it simulates power failure.
 _load_flakey_table()
 {
-
 	table="$FLAKEY_TABLE"
 	[ $1 -eq $FLAKEY_DROP_WRITES ] && table="$FLAKEY_TABLE_DROP"
+	[ $1 -eq $FLAKEY_ERROR_WRITES ] && table="$FLAKEY_TABLE_ERROR"
 
 	suspend_opt="--nolockfs"
 	[ $# -gt 1 ] && [ $2 -eq 1 ] && suspend_opt=""
-- 
2.9.5

[PATCH v3 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..22f7ba3
--- /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
+
+	log_ofs=$(_scratch_xfs_db -r -c "convert fsb ${log_ofs} bb" | \
+			awk '{gsub("[()]", "", $2); print $2}')
+	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
+_require_flakey_with_error_writes
+_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_ERROR=$(make_xfs_scratch_flakey_table)
+_load_flakey_table ${FLAKEY_ERROR_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

[PATCH v3 4/4] common/rc: factor out _scratch_xfs_[get|set]_sb_field

[Hou Tao]

It's common to get and set the values of fields in XFS super block,
so factor them out as scratch_xfs_[get|set]_sb_field, reimplement
them based on _scratch_xfs_[get|set]_metadata_field, and update the
related test cases accordingly.

Also move _scratch_xfs_[get|set]_metadata_field from common/fuzzy
to common/xfs.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 common/fuzzy  | 33 ---------------------------------
 common/xfs    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 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 | 14 +++-----------
 11 files changed, 67 insertions(+), 83 deletions(-)

diff --git a/common/fuzzy b/common/fuzzy
index 8453c29..9642809 100644
--- a/common/fuzzy
+++ b/common/fuzzy
@@ -120,39 +120,6 @@ _scratch_xfs_list_metadata_fields() {
 	_scratch_xfs_db "${cmds[@]}" -c print | __filter_xfs_db_print_fields "${filter}"
 }
 
-# Get a metadata field
-# The first arg is the field name
-# The rest of the arguments are xfs_db commands to find the metadata.
-_scratch_xfs_get_metadata_field() {
-	key="$1"
-	shift
-
-	grep_key="$(echo "${key}" | tr '[]()' '....')"
-	local cmds=()
-	for arg in "$@"; do
-		cmds+=("-c" "${arg}")
-	done
-	_scratch_xfs_db "${cmds[@]}" -c "print ${key}" | grep "^${grep_key}" | \
-		sed -e 's/^.* = //g'
-}
-
-# Set a metadata field
-# The first arg is the field name
-# The second arg is the new value
-# The rest of the arguments are xfs_db commands to find the metadata.
-_scratch_xfs_set_metadata_field() {
-	key="$1"
-	value="$2"
-	shift; shift
-
-	local cmds=()
-	for arg in "$@"; do
-		cmds+=("-c" "${arg}")
-	done
-	_scratch_xfs_db -x "${cmds[@]}" -c "write -d ${key} ${value}"
-	echo
-}
-
 # Fuzz a metadata field
 # The first arg is the field name
 # The second arg is the xfs_db fuzz verb
diff --git a/common/xfs b/common/xfs
index d4fef94..0509760 100644
--- a/common/xfs
+++ b/common/xfs
@@ -599,3 +599,50 @@ _require_no_xfs_debug()
 		_notrun "Require XFS built without CONFIG_XFS_DEBUG"
 	fi
 }
+
+# Get a metadata field
+# The first arg is the field name
+# The rest of the arguments are xfs_db commands to find the metadata.
+_scratch_xfs_get_metadata_field()
+{
+	local key="$1"
+	shift
+
+	local grep_key="$(echo "${key}" | tr '[]()' '....')"
+	local cmds=()
+	local arg
+	for arg in "$@"; do
+		cmds+=("-c" "${arg}")
+	done
+	_scratch_xfs_db "${cmds[@]}" -c "print ${key}" | grep "^${grep_key}" | \
+		sed -e 's/^.* = //g'
+}
+
+# Set a metadata field
+# The first arg is the field name
+# The second arg is the new value
+# The rest of the arguments are xfs_db commands to find the metadata.
+_scratch_xfs_set_metadata_field()
+{
+	local key="$1"
+	local value="$2"
+	shift; shift
+
+	local cmds=()
+	local arg
+	for arg in "$@"; do
+		cmds+=("-c" "${arg}")
+	done
+	_scratch_xfs_db -x "${cmds[@]}" -c "write -d ${key} ${value}"
+	echo
+}
+
+_scratch_xfs_get_sb_field()
+{
+	_scratch_xfs_get_metadata_field "$1" "sb 0"
+}
+
+_scratch_xfs_set_sb_field()
+{
+	_scratch_xfs_set_metadata_field "$1" "$2" "sb 0"
+}
diff --git a/tests/xfs/007 b/tests/xfs/007
index d80d380..215bd1d 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=`_scratch_xfs_get_sb_field $qino_1`
+	QINO_2=`_scratch_xfs_get_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..9bcd94b 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="$(_scratch_xfs_get_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="$(_scratch_xfs_get_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..a4527ed 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=`_scratch_xfs_get_sb_field rootino`
 fork_dir=$SCRATCH_MNT
 _print_inode
 
diff --git a/tests/xfs/199 b/tests/xfs/199
index ee26439..a0f48ad 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=`_scratch_xfs_get_sb_field features2`
+bf2=`_scratch_xfs_get_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=`_scratch_xfs_get_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=`_scratch_xfs_get_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..d829524 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}'
+	_scratch_xfs_get_sb_field "$@"
 }
 
 _set_sb_data() {
-	field="$1"
-	value="$2"
-	shift; shift
-
-	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"  >> $seqres.full
+	_scratch_xfs_set_sb_field "$@" >> $seqres.full
 }
 
 _filter_leftover() {
diff --git a/tests/xfs/308 b/tests/xfs/308
index e9d7f76..7e7adac 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}'
+	_scratch_xfs_get_sb_field "$@"
 }
 
 _set_sb_data() {
-	field="$1"
-	value="$2"
-	shift; shift
-
-	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"  >> $seqres.full
+	_scratch_xfs_set_sb_field "$@" >> $seqres.full
 }
 
 _filter_leftover() {
diff --git a/tests/xfs/339 b/tests/xfs/339
index 734d47b..78b714f 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=`_scratch_xfs_get_sb_field rrmapino`
+_scratch_xfs_set_metadata_field "u3.sfdir3.list[3].inumber.i4" $rrmapino \
+	'sb 0' 'addr rootino' >> $seqres.full
 _scratch_mount
 
 echo "Check files"
diff --git a/tests/xfs/340 b/tests/xfs/340
index 8cc50a2..7b1b7d1 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=$(_scratch_xfs_get_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 22f7ba3..2b0b1e5 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()
 {
@@ -72,9 +64,9 @@ make_xfs_scratch_flakey_table()
 		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 blk_sz=$(_scratch_xfs_get_sb_field blocksize)
+	local log_ofs=$(_scratch_xfs_get_sb_field logstart)
+	local log_sz=$(_scratch_xfs_get_sb_field logblocks)
 	local table=""
 	local ofs=0
 	local sz
-- 
2.9.5

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

[Eryu Guan]

On Thu, Nov 09, 2017 at 03:32:48PM +0800, Hou Tao wrote:
> Hi,
> 
> This is the v3 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].
> 
> The bug that the test case can not reproduced on a 15GB scratch device
> has been fixed, and many thanks to Eryu Guan. I had tested it on 4.14-rc8,
> and the problem can be reproduced reliably now.
> 
> Comments and questions are welcome.

I finally could reproduce the hang, thanks a lot!

But I won't merge the test until the fix is upstream, otherwise it would
interrupt all test runs on XFS.

And I have some nitpicks on individual patches :)

Thanks,
Eryu

> 
> Regards,
> Tao
> 
> ---
> Changes since v2:
> 
> v3:
> * dmflakey: support error_writes feature for dm-flakey
> 	* check the availability of error_writes by trial instead of version number
> 
> * xfs: test for umount hang caused by the pending dquota log item in AIL
> 	* calculate log_ofs correctly by using convert cmd in xfs_db
> 
> * common/rc: factor out _scratch_xfs_[get|set]_sb_field
> 	* reimplement _scratch_xfs_[get|set]_sb_field based on
> 	  _scratch_xfs_[get|set]_metadata_field.
> 
> v2:
> 	* http://www.spinics.net/lists/fstests/msg07792.html
> 
> v1:
> 	* https://www.spinics.net/lists/fstests/msg07622.html
> 
> [1]: https://www.spinics.net/lists/linux-xfs/msg11614.html
> 
> Hou Tao (4):
>   dmflakey: support multiple dm targets for a dm-flakey device
>   dmflakey: support error_writes feature for dm-flakey
>   xfs: test for umount hang caused by the pending dquota log item in AIL
>   common/rc: factor out _scratch_xfs_[get|set]_sb_field
> 
>  common/dmflakey   |  29 +++++++++-
>  common/fuzzy      |  33 -----------
>  common/xfs        |  47 ++++++++++++++++
>  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, 257 insertions(+), 74 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 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 v3 1/4] dmflakey: support multiple dm targets for a dm-flakey device

[Eryu Guan]

On Thu, Nov 09, 2017 at 03:32:49PM +0800, Hou Tao wrote:
> 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

Sorry for not bringing this up earlier, but adding some comments would
be good here.

Thanks,
Eryu

>  	[ $? -ne 0 ] && _fatal "failed to load table into flakey-test"
>  
>  	$DMSETUP_PROG resume flakey-test
> -- 
> 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 v3 2/4] dmflakey: support error_writes feature for dm-flakey

[Eryu Guan]

On Thu, Nov 09, 2017 at 03:32:50PM +0800, Hou Tao wrote:
> Add _require_flakey_with_error_writes() to check the availability of
> dm-flakey target and its error_writes feature, and support for enabling
> FLAKEY_TABLE_ERROR table by passing FLAKEY_ERROR_WRITES to
> _load_flakey_table().
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  common/dmflakey | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/common/dmflakey b/common/dmflakey
> index 16b82d2..845b466 100644
> --- a/common/dmflakey
> +++ b/common/dmflakey
> @@ -20,18 +20,43 @@
>  
>  FLAKEY_ALLOW_WRITES=0
>  FLAKEY_DROP_WRITES=1
> +FLAKEY_ERROR_WRITES=2
>  
>  echo $MOUNT_OPTIONS | grep -q dax
>  if [ $? -eq 0 ]; then
>  	_notrun "Cannot run tests with DAX on dmflakey devices"
>  fi
>  
> +_require_flakey_with_error_writes()
> +{
> +	local SIZE
> +	local TABLE
> +	local NAME=flakey-test
> +
> +	_require_dm_target flakey
> +
> +	SIZE=`blockdev --getsz $SCRATCH_DEV`
> +	TABLE="0 $SIZE flakey $SCRATCH_DEV 0 0 180 1 error_writes"
> +
> +	$DMSETUP_PROG create $NAME --table "$TABLE" >/dev/null 2>&1
> +	if [ $? -ne 0 ]; then
> +		_notrun "This test requires error_writes feature in dm-flakey"
> +	fi
> +	$DMSETUP_PROG mknodes >/dev/null 2>&1
> +
> +	# copy from _cleanup_flakey()
> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
> +	$DMSETUP_PROG remove $NAME >/dev/null 2>&1
> +	$DMSETUP_PROG mknodes >/dev/null 2>&1

How about just call _cleanup_flakey() here?

Thanks,
Eryu

> +}
> +
>  _init_flakey()
>  {
>  	local BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
>  	FLAKEY_DEV=/dev/mapper/flakey-test
>  	FLAKEY_TABLE="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 180 0"
>  	FLAKEY_TABLE_DROP="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 0 180 1 drop_writes"
> +	FLAKEY_TABLE_ERROR="0 $BLK_DEV_SIZE flakey $SCRATCH_DEV 0 0 180 1 error_writes"
>  	$DMSETUP_PROG create flakey-test --table "$FLAKEY_TABLE" || \
>  		_fatal "failed to create flakey device"
>  	$DMSETUP_PROG mknodes > /dev/null 2>&1
> @@ -67,9 +92,9 @@ _cleanup_flakey()
>  # table, so it simulates power failure.
>  _load_flakey_table()
>  {
> -
>  	table="$FLAKEY_TABLE"
>  	[ $1 -eq $FLAKEY_DROP_WRITES ] && table="$FLAKEY_TABLE_DROP"
> +	[ $1 -eq $FLAKEY_ERROR_WRITES ] && table="$FLAKEY_TABLE_ERROR"
>  
>  	suspend_opt="--nolockfs"
>  	[ $# -gt 1 ] && [ $2 -eq 1 ] && suspend_opt=""
> -- 
> 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 v3 3/4] xfs: test for umount hang caused by the pending dquota log item in AIL

[Eryu Guan]

On Thu, Nov 09, 2017 at 03:32:51PM +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>
> ---
>  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..22f7ba3
> --- /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}'
> +}

Hmm, I think we can do the refactor first, i.e. introduce all needed
functions and fix existing tests first and introduce the new test as the
last patch, so we don't need to update the new test again in the same
patch series, and it's easier for me to defer the test without holding
other patches too.

> +
> +# 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
> +
> +	log_ofs=$(_scratch_xfs_db -r -c "convert fsb ${log_ofs} bb" | \
> +			awk '{gsub("[()]", "", $2); print $2}')
                        ^^^ $AWK_PROG

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

This part is obvious to follow, better to have some comments too.

> +
> +	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
> +_require_flakey_with_error_writes
> +_require_user
> +_require_xfs_quota
> +_require_freeze
> +
> +rm -f $seqres.full
> +
> +echo "Silence is golden"
> +
> +_scratch_mkfs_xfs > $seqres.full 2>&1

_scratch_mkfs should be fine.

> +
> +# 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 hard-coded 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_ERROR=$(make_xfs_scratch_flakey_table)
> +_load_flakey_table ${FLAKEY_ERROR_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 v3 4/4] common/rc: factor out _scratch_xfs_[get|set]_sb_field

[Darrick J. Wong]

On Thu, Nov 09, 2017 at 03:32:52PM +0800, Hou Tao wrote:
> It's common to get and set the values of fields in XFS super block,
> so factor them out as scratch_xfs_[get|set]_sb_field, reimplement
> them based on _scratch_xfs_[get|set]_metadata_field, and update the
> related test cases accordingly.
> 
> Also move _scratch_xfs_[get|set]_metadata_field from common/fuzzy
> to common/xfs.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  common/fuzzy  | 33 ---------------------------------
>  common/xfs    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  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 | 14 +++-----------
>  11 files changed, 67 insertions(+), 83 deletions(-)
> 
> diff --git a/common/fuzzy b/common/fuzzy
> index 8453c29..9642809 100644
> --- a/common/fuzzy
> +++ b/common/fuzzy
> @@ -120,39 +120,6 @@ _scratch_xfs_list_metadata_fields() {
>  	_scratch_xfs_db "${cmds[@]}" -c print | __filter_xfs_db_print_fields "${filter}"
>  }
>  
> -# Get a metadata field
> -# The first arg is the field name
> -# The rest of the arguments are xfs_db commands to find the metadata.
> -_scratch_xfs_get_metadata_field() {
> -	key="$1"
> -	shift
> -
> -	grep_key="$(echo "${key}" | tr '[]()' '....')"
> -	local cmds=()
> -	for arg in "$@"; do
> -		cmds+=("-c" "${arg}")
> -	done
> -	_scratch_xfs_db "${cmds[@]}" -c "print ${key}" | grep "^${grep_key}" | \
> -		sed -e 's/^.* = //g'
> -}
> -
> -# Set a metadata field
> -# The first arg is the field name
> -# The second arg is the new value
> -# The rest of the arguments are xfs_db commands to find the metadata.
> -_scratch_xfs_set_metadata_field() {
> -	key="$1"
> -	value="$2"
> -	shift; shift
> -
> -	local cmds=()
> -	for arg in "$@"; do
> -		cmds+=("-c" "${arg}")
> -	done
> -	_scratch_xfs_db -x "${cmds[@]}" -c "write -d ${key} ${value}"
> -	echo
> -}
> -
>  # Fuzz a metadata field
>  # The first arg is the field name
>  # The second arg is the xfs_db fuzz verb
> diff --git a/common/xfs b/common/xfs
> index d4fef94..0509760 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -599,3 +599,50 @@ _require_no_xfs_debug()
>  		_notrun "Require XFS built without CONFIG_XFS_DEBUG"
>  	fi
>  }
> +
> +# Get a metadata field
> +# The first arg is the field name
> +# The rest of the arguments are xfs_db commands to find the metadata.
> +_scratch_xfs_get_metadata_field()
> +{
> +	local key="$1"
> +	shift
> +
> +	local grep_key="$(echo "${key}" | tr '[]()' '....')"
> +	local cmds=()
> +	local arg
> +	for arg in "$@"; do
> +		cmds+=("-c" "${arg}")
> +	done
> +	_scratch_xfs_db "${cmds[@]}" -c "print ${key}" | grep "^${grep_key}" | \
> +		sed -e 's/^.* = //g'
> +}
> +
> +# Set a metadata field
> +# The first arg is the field name
> +# The second arg is the new value
> +# The rest of the arguments are xfs_db commands to find the metadata.
> +_scratch_xfs_set_metadata_field()
> +{
> +	local key="$1"
> +	local value="$2"
> +	shift; shift
> +
> +	local cmds=()
> +	local arg
> +	for arg in "$@"; do
> +		cmds+=("-c" "${arg}")
> +	done
> +	_scratch_xfs_db -x "${cmds[@]}" -c "write -d ${key} ${value}"

/me wonders if this should be "write -d ${key} -- ${value}" since I
omitted that in the initial patch.  (That could be a separate patch.)

Otherwise looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> +	echo
> +}
> +
> +_scratch_xfs_get_sb_field()
> +{
> +	_scratch_xfs_get_metadata_field "$1" "sb 0"
> +}
> +
> +_scratch_xfs_set_sb_field()
> +{
> +	_scratch_xfs_set_metadata_field "$1" "$2" "sb 0"
> +}
> diff --git a/tests/xfs/007 b/tests/xfs/007
> index d80d380..215bd1d 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=`_scratch_xfs_get_sb_field $qino_1`
> +	QINO_2=`_scratch_xfs_get_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..9bcd94b 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="$(_scratch_xfs_get_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="$(_scratch_xfs_get_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..a4527ed 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=`_scratch_xfs_get_sb_field rootino`
>  fork_dir=$SCRATCH_MNT
>  _print_inode
>  
> diff --git a/tests/xfs/199 b/tests/xfs/199
> index ee26439..a0f48ad 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=`_scratch_xfs_get_sb_field features2`
> +bf2=`_scratch_xfs_get_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=`_scratch_xfs_get_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=`_scratch_xfs_get_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..d829524 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}'
> +	_scratch_xfs_get_sb_field "$@"
>  }
>  
>  _set_sb_data() {
> -	field="$1"
> -	value="$2"
> -	shift; shift
> -
> -	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"  >> $seqres.full
> +	_scratch_xfs_set_sb_field "$@" >> $seqres.full
>  }
>  
>  _filter_leftover() {
> diff --git a/tests/xfs/308 b/tests/xfs/308
> index e9d7f76..7e7adac 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}'
> +	_scratch_xfs_get_sb_field "$@"
>  }
>  
>  _set_sb_data() {
> -	field="$1"
> -	value="$2"
> -	shift; shift
> -
> -	_scratch_xfs_db -x -c 'sb 0' "$@" -c "write $field -- $value"  >> $seqres.full
> +	_scratch_xfs_set_sb_field "$@" >> $seqres.full
>  }
>  
>  _filter_leftover() {
> diff --git a/tests/xfs/339 b/tests/xfs/339
> index 734d47b..78b714f 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=`_scratch_xfs_get_sb_field rrmapino`
> +_scratch_xfs_set_metadata_field "u3.sfdir3.list[3].inumber.i4" $rrmapino \
> +	'sb 0' 'addr rootino' >> $seqres.full
>  _scratch_mount
>  
>  echo "Check files"
> diff --git a/tests/xfs/340 b/tests/xfs/340
> index 8cc50a2..7b1b7d1 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=$(_scratch_xfs_get_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 22f7ba3..2b0b1e5 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()
>  {
> @@ -72,9 +64,9 @@ make_xfs_scratch_flakey_table()
>  		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 blk_sz=$(_scratch_xfs_get_sb_field blocksize)
> +	local log_ofs=$(_scratch_xfs_get_sb_field logstart)
> +	local log_sz=$(_scratch_xfs_get_sb_field logblocks)
>  	local table=""
>  	local ofs=0
>  	local sz
> -- 
> 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

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

[Carlos Maiolino]

On Thu, Nov 09, 2017 at 03:32:48PM +0800, Hou Tao wrote:
> Hi,
> 
> This is the v3 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].
> 
> The bug that the test case can not reproduced on a 15GB scratch device
> has been fixed, and many thanks to Eryu Guan. I had tested it on 4.14-rc8,
> and the problem can be reproduced reliably now.
> 

Ok, found it :) I'll take a look into this tomorrow together with my patch.

Thanks for the test case.

> Comments and questions are welcome.
> 
> Regards,
> Tao
> 
> ---
> Changes since v2:
> 
> v3:
> * dmflakey: support error_writes feature for dm-flakey
> 	* check the availability of error_writes by trial instead of version number
> 
> * xfs: test for umount hang caused by the pending dquota log item in AIL
> 	* calculate log_ofs correctly by using convert cmd in xfs_db
> 
> * common/rc: factor out _scratch_xfs_[get|set]_sb_field
> 	* reimplement _scratch_xfs_[get|set]_sb_field based on
> 	  _scratch_xfs_[get|set]_metadata_field.
> 
> v2:
> 	* http://www.spinics.net/lists/fstests/msg07792.html
> 
> v1:
> 	* https://www.spinics.net/lists/fstests/msg07622.html
> 
> [1]: https://www.spinics.net/lists/linux-xfs/msg11614.html
> 
> Hou Tao (4):
>   dmflakey: support multiple dm targets for a dm-flakey device
>   dmflakey: support error_writes feature for dm-flakey
>   xfs: test for umount hang caused by the pending dquota log item in AIL
>   common/rc: factor out _scratch_xfs_[get|set]_sb_field
> 
>  common/dmflakey   |  29 +++++++++-
>  common/fuzzy      |  33 -----------
>  common/xfs        |  47 ++++++++++++++++
>  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, 257 insertions(+), 74 deletions(-)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> -- 
> 2.9.5
> 

-- 
Carlos

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

[Carlos Maiolino]

On Thu, Nov 09, 2017 at 03:32:48PM +0800, Hou Tao wrote:
> Hi,
> 
> This is the v3 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].
> 
> The bug that the test case can not reproduced on a 15GB scratch device
> has been fixed, and many thanks to Eryu Guan. I had tested it on 4.14-rc8,
> and the problem can be reproduced reliably now.
> 
> Comments and questions are welcome.
> 

Thanks, I'm testing my patch against this test, it might take a few more time to
properly work on that patch, but with the test it is better.

I didn't review the test yet, but I tried it once, and at least the result is
what I expected, I'll get some time to review it.

> Regards,
> Tao
> 
> ---
> Changes since v2:
> 
> v3:
> * dmflakey: support error_writes feature for dm-flakey
> 	* check the availability of error_writes by trial instead of version number
> 
> * xfs: test for umount hang caused by the pending dquota log item in AIL
> 	* calculate log_ofs correctly by using convert cmd in xfs_db
> 
> * common/rc: factor out _scratch_xfs_[get|set]_sb_field
> 	* reimplement _scratch_xfs_[get|set]_sb_field based on
> 	  _scratch_xfs_[get|set]_metadata_field.
> 
> v2:
> 	* http://www.spinics.net/lists/fstests/msg07792.html
> 
> v1:
> 	* https://www.spinics.net/lists/fstests/msg07622.html
> 
> [1]: https://www.spinics.net/lists/linux-xfs/msg11614.html
> 
> Hou Tao (4):
>   dmflakey: support multiple dm targets for a dm-flakey device
>   dmflakey: support error_writes feature for dm-flakey
>   xfs: test for umount hang caused by the pending dquota log item in AIL
>   common/rc: factor out _scratch_xfs_[get|set]_sb_field
> 
>  common/dmflakey   |  29 +++++++++-
>  common/fuzzy      |  33 -----------
>  common/xfs        |  47 ++++++++++++++++
>  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, 257 insertions(+), 74 deletions(-)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> -- 
> 2.9.5
> 

-- 
Carlos

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

[Carlos Maiolino]

On Thu, Nov 09, 2017 at 04:54:06PM +0800, Eryu Guan wrote:
> On Thu, Nov 09, 2017 at 03:32:48PM +0800, Hou Tao wrote:
> > Hi,
> > 
> > This is the v3 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].
> > 
> > The bug that the test case can not reproduced on a 15GB scratch device
> > has been fixed, and many thanks to Eryu Guan. I had tested it on 4.14-rc8,
> > and the problem can be reproduced reliably now.
> > 
> > Comments and questions are welcome.
> 
> I finally could reproduce the hang, thanks a lot!
> 
> But I won't merge the test until the fix is upstream, otherwise it would
> interrupt all test runs on XFS.

Just FYI, I should have the patch ready soon, it already passes this test, I
just need to make sure I'm not screwing up with the locks.

> 
> And I have some nitpicks on individual patches :)
> 
> Thanks,
> Eryu
> 
> > 
> > Regards,
> > Tao
> > 
> > ---
> > Changes since v2:
> > 
> > v3:
> > * dmflakey: support error_writes feature for dm-flakey
> > 	* check the availability of error_writes by trial instead of version number
> > 
> > * xfs: test for umount hang caused by the pending dquota log item in AIL
> > 	* calculate log_ofs correctly by using convert cmd in xfs_db
> > 
> > * common/rc: factor out _scratch_xfs_[get|set]_sb_field
> > 	* reimplement _scratch_xfs_[get|set]_sb_field based on
> > 	  _scratch_xfs_[get|set]_metadata_field.
> > 
> > v2:
> > 	* http://www.spinics.net/lists/fstests/msg07792.html
> > 
> > v1:
> > 	* https://www.spinics.net/lists/fstests/msg07622.html
> > 
> > [1]: https://www.spinics.net/lists/linux-xfs/msg11614.html
> > 
> > Hou Tao (4):
> >   dmflakey: support multiple dm targets for a dm-flakey device
> >   dmflakey: support error_writes feature for dm-flakey
> >   xfs: test for umount hang caused by the pending dquota log item in AIL
> >   common/rc: factor out _scratch_xfs_[get|set]_sb_field
> > 
> >  common/dmflakey   |  29 +++++++++-
> >  common/fuzzy      |  33 -----------
> >  common/xfs        |  47 ++++++++++++++++
> >  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, 257 insertions(+), 74 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 fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Carlos

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

[Darrick J. Wong]

On Thu, Nov 09, 2017 at 03:32:51PM +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>
> ---
>  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..22f7ba3
> --- /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.

Upstream commit is 373b0589dc8d58bc09c9a28d03611ae4fb216057 ("xfs:
Properly retry failed dquot items in case of error during buffer
writeback"), etc.

--D

> +#
> +#-----------------------------------------------------------------------
> +# 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
> +
> +	log_ofs=$(_scratch_xfs_db -r -c "convert fsb ${log_ofs} bb" | \
> +			awk '{gsub("[()]", "", $2); print $2}')
> +	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
> +_require_flakey_with_error_writes
> +_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_ERROR=$(make_xfs_scratch_flakey_table)
> +_load_flakey_table ${FLAKEY_ERROR_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 v3 3/4] xfs: test for umount hang caused by the pending dquota log item in AIL

[Darrick J. Wong]

On Mon, Dec 04, 2017 at 04:47:35PM -0800, Darrick J. Wong wrote:
> On Thu, Nov 09, 2017 at 03:32:51PM +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>
> > ---
> >  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..22f7ba3
> > --- /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.
> 
> Upstream commit is 373b0589dc8d58bc09c9a28d03611ae4fb216057 ("xfs:
> Properly retry failed dquot items in case of error during buffer
> writeback"), etc.

Ping?  This has been fixed upstream for a month, still waiting for tests
to get revised & resent....

--D

> --D
> 
> > +#
> > +#-----------------------------------------------------------------------
> > +# 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
> > +
> > +	log_ofs=$(_scratch_xfs_db -r -c "convert fsb ${log_ofs} bb" | \
> > +			awk '{gsub("[()]", "", $2); print $2}')
> > +	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
> > +_require_flakey_with_error_writes
> > +_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_ERROR=$(make_xfs_scratch_flakey_table)
> > +_load_flakey_table ${FLAKEY_ERROR_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
> --
> 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