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

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

[Hou Tao]

Hi,

This is the v4 patchset for testing the XFS umount hang problem caused by
the pending dquota log item in AIL. The problem had been fixed by commit
373b058 ("xfs: Properly retry failed dquot items in case of error during
buffer writeback"). The changes for v4 are appended in the commit log
of each patches.

Comments and questions are welcome.

Regards,
Tao

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   |  31 +++++++++-
 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     | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/999.out |   2 +
 tests/xfs/group   |   1 +
 14 files changed, 269 insertions(+), 75 deletions(-)
 create mode 100755 tests/xfs/999
 create mode 100644 tests/xfs/999.out

-- 
2.9.5

[PATCH v4 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>
---
v4: add comments for reading dm target table from standard input
---
 common/dmflakey | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/common/dmflakey b/common/dmflakey
index 4434307..e34fce8 100644
--- a/common/dmflakey
+++ b/common/dmflakey
@@ -77,11 +77,14 @@ _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"
+	# There may be multiple dm targets in the table, and these dm targets
+	# will be joined by the newline ("\n"). Option --table can not cope with
+	# the multiple-targets case, so get them by reading from standard input.
+	echo -e "$table" | $DMSETUP_PROG load flakey-test
 	[ $? -ne 0 ] && _fatal "failed to load table into flakey-test"
 
 	$DMSETUP_PROG resume flakey-test
-	[ $? -ne 0 ] && _fatal  "failed to resumeflakey-test"
+	[ $? -ne 0 ] && _fatal  "failed to resume flakey-test"
 }
 
 # Silently drop all writes and unmount/remount to simulate a crash/power
-- 
2.9.5

[PATCH v4 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>
---
v4: call _cleanup_flakey() directly instead of copy & paste
---
 common/dmflakey | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/common/dmflakey b/common/dmflakey
index e34fce8..06eee37 100644
--- a/common/dmflakey
+++ b/common/dmflakey
@@ -20,6 +20,7 @@
 
 FLAKEY_ALLOW_WRITES=0
 FLAKEY_DROP_WRITES=1
+FLAKEY_ERROR_WRITES=2
 
 echo $MOUNT_OPTIONS | grep -q dax
 if [ $? -eq 0 ]; then
@@ -32,6 +33,7 @@ _init_flakey()
 	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 +69,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=""
@@ -101,3 +103,23 @@ _flakey_drop_and_remount()
 	_load_flakey_table $FLAKEY_ALLOW_WRITES
 	_mount_flakey
 }
+
+_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
+
+	_cleanup_flakey
+}
-- 
2.9.5

[PATCH v4 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>
---
v4:
    * indicate the upstream commit fixes the problem
    * add more comments for make_xfs_scratch_flakey_table()
    * use $AWK_PROG instead of awk
    * use $qa_user instead of fsgqa
---
 tests/xfs/999     | 182 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/999.out |   2 +
 tests/xfs/group   |   1 +
 3 files changed, 185 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..10a2158
--- /dev/null
+++ b/tests/xfs/999
@@ -0,0 +1,182 @@
+#! /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.
+#
+# Fixed by upstream commit 373b058 ("xfs: Properly retry failed dquot
+# items in case of error during buffer writeback")
+#-----------------------------------------------------------------------
+# 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_PROG -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 using an external log device, just making the writing of
+	# entire data/metadata area fail forever.
+	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_PROG '{gsub("[()]", "", $2); print $2}')
+	let "log_sz *= blk_sz / 512"
+
+	# Add a flakey target for the area before the log section
+	# to make the data/metadata write fail forever
+	if [ "$ofs" -lt "${log_ofs}" ]; then
+		let "sz = log_ofs - ofs"
+		table="$ofs $sz $tgt $dev $ofs $opt"
+	fi
+
+	# Add a linear target for the log section, so the log write
+	# will work normally
+	table="$table\n${log_ofs} ${log_sz} linear $dev ${log_ofs}"
+
+	# Add a flakey target for the area after the log section
+	# to make the data/metadata write fail forever
+	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 > $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 $qa_user" $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 $qa_user" $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 d230060..6c14ddd 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -434,3 +434,4 @@
 434 auto quick clone fsr
 435 auto quick clone
 436 auto quick clone fsr
+999 auto quick quota dangerous
-- 
2.9.5

[PATCH v4 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.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
v4: * add double dashes between key and value to support negative value
      or value prefixed by dash
---
 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 d70964f..9a0456f 100644
--- a/common/fuzzy
+++ b/common/fuzzy
@@ -126,39 +126,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 5cfa761..45b84a0 100644
--- a/common/xfs
+++ b/common/xfs
@@ -624,3 +624,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 10a2158..55d324f 100755
--- a/tests/xfs/999
+++ b/tests/xfs/999
@@ -54,14 +54,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_PROG -v field=$field '$0 ~ field {print $3}'
-}
-
 # inject IO write error for the XFS filesystem except its log section
 make_xfs_scratch_flakey_table()
 {
@@ -77,9 +69,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 v4 0/4] test for XFS umount hang caused by the pending dquota log item in AIL

[Eryu Guan]

On Thu, Jan 11, 2018 at 02:49:29PM +0800, Hou Tao wrote:
> Hi,
> 
> This is the v4 patchset for testing the XFS umount hang problem caused by
> the pending dquota log item in AIL. The problem had been fixed by commit
> 373b058 ("xfs: Properly retry failed dquot items in case of error during
> buffer writeback"). The changes for v4 are appended in the commit log
> of each patches.
> 
> Comments and questions are welcome.

The patches all look good, thanks a lot for the revision!

> 
> Regards,
> Tao
> 
> 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

I switched the order of the last two patches, i.e. refactor
_scratch_xfs_[get|set]_sb_field first then use the new helper in the new
test in the end, so we don't add new test then update it again in the
same patch set.

Thanks,
Eryu

> 
>  common/dmflakey   |  31 +++++++++-
>  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     | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |   2 +
>  tests/xfs/group   |   1 +
>  14 files changed, 269 insertions(+), 75 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

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

[Hou Tao]

Hi Eryu,

On 2018/1/11 16:02, Eryu Guan wrote:
> On Thu, Jan 11, 2018 at 02:49:29PM +0800, Hou Tao wrote:
>> Hi,
>>
>> This is the v4 patchset for testing the XFS umount hang problem caused by
>> the pending dquota log item in AIL. The problem had been fixed by commit
>> 373b058 ("xfs: Properly retry failed dquot items in case of error during
>> buffer writeback"). The changes for v4 are appended in the commit log
>> of each patches.
>>
>> Comments and questions are welcome.
> 
> The patches all look good, thanks a lot for the revision!
> 
>>
>> Regards,
>> Tao
>>
>> 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
> 
> I switched the order of the last two patches, i.e. refactor
> _scratch_xfs_[get|set]_sb_field first then use the new helper in the new
> test in the end, so we don't add new test then update it again in the
> same patch set.

Yes, it's better after the order switch and thanks for that.

Regards,
Tao

> Thanks,
> Eryu
> 
>>
>>  common/dmflakey   |  31 +++++++++-
>>  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     | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/xfs/999.out |   2 +
>>  tests/xfs/group   |   1 +
>>  14 files changed, 269 insertions(+), 75 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
> 
> .
>