quota: clear speculative delalloc when checking quota usage

PATCH In review Darrick J. Wong 2 participants 32 messages Oct 26, 2017 – Nov 1, 2017

lore

Share & tools

lore

Darrick J. Wong

author

2017-10-26 05:51 UTC #0 lore raw

PATCH 1/6 quota: clear speculative delalloc when checking quota usage

From: Darrick J. Wong <[email protected]>

Occasionally speculative preallocation kicks in when writing files to a filesystem under test. These preallocations consume quota and /usually/ aren't around after we drop_caches, but there's nothing to guarantee that they actually have, so the quota reports will be different before and after the fs remount, causing sporadic test failures in generic/{23[123],270}.

We now have xfs_spaceman which can instruct XFS to forcibly remove the speculative preallocations. This fixes the sporadic failures, at least for XFS.

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

---
 common/config |    1 +
 common/quota  |    6 ++++++
 2 files changed, 7 insertions(+)

common/config +1 -0

index 8844173..96503c6 100644
@@ -149,6 +149,7 @@ export XFS_LOGPRINT_PROG="`set_prog_path xfs_logprint`"
 export XFS_REPAIR_PROG="`set_prog_path xfs_repair`"
 export XFS_DB_PROG="`set_prog_path xfs_db`"
 export XFS_GROWFS_PROG=`set_prog_path xfs_growfs`
+export XFS_SPACEMAN_PROG="`set_prog_path xfs_spaceman`"
 export XFS_SCRUB_PROG="`set_prog_path xfs_scrub`"
 export XFS_PARALLEL_REPAIR_PROG="`set_prog_path xfs_prepair`"
 export XFS_PARALLEL_REPAIR64_PROG="`set_prog_path xfs_prepair64`"

common/quota +6 -0

index d027a8c..2611c48 100644
@@ -267,6 +267,12 @@ _check_quota_usage()
 		VFS_QUOTA=1
 		quotaon -f -u -g $SCRATCH_MNT 2>/dev/null
 		;;
+	xfs)
+		# Clear out speculative preallocations to eliminate them
+		# as a source of intermittent orig/checked differences.
+		test -x "$XFS_SPACEMAN_PROG" && \
+			"$XFS_SPACEMAN_PROG" -c 'prealloc -s' $SCRATCH_MNT
+		;;
 	*)
 		;;
 	esac

Darrick J. Wong

author

2017-10-26 05:51 UTC #1 lore raw

PATCH 2/6 common/xfs: refactor xfs_scrub presence testing

From: Darrick J. Wong <[email protected]>

Move all the requirements checking for xfs_scrub into a helper function. Make sure the helper properly detects the presence of the scrub ioctl and situations where we can't run scrub (e.g. norecovery).

Refactor the existing three xfs_scrub call sites to use the helper to check if it's appropriate to run scrub.

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

---
 README            |    6 +++---
 common/rc         |    2 +-
 common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
 tests/generic/453 |   11 +----------
 tests/generic/454 |   11 +----------
 5 files changed, 39 insertions(+), 31 deletions(-)

README +3 -3

index 4963d28..a9da4f0 100644
@@ -88,9 +88,9 @@ Preparing system for tests:
                run xfs_repair -n to check the filesystem; xfs_repair to rebuild
                metadata indexes; and xfs_repair -n (a third time) to check the
                results of the rebuilding.
-             - set TEST_XFS_SCRUB=1 to have _check_xfs_filesystem run
-               xfs_scrub -vd to scrub the filesystem metadata online before
-               unmounting to run the offline check.
+             - xfs_scrub, if present, will always check the test and scratch
+               filesystems if they are still online at the end of the test.
+               It is no longer necessary to set TEST_XFS_SCRUB.
              - setenv LOGWRITES_DEV to a block device to use for power fail
                testing.

common/rc +1 -1

index 1a4d81e..83aaced 100644
@@ -2091,7 +2091,7 @@ _require_xfs_io_command()
 			_notrun "xfs_io $command support is missing"
 		;;
 	"scrub"|"repair")
-		testio=`$XFS_IO_PROG -x -c "$command test 0" $TEST_DIR 2>&1`
+		testio=`$XFS_IO_PROG -x -c "$command probe 0" $TEST_DIR 2>&1`
 		echo $testio | grep -q "Inappropriate ioctl" && \
 			_notrun "xfs_io $command support is missing"
 		;;

common/xfs +33 -7

index dff8454..df7d029 100644
@@ -298,6 +298,29 @@ _require_xfs_db_command()
 		_notrun "xfs_db $command support is missing"
 }
 
+# Does the filesystem mounted from a particular device support scrub?
+_supports_xfs_scrub()
+{
+	mountpoint="$1"
+	device="$2"
+
+	if [ ! -b "$device" ] || [ ! -e "$mountpoint" ]; then
+		echo "Usage: _supports_xfs_scrub mountpoint device"
+		exit 1
+	fi
+
+	test "$FSTYP" = "xfs" || return 1
+	test -x "$XFS_SCRUB_PROG" || return 1
+
+	# Probe for kernel support...
+	$XFS_IO_PROG -x -c "scrub probe 0" "$mountpoint" 2>&1 | grep -q "Inappropriate ioctl" && return 1
+
+	# Scrub can't run on norecovery mounts
+	_fs_options "$device" | grep -q "norecovery" && return 1
+
+	return 0
+}
+
 # run xfs_check and friends on a FS.
 _check_xfs_filesystem()
 {

@@ -330,14 +353,17 @@ _check_xfs_filesystem()
 	type=`_fs_type $device`
 	ok=1
 
-	if [ "$type" = "xfs" ]; then
-		if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
-			"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
-			if [ $? -ne 0 ]; then
-				_log_err "filesystem on $device failed scrub"
-				ok=0
-			fi
+	# Run online scrub if we can.
+	mntpt="$(_is_mounted $device)"
+	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
+		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full 2>&1
+		if [ $? -ne 0 ]; then
+			_log_err "filesystem on $device failed scrub"
+			ok=0
 		fi
+	fi
+
+	if [ "$type" = "xfs" ]; then
 		# mounted ...
 		mountpoint=`_umount_or_remount_ro $device`
 	fi

tests/generic/453 +1 ~~-10~~

index ff29736..40fae91 100755
@@ -136,10 +136,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \

@@ -147,12 +144,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }

tests/generic/454 +1 ~~-10~~

index 01279ee..462185a 100755
@@ -132,10 +132,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \

@@ -143,12 +140,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }

Eryu Guan

2017-10-27 04:37 UTC #2 lore raw

Re: [PATCH 2/6] common/xfs: refactor xfs_scrub presence testing

Quoted 25 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>
Move all the requirements checking for xfs_scrub into a helper function. Make sure the helper properly detects the presence of the scrub ioctl and situations where we can't run scrub (e.g. norecovery).
Refactor the existing three xfs_scrub call sites to use the helper to check if it's appropriate to run scrub.
Signed-off-by Darrick J. Wong <[email protected]>
---
 README            |    6 +++---
 common/rc         |    2 +-
 common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
 tests/generic/453 |   11 +----------
 tests/generic/454 |   11 +----------
 5 files changed, 39 insertions(+), 31 deletions(-)
README +0 -0
index 4963d28..a9da4f0 100644
@@ -88,9 +88,9 @@ Preparing system for tests:
                run xfs_repair -n to check the filesystem; xfs_repair to rebuild

README Preparing system for tests: · line 95

Quoted 10 lines

README +3 -3

                metadata indexes; and xfs_repair -n (a third time) to check the
                results of the rebuilding.
-             - set TEST_XFS_SCRUB=1 to have _check_xfs_filesystem run
-               xfs_scrub -vd to scrub the filesystem metadata online before
-               unmounting to run the offline check.
+             - xfs_scrub, if present, will always check the test and scratch
+               filesystems if they are still online at the end of the test.
+               It is no longer necessary to set TEST_XFS_SCRUB.
              - setenv LOGWRITES_DEV to a block device to use for power fail
                testing.

With xfs_scrub check enabled by default, I always see scrub failures like this:

_check_xfs_filesystem: filesystem on /dev/vda5 failed scrub (see /root/xfstests/results//check.full for details)
xfs_growfs: /mnt/testarea/test is not a mounted XFS filesystem generic/445 1s ... 1s
_check_xfs_filesystem: filesystem on /dev/vda5 failed scrub (see /root/xfstests/results//xfs_4k_crc/generic/445.full for details)
xfs_growfs: /mnt/testarea/test is not a mounted XFS filesystem
Ran: generic/445
Passed all 1 tests

And in check.full or 445.full:
*** xfs_scrub -v -d -n output ***
Phase 1: Find filesystem geometry.
Info: /mnt/testarea/test: Kernel metadata scrub is required. (ioctl.c line 946)
Info: /mnt/testarea/test: Scrub aborted after phase 1. (scrub.c line 801)

And this happens to all tests and all test configs. Is this a known issue of scrub?

The xfs_growfs message probably comes from _check_xfs_test_fs(), stderr of xfs_growfs should go to the pipe too.

Thanks,
Eryu

Quoted 132 lines

common/rc +1 -1

index 1a4d81e..83aaced 100644
@@ -2091,7 +2091,7 @@ _require_xfs_io_command()
 			_notrun "xfs_io $command support is missing"
 		;;
 	"scrub"|"repair")
-		testio=`$XFS_IO_PROG -x -c "$command test 0" $TEST_DIR 2>&1`
+		testio=`$XFS_IO_PROG -x -c "$command probe 0" $TEST_DIR 2>&1`
 		echo $testio | grep -q "Inappropriate ioctl" && \
 			_notrun "xfs_io $command support is missing"
 		;;

common/xfs +33 -7

index dff8454..df7d029 100644
@@ -298,6 +298,29 @@ _require_xfs_db_command()
 		_notrun "xfs_db $command support is missing"
 }
 
