Fix/harden "quick" tests for realtime subvolumes

PATCH In review Richard Wareing 2 participants 6 messages Jan 12, 2018 – Jan 19, 2018

lore

Share & tools

lore

Richard Wareing

author

2018-01-12 04:16 UTC #0 lore raw

v4 cover Fix/harden "quick" tests for realtime subvolumes

See v3 cover letter, everything still applies.

Richard Wareing (3):
xfs/realtime: Add require_no_realtime function xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function xfs/realtime: Fix direct invocations of xfs_repair

 common/rc         | 28 ++++++++++++++++++++++++++--
 tests/generic/250 |  3 +++
 tests/generic/252 |  3 +++
 tests/generic/441 |  3 +++
 tests/xfs/070     |  5 ++++-
 tests/xfs/077     |  2 ++
 tests/xfs/170     |  1 +
 tests/xfs/189     |  1 +
 tests/xfs/284     |  2 ++
 tests/xfs/291     |  5 ++++-
 10 files changed, 49 insertions(+), 4 deletions(-)

Signature · 1 line

2.9.5

Richard Wareing

author

2018-01-12 04:16 UTC #1 lore raw

PATCH v4 1/3 xfs/realtime: Add require_no_realtime function

Some tests do not play well with realtime devices, in an effort to produce a stable set of test which exercise the realtime code paths we introduce a _require_no_realtime function to allow tests to opt out of realtime subvolume test runs.

Signed-off-by Richard Wareing <[email protected]>

---
Changes since v3:
* Fixed formatting nit
* Added USE_EXTERNAL test in _require_no_realtime
* Bashism fix in _get_mount

Changes since v2:
* Fixed tests generic/409-411, _get_mount now honors $SCRATCH_OPTIONS
* tests/xfs/202 reverted, the test does indeed work with larger test device
* Added comments explaining why _require_no_realtime was declared

Changes since v1:
* None

 common/rc     | 18 ++++++++++++++++--
 tests/xfs/077 |  2 ++
 tests/xfs/189 |  1 +
 tests/xfs/284 |  2 ++
 4 files changed, 21 insertions(+), 2 deletions(-)

common/rc +16 -2

