[PATCHSET v2] fstests: random fixes for v2024.11.17

[PATCHSET v2] fstests: random fixes for v2024.11.17

[Darrick J. Wong]

Hi all,

Here's the usual odd fixes for fstests.  Most of these are cleanups and
bug fixes that have been aging in my djwong-wtf branch forever.  Now
with more cleanups to the logwrites code and better loop control for
251, as discussed on the v1 patchset.

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

With a bit of luck, this should all go splendidly.
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
---
Commits in this patchset:
 * generic/757: fix various bugs in this test
 * generic/757: convert to thinp
 * logwrites: warn if we don't think read after discard returns zeroes
 * logwrites: use BLKZEROOUT if it's available
 * logwrites: only use BLKDISCARD if we know discard zeroes data
 * xfs/113: fix failure to corrupt the entire directory
 * xfs/508: fix test for 64k blocksize
 * common/rc: capture dmesg when oom kills happen
 * generic/562: handle ENOSPC while cloning gracefully
 * xfs/163: skip test if we can't shrink due to enospc issues
 * xfs/009: allow logically contiguous preallocations
 * generic/251: use sentinel files to kill the fstrim loop
 * generic/251: constrain runtime via time/load/soak factors
 * generic/251: don't copy the fsstress source code
 * common/rc: _scratch_mkfs_sized supports extra arguments
 * xfs/157: do not drop necessary mkfs options
 * generic/366: fix directio requirements checking
---
 common/dmlogwrites          |   39 ++++++++++++++++++++++++++++++++++++++-
 common/rc                   |   35 +++++++++++++++++++----------------
 src/log-writes/log-writes.c |   10 ++++++++++
 src/log-writes/log-writes.h |    1 +
 src/log-writes/replay-log.c |    8 ++++++++
 tests/generic/251           |   42 +++++++++++++++++++++---------------------
 tests/generic/366           |    2 +-
 tests/generic/562           |   10 ++++++++--
 tests/generic/757           |   26 ++++++++++++++++++++------
 tests/xfs/009               |   29 ++++++++++++++++++++++++++++-
 tests/xfs/113               |   33 +++++++++++++++++++++++++++------
 tests/xfs/157               |    3 +--
 tests/xfs/163               |    9 ++++++++-
 tests/xfs/508               |    4 ++--
 14 files changed, 192 insertions(+), 59 deletions(-)

[PATCH 01/17] generic/757: fix various bugs in this test

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Fix this test so the check doesn't fail on XFS, and restrict runtime to
100 loops because otherwise this test takes many hours.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 tests/generic/757 |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


diff --git a/tests/generic/757 b/tests/generic/757
index 0ff5a8ac00182b..37cf49e6bc7fd9 100755
--- a/tests/generic/757
+++ b/tests/generic/757
@@ -63,9 +63,14 @@ prev=$(_log_writes_mark_to_entry_number mkfs)
 cur=$(_log_writes_find_next_fua $prev)
 [ -z "$cur" ] && _fail "failed to locate next FUA write"
 
-while [ ! -z "$cur" ]; do
+while _soak_loop_running $((100 * TIME_FACTOR)); do
 	_log_writes_replay_log_range $cur $SCRATCH_DEV >> $seqres.full
 
+	# xfs_repair won't run if the log is dirty
+	if [ $FSTYP = "xfs" ]; then
+		_scratch_mount
+		_scratch_unmount
+	fi
 	_check_scratch_fs
 
 	prev=$cur

[PATCH 02/17] generic/757: convert to thinp

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Convert this test to use dm-thinp so that discards always zero the data.
This prevents weird replay problems if the scratch device doesn't
guarantee that read after discard returns zeroes.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/generic/757 |   23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)


diff --git a/tests/generic/757 b/tests/generic/757
index 37cf49e6bc7fd9..6c13c6af41c57c 100755
--- a/tests/generic/757
+++ b/tests/generic/757
@@ -8,12 +8,13 @@
 # This can be seen on subpage FSes on Linux 6.4.
 #
 . ./common/preamble
-_begin_fstest auto quick metadata log recoveryloop aio
+_begin_fstest auto quick metadata log recoveryloop aio thin
 
 _cleanup()
 {
 	cd /
 	_log_writes_cleanup &> /dev/null
+	_dmthin_cleanup
 	rm -f $tmp.*
 }
 
@@ -23,11 +24,14 @@ _cleanup()
 
 fio_config=$tmp.fio
 
+. ./common/dmthin
 . ./common/dmlogwrites
 
-_require_scratch
+# Use thin device as replay device, which requires $SCRATCH_DEV
+_require_scratch_nocheck
 _require_aiodio
 _require_log_writes
+_require_dm_target thin-pool
 
 cat >$fio_config <<EOF
 [global]
@@ -47,7 +51,13 @@ _require_fio $fio_config
 
 cat $fio_config >> $seqres.full
 
-_log_writes_init $SCRATCH_DEV
+# Use a thin device to provide deterministic discard behavior. Discards are used
+# by the log replay tool for fast zeroing to prevent out-of-order replay issues.
+_test_unmount
+sectors=$(blockdev --getsz $SCRATCH_DEV)
+sectors=$((sectors * 90 / 100))
+_dmthin_init $sectors $sectors
+_log_writes_init $DMTHIN_VOL_DEV
 _log_writes_mkfs >> $seqres.full 2>&1
 _log_writes_mark mkfs
 
@@ -64,14 +74,13 @@ cur=$(_log_writes_find_next_fua $prev)
 [ -z "$cur" ] && _fail "failed to locate next FUA write"
 
 while _soak_loop_running $((100 * TIME_FACTOR)); do
-	_log_writes_replay_log_range $cur $SCRATCH_DEV >> $seqres.full
+	_log_writes_replay_log_range $cur $DMTHIN_VOL_DEV >> $seqres.full
 
 	# xfs_repair won't run if the log is dirty
 	if [ $FSTYP = "xfs" ]; then
-		_scratch_mount
-		_scratch_unmount
+		_dmthin_mount
 	fi
-	_check_scratch_fs
+	_dmthin_check_fs
 
 	prev=$cur
 	cur=$(_log_writes_find_next_fua $(($cur + 1)))