+# Does the filesystem mounted from a particular device support scrub?
+_supports_xfs_scrub()
+{
+	mountpoint="$1"
+	device="$2"
+
+	if [ ! -b "$device" ] || [ ! -e "$mountpoint" ]; then
+		echo "Usage: _supports_xfs_scrub mountpoint device"
+		exit 1
+	fi
+
+	test "$FSTYP" = "xfs" || return 1
+	test -x "$XFS_SCRUB_PROG" || return 1
+
+	# Probe for kernel support...
+	$XFS_IO_PROG -x -c "scrub probe 0" "$mountpoint" 2>&1 | grep -q "Inappropriate ioctl" && return 1
+
+	# Scrub can't run on norecovery mounts
+	_fs_options "$device" | grep -q "norecovery" && return 1
+
+	return 0
+}
+
 # run xfs_check and friends on a FS.
 _check_xfs_filesystem()
 {

@@ -330,14 +353,17 @@ _check_xfs_filesystem()
 	type=`_fs_type $device`
 	ok=1
 
-	if [ "$type" = "xfs" ]; then
-		if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
-			"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
-			if [ $? -ne 0 ]; then
-				_log_err "filesystem on $device failed scrub"
-				ok=0
-			fi
+	# Run online scrub if we can.
+	mntpt="$(_is_mounted $device)"
+	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
+		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full 2>&1
+		if [ $? -ne 0 ]; then
+			_log_err "filesystem on $device failed scrub"
+			ok=0
 		fi
+	fi
+
+	if [ "$type" = "xfs" ]; then
 		# mounted ...
 		mountpoint=`_umount_or_remount_ro $device`
 	fi

tests/generic/453 +1 ~~-10~~

index ff29736..40fae91 100755
@@ -136,10 +136,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \

@@ -147,12 +144,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }

tests/generic/454 +1 ~~-10~~

index 01279ee..462185a 100755
@@ -132,10 +132,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \

@@ -143,12 +140,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }

Darrick J. Wong

author

2017-10-27 18:04 UTC #3 lore raw

Re: [PATCH 2/6] common/xfs: refactor xfs_scrub presence testing

Quoted 50 lines · Eryu Guan

Quoted 25 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Move all the requirements checking for xfs_scrub into a helper function. Make sure the helper properly detects the presence of the scrub ioctl and situations where we can't run scrub (e.g. norecovery).
Refactor the existing three xfs_scrub call sites to use the helper to check if it's appropriate to run scrub.
Signed-off-by Darrick J. Wong <[email protected]>
---
 README            |    6 +++---
 common/rc         |    2 +-
 common/xfs        |   40 +++++++++++++++++++++++++++++++++-------
 tests/generic/453 |   11 +----------
 tests/generic/454 |   11 +----------
 5 files changed, 39 insertions(+), 31 deletions(-)
README +0 -0
index 4963d28..a9da4f0 100644
@@ -88,9 +88,9 @@ Preparing system for tests:
                run xfs_repair -n to check the filesystem; xfs_repair to rebuild
README Preparing system for tests: · line 95
Quoted 10 lines
README +3 -3
                metadata indexes; and xfs_repair -n (a third time) to check the
                results of the rebuilding.
-             - set TEST_XFS_SCRUB=1 to have _check_xfs_filesystem run
-               xfs_scrub -vd to scrub the filesystem metadata online before
-               unmounting to run the offline check.
+             - xfs_scrub, if present, will always check the test and scratch
+               filesystems if they are still online at the end of the test.
+               It is no longer necessary to set TEST_XFS_SCRUB.
              - setenv LOGWRITES_DEV to a block device to use for power fail
                testing.
With xfs_scrub check enabled by default, I always see scrub failures like this:
_check_xfs_filesystem: filesystem on /dev/vda5 failed scrub (see /root/xfstests/results//check.full for details)
xfs_growfs: /mnt/testarea/test is not a mounted XFS filesystem generic/445 1s ... 1s
_check_xfs_filesystem: filesystem on /dev/vda5 failed scrub (see /root/xfstests/results//xfs_4k_crc/generic/445.full for details)
xfs_growfs: /mnt/testarea/test is not a mounted XFS filesystem
Ran: generic/445
Passed all 1 tests

Quoted 8 lines

And in check.full or 445.full:
*** xfs_scrub -v -d -n output ***
Phase 1: Find filesystem geometry.
Info: /mnt/testarea/test: Kernel metadata scrub is required. (ioctl.c line 946)
Info: /mnt/testarea/test: Scrub aborted after phase 1. (scrub.c line 801)
And this happens to all tests and all test configs. Is this a known issue of scrub?

It shouldn't be... but we /did/ change the name of the "does this kernel support scrub" xfs_io command to 'probe' from 'test', so if you have an old branch of djwong-dev sitting around that might be why it fails to notice that the running kernel doesn't support scrub.

Ok, I should test that xfs_io has a scrub command and that it has a probe subcommand.

Quoted 2 lines

The xfs_growfs message probably comes from _check_xfs_test_fs(), stderr of xfs_growfs should go to the pipe too.

The xfs_growfs error happens because we fail scrub, which sets ok=0. Then we unmount the filesystem to run repair, but we don't remount the filesystem after that because ok=0. _check_xfs_test_fs doesn't check the return value of _check_xfs_filesystem, so it blindly assumes that the testfs is still mounted and runs xfs_info, which fails as you describe. That whole thing is pointless anyway because we don't support "inode-paths" (i.e. IRIX parent pointers) so we might as well rip it out.

--D

Quoted 139 lines

Thanks,
Eryu

Quoted 122 lines

common/rc +1 -1

index 1a4d81e..83aaced 100644
@@ -2091,7 +2091,7 @@ _require_xfs_io_command()
 			_notrun "xfs_io $command support is missing"
 		;;
 	"scrub"|"repair")
-		testio=`$XFS_IO_PROG -x -c "$command test 0" $TEST_DIR 2>&1`
+		testio=`$XFS_IO_PROG -x -c "$command probe 0" $TEST_DIR 2>&1`
 		echo $testio | grep -q "Inappropriate ioctl" && \
 			_notrun "xfs_io $command support is missing"
 		;;

common/xfs +33 -7

index dff8454..df7d029 100644
@@ -298,6 +298,29 @@ _require_xfs_db_command()
 		_notrun "xfs_db $command support is missing"
 }
 
+# Does the filesystem mounted from a particular device support scrub?
+_supports_xfs_scrub()
+{
+	mountpoint="$1"
+	device="$2"
+
+	if [ ! -b "$device" ] || [ ! -e "$mountpoint" ]; then
+		echo "Usage: _supports_xfs_scrub mountpoint device"
+		exit 1
+	fi
+
+	test "$FSTYP" = "xfs" || return 1
+	test -x "$XFS_SCRUB_PROG" || return 1
+
+	# Probe for kernel support...
+	$XFS_IO_PROG -x -c "scrub probe 0" "$mountpoint" 2>&1 | grep -q "Inappropriate ioctl" && return 1
+
+	# Scrub can't run on norecovery mounts
+	_fs_options "$device" | grep -q "norecovery" && return 1
+
+	return 0
+}
+
 # run xfs_check and friends on a FS.
 _check_xfs_filesystem()
 {

@@ -330,14 +353,17 @@ _check_xfs_filesystem()
 	type=`_fs_type $device`
 	ok=1
 
-	if [ "$type" = "xfs" ]; then
-		if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
-			"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full
-			if [ $? -ne 0 ]; then
-				_log_err "filesystem on $device failed scrub"
-				ok=0
-			fi
+	# Run online scrub if we can.
+	mntpt="$(_is_mounted $device)"
+	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
+		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full 2>&1
+		if [ $? -ne 0 ]; then
+			_log_err "filesystem on $device failed scrub"
+			ok=0
 		fi
+	fi
+
+	if [ "$type" = "xfs" ]; then
 		# mounted ...
 		mountpoint=`_umount_or_remount_ro $device`
 	fi

tests/generic/453 +1 ~~-10~~

index ff29736..40fae91 100755
@@ -136,10 +136,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \

@@ -147,12 +144,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }

tests/generic/454 +1 -4

index 01279ee..462185a 100755
@@ -132,10 +132,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \

@@ -143,12 +140,6 @@ check_xfs_scrub() {
 		return 1
 	fi

tests/generic/454 check_xfs_scrub() { · line 145

Quoted 10 lines

tests/generic/454 +0 -6

 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-27 20:21 UTC #4 lore raw

PATCH v2 2/6 common/xfs: refactor xfs_scrub presence testing

In reply to Darrick J. Wong

Refactor the existing three xfs_scrub call sites to use the helper to check if it's appropriate to run scrub.

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

---
v2: make sure xfs_io has a scrub/probe command at all...

---
 README            |    6 +++---
 common/rc         |    2 +-
 common/xfs        |   41 ++++++++++++++++++++++++++++++++++-------
 tests/generic/453 |   11 +----------
 tests/generic/454 |   11 +----------
 5 files changed, 40 insertions(+), 31 deletions(-)

README +3 -3

index 4963d28..a9da4f0 100644
@@ -88,9 +88,9 @@ Preparing system for tests:
                run xfs_repair -n to check the filesystem; xfs_repair to rebuild
                metadata indexes; and xfs_repair -n (a third time) to check the
                results of the rebuilding.
-             - set TEST_XFS_SCRUB=1 to have _check_xfs_filesystem run
-               xfs_scrub -vd to scrub the filesystem metadata online before
-               unmounting to run the offline check.
+             - xfs_scrub, if present, will always check the test and scratch
+               filesystems if they are still online at the end of the test.
+               It is no longer necessary to set TEST_XFS_SCRUB.
              - setenv LOGWRITES_DEV to a block device to use for power fail
                testing.

common/rc +1 -1

index 1a4d81e..83aaced 100644
@@ -2091,7 +2091,7 @@ _require_xfs_io_command()
 			_notrun "xfs_io $command support is missing"
 		;;
 	"scrub"|"repair")
-		testio=`$XFS_IO_PROG -x -c "$command test 0" $TEST_DIR 2>&1`
+		testio=`$XFS_IO_PROG -x -c "$command probe 0" $TEST_DIR 2>&1`
 		echo $testio | grep -q "Inappropriate ioctl" && \
 			_notrun "xfs_io $command support is missing"
 		;;

common/xfs +34 -7

54 lines not on this page · raw message

tests/generic/453 +1 ~~-10~~

index ff29736..40fae91 100755
@@ -136,10 +136,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \

@@ -147,12 +144,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }

tests/generic/454 +1 ~~-10~~

index 01279ee..462185a 100755
@@ -132,10 +132,7 @@ echo "Test XFS online scrub, if applicable"
 
 # Only run this on xfs if xfs_scrub is available and has the unicode checker
 check_xfs_scrub() {
-	# Ignore non-XFS fs or no scrub program...
-	if [ "${FSTYP}" != "xfs" ] || [ ! -x "${XFS_SCRUB_PROG}" ]; then
-		return 1
-	fi
+	_supports_xfs_scrub "$SCRATCH_MNT" "$SCRATCH_DEV" || return 1
 
 	# We only care if xfs_scrub has unicode string support...
 	if ! type ldd > /dev/null 2>&1 || \

@@ -143,12 +140,6 @@ check_xfs_scrub() {
 		return 1
 	fi
 
-	# Does the ioctl work?
-	if $XFS_IO_PROG -x -c "scrub probe 0" $SCRATCH_MNT 2>&1 | \
-	   grep -q "Inappropriate ioctl"; then
-		return 1
-	fi
-
 	return 0
 }

Darrick J. Wong

author

2017-10-26 05:51 UTC #5 lore raw

PATCH 3/6 common/xfs: standardize the xfs_scrub output that gets recorded to $seqres.full

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Make the xfs_scrub output that gets recorded to $seqres.full follow the format of xfs_repair checks.

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

---
 common/xfs |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

common/xfs +6 -2

index df7d029..25c2ce9 100644
@@ -356,11 +356,15 @@ _check_xfs_filesystem()
 	# Run online scrub if we can.
 	mntpt="$(_is_mounted $device)"
 	if [ -n "$mntpt" ] && _supports_xfs_scrub "$mntpt" "$device"; then
-		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device >>$seqres.full 2>&1
+		"$XFS_SCRUB_PROG" $scrubflag -v -d -n $device > $tmp.scrub 2>&1
 		if [ $? -ne 0 ]; then
-			_log_err "filesystem on $device failed scrub"
+			_log_err "_check_xfs_filesystem: filesystem on $device failed scrub"
+			echo "*** xfs_scrub $scrubflag -v -d -n output ***" >> $seqres.full
+			cat $tmp.scrub >> $seqres.full
+			echo "*** end xfs_scrub output" >> $serqres.full
 			ok=0
 		fi
+		rm -f $tmp.scrub
 	fi
 
 	if [ "$type" = "xfs" ]; then

Darrick J. Wong

author

2017-10-26 05:52 UTC #6 lore raw

PATCH 4/6 generic/45[34]: force UTF-8 codeset to enable utf-8 namer checks in xfs_scrub

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

The upcoming xfs_scrub tool will have the ability to warn about suspicious UTF-8 normalization collisions. We want generic/45[34] to be able to test this functionality, but to do that we have to forcibly set the codeset to UTF-8 via LC_ALL since the rest of xfstests only uses LC_ALL=C.

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

---
 tests/generic/453 |    2 +-
 tests/generic/454 |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

tests/generic/453 +1 -1

index 40fae91..16589d1 100755
@@ -148,7 +148,7 @@ check_xfs_scrub() {
 }
 
 if check_xfs_scrub; then
-	output="$(${XFS_SCRUB_PROG} -n "${SCRATCH_MNT}" 2>&1 | filter_scrub)"
+	output="$(LC_ALL="C.UTF-8" ${XFS_SCRUB_PROG} -n "${SCRATCH_MNT}" 2>&1 | filter_scrub)"
 	echo "${output}" | grep -q "french_" || echo "No complaints about french e accent?"
 	echo "${output}" | grep -q "chinese_" || echo "No complaints about chinese width-different?"
 	echo "${output}" | grep -q "greek_" || echo "No complaints about greek letter mess?"

tests/generic/454 +1 -1

index 462185a..efac860 100755
@@ -144,7 +144,7 @@ check_xfs_scrub() {
 }
 
 if check_xfs_scrub; then
-	output="$(${XFS_SCRUB_PROG} -n "${SCRATCH_MNT}" 2>&1 | filter_scrub)"
+	output="$(LC_ALL="C.UTF-8" ${XFS_SCRUB_PROG} -n "${SCRATCH_MNT}" 2>&1 | filter_scrub)"
 	echo "${output}" | grep -q "french_" || echo "No complaints about french e accent?"
 	echo "${output}" | grep -q "chinese_" || echo "No complaints about chinese width-different?"
 	echo "${output}" | grep -q "greek_" || echo "No complaints about greek letter mess?"

Darrick J. Wong

author

2017-10-26 05:52 UTC #7 lore raw

PATCH 5/6 misc: add module reloading helpers

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Add some helper functions to require that we can reload a given module, and add a helper to actually do that. Refactor the existing users to use the generics.

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

---
 common/btrfs    |   12 ------------
 common/rc       |   15 +++++++++++++++
 tests/btrfs/124 |    9 +++------
 tests/btrfs/125 |    9 +++------
 4 files changed, 21 insertions(+), 24 deletions(-)

common/btrfs +0 ~~-12~~

index fd762ef..c09206c 100644
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib

common/rc +15 -0

index 83aaced..5375f7b 100644
@@ -3580,6 +3580,21 @@ _require_scratch_feature()
 	esac
 }
 
+# Check that we have a module that can be loaded.
+_require_loadable_module()
+{
+	module="$1"
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+}
+
+# Reload a particular module.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
 
 init_rc

tests/btrfs/124 +3 -6

index 7206094..1b6cb24 100755
@@ -64,10 +64,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
-_test_unmount
-_require_btrfs_loadable
+_require_loadable_module "btrfs"
 
 _scratch_dev_pool_get 2

@@ -102,7 +99,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full

@@ -141,7 +138,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`

tests/btrfs/125 +3 -6

index 91aa8d8..dbb226c 100755
@@ -63,10 +63,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
-_test_unmount
-_require_btrfs_loadable
+_require_loadable_module "btrfs"
 
 _scratch_dev_pool_get 3

@@ -118,7 +115,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1

@@ -154,7 +151,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1

Eryu Guan

2017-10-26 06:43 UTC #8 lore raw

Re: [PATCH 5/6] misc: add module reloading helpers

Quoted 43 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Add some helper functions to require that we can reload a given module, and add a helper to actually do that. Refactor the existing users to use the generics.

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

---
 common/btrfs    |   12 ------------
 common/rc       |   15 +++++++++++++++
 tests/btrfs/124 |    9 +++------
 tests/btrfs/125 |    9 +++------
 4 files changed, 21 insertions(+), 24 deletions(-)

common/btrfs +0 ~~-12~~

index fd762ef..c09206c 100644
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib

common/rc +0 -0

index 83aaced..5375f7b 100644

common/rc _require_scratch_feature() · line 3587

Quoted 8 lines

common/rc +5 -0

@@ -3580,6 +3580,21 @@ _require_scratch_feature()
 	esac
 }
 
+# Check that we have a module that can be loaded.
+_require_loadable_module()
+{
+	module="$1"
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."

I think we should try to unload the given module too to see if we really could unload it, otherwise a later _reload_module() still fails if rootfs is mounted as $module type, even if we already umounted TEST_DEV and SCRATCH_DEV.

Quoted 18 lines

common/rc +10 -0

+}
+
+# Reload a particular module.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
 
 init_rc

tests/btrfs/124 +0 -0

index 7206094..1b6cb24 100755

tests/btrfs/124 rm -f $seqres.full · line 67

Quoted 8 lines

tests/btrfs/124 +1 -4

@@ -64,10 +64,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
-_test_unmount
-_require_btrfs_loadable
+_require_loadable_module "btrfs"

Then keep the above comments and _test_unmount?

Thanks,
Eryu

Quoted 56 lines

tests/btrfs/124 +2 -2

 _scratch_dev_pool_get 2

@@ -102,7 +99,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full

@@ -141,7 +138,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`

tests/btrfs/125 +3 -6

index 91aa8d8..dbb226c 100755
@@ -63,10 +63,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
-_test_unmount
-_require_btrfs_loadable
+_require_loadable_module "btrfs"
 
 _scratch_dev_pool_get 3

@@ -118,7 +115,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1

@@ -154,7 +151,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1

Darrick J. Wong

author

2017-10-27 00:35 UTC #9 lore raw