index 9216efd..76f9d69 100644
@@ -198,8 +198,14 @@ _mount()
 _get_mount()
 {
 	local mnt_point=${!#}
+	local mnt_dev=${@:(-2):1}
+	local scratch_opts=""
+	if [ "$mnt_dev" = "$SCRATCH_DEV" ]; then
+		_scratch_options mount
+		scratch_opts="$SCRATCH_OPTIONS"
+	fi
 
-	_mount $*
+	_mount $scratch_opts $*
 	if [ $? -eq 0 ]; then
 		MOUNTED_POINT_STACK="$mnt_point $MOUNTED_POINT_STACK"
 	else

@@ -230,7 +236,7 @@ _clear_mount_stack()
 
 _scratch_options()
 {
-    type=$1
+    local type=$1
     SCRATCH_OPTIONS=""
 
     if [ "$FSTYP" != "xfs" ]; then

@@ -1737,6 +1743,14 @@ _require_realtime()
 	_notrun "Realtime device required, skipped this test"
 }
 
+# This test requires that a realtime subvolume is not in use
+#
+_require_no_realtime()
+{
+    [ "$USE_EXTERNAL" = "yes" ] && [ -n "$SCRATCH_RTDEV" ] && \
+	_notrun "Test not compatible with realtime subvolumes, skipped this test"
+}
+
 # this test requires that a specified command (executable) exists
 # $1 - command, $2 - name for error message
 #

tests/xfs/077 +2 -0

index eba4f08..6d5ac1a 100755
@@ -50,6 +50,8 @@ _cleanup()
 
 _supported_fs xfs
 _supported_os Linux
+# xfs_copy does not support realtime devices
+_require_no_realtime
 _require_scratch
 _require_xfs_crc
 _require_meta_uuid

tests/xfs/189 +1 -0

index 636f6f0..699eb3c 100755
@@ -236,6 +236,7 @@ _putback_scratch_fstab()
 _supported_fs xfs
 _supported_os Linux
 
+_require_no_realtime
 _require_scratch
 _require_noattr2

tests/xfs/284 +2 -0

index e3625fe..d0eb5bd 100755
@@ -49,6 +49,8 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs xfs
 _supported_os Linux
+# xfs_copy does not support realtime devices
+_require_no_realtime
 _require_test
 _require_scratch

Signature · 1 line

2.9.5

Richard Wareing

author

2018-01-12 04:16 UTC #2 lore raw

PATCH v4 2/3 xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function

In reply to Richard Wareing

To better exercise the data path code of realtime subvolumes, we will set rtinherit=1 during mkfs calls. For tests which this is not desired we introduce a _require_no_rtinherit function to opt out of this behavior.

Signed-off-by Richard Wareing <[email protected]>

---
Changes since v3:
* XFS FS check in _require_no_rtinherit
* Fixed comment for _require_no_rtinherit to reflect (reworked) function

Changes since v2:
* Removed use of RT_INHERT, instead we now simply bail from the test. Users will have to create two separate configs for realtime one with rtinherit=1 in the mkfs options, one without and do separate runs to get full test coverage.
* Added comments explaining reasons for _require_no_rtinherit declarations

Changes since v1:
* None

 common/rc         | 10 ++++++++++
 tests/generic/250 |  3 +++
 tests/generic/252 |  3 +++
 tests/generic/441 |  3 +++
 tests/xfs/170     |  1 +
 5 files changed, 20 insertions(+)

common/rc +10 -0

index 76f9d69..cceabce 100644
@@ -33,6 +33,16 @@ BC=$(which bc 2> /dev/null) || BC=
 VALID_TEST_ID="[0-9]\{3\}"
 VALID_TEST_NAME="$VALID_TEST_ID-\?[[:alnum:]-]*"
 
+# Some tests are not relevant or functional when testing XFS realtime
+# subvolumes along with the rtinherit=1 mkfs option.  In these cases,
+# this test will opt-out of the test.
+_require_no_rtinherit()
+{
+  [ "$FSTYP" = "xfs" ] && echo "$MKFS_OPTIONS" | 
+    egrep -q "rtinherit([^=]|=1)" && \
+    _notrun "rtinherit mkfs option is not supported by this test."
+}
+
 _require_math()
 {
 	if [ -z "$BC" ]; then

tests/generic/250 +3 -0

index 3c4fe6d..a8fd97e 100755
@@ -48,6 +48,9 @@ _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
 _require_odirect
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
 
 rm -f $seqres.full

tests/generic/252 +3 -0

index ffedd56..b506d59 100755
@@ -47,6 +47,9 @@ _supported_os Linux
 _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
 _require_aiodio "aiocp"
 AIO_TEST="src/aio-dio-regress/aiocp"

tests/generic/441 +3 -0

index 075d877..5fbfece 100755
@@ -47,6 +47,9 @@ _cleanup()
 # real QA test starts here
 _supported_os Linux
 _require_scratch
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
 
 # Generally, we want to avoid journal errors on the extended testcase. Only
 # unset the -s flag if we have a logdev

tests/xfs/170 +1 -0

index c5ae8e4..6deef1b 100755
@@ -50,6 +50,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"

Signature · 1 line

2.9.5

Eryu Guan

2018-01-19 04:39 UTC #3 lore raw

Re: [PATCH v4 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function

Quoted 33 lines · Richard Wareing

To better exercise the data path code of realtime subvolumes, we will set rtinherit=1 during mkfs calls. For tests which this is not desired we introduce a _require_no_rtinherit function to opt out of this behavior.
Signed-off-by Richard Wareing <[email protected]>
---
Changes since v3:
* XFS FS check in _require_no_rtinherit
* Fixed comment for _require_no_rtinherit to reflect (reworked) function
Changes since v2:
* Removed use of RT_INHERT, instead we now simply bail from the test. Users will have to create two separate configs for realtime one with rtinherit=1 in the mkfs options, one without and do separate runs to get full test coverage.
* Added comments explaining reasons for _require_no_rtinherit declarations
Changes since v1:
* None
 common/rc         | 10 ++++++++++
 tests/generic/250 |  3 +++
 tests/generic/252 |  3 +++
 tests/generic/441 |  3 +++
 tests/xfs/170     |  1 +
 5 files changed, 20 insertions(+)
common/rc +0 -0
index 76f9d69..cceabce 100644

common/rc BC=$(which bc 2> /dev/null) || BC= · line 41

Quoted 9 lines

common/rc +6 -0

@@ -33,6 +33,16 @@ BC=$(which bc 2> /dev/null) || BC=
 VALID_TEST_ID="[0-9]\{3\}"
 VALID_TEST_NAME="$VALID_TEST_ID-\?[[:alnum:]-]*"
 
+# Some tests are not relevant or functional when testing XFS realtime
+# subvolumes along with the rtinherit=1 mkfs option.  In these cases,
+# this test will opt-out of the test.
+_require_no_rtinherit()
+{
+  [ "$FSTYP" = "xfs" ] && echo "$MKFS_OPTIONS" |

Trailing whitespace in above line :)

common/rc BC=$(which bc 2> /dev/null) || BC= · line 42

Quoted 1 line

common/rc +1 -0

+    egrep -q "rtinherit([^=]|=1)" && \

Hmm, this works with "-drtinherit=1" and "-drtinherit,<other opts>" but fails to detect the "-d rtinherit" case, because the regexp expects some character after "rtinherit". I think this change could fix it:

-   egrep -q "rtinherit([^=]|=1)" && \
+   egrep -q "rtinherit([^=]|=1|$)" && \

If this looks ok to you, I can fix it on commit (along with the whitespace issue).

Thanks,
Eryu

Quoted 63 lines

common/rc +3 -0

+    _notrun "rtinherit mkfs option is not supported by this test."
+}
+
 _require_math()
 {
 	if [ -z "$BC" ]; then

tests/generic/250 +3 -0

index 3c4fe6d..a8fd97e 100755
@@ -48,6 +48,9 @@ _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
 _require_odirect
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
 
 rm -f $seqres.full

tests/generic/252 +3 -0

index ffedd56..b506d59 100755
@@ -47,6 +47,9 @@ _supported_os Linux
 _require_scratch
 _require_dm_target error
 _require_xfs_io_command "falloc"
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
 _require_aiodio "aiocp"
 AIO_TEST="src/aio-dio-regress/aiocp"

tests/generic/441 +3 -0

index 075d877..5fbfece 100755
@@ -47,6 +47,9 @@ _cleanup()
 # real QA test starts here
 _supported_os Linux
 _require_scratch
+# This test uses "dm" without taking into account the data could be on
+# realtime subvolume, thus the test will fail with rtinherit=1
+_require_no_rtinherit
 
 # Generally, we want to avoid journal errors on the extended testcase. Only
 # unset the -s flag if we have a logdev

tests/xfs/170 +1 -0

index c5ae8e4..6deef1b 100755
@@ -50,6 +50,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"

Signature · 1 line

2.9.5

Eryu Guan

2018-01-19 08:07 UTC #4 lore raw

Re: [PATCH v4 2/3] xfs/realtime: Default rtinherit=1, add _require_no_rtinherit function

In reply to Richard Wareing

Quoted 6 lines · Richard Wareing

To better exercise the data path code of realtime subvolumes, we will set rtinherit=1 during mkfs calls. For tests which this is not desired we introduce a _require_no_rtinherit function to opt out of this behavior.
Signed-off-by Richard Wareing <[email protected]>

BTW, generic/256 triggered xfs shutdown with rtdev enabled and MKFS_OPTIONS="-d rtinherit".

Thanks,
Eryu

[59132.903870] XFS (vda6): _xfs_buf_find: Block out of range: block 0xc00000003fff0, EOFS 0x300000
[59132.904675] WARNING: CPU: 3 PID: 1199 at fs/xfs/xfs_buf.c:590 _xfs_buf_find+0x3e8/0x520 [xfs]
[59132.905352] Modules linked in: xfs dm_delay dm_thin_pool dm_persistent_data dm_bio_prison sd_mod sg dm_snapshot dm_bufio dm_flakey loop dm_mod ip6t_rpfilter ipt_REJECT nf_reject_ipv4 ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter sunrpc btrfs xor zstd_decompress zstd_compress xxhash raid6_pq i2c_piix4 i2c_core virtio_balloon joydev pcspkr ip_tables ext4 mbcache jbd2 ata_generic pata_acpi virtio_net virtio_blk ata_piix libata virtio_pci virtio_ring virtio
[59132.905607]  serio_raw floppy [last unloaded: scsi_debug]
[59132.905607] CPU: 3 PID: 1199 Comm: xfs_io Tainted: G        W        4.15.0-rc6 #28
[59132.905607] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2007
[59132.905607] RIP: 0010:_xfs_buf_find+0x3e8/0x520 [xfs]
[59132.905607] RSP: 0018:ffff9eb8c1d83718 EFLAGS: 00010246
[59132.905607] RAX: 0000000000000000 RBX: ffff9eb8c1d83878 RCX: 0000000000000000
[59132.905607] RDX: ffff9eb8c1d83638 RSI: 000000000000000a RDI: ffffffffc0758ebb
[59132.905607] RBP: 0000000000000001 R08: 0000000000000000 R09: 0000000000000021
[59132.905607] R10: 0000000000000000 R11: 000000000000000a R12: ffff93846fb19b40
[59132.905607] R13: ffff93846fb19b40 R14: 0000000000000001 R15: ffff9eb8c1d83878
[59132.905607] FS:  00007f8cb01ea740(0000) GS:ffff93849fd80000(0000) knlGS:0000000000000000
[59132.905607] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[59132.905607] CR2: 00007f8caf9eefb8 CR3: 0000000215749000 CR4: 00000000000006e0
[59132.905607] Call Trace:
[59132.905607]  ? _cond_resched+0x15/0x30
[59132.905607]  xfs_buf_get_map+0x23/0x260 [xfs]
[59132.905607]  xfs_buf_read_map+0x29/0x180 [xfs]
[59132.905607]  xfs_trans_read_buf_map+0xec/0x300 [xfs]
[59132.905607]  xfs_rtbuf_get+0xf5/0x130 [xfs]
[59132.905607]  xfs_rtcheck_range+0x10a/0x300 [xfs]
[59132.905607]  xfs_rtallocate_extent_block+0x137/0x240 [xfs]
[59132.905607]  xfs_rtallocate_extent_near+0x287/0x390 [xfs]
[59132.905607]  ? up+0x12/0x46
[59132.905607]  ? xfs_buf_rele+0x5b/0x390 [xfs]
[59132.905607]  xfs_rtallocate_extent+0x121/0x1b0 [xfs]
[59132.905607]  xfs_bmap_rtalloc+0x180/0x2b0 [xfs]
[59132.905607]  xfs_bmapi_write+0x5dc/0xce0 [xfs]
[59132.905607]  ? lookup_fast+0xcb/0x2b0
[59132.905607]  xfs_alloc_file_space+0x15a/0x3a0 [xfs]
[59132.905607]  ? account_entity_enqueue+0xc5/0xf0
[59132.905607]  xfs_file_fallocate+0x1e2/0x330 [xfs]
[59132.905607]  ? check_preempt_curr+0x74/0xa0
[59132.905607]  ? wake_up_new_task+0x1c4/0x280
[59132.905607]  vfs_fallocate+0x151/0x270
[59132.905607]  SyS_fallocate+0x3f/0x60
[59132.905607]  do_syscall_64+0x61/0x1a0
[59132.905607]  entry_SYSCALL64_slow_path+0x25/0x25
[59132.905607] RIP: 0033:0x7f8cafae47e0
[59132.905607] RSP: 002b:00007ffd3ad5eb50 EFLAGS: 00000293 ORIG_RAX: 000000000000011d
[59132.905607] RAX: ffffffffffffffda RBX: 000000000000011d RCX: 00007f8cafae47e0
[59132.905607] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
[59132.905607] RBP: 0000000001111130 R08: 0000000000000000 R09: 00000000011123e2
[59132.905607] R10: 0000000010000000 R11: 0000000000000293 R12: 0000000000000000
[59132.905607] R13: 00000000011127c0 R14: 0000000000000001 R15: 0000000000000000
[59132.905607] Code: 48 89 de ff d0 49 8b 45 00 48 85 c0 75 e5 e9 57 ff ff ff 48 89 c1 48 c7 c2 c0 56 75 c0 48 c7 c6 80 c7 75 c0 31 c0 e8 68 89 01 00 <0f> ff 31 c0 e9 74 ff ff ff 39 ca 0f 82 56 fe ff ff 48 8b 4c 24
[59132.905607] ---[ end trace 2525be156b26eb0c ]---

Richard Wareing

author

2018-01-12 04:16 UTC #5 lore raw

PATCH v4 3/3 xfs/realtime: Fix direct invocations of xfs_repair

In reply to Richard Wareing

Fixes direct invocations of xfs_repair to add in -r option if required.

Reviewed-by Darrick J. Wong <[email protected]>

Signed-off-by Richard Wareing <[email protected]>

---
Changes since v3:
* Formatting nits

Changes since v2:
* Added check for USE_EXTERNAL in tests/xfs/291 & tests/xfs/070
* Call xfs_repair via $XFS_REPAIR_PROG

Changes since v1:
* Fixed kill -9 in test xfs/070

 tests/xfs/070 | 5 ++++-
 tests/xfs/291 | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

tests/xfs/070 +4 -1

index 0ae6eff..81111d5 100755
@@ -56,7 +56,10 @@ _cleanup()
 _xfs_repair_noscan()
 {
 	# invoke repair directly so we can kill the process if need be
-	$XFS_REPAIR_PROG $SCRATCH_DEV 2>&1 | tee -a $seqres.full > $tmp.repair &
+	[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \
+		rt_repair_opts="-r $SCRATCH_RTDEV"
+	$XFS_REPAIR_PROG $rt_repair_opts $SCRATCH_DEV 2>&1 |
+		tee -a $seqres.full > $tmp.repair &
 	repair_pid=$!
 
 	# monitor progress for as long as it is running

tests/xfs/291 +4 -1

index 3f5295c..fd5c1f1 100755
@@ -122,7 +122,10 @@ _xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed"
 # Can xfs_metadump cope with this monster?
 _scratch_metadump $tmp.metadump || _fail "xfs_metadump failed"
 xfs_mdrestore $tmp.metadump $tmp.img || _fail "xfs_mdrestore failed"
-xfs_repair -f $tmp.img >> $seqres.full 2>&1 || _fail "xfs_repair of metadump failed"
+[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \
+	rt_repair_opts="-r $SCRATCH_RTDEV"
+$XFS_REPAIR_PROG $rt_repair_opts -f $tmp.img >> $seqres.full 2>&1 || \
+	_fail "xfs_repair of metadump failed"
 
 # Yes it can; success, all done
 status=0

Signature · 1 line

2.9.5