[PATCH 03/17] logwrites: warn if we don't think read after discard returns zeroes

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

The logwrites replay program expects that it can issue a DISCARD against
the block device passed to _log_writes_init and that will cause all
subsequent reads to return zeroes.  This is required for correct log
recovery on filesystems such as XFS that skip recovering buffers if
newer ones are found on disk.

Unfortunately, there's no way to discover if a device's discard
implementation actually guarantees zeroes.  There used to be a sysfs
knob keyed to an allowlist, but it is now hardwired to return 0.  So
either we need a magic device that does discard-and-zero, or we need to
do the zeroing ourselves.  The logwrites program does its own zeroing if
there is no discard support, and some tests do their own zeroing.

The only devices we know to work reliably are the software defined ones
that are provided by the kernel itself -- which means dm-thinp.  Warn if
we have a device that supports discard that isn't thinp and the test
fails.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 common/dmlogwrites |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)


diff --git a/common/dmlogwrites b/common/dmlogwrites
index c1c85de9dd43ac..24a8a25ace277f 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -59,6 +59,35 @@ _require_log_writes_dax_mountopt()
 	fi
 }
 
+_log_writes_check_bdev()
+{
+	local sysfs="/sys/block/$(_short_dev $1)"
+
+	# Some filesystems (e.g. XFS) optimize log recovery by assuming that
+	# they can elide replay of metadata blocks if the block has a higher
+	# log serial number than the transaction being recovered.  This is a
+	# problem if the filesystem log contents can go back in time, which is
+	# what the logwrites replay program does.
+	#
+	# The logwrites replay program begins by erasing the block device's
+	# contents.  This can be done very quickly with DISCARD provided the
+	# device guarantees that all reads after a DISCARD return zeroes, or
+	# very slowly by writing zeroes to the device.  Fast is preferable, but
+	# there's no longer any way to detect that DISCARD actually unmaps
+	# zeroes, so warn the user about this requirement if the test happens
+	# to fail.
+
+	# No discard support means the logwrites will do its own zeroing
+	test "$(cat "$sysfs/queue/discard_max_bytes")" -eq 0 && return
+
+	# dm-thinp guarantees that reads after discards return zeroes
+	dmsetup status "$blkdev" 2>/dev/null | grep -q '^0.* thin ' && return
+
+	echo "HINT: $blkdev doesn't guarantee that reads after DISCARD will return zeroes" >> $seqres.hints
+	echo "      This is required for correct journal replay on some filesystems (e.g. xfs)" >> $seqres.hints
+	echo >> $seqres.hints
+}
+
 # Set up a dm-log-writes device
 #
 # blkdev: the specified target device
@@ -84,6 +113,8 @@ _log_writes_init()
 	LOGWRITES_NAME=logwrites-test
 	LOGWRITES_DMDEV=/dev/mapper/$LOGWRITES_NAME
 	LOGWRITES_TABLE="0 $BLK_DEV_SIZE log-writes $blkdev $LOGWRITES_DEV"
+
+	_log_writes_check_bdev "$blkdev"
 	_dmsetup_create $LOGWRITES_NAME --table "$LOGWRITES_TABLE" || \
 		_fail "failed to create log-writes device"
 }

[PATCH 04/17] logwrites: use BLKZEROOUT if it's available

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Use the BLKZEROOUT ioctl instead of writing zeroed buffers if the kernel
supports it.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 src/log-writes/log-writes.c |   10 ++++++++++
 src/log-writes/log-writes.h |    1 +
 2 files changed, 11 insertions(+)


diff --git a/src/log-writes/log-writes.c b/src/log-writes/log-writes.c
index aa53473974d9e8..8f94ae5629e085 100644
--- a/src/log-writes/log-writes.c
+++ b/src/log-writes/log-writes.c
@@ -42,6 +42,7 @@ static int discard_range(struct log *log, u64 start, u64 len)
 
 static int zero_range(struct log *log, u64 start, u64 len)
 {
+	u64 range[2] = { start, len };
 	u64 bufsize = len;
 	ssize_t ret;
 	char *buf = NULL;
@@ -54,6 +55,15 @@ static int zero_range(struct log *log, u64 start, u64 len)
 		return 0;
 	}
 
+	if (!(log->flags & LOG_ZEROOUT_NOT_SUPP)) {
+		if (ioctl(log->replayfd, BLKZEROOUT, &range) < 0) {
+			if (log_writes_verbose)
+				printf(
+ "replay device doesn't support zeroout, switching to writing zeros\n");
+			log->flags |= LOG_ZEROOUT_NOT_SUPP;
+		}
+	}
+
 	while (!buf) {
 		buf = malloc(bufsize);
 		if (!buf)
diff --git a/src/log-writes/log-writes.h b/src/log-writes/log-writes.h
index b9f571ac3b2384..f659931634e64a 100644
--- a/src/log-writes/log-writes.h
+++ b/src/log-writes/log-writes.h
@@ -63,6 +63,7 @@ struct log_write_entry {
 
 #define LOG_IGNORE_DISCARD (1 << 0)
 #define LOG_DISCARD_NOT_SUPP (1 << 1)
+#define LOG_ZEROOUT_NOT_SUPP (1 << 2)
 
 struct log {
 	int logfd;

[PATCH 05/17] logwrites: only use BLKDISCARD if we know discard zeroes data

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Building off the checks established in the previous patch, only enable
the use of BLKDISCARD if we know that the logwrites device guarantees
that reads after a discard return zeroes.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 common/dmlogwrites          |   10 ++++++++--
 src/log-writes/replay-log.c |    8 ++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)


diff --git a/common/dmlogwrites b/common/dmlogwrites
index 24a8a25ace277f..7a8a9078cb8b65 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -81,7 +81,10 @@ _log_writes_check_bdev()
 	test "$(cat "$sysfs/queue/discard_max_bytes")" -eq 0 && return
 
 	# dm-thinp guarantees that reads after discards return zeroes
-	dmsetup status "$blkdev" 2>/dev/null | grep -q '^0.* thin ' && return
+	if dmsetup status "$blkdev" 2>/dev/null | grep -q '^0.* thin '; then
+		LOGWRITES_REPLAY_ARGS+=(--discard-zeroes-data)
+		return
+	fi
 
 	echo "HINT: $blkdev doesn't guarantee that reads after DISCARD will return zeroes" >> $seqres.hints
 	echo "      This is required for correct journal replay on some filesystems (e.g. xfs)" >> $seqres.hints
@@ -110,6 +113,7 @@ _log_writes_init()
 		BLK_DEV_SIZE=$((length / blksz))
 	fi
 
+	LOGWRITES_REPLAY_ARGS=()
 	LOGWRITES_NAME=logwrites-test
 	LOGWRITES_DMDEV=/dev/mapper/$LOGWRITES_NAME
 	LOGWRITES_TABLE="0 $BLK_DEV_SIZE log-writes $blkdev $LOGWRITES_DEV"
@@ -161,7 +165,8 @@ _log_writes_replay_log()
 	[ $? -ne 0 ] && _fail "mark '$_mark' does not exist"
 
 	$here/src/log-writes/replay-log --log $LOGWRITES_DEV --replay $_blkdev \
-		--end-mark $_mark >> $seqres.full 2>&1
+		--end-mark $_mark "${LOGWRITES_REPLAY_ARGS[@]}" \
+		>> $seqres.full 2>&1
 	[ $? -ne 0 ] && _fail "replay failed"
 }
 
@@ -231,6 +236,7 @@ _log_writes_replay_log_range()
 	echo "=== replay to $end ===" >> $seqres.full
 	$here/src/log-writes/replay-log -vv --log $LOGWRITES_DEV \
 		--replay $blkdev --limit $(($end + 1)) \
+		"${LOGWRITES_REPLAY_ARGS[@]}" \
 		>> $seqres.full 2>&1
 	[ $? -ne 0 ] && _fail "replay failed"
 }