Re: [PATCH 5/6] misc: add module reloading helpers

Quoted 47 lines · Eryu Guan

Quoted 46 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Add some helper functions to require that we can reload a given module, and add a helper to actually do that. Refactor the existing users to use the generics.

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

---
 common/btrfs    |   12 ------------
 common/rc       |   15 +++++++++++++++
 tests/btrfs/124 |    9 +++------
 tests/btrfs/125 |    9 +++------
 4 files changed, 21 insertions(+), 24 deletions(-)

common/btrfs +0 ~~-12~~

index fd762ef..c09206c 100644
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib

common/rc +0 -0

index 83aaced..5375f7b 100644
@@ -3580,6 +3580,21 @@ _require_scratch_feature()
 	esac
 }

Quoted 10 lines

common/rc _require_scratch_feature() · line 3587
Quoted 5 lines
common/rc +5 -0
+# Check that we have a module that can be loaded.
+_require_loadable_module()
+{
+	module="$1"
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
I think we should try to unload the given module too to see if we really could unload it, otherwise a later _reload_module() still fails if rootfs is mounted as $module type, even if we already umounted TEST_DEV and SCRATCH_DEV.

Yeah, ok. While I'm at it I'll drag all the module helpers out into a separate file too.

--D

Quoted 92 lines

Quoted 18 lines

common/rc +10 -0

+}
+
+# Reload a particular module.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
 
 init_rc

tests/btrfs/124 +0 -0

index 7206094..1b6cb24 100755

tests/btrfs/124 rm -f $seqres.full · line 67

Quoted 8 lines

tests/btrfs/124 +1 -4

@@ -64,10 +64,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
-_test_unmount
-_require_btrfs_loadable
+_require_loadable_module "btrfs"

Then keep the above comments and _test_unmount?

Thanks,
Eryu

Quoted 48 lines

tests/btrfs/124 +2 -2

 _scratch_dev_pool_get 2

@@ -102,7 +99,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full

@@ -141,7 +138,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`

tests/btrfs/125 +2 -5

index 91aa8d8..dbb226c 100755
@@ -63,10 +63,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
-_test_unmount
-_require_btrfs_loadable
+_require_loadable_module "btrfs"
 
 _scratch_dev_pool_get 3

@@ -118,7 +115,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1

tests/btrfs/125 echo "Mount degraded but with other dev" · line 157

Quoted 8 lines

tests/btrfs/125 +1 -1

@@ -154,7 +151,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1

--
To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-27 00:38 UTC #10 lore raw

PATCH v2 5/6 misc: add module reloading helpers

In reply to Darrick J. Wong

Add some helper functions to require that we can reload a given module, and add a helper to actually do that. Refactor the existing users to use the generics.

We need to hoist completely the behaviors of the old btrfs module helper because we need to confirm before starting the test that we actually can remove the module.

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

---
 common/btrfs    |   12 --------
 common/module   |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/overlay  |    1 +
 common/rc       |   11 -------
 tests/btrfs/124 |    9 +++---
 tests/btrfs/125 |    9 +++---
 6 files changed, 92 insertions(+), 33 deletions(-)
 create mode 100644 common/module

common/btrfs +0 ~~-12~~

index fd762ef..c09206c 100644
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib

common/module new file +83 -0

83 lines not on this page · raw message

common/overlay +1 -0

index 79097ba..1da4ab1 100644
@@ -1,6 +1,7 @@
 #
 # overlayfs specific common functions.
 #
+. ./common/module
 
 # helper function to do the actual overlayfs mount operation
 _overlay_mount_dirs()

common/rc +0 ~~-10~~

index 83aaced..c708bfa 100644
@@ -3554,16 +3554,6 @@ _get_fs_sysfs_attr()
 	cat /sys/fs/${FSTYP}/${dname}/${attr}
 }
 
-# Print the value of a filesystem module parameter
-# at /sys/module/$FSTYP/parameters/$PARAM
-#
-# Usage example (FSTYP=overlay):
-#   _get_fs_module_param index
-_get_fs_module_param()
-{
-	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
-}
-
 # Generic test for specific filesystem feature.
 # Currently only implemented to test overlayfs features.
 _require_scratch_feature()

tests/btrfs/124 +4 -5

index 7206094..a648627 100755
@@ -55,6 +55,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full

@@ -64,10 +65,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 2

@@ -102,7 +101,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full

@@ -141,7 +140,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`

tests/btrfs/125 +4 -5

index 91aa8d8..19961a1 100755
@@ -54,6 +54,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full

@@ -63,10 +64,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 3

@@ -118,7 +117,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1

@@ -154,7 +153,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1

Eryu Guan

2017-10-27 04:41 UTC #11 lore raw

Re: [PATCH v2 5/6] misc: add module reloading helpers

Quoted 110 lines · Darrick J. Wong

Add some helper functions to require that we can reload a given module, and add a helper to actually do that. Refactor the existing users to use the generics.

We need to hoist completely the behaviors of the old btrfs module helper because we need to confirm before starting the test that we actually can remove the module.

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

---
 common/btrfs    |   12 --------
 common/module   |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/overlay  |    1 +
 common/rc       |   11 -------
 tests/btrfs/124 |    9 +++---
 tests/btrfs/125 |    9 +++---
 6 files changed, 92 insertions(+), 33 deletions(-)
 create mode 100644 common/module

common/btrfs +0 ~~-12~~

index fd762ef..c09206c 100644
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib

common/module new file +62 -0

new file mode 100644
index 0000000..59cbd8a
@@ -0,0 +1,83 @@
+##/bin/bash
+
+# Routines for messing around with loadable kernel modules
+#
+#-----------------------------------------------------------------------
+#  Copyright (c) 2017 Oracle.  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; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will 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 to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
+#  USA
+#-----------------------------------------------------------------------
+
+# Return the module name for this fs.
+_module_for_fs()
+{
+	echo "$FSTYP"
+}
+
+# Reload a particular module.  This module MUST NOT be the module that
+# underlies the filesystem.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
+
+# Reload the filesystem module.
+_reload_fs_module()
+{
+	module="$1"
+
+	# Unload test fs, try to reload module, remount
+	had_testfs=""
+	had_scratchfs=""
+	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
+	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
+	test -n "$had_testfs" && _test_unmount
+	test -n "$had_scratchfs" && _scratch_unmount
+	_reload_module "$module"
+	test -n "$had_scratchfs" && _scratch_mount 2> /dev/null
+	test -n "$had_testfs" && _test_mount 2> /dev/null
+}
+
+# Check that we have a module that can be loaded.  This module MUST NOT
+# be the module that underlies the filesystem.
+_require_loadable_module()
+{
+	module="$1"
+
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."

common/module line 72

Quoted 10 lines

common/module new file +10 -0

+	_reload_module "$module"
+}
+
+# Check that the module for FSTYP can be loaded.
+_require_loadable_fs_module()
+{
+	module="$1"
+
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+	_reload_fs_module "$module"

_reload_module() or _reload_fs_module() fails the test directly if the module is still in use. IMHO we should _notrun in this case.

Currently xfs/70[345] still fail if rootfs is xfs.

Thanks,
Eryu

Quoted 128 lines

common/module new file +11 -0

+}
+
+# Print the value of a filesystem module parameter
+# at /sys/module/$FSTYP/parameters/$PARAM
+#
+# Usage example (FSTYP=overlay):
+#   _get_fs_module_param index
+_get_fs_module_param()
+{
+	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
+}

common/overlay +1 -0

index 79097ba..1da4ab1 100644
@@ -1,6 +1,7 @@
 #
 # overlayfs specific common functions.
 #
+. ./common/module
 
 # helper function to do the actual overlayfs mount operation
 _overlay_mount_dirs()

common/rc +0 ~~-10~~

index 83aaced..c708bfa 100644
@@ -3554,16 +3554,6 @@ _get_fs_sysfs_attr()
 	cat /sys/fs/${FSTYP}/${dname}/${attr}
 }
 
-# Print the value of a filesystem module parameter
-# at /sys/module/$FSTYP/parameters/$PARAM
-#
-# Usage example (FSTYP=overlay):
-#   _get_fs_module_param index
-_get_fs_module_param()
-{
-	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
-}
-
 # Generic test for specific filesystem feature.
 # Currently only implemented to test overlayfs features.
 _require_scratch_feature()

tests/btrfs/124 +4 -5

index 7206094..a648627 100755
@@ -55,6 +55,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full

@@ -64,10 +65,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 2

@@ -102,7 +101,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full

@@ -141,7 +140,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`

tests/btrfs/125 +4 -5

index 91aa8d8..19961a1 100755
@@ -54,6 +54,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full

@@ -63,10 +64,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 3

@@ -118,7 +117,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1

@@ -154,7 +153,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1

Darrick J. Wong

author

2017-10-27 18:19 UTC #12 lore raw

Re: [PATCH v2 5/6] misc: add module reloading helpers

Quoted 116 lines · Eryu Guan

Quoted 115 lines · Darrick J. Wong

Add some helper functions to require that we can reload a given module, and add a helper to actually do that. Refactor the existing users to use the generics.

We need to hoist completely the behaviors of the old btrfs module helper because we need to confirm before starting the test that we actually can remove the module.

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

---
 common/btrfs    |   12 --------
 common/module   |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/overlay  |    1 +
 common/rc       |   11 -------
 tests/btrfs/124 |    9 +++---
 tests/btrfs/125 |    9 +++---
 6 files changed, 92 insertions(+), 33 deletions(-)
 create mode 100644 common/module

common/btrfs +0 ~~-12~~

index fd762ef..c09206c 100644
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib

common/module new file +67 -0

new file mode 100644
index 0000000..59cbd8a
@@ -0,0 +1,83 @@
+##/bin/bash
+
+# Routines for messing around with loadable kernel modules
+#
+#-----------------------------------------------------------------------
+#  Copyright (c) 2017 Oracle.  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; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will 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 to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
+#  USA
+#-----------------------------------------------------------------------
+
+# Return the module name for this fs.
+_module_for_fs()
+{
+	echo "$FSTYP"
+}
+
+# Reload a particular module.  This module MUST NOT be the module that
+# underlies the filesystem.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
+
+# Reload the filesystem module.
+_reload_fs_module()
+{
+	module="$1"
+
+	# Unload test fs, try to reload module, remount
+	had_testfs=""
+	had_scratchfs=""
+	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
+	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
+	test -n "$had_testfs" && _test_unmount
+	test -n "$had_scratchfs" && _scratch_unmount
+	_reload_module "$module"
+	test -n "$had_scratchfs" && _scratch_mount 2> /dev/null
+	test -n "$had_testfs" && _test_mount 2> /dev/null
+}
+
+# Check that we have a module that can be loaded.  This module MUST NOT
+# be the module that underlies the filesystem.
+_require_loadable_module()
+{
+	module="$1"
+
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+	_reload_module "$module"
+}
+
+# Check that the module for FSTYP can be loaded.
+_require_loadable_fs_module()

Quoted 8 lines

common/module line 72
Quoted 5 lines
common/module new file +5 -0
+{
+	module="$1"
+
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+	_reload_fs_module "$module"
_reload_module() or _reload_fs_module() fails the test directly if the module is still in use. IMHO we should _notrun in this case.

Ok, will fix.

Quoted 1 line

Currently xfs/70[345] still fail if rootfs is xfs.

I don't consider it a good idea to store xfsprogs/xfstests on a $FSTYP filesystem because you're depending on the reliability of the same filesystem whose reliability you're trying to prove.

That said, I suppose that pretty much requires nfsroot or a giant initramfs...

--D

Quoted 135 lines

Thanks,
Eryu

Quoted 120 lines

common/module new file +11 -0

+}
+
+# Print the value of a filesystem module parameter
+# at /sys/module/$FSTYP/parameters/$PARAM
+#
+# Usage example (FSTYP=overlay):
+#   _get_fs_module_param index
+_get_fs_module_param()
+{
+	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
+}

common/overlay +1 -0

index 79097ba..1da4ab1 100644
@@ -1,6 +1,7 @@
 #
 # overlayfs specific common functions.
 #
+. ./common/module
 
 # helper function to do the actual overlayfs mount operation
 _overlay_mount_dirs()

common/rc +0 ~~-10~~

index 83aaced..c708bfa 100644
@@ -3554,16 +3554,6 @@ _get_fs_sysfs_attr()
 	cat /sys/fs/${FSTYP}/${dname}/${attr}
 }
 
-# Print the value of a filesystem module parameter
-# at /sys/module/$FSTYP/parameters/$PARAM
-#
-# Usage example (FSTYP=overlay):
-#   _get_fs_module_param index
-_get_fs_module_param()
-{
-	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
-}
-
 # Generic test for specific filesystem feature.
 # Currently only implemented to test overlayfs features.
 _require_scratch_feature()

tests/btrfs/124 +4 -5

index 7206094..a648627 100755
@@ -55,6 +55,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full

@@ -64,10 +65,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 2

@@ -102,7 +101,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full

@@ -141,7 +140,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`

tests/btrfs/125 +3 -4

index 91aa8d8..19961a1 100755
@@ -54,6 +54,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full

@@ -63,10 +64,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 3

@@ -118,7 +117,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1

tests/btrfs/125 echo "Mount degraded but with other dev" · line 159

Quoted 8 lines

tests/btrfs/125 +1 -1

@@ -154,7 +153,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Eryu Guan

2017-10-28 05:47 UTC #13 lore raw

Re: [PATCH v2 5/6] misc: add module reloading helpers

Quoted 128 lines · Darrick J. Wong

Quoted 116 lines · Eryu Guan

Quoted 115 lines · Darrick J. Wong

Add some helper functions to require that we can reload a given module, and add a helper to actually do that. Refactor the existing users to use the generics.

We need to hoist completely the behaviors of the old btrfs module helper because we need to confirm before starting the test that we actually can remove the module.

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

---
 common/btrfs    |   12 --------
 common/module   |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/overlay  |    1 +
 common/rc       |   11 -------
 tests/btrfs/124 |    9 +++---
 tests/btrfs/125 |    9 +++---
 6 files changed, 92 insertions(+), 33 deletions(-)
 create mode 100644 common/module

common/btrfs +0 ~~-12~~

index fd762ef..c09206c 100644
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib

common/module new file +67 -0

new file mode 100644
index 0000000..59cbd8a
@@ -0,0 +1,83 @@
+##/bin/bash
+
+# Routines for messing around with loadable kernel modules
+#
+#-----------------------------------------------------------------------
+#  Copyright (c) 2017 Oracle.  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; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will 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 to the Free Software
+#  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307
+#  USA
+#-----------------------------------------------------------------------
+
+# Return the module name for this fs.
+_module_for_fs()
+{
+	echo "$FSTYP"
+}
+
+# Reload a particular module.  This module MUST NOT be the module that
+# underlies the filesystem.
+_reload_module()
+{
+	module="$1"
+
+	modprobe -r "${module}" || _fail "${module} unload failed"
+	modprobe "${module}" || _fail "${module} load failed"
+}
+
+# Reload the filesystem module.
+_reload_fs_module()
+{
+	module="$1"
+
+	# Unload test fs, try to reload module, remount
+	had_testfs=""
+	had_scratchfs=""
+	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR && had_testfs="true"
+	_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT && had_scratchfs="true"
+	test -n "$had_testfs" && _test_unmount
+	test -n "$had_scratchfs" && _scratch_unmount
+	_reload_module "$module"
+	test -n "$had_scratchfs" && _scratch_mount 2> /dev/null
+	test -n "$had_testfs" && _test_mount 2> /dev/null
+}
+
+# Check that we have a module that can be loaded.  This module MUST NOT
+# be the module that underlies the filesystem.
+_require_loadable_module()
+{
+	module="$1"
+
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+	_reload_module "$module"
+}
+
+# Check that the module for FSTYP can be loaded.
+_require_loadable_fs_module()

Quoted 8 lines

common/module line 72
Quoted 5 lines
common/module new file +5 -0
+{
+	module="$1"
+
+	modinfo "$module" > /dev/null 2>&1 || _notrun "${module}: must be a module."
+	_reload_fs_module "$module"
_reload_module() or _reload_fs_module() fails the test directly if the module is still in use. IMHO we should _notrun in this case.

Ok, will fix.

Quoted 7 lines

Quoted 1 line
Currently xfs/70[345] still fail if rootfs is xfs.
I don't consider it a good idea to store xfsprogs/xfstests on a $FSTYP filesystem because you're depending on the reliability of the same filesystem whose reliability you're trying to prove.
That said, I suppose that pretty much requires nfsroot or a giant initramfs...

So I have two test vms, one with ext4 rootfs to test xfs, one with xfs rootfs to test ext4, I didn't use btrfs root because RHEL dropped btrfs support :)

BTW, I took patch 1, 4 and 7 for this week's update, the other patches probably will go in next week. Thanks a lot for all the updates!

Eryu

Darrick J. Wong

author

2017-10-27 20:23 UTC #14 lore raw

PATCH v3 5/6 misc: add module reloading helpers

In reply to Darrick J. Wong

Add some helper functions to require that we can reload a given module, and add a helper to actually do that. Refactor the existing users to use the generics.

We need to hoist completely the behaviors of the old btrfs module helper because we need to confirm before starting the test that we actually can remove the module.

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

---
v3: be more judicious in our use of _notrun/_fail

---
 common/btrfs    |   12 -------
 common/module   |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/overlay  |    1 +
 common/rc       |   11 ------
 tests/btrfs/124 |    9 ++---
 tests/btrfs/125 |    9 ++---
 6 files changed, 108 insertions(+), 33 deletions(-)
 create mode 100644 common/module

common/btrfs +0 ~~-12~~

index fd762ef..c09206c 100644
@@ -349,18 +349,6 @@ _btrfs_qgroup_units()
 	$BTRFS_UTIL_PROG qgroup show --help 2>&1 | grep -q -- --raw && echo "--raw"
 }
 
-_require_btrfs_loadable()
-{
-	modprobe -r btrfs || _notrun "btrfs unloadable"
-	modprobe btrfs || _notrun "Can't load btrfs"
-}
-
-_reload_btrfs_ko()
-{
-	modprobe -r btrfs || _fail "btrfs unload failed"
-	modprobe btrfs || _fail "btrfs load failed"
-}
-
 _btrfs_compression_algos()
 {
 	echo zlib

common/module new file +99 -0

99 lines not on this page · raw message

common/overlay +1 -0

index 79097ba..1da4ab1 100644
@@ -1,6 +1,7 @@
 #
 # overlayfs specific common functions.
 #
+. ./common/module
 
 # helper function to do the actual overlayfs mount operation
 _overlay_mount_dirs()

common/rc +0 ~~-11~~

index 83aaced..c708bfa 100644
@@ -3554,16 +3554,6 @@ _get_fs_sysfs_attr()
 	cat /sys/fs/${FSTYP}/${dname}/${attr}
 }
 