diff --git a/src/log-writes/replay-log.c b/src/log-writes/replay-log.c
index 968c82ab64a9ad..e07401f63af573 100644
--- a/src/log-writes/replay-log.c
+++ b/src/log-writes/replay-log.c
@@ -18,6 +18,7 @@ enum option_indexes {
 	FIND,
 	NUM_ENTRIES,
 	NO_DISCARD,
+	DISCARD_ZEROES_DATA,
 	FSCK,
 	CHECK,
 	START_MARK,
@@ -37,6 +38,7 @@ static struct option long_options[] = {
 	{"find", no_argument, NULL, 0},
 	{"num-entries", no_argument, NULL, 0},
 	{"no-discard", no_argument, NULL, 0},
+	{"discard-zeroes-data", no_argument, NULL, 0},
 	{"fsck", required_argument, NULL, 0},
 	{"check", required_argument, NULL, 0},
 	{"start-mark", required_argument, NULL, 0},
@@ -155,6 +157,7 @@ int main(int argc, char **argv)
 	int ret;
 	int print_num_entries = 0;
 	int discard = 1;
+	int use_kernel_discard = 0;
 	enum log_replay_check_mode check_mode = 0;
 
 	while ((c = getopt_long(argc, argv, "v", long_options,
@@ -242,6 +245,9 @@ int main(int argc, char **argv)
 		case NO_DISCARD:
 			discard = 0;
 			break;
+		case DISCARD_ZEROES_DATA:
+			use_kernel_discard = 1;
+			break;
 		case FSCK:
 			fsck_command = strdup(optarg);
 			if (!fsck_command) {
@@ -299,6 +305,8 @@ int main(int argc, char **argv)
 
 	if (!discard)
 		log->flags |= LOG_IGNORE_DISCARD;
+	if (!use_kernel_discard)
+		log->flags |= LOG_DISCARD_NOT_SUPP;
 
 	log->start_sector = start_sector;
 	log->end_sector = end_sector;

[PATCH 06/17] xfs/113: fix failure to corrupt the entire directory

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

This test tries to corrupt the data blocks of a directory, but it
doesn't take into account the fact that __populate_check_xfs_dir can
remove enough entries to cause sparse holes in the directory.  If that
happens, this "file data block is unmapped" logic will cause the
corruption loop to exit early.  Then we can add to the directory, which
causes the test to fail.

Instead, create a list of mappable dir block offsets, and run 100
corruptions at a time to reduce the amount of time we spend initializing
xfs_db.  This fixes the regressions that I see with 32k/64k block sizes.

Cc: <fstests@vger.kernel.org> # v2022.05.01
Fixes: c8e6dbc8812653 ("xfs: test directory metadata corruption checking and repair")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/xfs/113 |   33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)


diff --git a/tests/xfs/113 b/tests/xfs/113
index 094ab71f2aefec..22ac8c3fd51b80 100755
--- a/tests/xfs/113
+++ b/tests/xfs/113
@@ -52,13 +52,34 @@ _scratch_xfs_repair -n >> $seqres.full 2>&1 || _fail "xfs_repair should not fail
 echo "+ check dir"
 __populate_check_xfs_dir "${inode}" btree
 
+dir_data_offsets() {
+	_scratch_xfs_db -c "inode ${inode}" -c 'bmap' | \
+		awk -v leaf_lblk=$leaf_lblk \
+		'{
+			if ($3 >= leaf_lblk)
+				exit;
+			for (i = 0; i < $8; i++)
+				printf("%d\n", $3 + i);
+		}'
+}
+
 echo "+ corrupt dir"
-loff=0
-while true; do
-	_scratch_xfs_db -x -c "inode ${inode}" -c "dblock ${loff}" -c "stack" | grep -q 'file data block is unmapped' && break
-	_scratch_xfs_db -x -c "inode ${inode}" -c "dblock ${loff}" -c "stack" -c "blocktrash -x 32 -y $((blksz * 8)) -z ${FUZZ_ARGS}" >> $seqres.full
-	loff="$((loff + 1))"
-done
+subcommands=()
+while read loff; do
+	# run 100 commands at a time
+	if [ "${#subcommands[@]}" -lt 600 ]; then
+		subcommands+=(-c "inode ${inode}")
+		subcommands+=(-c "dblock ${loff}")
+		subcommands+=(-c "blocktrash -x 32 -y $((blksz * 8)) -z ${FUZZ_ARGS}")
+		continue
+	fi
+
+	_scratch_xfs_db -x "${subcommands[@]}" >> $seqres.full
+	subcommands=()
+done < <(dir_data_offsets)
+if [ "${#subcommands[@]}" -gt 0 ]; then
+	_scratch_xfs_db -x "${subcommands[@]}" >> $seqres.full
+fi
 
 echo "+ mount image && modify dir"
 if _try_scratch_mount >> $seqres.full 2>&1; then

[PATCH 07/17] xfs/508: fix test for 64k blocksize

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

It turns out that icreate transactions will try to reserve quite a bit
of space on a 64k fsblock filesystem -- enough to handle the worst case
parent directory expansion, a new inode chunk, and these days a parent
pointer as well.  This can work out to quite a bit of space:

fsblock		reservation
1k		172K
4k		368K
16k		1136K
64k		3650K

Unfortunately, this test sets its block quota limits at 1-2MB, so we
can't even create a child file.  Bump the limits up by 10x so that this
test will pass even if there's more metadata size creep in the future.

Fixes: f769a923f576df ("xfs: project quota ineritance flag test")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/xfs/508 |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/tests/xfs/508 b/tests/xfs/508
index ee1a0371db7d6d..1bd13e98c9f641 100755
--- a/tests/xfs/508
+++ b/tests/xfs/508
@@ -44,7 +44,7 @@ do_quota_nospc()
 	local exp=$2
 
 	echo "Write $file, expect $exp:" | _filter_scratch
-	$XFS_IO_PROG -t -f -c "pwrite 0 5m" $file 2>&1 >/dev/null | \
+	$XFS_IO_PROG -t -f -c "pwrite 0 50m" $file 2>&1 >/dev/null | \
 		_filter_xfs_io_error
 	rm -f $file
 }
@@ -56,7 +56,7 @@ _require_prjquota $SCRATCH_DEV
 
 mkdir $SCRATCH_MNT/dir
 $QUOTA_CMD -x -c 'project -s test' $SCRATCH_MNT >>$seqres.full 2>&1
-$QUOTA_CMD -x -c 'limit -p bsoft=1m bhard=2m test' $SCRATCH_MNT
+$QUOTA_CMD -x -c 'limit -p bsoft=10m bhard=20m test' $SCRATCH_MNT
 
 # test the Project inheritance bit is a directory only flag, and it's set on
 # directory by default. Expect no complain about "project inheritance flag is

[PATCH 08/17] common/rc: capture dmesg when oom kills happen

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Capture the dmesg output if the OOM killer is invoked during fstests.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 common/rc |    1 +
 1 file changed, 1 insertion(+)


diff --git a/common/rc b/common/rc
index 2ee46e5101e168..70a0f1d1c6acd9 100644
--- a/common/rc
+++ b/common/rc
@@ -4538,6 +4538,7 @@ _check_dmesg()
 	     -e "INFO: possible circular locking dependency detected" \
 	     -e "general protection fault:" \
 	     -e "BUG .* remaining" \
+	     -e "oom-kill" \
 	     -e "UBSAN:" \
 	     $seqres.dmesg
 	if [ $? -eq 0 ]; then

[PATCH 09/17] generic/562: handle ENOSPC while cloning gracefully

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

This test creates a couple of patterned files on a tiny filesystem,
fragments the free space, clones one patterned file to the other, and
checks that the entire file was cloned.

However, this test doesn't work on a 64k fsblock filesystem because
we've used up all the free space reservation for the rmapbt, and that
causes the FICLONE to error out with ENOSPC partway through.  Hence we
need to detect the ENOSPC and _notrun the test.

That said, it turns out that XFS has been silently dropping error codes
if we managed to make some progress cloning extents.  That's ok if the
operation has REMAP_FILE_CAN_SHORTEN like copy_file_range does, but
FICLONE/FICLONERANGE do not permit partial results, so the dropped error
codes is actually an error.

Therefore, this testcase now becomes a regression test for the patch to
fix that.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/generic/562 |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


diff --git a/tests/generic/562 b/tests/generic/562
index 91360c4154a6a2..36bd02911c96b8 100755
--- a/tests/generic/562
+++ b/tests/generic/562
@@ -15,6 +15,9 @@ _begin_fstest auto clone punch
 . ./common/filter
 . ./common/reflink
 
+test "$FSTYP" = "xfs" && \
+	_fixed_by_kernel_commit XXXXXXXXXX "xfs: don't drop errno values when we fail to ficlone the entire range"
+
 _require_scratch_reflink
 _require_test_program "punch-alternating"
 _require_xfs_io_command "fpunch"
@@ -48,8 +51,11 @@ while true; do
 done
 
 # Now clone file bar into file foo. This is supposed to succeed and not fail
-# with ENOSPC for example.
-_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full
+# with ENOSPC for example.  However, XFS will sometimes run out of space.
+_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full 2> $tmp.err
+cat $tmp.err
+test "$FSTYP" = "xfs" && grep -q 'No space left on device' $tmp.err && \
+	_notrun "ran out of space while cloning"
 
 # Unmount and mount the filesystem again to verify the operation was durably
 # persisted.

[PATCH 10/17] xfs/163: skip test if we can't shrink due to enospc issues

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

If this test fails due to insufficient space, skip this test.  This can
happen if a realtime volume is enabled on the filesystem and we cannot
shrink due to the rtbitmap.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 tests/xfs/163 |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/163 b/tests/xfs/163
index 2bd94060222f96..75c3113dc2fd03 100755
--- a/tests/xfs/163
+++ b/tests/xfs/163
@@ -17,13 +17,20 @@ _begin_fstest auto quick growfs shrinkfs
 
 test_shrink()
 {
-	$XFS_GROWFS_PROG -D"$1" $SCRATCH_MNT >> $seqres.full 2>&1
+	$XFS_GROWFS_PROG -D"$1" $SCRATCH_MNT &> $tmp.growfs
 	ret=$?
 
 	_scratch_unmount
 	_check_scratch_fs
 	_scratch_mount
 
+	# If we couldn't shrink the filesystem due to lack of space, we're
+	# done with this test.
+	[ $1 -ne $dblocks ] && \
+		grep -q 'No space left on device' $tmp.growfs && \
+		_notrun "Could not shrink due to lack of space"
+	cat $tmp.growfs >> $seqres.full
+
 	$XFS_INFO_PROG $SCRATCH_MNT 2>&1 | _filter_mkfs 2>$tmp.growfs >/dev/null
 	. $tmp.growfs
 	[ $ret -eq 0 -a $1 -eq $dblocks ]

[PATCH 11/17] xfs/009: allow logically contiguous preallocations

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

The new rtgroups feature implements a simplistic rotor to pick the
rtgroup for an initial allocation to a file.  This causes test failures
if the preallocations are spread across two rtgroups, which happens if
there are more subtests than rtgroups.

One way to fix this would be to reset the rotor then each subtest starts
allocating from rtgroup 0, but the only way to do that is to cycle the
scratch mount, which is a bit gross.

Instead, report logically contiguous mappings as a single mapping even
if the physical space is not contiguous.  Unfortunately, there's not
enough context in the comments to know if the test actually was checking
for physical contiguity?  Or if this is just an exerciser of the old
preallocation calls, and it's fine as long as the file ranges are mapped
(or unmapped) as desired.

Messing with some awk is a lot cheaper than umount/mount cycling.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 tests/xfs/009 |   29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/009 b/tests/xfs/009
index dde505f079f4f8..bb42ce32490df5 100755
--- a/tests/xfs/009
+++ b/tests/xfs/009
@@ -49,13 +49,26 @@ _filesize()
 _block_filter()
 {
 	$AWK_PROG -v bsize="$bsize" '
+	BEGIN {
+		br_pos = 0
+		br_len = 0
+	}
+	function dump_blockrange() {
+		if (br_len == 0)
+			return
+		printf("        [%d,%d]: BLOCKRANGE\n", br_pos, br_len)
+		br_pos = 0
+		br_len = 0
+	}
 	/blocksize/ {
+		dump_blockrange()
 		printf("    blocksize BSIZE\n")
 
 		next
 	}
 
 	/CMD/ {
+		dump_blockrange()
 		split($3, off, "=")
 		offset = strtonum(off[2])
 		if (offset != -1)
@@ -72,6 +85,7 @@ _block_filter()
 	}
 
 	/MAP/ {
+		dump_blockrange()
 		split($2, off, "=")
 		offset = strtonum(off[2])
 		if (offset != -1)
@@ -90,6 +104,7 @@ _block_filter()
 	}
 
 	/TRUNCATE/ {
+		dump_blockrange()
 		split($2, off, "=")
 		offset = strtonum(off[2]) / bsize
 
@@ -99,16 +114,28 @@ _block_filter()
 	}
 
 	/\[[0-9]+,[0-9]+\]:/ {
-		printf("        %s BLOCKRANGE\n", $1)
+		rangestr = gensub(/\[([0-9]+),([0-9]+)\]:/, "\\1,\\2", "g", $1);
+		split(rangestr, off, ",")
+		if (br_pos + br_len == off[1]) {
+			br_len += off[2];
+		} else {
+			dump_blockrange()
+			br_pos = off[1];
+			br_len = off[2];
+		}
 
 		next
 	}
 
 	{
+		dump_blockrange()
 		print
 
 		next
 	}
+	END {
+		dump_blockrange()
+	}
 	'
 }
 

[PATCH 12/17] generic/251: use sentinel files to kill the fstrim loop

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Apparently the subshell kill doesn't always take, and then the test runs
for hours and hours because nothing stops it.  Instead, use a sentinel
file to detect when fstrim_loop should stop execing background fstrims.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 tests/generic/251 |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


diff --git a/tests/generic/251 b/tests/generic/251
index b432fb11937911..d59e91c3e0a33a 100755
--- a/tests/generic/251
+++ b/tests/generic/251
@@ -125,12 +125,15 @@ fstrim_loop()
 			wait $fpid
 		fi
 		while [ $start -lt $fsize ] ; do
+			test -s $tmp.fstrim_loop || break
 			$FSTRIM_PROG -m ${minlen}k -o ${start}k -l ${step}k $SCRATCH_MNT &
 			fpid=$!
 			wait $fpid
 			start=$(( $start + $step ))
 		done
+		test -s $tmp.fstrim_loop || break
 	done
+	rm -f $tmp.fstrim_loop
 }
 
 function check_sums() {
@@ -188,6 +191,7 @@ find -P . -xdev -type f -print0 | xargs -0 md5sum | sort -o $tmp/content.sums
 
 echo -n "Running the test: "
 pids=""
+echo run > $tmp.fstrim_loop
 fstrim_loop &
 fstrim_pid=$!
 p=1
@@ -199,8 +203,10 @@ done
 echo "done."
 
 wait $pids
-kill $fstrim_pid
-wait $fstrim_pid
+truncate -s 0 $tmp.fstrim_loop
+while test -e $tmp.fstrim_loop; do
+	sleep 1
+done
 
 status=0
 

[PATCH 13/17] generic/251: constrain runtime via time/load/soak factors

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

On my test fleet, this test can run for well in excess of 20 minutes:

   613 generic/251
   616 generic/251
   624 generic/251
   630 generic/251
   634 generic/251
   652 generic/251
   675 generic/251
   749 generic/251
   777 generic/251
   808 generic/251
   832 generic/251
   946 generic/251
  1082 generic/251
  1221 generic/251
  1241 generic/251
  1254 generic/251
  1305 generic/251
  1366 generic/251
  1646 generic/251
  1936 generic/251
  1952 generic/251
  2358 generic/251
  4359 generic/251
  5325 generic/251
 34046 generic/251

because it hardcodes 20 threads and 10 copies.  It's not great to have a
test that results in a significant fraction of the total test runtime.
Fix the looping and load on this test to use LOAD and TIME_FACTOR to
scale up its operations, along with the usual SOAK_DURATION override.
That brings the default runtime down to less than a minute.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/generic/251 |   24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)


diff --git a/tests/generic/251 b/tests/generic/251
index d59e91c3e0a33a..b4ddda10cef403 100755
--- a/tests/generic/251
+++ b/tests/generic/251
@@ -15,7 +15,6 @@ _begin_fstest ioctl trim auto
 tmp=`mktemp -d`
 trap "_cleanup; exit \$status" 0 1 3
 trap "_destroy; exit \$status" 2 15
-chpid=0
 mypid=$$
 
 # Import common functions.
@@ -151,29 +150,28 @@ function check_sums() {
 
 function run_process() {
 	local p=$1
-	repeat=10
+	if [ -n "$SOAK_DURATION" ]; then
+		local duration="$SOAK_DURATION"
+	else
+		local duration="$((30 * TIME_FACTOR))"
+	fi
+	local stopat="$(( $(date +%s) + duration))"
 
-	sleep $((5*$p))s &
-	export chpid=$! && wait $chpid &> /dev/null
-	chpid=0
-
-	while [ $repeat -gt 0 ]; do
+	sleep $((5*$p))s
 
+	while [ "$(date +%s)" -lt "$stopat" ]; do
 		# Remove old directories.
 		rm -rf $SCRATCH_MNT/$p
-		export chpid=$! && wait $chpid &> /dev/null
 
 		# Copy content -> partition.
 		mkdir $SCRATCH_MNT/$p
 		cp -axT $content/ $SCRATCH_MNT/$p/
-		export chpid=$! && wait $chpid &> /dev/null
 
 		check_sums
-		repeat=$(( $repeat - 1 ))
 	done
 }
 
-nproc=20
+nproc=$((4 * LOAD_FACTOR))
 
 # Copy $here to the scratch fs and make coipes of the replica.  The fstests
 # output (and hence $seqres.full) could be in $here, so we need to snapshot
@@ -194,11 +192,9 @@ pids=""
 echo run > $tmp.fstrim_loop
 fstrim_loop &
 fstrim_pid=$!
-p=1
-while [ $p -le $nproc ]; do
+for ((p = 1; p < nproc; p++)); do
 	run_process $p &
 	pids="$pids $!"
-	p=$(($p+1))
 done
 echo "done."
 

[PATCH 14/17] generic/251: don't copy the fsstress source code

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Run fsstress for a short time to generate test data to replicate on the
scratch device so that we don't blow out the test runtimes on
unintentionally copying .git directories or large corefiles from the
developer's systems, etc.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/generic/251 |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)


diff --git a/tests/generic/251 b/tests/generic/251
index b4ddda10cef403..ec486c277c6828 100755
--- a/tests/generic/251
+++ b/tests/generic/251
@@ -173,13 +173,11 @@ function run_process() {
 
 nproc=$((4 * LOAD_FACTOR))
 
-# Copy $here to the scratch fs and make coipes of the replica.  The fstests
-# output (and hence $seqres.full) could be in $here, so we need to snapshot
-# $here before computing file checksums.
+# Use fsstress to create a directory tree with some variability
 content=$SCRATCH_MNT/orig
 mkdir -p $content
-cp -axT $here/ $content/
-
+FSSTRESS_ARGS=$(_scale_fsstress_args -p 4 -d $content -n 1000 $FSSTRESS_AVOID)
+$FSSTRESS_PROG $FSSTRESS_ARGS >> $seqres.full
 mkdir -p $tmp
 
 (

[PATCH 15/17] common/rc: _scratch_mkfs_sized supports extra arguments

[Darrick J. Wong]

From: Zorro Lang <zlang@kernel.org>

To give more arguments to _scratch_mkfs_sized, we generally do as:

  MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size

to give "-L oldlabel" to it. But if _scratch_mkfs_sized fails, it
will get rid of the whole MKFS_OPTIONS and try to mkfs again.
Likes:

  ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 **
  ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 **

But that's not the fault of "-L oldlabel". So for keeping the mkfs
options ("-L oldlabel") we need, we'd better to let the
scratch_mkfs_sized to support extra arguments, rather than using
global MKFS_OPTIONS.

Signed-off-by: Zorro Lang <zlang@kernel.org>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
[djwong: fix string quoting issues]
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 common/rc |   34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)


diff --git a/common/rc b/common/rc
index 70a0f1d1c6acd9..6acacbe4c88eea 100644
--- a/common/rc
+++ b/common/rc
@@ -1026,11 +1026,13 @@ _small_fs_size_mb()
 }
 
 # Create fs of certain size on scratch device
-# _try_scratch_mkfs_sized <size in bytes> [optional blocksize]
+# _try_scratch_mkfs_sized <size in bytes> [optional blocksize] [other options]
 _try_scratch_mkfs_sized()
 {
 	local fssize=$1
-	local blocksize=$2
+	shift
+	local blocksize=$1
+	shift
 	local def_blksz
 	local blocksize_opt
 	local rt_ops
@@ -1094,10 +1096,10 @@ _try_scratch_mkfs_sized()
 		# don't override MKFS_OPTIONS that set a block size.
 		echo $MKFS_OPTIONS |grep -E -q "b\s*size="
 		if [ $? -eq 0 ]; then
-			_try_scratch_mkfs_xfs -d size=$fssize $rt_ops
+			_try_scratch_mkfs_xfs -d size=$fssize $rt_ops "$@"
 		else
 			_try_scratch_mkfs_xfs -d size=$fssize $rt_ops \
-				-b size=$blocksize
+				-b size=$blocksize "$@"
 		fi
 		;;
 	ext2|ext3|ext4)
@@ -1108,7 +1110,7 @@ _try_scratch_mkfs_sized()
 				_notrun "Could not make scratch logdev"
 			MKFS_OPTIONS="$MKFS_OPTIONS -J device=$SCRATCH_LOGDEV"
 		fi
-		${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
+		${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks
 		;;
 	gfs2)
 		# mkfs.gfs2 doesn't automatically shrink journal files on small
@@ -1123,13 +1125,13 @@ _try_scratch_mkfs_sized()
 			(( journal_size >= min_journal_size )) || journal_size=$min_journal_size
 			MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS"
 		fi
-		${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks
+		${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize "$@" $SCRATCH_DEV $blocks
 		;;
 	ocfs2)
-		yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
+		yes | ${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks
 		;;
 	udf)
-		$MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
+		$MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks
 		;;
 	btrfs)
 		local mixed_opt=
@@ -1137,33 +1139,33 @@ _try_scratch_mkfs_sized()
 		# the device is not zoned. Ref: btrfs-progs: btrfs_min_dev_size()
 		(( fssize < $((256 * 1024 * 1024)) )) &&
 			! _scratch_btrfs_is_zoned && mixed_opt='--mixed'
-		$MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV
+		$MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize "$@" $SCRATCH_DEV
 		;;
 	jfs)
-		${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS $SCRATCH_DEV $blocks
+		${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS "$@" $SCRATCH_DEV $blocks
 		;;
 	reiserfs)
-		${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
+		${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -b $blocksize "$@" $SCRATCH_DEV $blocks
 		;;
 	reiser4)
 		# mkfs.resier4 requires size in KB as input for creating filesystem
-		$MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize $SCRATCH_DEV \
+		$MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize "$@" $SCRATCH_DEV \
 				   `expr $fssize / 1024`
 		;;
 	f2fs)
 		# mkfs.f2fs requires # of sectors as an input for the size
 		local sector_size=`blockdev --getss $SCRATCH_DEV`
-		$MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size`
+		$MKFS_F2FS_PROG $MKFS_OPTIONS "$@" $SCRATCH_DEV `expr $fssize / $sector_size`
 		;;
 	tmpfs)
 		local free_mem=`_free_memory_bytes`
 		if [ "$free_mem" -lt "$fssize" ] ; then
 		   _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes"
 		fi
-		export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS"
+		export MOUNT_OPTIONS="-o size=$fssize "$@" $TMPFS_MOUNT_OPTIONS"
 		;;
 	bcachefs)
-		$MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt $SCRATCH_DEV
+		$MKFS_BCACHEFS_PROG $MKFS_OPTIONS --fs_size=$fssize $blocksize_opt "$@" $SCRATCH_DEV
 		;;
 	*)
 		_notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized"
@@ -1173,7 +1175,7 @@ _try_scratch_mkfs_sized()
 
 _scratch_mkfs_sized()
 {
-	_try_scratch_mkfs_sized $* || _notrun "_scratch_mkfs_sized failed with ($*)"
+	_try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($*)"
 }
 
 # Emulate an N-data-disk stripe w/ various stripe units

[PATCH 16/17] xfs/157: do not drop necessary mkfs options

[Darrick J. Wong]

From: Zorro Lang <zlang@kernel.org>

To give the test option "-L oldlabel" to _scratch_mkfs_sized, xfs/157
does:

  MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size

but the _scratch_mkfs_sized trys to keep the $fs_size, when mkfs
fails with incompatible $MKFS_OPTIONS options, likes this:

  ** mkfs failed with extra mkfs options added to "-L oldlabel -m rmapbt=1" by test 157 **
  ** attempting to mkfs using only test 157 options: -d size=524288000 -b size=4096 **

but the "-L oldlabel" is necessary, we shouldn't drop it. To avoid
that, we give the "-L oldlabel" to _scratch_mkfs_sized through
function parameters, not through global MKFS_OPTIONS.

Signed-off-by: Zorro Lang <zlang@kernel.org>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
[djwong: fix more string quoting issues]
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/xfs/157 |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


diff --git a/tests/xfs/157 b/tests/xfs/157
index 9b5badbaeb3c76..e102a5a10abe4b 100755
--- a/tests/xfs/157
+++ b/tests/xfs/157
@@ -66,8 +66,7 @@ scenario() {
 }
 
 check_label() {
-	MKFS_OPTIONS="-L oldlabel $MKFS_OPTIONS" _scratch_mkfs_sized $fs_size \
-		>> $seqres.full
+	_scratch_mkfs_sized "$fs_size" "" -L oldlabel >> $seqres.full 2>&1
 	_scratch_xfs_db -c label
 	_scratch_xfs_admin -L newlabel "$@" >> $seqres.full
 	_scratch_xfs_db -c label

[PATCH 17/17] generic/366: fix directio requirements checking

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

On a system with 4k-sector storage devices, this test fails with:

 --- /tmp/fstests/tests/generic/366.out	2024-11-17 09:04:53.161104479 -0800
 +++ /var/tmp/fstests/generic/366.out.bad	2024-11-20 21:02:30.948000000 -0800
 @@ -1,2 +1,34 @@
  QA output created by 366
 +fio: io_u error on file /opt/file1: Invalid argument: read offset=15360, buflen=512
 +fio: io_u error on file /opt/file1: Invalid argument: read offset=15360, buflen=512

The cause of this failure is that we cannot do 512byte directios to a
device with 4k LBAs.  Update the precondition checking to exclude this
scenario.

Cc: <fstests@vger.kernel.org> # v2024.11.17
Fixes: 4c1629ae3a3a56 ("generic: new test case to verify if certain fio load will hang the filesystem")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/generic/366 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/tests/generic/366 b/tests/generic/366
index 6e7dd7279218c4..b322bcca72fecc 100755
--- a/tests/generic/366
+++ b/tests/generic/366
@@ -20,7 +20,7 @@ _begin_fstest auto quick rw
 . ./common/filter
 
 _require_scratch
-_require_odirect
+_require_odirect 512	# see fio job1 config below
 _require_aio
 
 _fixed_by_kernel_commit xxxxxxxxxxxx \

Re: [PATCH 02/17] generic/757: convert to thinp

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 03/17] logwrites: warn if we don't think read after discard returns zeroes

[Christoph Hellwig]

This feels like it's heading in the wrong direction, as it just adds
more hacks.

IMHO the right thing is to:

 a) document current assumptions and tell users very clearly to use
    dm-thin to get the expected semantics
 b) if we have a use for it, provide an option to use write zeroes
    instead that can be used on any device

I'm happy to do that work, but until then I'd suggest to skip this and
the next two patches.

Re: [PATCH 06/17] xfs/113: fix failure to corrupt the entire directory

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 07/17] xfs/508: fix test for 64k blocksize

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 08/17] common/rc: capture dmesg when oom kills happen

[Christoph Hellwig]

On Fri, Nov 22, 2024 at 08:52:32AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Capture the dmesg output if the OOM killer is invoked during fstests.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 09/17] generic/562: handle ENOSPC while cloning gracefully

[Christoph Hellwig]

On Fri, Nov 22, 2024 at 08:52:48AM -0800, Darrick J. Wong wrote:
> +# with ENOSPC for example.  However, XFS will sometimes run out of space.
> +_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full 2> $tmp.err
> +cat $tmp.err
> +test "$FSTYP" = "xfs" && grep -q 'No space left on device' $tmp.err && \
> +	_notrun "ran out of space while cloning"

Should this simply be unconditional instead of depend on XFS?

Re: [PATCH 13/17] generic/251: constrain runtime via time/load/soak factors

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 14/17] generic/251: don't copy the fsstress source code

[Christoph Hellwig]

On Fri, Nov 22, 2024 at 08:54:06AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Run fsstress for a short time to generate test data to replicate on the
> scratch device so that we don't blow out the test runtimes on
> unintentionally copying .git directories or large corefiles from the
> developer's systems, etc.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 09/17] generic/562: handle ENOSPC while cloning gracefully

[Darrick J. Wong]

On Sun, Nov 24, 2024 at 09:14:43PM -0800, Christoph Hellwig wrote:
> On Fri, Nov 22, 2024 at 08:52:48AM -0800, Darrick J. Wong wrote:
> > +# with ENOSPC for example.  However, XFS will sometimes run out of space.
> > +_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full 2> $tmp.err
> > +cat $tmp.err
> > +test "$FSTYP" = "xfs" && grep -q 'No space left on device' $tmp.err && \
> > +	_notrun "ran out of space while cloning"
> 
> Should this simply be unconditional instead of depend on XFS?

Felipe said no:
https://lore.kernel.org/fstests/CAL3q7H5KjvXsXzt4n0XP1FTUt=A5cKom7p+dGD6GG-iL7CyDXQ@mail.gmail.com/

(which I should reply to)

--D

Re: [PATCH 17/17] generic/366: fix directio requirements checking

[Christoph Hellwig]

On Fri, Nov 22, 2024 at 08:54:53AM -0800, Darrick J. Wong wrote:
> -_require_odirect
> +_require_odirect 512	# see fio job1 config below

Hah, I did not know about this optional argument to
_require_odirect, which sounds pretty useful.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 09/17] generic/562: handle ENOSPC while cloning gracefully

[Christoph Hellwig]

On Sun, Nov 24, 2024 at 09:16:39PM -0800, Darrick J. Wong wrote:
> On Sun, Nov 24, 2024 at 09:14:43PM -0800, Christoph Hellwig wrote:
> > On Fri, Nov 22, 2024 at 08:52:48AM -0800, Darrick J. Wong wrote:
> > > +# with ENOSPC for example.  However, XFS will sometimes run out of space.
> > > +_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full 2> $tmp.err
> > > +cat $tmp.err
> > > +test "$FSTYP" = "xfs" && grep -q 'No space left on device' $tmp.err && \
> > > +	_notrun "ran out of space while cloning"
> > 
> > Should this simply be unconditional instead of depend on XFS?
> 
> Felipe said no:
> https://lore.kernel.org/fstests/CAL3q7H5KjvXsXzt4n0XP1FTUt=A5cKom7p+dGD6GG-iL7CyDXQ@mail.gmail.com/

Hmm.   Being able to totally fill the fs without ENOSPC seems odd.
Maybe we need to figure out a way to scale down the size for the generic
test and have a separate one for the XFS ENOSPC case?  Not a huge fan
of that, but the current version also seems odd.

Re: [PATCH 03/17] logwrites: warn if we don't think read after discard returns zeroes

[Darrick J. Wong]

On Sun, Nov 24, 2024 at 09:12:26PM -0800, Christoph Hellwig wrote:
> This feels like it's heading in the wrong direction, as it just adds
> more hacks.
> 
> IMHO the right thing is to:
> 
>  a) document current assumptions and tell users very clearly to use
>     dm-thin to get the expected semantics
>  b) if we have a use for it, provide an option to use write zeroes
>     instead that can be used on any device
> 
> I'm happy to do that work, but until then I'd suggest to skip this and
> the next two patches.

Works for me. :)

Also feel free to grab/modify patch 4 if you like.

--D

Re: [PATCH 09/17] generic/562: handle ENOSPC while cloning gracefully

[Darrick J. Wong]

On Sun, Nov 24, 2024 at 09:20:29PM -0800, Christoph Hellwig wrote:
> On Sun, Nov 24, 2024 at 09:16:39PM -0800, Darrick J. Wong wrote:
> > On Sun, Nov 24, 2024 at 09:14:43PM -0800, Christoph Hellwig wrote:
> > > On Fri, Nov 22, 2024 at 08:52:48AM -0800, Darrick J. Wong wrote:
> > > > +# with ENOSPC for example.  However, XFS will sometimes run out of space.
> > > > +_reflink $SCRATCH_MNT/bar $SCRATCH_MNT/foo >>$seqres.full 2> $tmp.err
> > > > +cat $tmp.err
> > > > +test "$FSTYP" = "xfs" && grep -q 'No space left on device' $tmp.err && \
> > > > +	_notrun "ran out of space while cloning"
> > > 
> > > Should this simply be unconditional instead of depend on XFS?
> > 
> > Felipe said no:
> > https://lore.kernel.org/fstests/CAL3q7H5KjvXsXzt4n0XP1FTUt=A5cKom7p+dGD6GG-iL7CyDXQ@mail.gmail.com/
> 
> Hmm.   Being able to totally fill the fs without ENOSPC seems odd.
> Maybe we need to figure out a way to scale down the size for the generic
> test and have a separate one for the XFS ENOSPC case?  Not a huge fan
> of that, but the current version also seems odd.

Yeah, I definitely need to write a fstest that can trip this bug on
smaller fsblock filesystems.  In the meantime, this one should not fail
just because xfs runs out of space before the point where this test
would have thought that would happen; and then xfs_io spews an error
message into the golden output.

Though if this is really a test that computes when *btrfs* would run out
of space and drives towards that point just to see if ENOSPC does /not/
come out of the kernel, then maybe this belongs in tests/btrfs/ ?

--D