-# Print the value of a filesystem module parameter
-# at /sys/module/$FSTYP/parameters/$PARAM
-#
-# Usage example (FSTYP=overlay):
-#   _get_fs_module_param index
-_get_fs_module_param()
-{
-	cat /sys/module/${FSTYP}/parameters/${1} 2>/dev/null
-}
-
 # Generic test for specific filesystem feature.
 # Currently only implemented to test overlayfs features.
 _require_scratch_feature()

@@ -3580,7 +3570,6 @@ _require_scratch_feature()
 	esac
 }
 
-
 init_rc
 
 ################################################################################

tests/btrfs/124 +4 -5

index 7206094..a648627 100755
@@ -55,6 +55,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full

@@ -64,10 +65,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-
-# the mounted test dir prevent btrfs unload, we need to unmount
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 2

@@ -102,7 +101,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full

@@ -141,7 +140,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`

tests/btrfs/125 +4 -5

index 91aa8d8..19961a1 100755
@@ -54,6 +54,7 @@ _cleanup()
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
+. ./common/module
 
 # remove previous $seqres.full before test
 rm -f $seqres.full

@@ -63,10 +64,8 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
-
-# we need btrfs to unload, need test dir unmounted
 _test_unmount
-_require_btrfs_loadable
+_require_loadable_fs_module "btrfs"
 
 _scratch_dev_pool_get 3

@@ -118,7 +117,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1

@@ -154,7 +153,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_btrfs_ko
+_reload_fs_module "btrfs"
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1

Darrick J. Wong

author

2017-10-26 05:52 UTC #15 lore raw

PATCH 6/6 xfs: test that we don't leak inodes and dquots during failed cow recovery

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Add a couple of tests to check that we don't leak inodes or dquots if CoW recovery fails and therefore the mount fails.

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

---
 common/rc         |    1 
 tests/xfs/703     |  112 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/703.out |    9 ++++
 tests/xfs/704     |   90 +++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/704.out |    5 ++
 tests/xfs/705     |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/705.out |    9 ++++
 tests/xfs/group   |    3 +
 8 files changed, 335 insertions(+)
 create mode 100755 tests/xfs/703
 create mode 100644 tests/xfs/703.out
 create mode 100755 tests/xfs/704
 create mode 100644 tests/xfs/704.out
 create mode 100755 tests/xfs/705
 create mode 100644 tests/xfs/705.out

common/rc +1 -0

index 5375f7b..a0a348f 100644
@@ -3324,6 +3324,7 @@ _check_dmesg()
 	     -e "(INFO|ERR): suspicious RCU usage" \
 	     -e "INFO: possible circular locking dependency detected" \
 	     -e "general protection fault:" \
+	     -e "BUG .* remaining" \
 	     $seqres.dmesg
 	if [ $? -eq 0 ]; then
 		_dump_err "_check_dmesg: something found in dmesg (see $seqres.dmesg)"

tests/xfs/703 new file +112 -0

112 lines not on this page · raw message

tests/xfs/703.out new file +9 -0

new file mode 100644
index 0000000..d811880
@@ -0,0 +1,9 @@
+QA output created by 703
+Format and mount
+Create a many-block file
+Inject error
+Defrag the file
+FS should be shut down, touch will fail
+touch: cannot touch 'SCRATCH_MNT/badfs': Input/output error
+Remount to replay log
+See if we leak

tests/xfs/704 new file +90 -0

90 lines not on this page · raw message

tests/xfs/704.out new file +5 -0

new file mode 100644
index 0000000..b237b2d
@@ -0,0 +1,5 @@
+QA output created by 704
+Format and mount
+Create a many-block file
+Remount to check recovery
+See if we leak

tests/xfs/705 new file +106 -0

106 lines not on this page · raw message

tests/xfs/705.out new file +9 -0

new file mode 100644
index 0000000..0bcd9ab
@@ -0,0 +1,9 @@
+QA output created by 705
+Format and mount
+Create a many-block file
+Inject error
+Defrag the file
+FS should be shut down, touch will fail
+touch: cannot touch 'SCRATCH_MNT/badfs': Input/output error
+Remount to replay log
+See if we leak

tests/xfs/group +3 -0

index b439842..e56666b 100644
@@ -431,3 +431,6 @@
 431 auto quick dangerous
 432 auto quick dir metadata
 433 auto quick attr
+703 auto quick clone fsr
+704 auto quick clone
+705 auto quick clone fsr

Darrick J. Wong

author

2017-10-27 00:42 UTC #16 lore raw

PATCH v2 6/6 xfs: test that we don't leak inodes and dquots during failed cow recovery

Add a couple of tests to check that we don't leak inodes or dquots if CoW recovery fails and therefore the mount fails.

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

---
 common/rc         |    1 
 tests/xfs/703     |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/703.out |    9 ++++
 tests/xfs/704     |   89 ++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/704.out |    5 ++
 tests/xfs/705     |  105 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/705.out |    9 ++++
 tests/xfs/group   |    3 +
 8 files changed, 332 insertions(+)
 create mode 100755 tests/xfs/703
 create mode 100644 tests/xfs/703.out
 create mode 100755 tests/xfs/704
 create mode 100644 tests/xfs/704.out
 create mode 100755 tests/xfs/705
 create mode 100644 tests/xfs/705.out

common/rc +1 -0

index c708bfa..7e453e0 100644
@@ -3324,6 +3324,7 @@ _check_dmesg()
 	     -e "(INFO|ERR): suspicious RCU usage" \
 	     -e "INFO: possible circular locking dependency detected" \
 	     -e "general protection fault:" \
+	     -e "BUG .* remaining" \
 	     $seqres.dmesg
 	if [ $? -eq 0 ]; then
 		_dump_err "_check_dmesg: something found in dmesg (see $seqres.dmesg)"

tests/xfs/703 new file +111 -0

111 lines not on this page · raw message

tests/xfs/703.out new file +9 -0

new file mode 100644
index 0000000..d811880
@@ -0,0 +1,9 @@
+QA output created by 703
+Format and mount
+Create a many-block file
+Inject error
+Defrag the file
+FS should be shut down, touch will fail
+touch: cannot touch 'SCRATCH_MNT/badfs': Input/output error
+Remount to replay log
+See if we leak

tests/xfs/704 new file +89 -0

89 lines not on this page · raw message

tests/xfs/704.out new file +5 -0

new file mode 100644
index 0000000..b237b2d
@@ -0,0 +1,5 @@
+QA output created by 704
+Format and mount
+Create a many-block file
+Remount to check recovery
+See if we leak

tests/xfs/705 new file +105 -0

105 lines not on this page · raw message

tests/xfs/705.out new file +9 -0

new file mode 100644
index 0000000..0bcd9ab
@@ -0,0 +1,9 @@
+QA output created by 705
+Format and mount
+Create a many-block file
+Inject error
+Defrag the file
+FS should be shut down, touch will fail
+touch: cannot touch 'SCRATCH_MNT/badfs': Input/output error
+Remount to replay log
+See if we leak

tests/xfs/group +3 -0

index b439842..e56666b 100644
@@ -431,3 +431,6 @@
 431 auto quick dangerous
 432 auto quick dir metadata
 433 auto quick attr
+703 auto quick clone fsr
+704 auto quick clone
+705 auto quick clone fsr

Darrick J. Wong

author

2017-10-27 00:43 UTC #17 lore raw

[PATCH 7/6] common/fuzzy: online re-scrub should not preen

In reply to Darrick J. Wong

When we're doing the second online scrub (to figure out if the repair did any good) we shouldn't let that second scrub preen the filesystem in any way. If scrub finds things it can't/won't preen that turns into a nonzero return code which gets reported (incorrectly) as a failure.

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

---
 common/fuzzy |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

common/fuzzy +1 -1

index 7a4b03a..87f7acc 100644
@@ -275,7 +275,7 @@ __scratch_xfs_fuzz_field_test() {
 		# which scrub doesn't know how to fix.
 		echo "++ Online scrub"
 		if [ "$1" != "sb 0" ]; then
-			_scratch_scrub -e continue 2>&1
+			_scratch_scrub -n -e continue 2>&1
 			res=$?
 			test $res -ne 0 && \
 				(>&2 echo "online re-scrub ($res) with ${field} = ${fuzzverb}.")

Darrick J. Wong

author

2017-10-27 00:44 UTC #18 lore raw

[PATCH 8/6] xfs/333: fix errors with new inode pointer verifiers

In reply to Darrick J. Wong

Fix test failures with new inode pointer verifiers... and also make sure that the running xfs actually supports realtime rmap. (This should stop the current crop of weird test failures since nobody has rtrmap yet anyway...)

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

---
 tests/xfs/333     |    4 +++-
 tests/xfs/333.out |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

tests/xfs/333 +3 -1

index f7f233d..af52373 100755
@@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
 
 echo "Format and mount"
 _scratch_mkfs > "$seqres.full" 2>&1
+rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
+test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
 _scratch_mount
 
 echo "Create some files"

@@ -62,7 +64,7 @@ _scratch_unmount
 
 echo "Corrupt fs"
 _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
-_scratch_mount
+_scratch_mount 2>&1 | _filter_scratch
 
 echo "Check files"
 md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch

tests/xfs/333.out +3 -1

index bee9bbc..9f81ba4 100644
@@ -2,8 +2,10 @@ QA output created by 333
 Format and mount
 Create some files
 Corrupt fs
+mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
 Check files
-8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
+md5sum: SCRATCH_MNT/f1: No such file or directory
 Try to create more files
 Repair fs
+umount: SCRATCH_DEV: not mounted
 Try to create more files (again)

Eryu Guan

2017-10-27 06:04 UTC #19 lore raw

Re: [PATCH 8/6] xfs/333: fix errors with new inode pointer verifiers

Quoted 38 lines · Darrick J. Wong

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

---
 tests/xfs/333     |    4 +++-
 tests/xfs/333.out |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

tests/xfs/333 +3 -1

index f7f233d..af52373 100755
@@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
 
 echo "Format and mount"
 _scratch_mkfs > "$seqres.full" 2>&1
+rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
+test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
 _scratch_mount
 
 echo "Create some files"

@@ -62,7 +64,7 @@ _scratch_unmount
 
 echo "Corrupt fs"
 _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
-_scratch_mount
+_scratch_mount 2>&1 | _filter_scratch
 
 echo "Check files"
 md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch

tests/xfs/333.out +0 -0

index bee9bbc..9f81ba4 100644

tests/xfs/333.out QA output created by 333 · line 5

Quoted 4 lines

tests/xfs/333.out +1 -0

@@ -2,8 +2,10 @@ QA output created by 333
 Format and mount
 Create some files
 Corrupt fs
+mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning

So kernel refuses to mount the corrupted fs now, then all the subsequent test steps before _repair_scratch_fs seem meaningless now, e.g. check md5sum of the file, and since SCRATCH_DEV is not mounted, "Trying to create more files" is actually writing to rootfs. All these don't seem right and look confusing to me..

Thanks,
Eryu

Quoted 7 lines

tests/xfs/333.out +2 -1

 Check files
-8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
+md5sum: SCRATCH_MNT/f1: No such file or directory
 Try to create more files
 Repair fs
+umount: SCRATCH_DEV: not mounted
 Try to create more files (again)

Darrick J. Wong

author

2017-10-27 18:21 UTC #20 lore raw

Re: [PATCH 8/6] xfs/333: fix errors with new inode pointer verifiers

Quoted 41 lines · Eryu Guan

Quoted 40 lines · Darrick J. Wong

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

---
 tests/xfs/333     |    4 +++-
 tests/xfs/333.out |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

tests/xfs/333 +3 -1

index f7f233d..af52373 100755
@@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
 
 echo "Format and mount"
 _scratch_mkfs > "$seqres.full" 2>&1
+rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
+test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
 _scratch_mount
 
 echo "Create some files"

@@ -62,7 +64,7 @@ _scratch_unmount
 
 echo "Corrupt fs"
 _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
-_scratch_mount
+_scratch_mount 2>&1 | _filter_scratch
 
 echo "Check files"
 md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch

tests/xfs/333.out +0 -0

index bee9bbc..9f81ba4 100644
@@ -2,8 +2,10 @@ QA output created by 333
 Format and mount
 Create some files

Quoted 8 lines

tests/xfs/333.out QA output created by 333 · line 5
Quoted 2 lines
tests/xfs/333.out +1 -0
 Corrupt fs
+mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
So kernel refuses to mount the corrupted fs now, then all the subsequent test steps before _repair_scratch_fs seem meaningless now, e.g. check md5sum of the file, and since SCRATCH_DEV is not mounted, "Trying to create more files" is actually writing to rootfs. All these don't seem right and look confusing to me..

Ok, yeah, the rest of the test can get deleted now.

--D

Quoted 14 lines

Thanks,
Eryu
tests/xfs/333.out QA output created by 333 · line 11
Quoted 7 lines
tests/xfs/333.out +2 -1
 Check files
-8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
+md5sum: SCRATCH_MNT/f1: No such file or directory
 Try to create more files
 Repair fs
+umount: SCRATCH_DEV: not mounted
 Try to create more files (again)
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-27 20:24 UTC #21 lore raw

[PATCH v2 8/6] xfs/333: fix errors with new inode pointer verifiers

In reply to Darrick J. Wong

Fix test failures with new inode pointer verifiers... and also make sure that the running xfs actually supports realtime rmap.

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

---
v2: remove the rest of the test after the mount failure

---
 tests/xfs/333     |   18 ++++--------------
 tests/xfs/333.out |    7 ++-----
 2 files changed, 6 insertions(+), 19 deletions(-)

tests/xfs/333 +4 ~~-14~~

index f7f233d..bf0c811 100755
@@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
 
 echo "Format and mount"
 _scratch_mkfs > "$seqres.full" 2>&1
+rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
+test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
 _scratch_mount
 
 echo "Create some files"

@@ -62,21 +64,9 @@ _scratch_unmount
 
 echo "Corrupt fs"
 _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
-_scratch_mount
-
-echo "Check files"
-md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch
-
-echo "Try to create more files"
-$XFS_IO_PROG -f -c "pwrite -S 0x68 0 9999" $SCRATCH_MNT/f3 >> $seqres.full 2>&1
+_scratch_mount 2>&1 | _filter_scratch
 
-echo "Repair fs"
-_scratch_unmount 2>&1 | _filter_scratch
-_repair_scratch_fs >> $seqres.full 2>&1
-
-echo "Try to create more files (again)"
-_scratch_mount
-$XFS_IO_PROG -f -c "pwrite -S 0x68 0 9999" $SCRATCH_MNT/f4 >> $seqres.full
+echo "Test done, mount should have failed"
 
 # success, all done
 status=0

tests/xfs/333.out +2 -5

index bee9bbc..f7518f4 100644
@@ -2,8 +2,5 @@ QA output created by 333
 Format and mount
 Create some files
 Corrupt fs
-Check files
-8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
-Try to create more files
-Repair fs
-Try to create more files (again)
+mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
+Test done, mount should have failed

Darrick J. Wong

author

2017-11-01 21:13 UTC #22 lore raw

Re: [PATCH v2 8/6] xfs/333: fix errors with new inode pointer verifiers

Quoted 35 lines · Darrick J. Wong

Fix test failures with new inode pointer verifiers... and also make sure that the running xfs actually supports realtime rmap.

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

---
v2: remove the rest of the test after the mount failure

---
 tests/xfs/333     |   18 ++++--------------
 tests/xfs/333.out |    7 ++-----
 2 files changed, 6 insertions(+), 19 deletions(-)

tests/xfs/333 +2 -7

index f7f233d..bf0c811 100755
@@ -51,6 +51,8 @@ unset SCRATCH_RTDEV
 
 echo "Format and mount"
 _scratch_mkfs > "$seqres.full" 2>&1
+rrmapino="$(_scratch_xfs_db -c 'sb 0' -c 'p rrmapino' 2>&1)"
+test "${rrmapino}" = "field rrmapino not found" && _notrun "realtime rmapbt not supported"
 _scratch_mount
 
 echo "Create some files"

@@ -62,21 +64,9 @@ _scratch_unmount
 
 echo "Corrupt fs"
 _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
-_scratch_mount
-
-echo "Check files"
-md5sum $SCRATCH_MNT/f1 2>&1 | _filter_scratch
-
-echo "Try to create more files"
-$XFS_IO_PROG -f -c "pwrite -S 0x68 0 9999" $SCRATCH_MNT/f3 >> $seqres.full 2>&1

tests/xfs/333 _scratch_unmount · line 69

Quoted 10 lines

tests/xfs/333 +2 -7

+_scratch_mount 2>&1 | _filter_scratch
 
-echo "Repair fs"
-_scratch_unmount 2>&1 | _filter_scratch
-_repair_scratch_fs >> $seqres.full 2>&1
-
-echo "Try to create more files (again)"
-_scratch_mount
-$XFS_IO_PROG -f -c "pwrite -S 0x68 0 9999" $SCRATCH_MNT/f4 >> $seqres.full
+echo "Test done, mount should have failed"

Hmm, we ought to repair the filesystem to make sure that repair actually knows how to deal with rrmapino problems.

(I already fixed this; will be sending a rollup of this week's patches shortly.)

--D

Quoted 22 lines

tests/xfs/333 +0 -0
 # success, all done
 status=0
tests/xfs/333.out +2 -6
index bee9bbc..f7518f4 100644
@@ -2,8 +2,5 @@ QA output created by 333
 Format and mount
 Create some files
 Corrupt fs
-Check files
-8f27047948255cb84872e2dd7c0bc56d  SCRATCH_MNT/f1
-Try to create more files
-Repair fs
-Try to create more files (again)
+mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
+Test done, mount should have failed
--
To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-27 00:44 UTC #23 lore raw

[PATCH 9/6] generic/459: fix test running errors

In reply to Darrick J. Wong

If the DISCARD of the thin device somehow fails with this message:

device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.

Then we can end up with arbitrary gunk in the thin device. This causes mkfs to fail because it's afraid to format the device. Don't be afraid, just zap it. FWIW mkfs.xfs thinks that the thinp device has an xfs external log because sometimes the thinp device just happen to be backed by the log of the previous test's scratch fs.

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

---
 tests/generic/459 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tests/generic/459 +1 -1

index 7c10c2a..3dd3681 100755
@@ -92,7 +92,7 @@ $LVM_PROG lvcreate  --virtualsize $virtsize \
 		    -T $vgname/$poolname \
 		    -n $lvname >>$seqres.full 2>&1
 
-_mkfs_dev /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
+_mkfs_dev -f /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
 
 
 # Running the test over the original volume doesn't reproduce the problem

Eryu Guan

2017-10-27 04:42 UTC #24 lore raw

Re: [PATCH 9/6] generic/459: fix test running errors

Quoted 20 lines · Darrick J. Wong

If the DISCARD of the thin device somehow fails with this message:
device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.
Then we can end up with arbitrary gunk in the thin device. This causes mkfs to fail because it's afraid to format the device. Don't be afraid, just zap it. FWIW mkfs.xfs thinks that the thinp device has an xfs external log because sometimes the thinp device just happen to be backed by the log of the previous test's scratch fs.
Signed-off-by Darrick J. Wong <[email protected]>
---
 tests/generic/459 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tests/generic/459 +0 -0
index 7c10c2a..3dd3681 100755

tests/generic/459 $LVM_PROG lvcreate --virtualsize $virtsize \ · line 95

Quoted 5 lines

tests/generic/459 +1 -1

@@ -92,7 +92,7 @@ $LVM_PROG lvcreate  --virtualsize $virtsize \
 		    -T $vgname/$poolname \
 		    -n $lvname >>$seqres.full 2>&1
 
-_mkfs_dev /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
+_mkfs_dev -f /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1

I think we should fix it in _mkfs_dev, add a new switch entry for xfs.

Thanks,
Eryu

Quoted 3 lines

tests/generic/459 +0 -0

 # Running the test over the original volume doesn't reproduce the problem

Darrick J. Wong

author

2017-10-27 18:22 UTC #25 lore raw

Re: [PATCH 9/6] generic/459: fix test running errors

Quoted 24 lines · Eryu Guan

Quoted 23 lines · Darrick J. Wong
If the DISCARD of the thin device somehow fails with this message:
device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.
Then we can end up with arbitrary gunk in the thin device. This causes mkfs to fail because it's afraid to format the device. Don't be afraid, just zap it. FWIW mkfs.xfs thinks that the thinp device has an xfs external log because sometimes the thinp device just happen to be backed by the log of the previous test's scratch fs.
Signed-off-by Darrick J. Wong <[email protected]>
---
 tests/generic/459 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tests/generic/459 +0 -0
index 7c10c2a..3dd3681 100755
@@ -92,7 +92,7 @@ $LVM_PROG lvcreate  --virtualsize $virtsize \
 		    -T $vgname/$poolname \
 		    -n $lvname >>$seqres.full 2>&1
 

Quoted 4 lines

tests/generic/459 $LVM_PROG lvcreate --virtualsize $virtsize \ · line 95
Quoted 2 lines
tests/generic/459 +1 -1
-_mkfs_dev /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
+_mkfs_dev -f /dev/mapper/$vgname-$lvname >>$seqres.full 2>&1
I think we should fix it in _mkfs_dev, add a new switch entry for xfs.

Will do.

--D

Quoted 10 lines

Thanks,
Eryu
tests/generic/459 $LVM_PROG lvcreate --virtualsize $virtsize \ · line 98
Quoted 3 lines
tests/generic/459 +0 -0
 # Running the test over the original volume doesn't reproduce the problem
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-27 20:25 UTC #26 lore raw

[PATCH v2 9/6] generic/459: fix test running errors

In reply to Darrick J. Wong

If the DISCARD of the thin device somehow fails with this message:

device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.

Fix this by making the _mkfs_dev helper always format the device, per Eryu Guan's suggestion.

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

---
 common/rc |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

common/rc +4 -1

index 7e453e0..a985f4d 100644
@@ -642,7 +642,10 @@ _mkfs_dev()
 	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
 	;;
-
+    xfs)
+	yes | $MKFS_PROG -f -t $FSTYP -- $MKFS_OPTIONS $* \
+		2>$tmp.mkfserr 1>$tmp.mkfsstd
+	;;
     *)
 	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd

Darrick J. Wong

author

2017-10-28 17:08 UTC #27 lore raw

Re: [PATCH v2 9/6] generic/459: fix test running errors

Quoted 23 lines · Darrick J. Wong

If the DISCARD of the thin device somehow fails with this message:
device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.
Then we can end up with arbitrary gunk in the thin device. This causes mkfs to fail because it's afraid to format the device. Don't be afraid, just zap it. FWIW mkfs.xfs thinks that the thinp device has an xfs external log because sometimes the thinp device just happen to be backed by the log of the previous test's scratch fs.
Fix this by making the _mkfs_dev helper always format the device, per Eryu Guan's suggestion.
Signed-off-by Darrick J. Wong <[email protected]>
---
 common/rc |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
common/rc +0 -0
index 7e453e0..a985f4d 100644

common/rc _mkfs_dev() · line 646

Quoted 6 lines

common/rc +2 -1

@@ -642,7 +642,10 @@ _mkfs_dev()
 	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
 	;;
-
+    xfs)
+	yes | $MKFS_PROG -f -t $FSTYP -- $MKFS_OPTIONS $* \

The '-f' should be after the '--', will resend patch.

--D

Quoted 9 lines

common/rc +2 -1
+		2>$tmp.mkfserr 1>$tmp.mkfsstd
+	;;
     *)
 	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-28 17:09 UTC #28 lore raw

[PATCH v3 9/6] generic/459: fix test running errors

In reply to Darrick J. Wong

If the DISCARD of the thin device somehow fails with this message:

device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.

Fix this by making the _mkfs_dev helper always format the device, per Eryu Guan's suggestion.

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

---
 common/rc |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

common/rc +4 -1

index 7e453e0..c441199 100644
@@ -642,7 +642,10 @@ _mkfs_dev()
 	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
 	;;
-
+    xfs)
+	yes | $MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* \
+		2>$tmp.mkfserr 1>$tmp.mkfsstd
+	;;
     *)
 	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd

Eryu Guan

2017-10-30 05:01 UTC #29 lore raw

Re: [PATCH v3 9/6] generic/459: fix test running errors

Quoted 23 lines · Darrick J. Wong

If the DISCARD of the thin device somehow fails with this message:
device-mapper: thin: Data device (dm-1) discard unsupported: Disabling discard passdown.
Then we can end up with arbitrary gunk in the thin device. This causes mkfs to fail because it's afraid to format the device. Don't be afraid, just zap it. FWIW mkfs.xfs thinks that the thinp device has an xfs external log because sometimes the thinp device just happen to be backed by the log of the previous test's scratch fs.
Fix this by making the _mkfs_dev helper always format the device, per Eryu Guan's suggestion.
Signed-off-by Darrick J. Wong <[email protected]>
---
 common/rc |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
common/rc +0 -0
index 7e453e0..c441199 100644

common/rc _mkfs_dev() · line 648

Quoted 8 lines

common/rc +4 -1

@@ -642,7 +642,10 @@ _mkfs_dev()
 	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
 	;;
-
+    xfs)
+	yes | $MKFS_PROG -t $FSTYP -- -f $MKFS_OPTIONS $* \
+		2>$tmp.mkfserr 1>$tmp.mkfsstd
+	;;

I think the 'yes' pipe can be omitted, I'll fix that on commit.

Thanks,
Eryu

Quoted 3 lines

common/rc +0 -0

     *)
 	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* \
 		2>$tmp.mkfserr 1>$tmp.mkfsstd

Darrick J. Wong

author

2017-10-27 20:25 UTC #30 lore raw

[PATCH 10/6] common/xfs: remove inode-paths cruft

In reply to Darrick J. Wong

Remove the inode-paths check from _check_xfs_test_fs because we don't support inode paths, xfsprogs doesn't have a xfs_{check,repair}_ipaths tool, and it's broken anyway because we ignore _check_xfs_filesystem (which tells whether or not the filesystem is even still mounted).

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

---
 common/xfs |    7 -------
 1 file changed, 7 deletions(-)

common/xfs +0 -7

index 5643adf..79efb78 100644
@@ -471,13 +471,6 @@ _check_xfs_test_fs()
 		TEST_RT="$TEST_RTDEV"
 
 	_check_xfs_filesystem $TEST_DEV $TEST_LOG $TEST_RT
-
-	# check for ipath consistency
-	if $XFS_GROWFS_PROG -n $TEST_DIR | grep -q 'inode-paths=1'; then
-		# errors go to stderr
-		xfs_check_ipaths $TEST_DIR >/dev/null
-		xfs_repair_ipaths -n $TEST_DIR >/dev/null
-	fi
 }
 
 _require_xfs_test_rmapbt()

Eryu Guan

2017-10-30 05:00 UTC #31 lore raw

Re: [PATCH 10/6] common/xfs: remove inode-paths cruft

Quoted 15 lines · Darrick J. Wong

Remove the inode-paths check from _check_xfs_test_fs because we don't support inode paths, xfsprogs doesn't have a xfs_{check,repair}_ipaths tool, and it's broken anyway because we ignore _check_xfs_filesystem (which tells whether or not the filesystem is even still mounted).
Signed-off-by Darrick J. Wong <[email protected]>
---
 common/xfs |    7 -------
 1 file changed, 7 deletions(-)
common/xfs +0 -0
index 5643adf..79efb78 100644

common/xfs _check_xfs_test_fs() · line 480

Quoted 10 lines

common/xfs +0 -7

@@ -471,13 +471,6 @@ _check_xfs_test_fs()
 		TEST_RT="$TEST_RTDEV"
 
 	_check_xfs_filesystem $TEST_DEV $TEST_LOG $TEST_RT
-
-	# check for ipath consistency
-	if $XFS_GROWFS_PROG -n $TEST_DIR | grep -q 'inode-paths=1'; then
-		# errors go to stderr
-		xfs_check_ipaths $TEST_DIR >/dev/null
-		xfs_repair_ipaths -n $TEST_DIR >/dev/null
-	fi

I added a "return $?" here to report _check_xfs_filesystem result to the caller, otherwise test was reported as a PASS even _check_xfs_filesystem found something and printed out the error messages.

Thanks,
Eryu

Quoted 3 lines

common/xfs +0 -0

 }
 
 _require_xfs_test_rmapbt()