Add more tests for multi fs block atomic writes

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Signed-off-by Ojaswin Mujoo <[email protected]>

---
 common/rc | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

common/rc +32 -0

index 35a1c835..f45b9a38 100644
@@ -5997,6 +5997,38 @@ _max() {
 	echo $ret
 }
 
+# Check the required fio version. Examples:
+#   _require_fio_version 3.38 (matches 3.38 only)
+#   _require_fio_version 3.38+ (matches 3.38 and above)
+#   _require_fio_version 3.38- (matches 3.38 and below)
+_require_fio_version() {
+	local req_ver="$1"
+	local fio_ver
+
+	_require_fio
+	_require_math
+
+	fio_ver=$(fio -v | cut -d"-" -f2)
+
+	case "$req_ver" in
+	*+)
+		req_ver=${req_ver%+}
+		test $(_math "$fio_ver >= $req_ver") -eq 1 || \
+			_notrun "need fio >= $req_ver (found $fio_ver)"
+		;;
+	*-)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver <= $req_ver") -eq 1 || \
+			_notrun "need fio <= $req_ver (found $fio_ver)"
+		;;
+	*)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver == $req_ver") -eq 1 || \
+			_notrun "need fio = $req_ver (found $fio_ver)"
+		;;
+	esac
+}
+
 ################################################################################
 # make sure this script returns success
 /bin/true

Signature · 1 line

2.49.0

Quoted 10 lines · Ojaswin Mujoo

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Actually I'm wondering if fstests really needs to care about this. This's just a temporary issue of fio, not kernel or any fs usespace program. Do we need to add a seperated helper only for a temporary fio issue? If fio doesn't break fstests running, let it run. Just the testers install proper fio (maybe latest) they need. What do you and others think?

Thanks,
Zorro

Quoted 53 lines

Signed-off-by Ojaswin Mujoo <[email protected]>

---
 common/rc | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

common/rc +32 -0

index 35a1c835..f45b9a38 100644
@@ -5997,6 +5997,38 @@ _max() {
 	echo $ret
 }
 
+# Check the required fio version. Examples:
+#   _require_fio_version 3.38 (matches 3.38 only)
+#   _require_fio_version 3.38+ (matches 3.38 and above)
+#   _require_fio_version 3.38- (matches 3.38 and below)
+_require_fio_version() {
+	local req_ver="$1"
+	local fio_ver
+
+	_require_fio
+	_require_math
+
+	fio_ver=$(fio -v | cut -d"-" -f2)
+
+	case "$req_ver" in
+	*+)
+		req_ver=${req_ver%+}
+		test $(_math "$fio_ver >= $req_ver") -eq 1 || \
+			_notrun "need fio >= $req_ver (found $fio_ver)"
+		;;
+	*-)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver <= $req_ver") -eq 1 || \
+			_notrun "need fio <= $req_ver (found $fio_ver)"
+		;;
+	*)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver == $req_ver") -eq 1 || \
+			_notrun "need fio = $req_ver (found $fio_ver)"
+		;;
+	esac
+}
+
 ################################################################################
 # make sure this script returns success
 /bin/true

Signature · 1 line

2.49.0

Quoted 9 lines · Zorro Lang

Quoted 8 lines · Ojaswin Mujoo

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Quoted 8 lines

Quoted 2 lines

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Actually I'm wondering if fstests really needs to care about this. This's just a temporary issue of fio, not kernel or any fs usespace program. Do we need to add a seperated helper only for a temporary fio issue? If fio doesn't break fstests running, let it run. Just the testers install proper fio (maybe latest) they need. What do you and others think?

Quoted 2 lines

Thanks,
Zorro

Hey Zorro,

Sure I'm okay with not keeping the helper and letting the user make sure the fio version is correct.

@John, does that sound okay?

Regards,
ojaswin

Quoted 54 lines

Quoted 53 lines

Signed-off-by Ojaswin Mujoo <[email protected]>

---
 common/rc | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

common/rc +32 -0

index 35a1c835..f45b9a38 100644
@@ -5997,6 +5997,38 @@ _max() {
 	echo $ret
 }
 
+# Check the required fio version. Examples:
+#   _require_fio_version 3.38 (matches 3.38 only)
+#   _require_fio_version 3.38+ (matches 3.38 and above)
+#   _require_fio_version 3.38- (matches 3.38 and below)
+_require_fio_version() {
+	local req_ver="$1"
+	local fio_ver
+
+	_require_fio
+	_require_math
+
+	fio_ver=$(fio -v | cut -d"-" -f2)
+
+	case "$req_ver" in
+	*+)
+		req_ver=${req_ver%+}
+		test $(_math "$fio_ver >= $req_ver") -eq 1 || \
+			_notrun "need fio >= $req_ver (found $fio_ver)"
+		;;
+	*-)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver <= $req_ver") -eq 1 || \
+			_notrun "need fio <= $req_ver (found $fio_ver)"
+		;;
+	*)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver == $req_ver") -eq 1 || \
+			_notrun "need fio = $req_ver (found $fio_ver)"
+		;;
+	esac
+}
+
 ################################################################################
 # make sure this script returns success
 /bin/true

Signature · 1 line

2.49.0

From: "Ritesh Harjani (IBM)" <[email protected]>

This test does a lot of parallel RWF_ATOMIC IO on a preallocated file to stress the write and end-io unwritten conversion code paths. We brute force this for different blocksize and clustersizes and after each iteration we ensure the data was not torn or corrupted using fio crc verification.

Note that in this test we use overlapping atomic writes of same io size. Although serializing racing writes is not guaranteed for RWF_ATOMIC, NVMe and SCSI provide this guarantee as an inseparable feature to power-fail atomicity. Keeping the iosize as same also ensures that ext4 doesn't tear the write due to racing ioend unwritten conversion.

The value of this test is that we make sure the RWF_ATOMIC is handled correctly by ext4 as well as test that the block layer doesn't split or only generate multiple bios for an atomic write.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

Signed-off-by Ojaswin Mujoo <[email protected]>

---
 tests/ext4/061     | 155 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/061.out |   2 +
 2 files changed, 157 insertions(+)
 create mode 100755 tests/ext4/061
 create mode 100644 tests/ext4/061.out

tests/ext4/061 new file +155 -0

new file mode 100755
index 00000000..0ccf9f69
@@ -0,0 +1,155 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 IBM Corporation. All Rights Reserved.
+#
+# FS QA Test 061
+#
+# This test does a lot of parallel RWF_ATOMIC IO on a preallocated file to
+# stress the write and end-io unwritten conversion code paths. We brute force
+# this for all possible blocksize and clustersizes and after each iteration we
+# ensure the data was not torn or corrupted using fio crc verification.
+#
+# Note that in this test we use overlapping atomic writes of same io size.
+# Although serializing racing writes is not guaranteed for RWF_ATOMIC, NVMe and
+# SCSI provide this guarantee as an inseparable feature to power-fail
+# atomicity. Keeping the iosize as same also ensures that ext4 doesn't tear the
+# write due to racing ioend unwritten conversion.
+#
+# The value of this test is that we make sure the RWF_ATOMIC is handled
+# correctly by ext4 as well as test that the block layer doesn't split or only
+# generate multiple bios for an atomic write.
+
+. ./common/preamble
+. ./common/atomicwrites
+
+_begin_fstest auto rw stress atomicwrites
+
+_require_scratch_write_atomic
+_require_aiodio
+_require_fio_version "3.38+"
+
+FIO_LOAD=$(($(nproc) * 2 * LOAD_FACTOR))
+SIZE=$((100*1024*1024))
+
+# Calculate fsblocksize as per bdev atomic write units.
+bdev_awu_min=$(_get_atomic_write_unit_min $SCRATCH_DEV)
+bdev_awu_max=$(_get_atomic_write_unit_max $SCRATCH_DEV)
+bs=$(_max 4096 "$bdev_awu_min")
+
+function create_fio_configs()
+{
+	local bsize=$1
+	create_fio_aw_config $bsize
+	create_fio_verify_config $bsize
+}
+
+function create_fio_verify_config()
+{
+	local bsize=$1
+cat >$fio_verify_config <<EOF
+	[aio-dio-aw-verify]
+	direct=1
+	ioengine=libaio
+	rw=read
+	bs=$bsize
+	fallocate=native
+	filename=$SCRATCH_MNT/test-file
+	size=$SIZE
+	iodepth=$FIO_LOAD
+	numjobs=$FIO_LOAD
+	atomic=1
+	group_reporting=1
+
+	verify_only=1
+	verify_state_save=0
+	verify=crc32c
+	verify_fatal=1
+	verify_write_sequence=0
+EOF
+}
+
+function create_fio_aw_config()
+{
+	local bsize=$1
+cat >$fio_aw_config <<EOF
+	[aio-dio-aw]
+	direct=1
+	ioengine=libaio
+	rw=randwrite
+	bs=$bsize
+	fallocate=native
+	filename=$SCRATCH_MNT/test-file
+	size=$SIZE
+	iodepth=$FIO_LOAD
+	numjobs=$FIO_LOAD
+	group_reporting=1
+	atomic=1
+
+	verify_state_save=0
+	verify=crc32c
+	do_verify=0
+
+EOF
+}
+
+run_test_one() {
+	local bs=$1
+	local cs=$2
+	local iosize=$3
+
+	MKFS_OPTIONS="-O bigalloc -b $bs -C $cs"
+	_scratch_mkfs_ext4  >> $seqres.full 2>&1 || continue
+	if _try_scratch_mount >> $seqres.full 2>&1; then
+		echo "== Testing: bs=$bs cs=$cs iosize=$iosize ==" >> $seqres.full
+
+		touch $SCRATCH_MNT/f1
+		create_fio_configs $iosize
+
+		cat $fio_aw_config >> $seqres.full
+		echo >> $seqres.full
+		cat $fio_verify_config >> $seqres.full
+
+		$FIO_PROG $fio_aw_config >> $seqres.full
+		ret1=$?
+
+		$FIO_PROG $fio_verify_config >> $seqres.full
+		ret2=$?
+
+		_scratch_unmount
+
+		[[ $ret1 -eq 0 && $ret2 -eq 0 ]] || _fail "fio with atomic write failed"
+	fi
+}
+
+run_test() {
+	local bs=$1
+
+	# cluster sizes above 16 x blocksize are experimental so avoid them
+	# Also, cap cluster size at 128kb to keep it reasonable for large
+	# blocks size
+	max_cs=$(_min $((16 * bs)) "$bdev_awu_max" $((128 * 1024)))
+
+	# Fuzz for combinations of blocksize, clustersize and
+	# iosize that cover most of the cases
+	run_test_one $bs $bs $bs
+	run_test_one $bs $max_cs $bs
+	run_test_one $bs $max_cs $max_cs
+	run_test_one $bs $max_cs $(_max "$((max_cs/2))" $bs)
+}
+
+# Let's create a sample fio config to check whether fio supports all options.
+fio_aw_config=$tmp.aw.fio
+fio_verify_config=$tmp.verify.fio
+fio_out=$tmp.fio.out
+
+create_fio_configs $bs
+_require_fio $fio_aw_config
+
+for ((bs=$bs; bs <= $(_get_page_size); bs = $bs << 1)); do
+	run_test $bs
+done
+
+# success, all done
+echo Silence is golden
+status=0
+exit

tests/ext4/061.out new file +2 -0

new file mode 100644
index 00000000..273be9e0
@@ -0,0 +1,2 @@
+QA output created by 061
+Silence is golden

Signature · 1 line

2.49.0

Quoted 10 lines · Ojaswin Mujoo

Quoted 9 lines · Zorro Lang

Quoted 8 lines · Ojaswin Mujoo

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Actually I'm wondering if fstests really needs to care about this. This's just a temporary issue of fio, not kernel or any fs usespace program. Do we need to add a seperated helper only for a temporary fio issue? If fio doesn't break fstests running, let it run. Just the testers install proper fio (maybe latest) they need. What do you and others think?

Are there obvious failures if you try to run these new atomic write tests on a system with the weird versions of fio that have the no-op atomic= functionality? I'm concerned that some QA person is going to do that unwittingly and report that everything is ok when in reality they didn't actually test anything.

--D

Quoted 67 lines

Quoted 2 lines

Thanks,
Zorro

Hey Zorro,

Sure I'm okay with not keeping the helper and letting the user make sure the fio version is correct.

@John, does that sound okay?

Regards,
ojaswin

Quoted 54 lines

Quoted 53 lines

Signed-off-by Ojaswin Mujoo <[email protected]>

---
 common/rc | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

common/rc +32 -0

index 35a1c835..f45b9a38 100644
@@ -5997,6 +5997,38 @@ _max() {
 	echo $ret
 }
 
+# Check the required fio version. Examples:
+#   _require_fio_version 3.38 (matches 3.38 only)
+#   _require_fio_version 3.38+ (matches 3.38 and above)
+#   _require_fio_version 3.38- (matches 3.38 and below)
+_require_fio_version() {
+	local req_ver="$1"
+	local fio_ver
+
+	_require_fio
+	_require_math
+
+	fio_ver=$(fio -v | cut -d"-" -f2)
+
+	case "$req_ver" in
+	*+)
+		req_ver=${req_ver%+}
+		test $(_math "$fio_ver >= $req_ver") -eq 1 || \
+			_notrun "need fio >= $req_ver (found $fio_ver)"
+		;;
+	*-)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver <= $req_ver") -eq 1 || \
+			_notrun "need fio <= $req_ver (found $fio_ver)"
+		;;
+	*)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver == $req_ver") -eq 1 || \
+			_notrun "need fio = $req_ver (found $fio_ver)"
+		;;
+	esac
+}
+
 ################################################################################
 # make sure this script returns success
 /bin/true

Signature · 1 line

2.49.0

Quoted 20 lines · Darrick J. Wong

Quoted 10 lines · Ojaswin Mujoo

Quoted 9 lines · Zorro Lang

Quoted 8 lines · Ojaswin Mujoo

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Actually I'm wondering if fstests really needs to care about this. This's just a temporary issue of fio, not kernel or any fs usespace program. Do we need to add a seperated helper only for a temporary fio issue? If fio doesn't break fstests running, let it run. Just the testers install proper fio (maybe latest) they need. What do you and others think?

Quoted 5 lines

Are there obvious failures if you try to run these new atomic write tests on a system with the weird versions of fio that have the no-op atomic= functionality? I'm concerned that some QA person is going to do that unwittingly and report that everything is ok when in reality they didn't actually test anything.

I think John has a bit more background but afaict, RWF_ATOMIC support was added (fio commit: d01612f3ae25) but then removed (commit:
a25ba6c64fe1) since the feature didn't make it to kernel in time. However the option seemed to be kept in place. Later, commit 40f1fc11d added the support back in a later version of fio.

So yes, I think there are some version where fio will accept atomic=1 but not act upon it and the tests may start failing with no apparent reason.

Regards,
ojaswin

Quoted 70 lines

--D

Quoted 67 lines

Quoted 2 lines

Thanks,
Zorro

Hey Zorro,

Sure I'm okay with not keeping the helper and letting the user make sure the fio version is correct.

@John, does that sound okay?

Regards,
ojaswin

Quoted 54 lines

Quoted 53 lines

Signed-off-by Ojaswin Mujoo <[email protected]>

---
 common/rc | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

common/rc +32 -0

index 35a1c835..f45b9a38 100644
@@ -5997,6 +5997,38 @@ _max() {
 	echo $ret
 }
 
+# Check the required fio version. Examples:
+#   _require_fio_version 3.38 (matches 3.38 only)
+#   _require_fio_version 3.38+ (matches 3.38 and above)
+#   _require_fio_version 3.38- (matches 3.38 and below)
+_require_fio_version() {
+	local req_ver="$1"
+	local fio_ver
+
+	_require_fio
+	_require_math
+
+	fio_ver=$(fio -v | cut -d"-" -f2)
+
+	case "$req_ver" in
+	*+)
+		req_ver=${req_ver%+}
+		test $(_math "$fio_ver >= $req_ver") -eq 1 || \
+			_notrun "need fio >= $req_ver (found $fio_ver)"
+		;;
+	*-)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver <= $req_ver") -eq 1 || \
+			_notrun "need fio <= $req_ver (found $fio_ver)"
+		;;
+	*)
+		req_ver=${req_ver%-}
+		test $(_math "$fio_ver == $req_ver") -eq 1 || \
+			_notrun "need fio = $req_ver (found $fio_ver)"
+		;;
+	esac
+}
+
 ################################################################################
 # make sure this script returns success
 /bin/true

Signature · 1 line

2.49.0

Quoted 33 lines · Ojaswin Mujoo

Quoted 20 lines · Darrick J. Wong

Quoted 10 lines · Ojaswin Mujoo

Quoted 9 lines · Zorro Lang

Quoted 8 lines · Ojaswin Mujoo

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Actually I'm wondering if fstests really needs to care about this. This's just a temporary issue of fio, not kernel or any fs usespace program. Do we need to add a seperated helper only for a temporary fio issue? If fio doesn't break fstests running, let it run. Just the testers install proper fio (maybe latest) they need. What do you and others think?

Quoted 5 lines

Are there obvious failures if you try to run these new atomic write tests on a system with the weird versions of fio that have the no-op atomic= functionality? I'm concerned that some QA person is going to do that unwittingly and report that everything is ok when in reality they didn't actually test anything.

I think John has a bit more background but afaict, RWF_ATOMIC support was added (fio commit: d01612f3ae25) but then removed (commit:
a25ba6c64fe1) since the feature didn't make it to kernel in time. However the option seemed to be kept in place. Later, commit 40f1fc11d added the support back in a later version of fio.

Quoted 3 lines

So yes, I think there are some version where fio will accept atomic=1 but not act upon it and the tests may start failing with no apparent reason.

The concern from Darrick might be a problem. May I ask which fio commit brought in this issue, and which fio commit fixed it? If this issue be brought in and fixed within a fio release, it might be better. But if it crosses fio release, that might be bad, then we might be better to have this helper.

Thanks,
Zorro

Quoted 8 lines

Regards,
ojaswin

Quoted 5 lines

--D

Quoted 2 lines

Quoted 2 lines

Thanks,
Zorro

Quoted 38 lines · Zorro Lang

Quoted 36 lines · Ojaswin Mujoo

Quoted 20 lines · Darrick J. Wong

Quoted 10 lines · Ojaswin Mujoo

Quoted 9 lines · Zorro Lang

Quoted 8 lines · Ojaswin Mujoo

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Actually I'm wondering if fstests really needs to care about this. This's just a temporary issue of fio, not kernel or any fs usespace program. Do we need to add a seperated helper only for a temporary fio issue? If fio doesn't break fstests running, let it run. Just the testers install proper fio (maybe latest) they need. What do you and others think?

Quoted 5 lines

Are there obvious failures if you try to run these new atomic write tests on a system with the weird versions of fio that have the no-op atomic= functionality? I'm concerned that some QA person is going to do that unwittingly and report that everything is ok when in reality they didn't actually test anything.

I think John has a bit more background but afaict, RWF_ATOMIC support was added (fio commit: d01612f3ae25) but then removed (commit:
a25ba6c64fe1) since the feature didn't make it to kernel in time. However the option seemed to be kept in place. Later, commit 40f1fc11d added the support back in a later version of fio.

So yes, I think there are some version where fio will accept atomic=1 but not act upon it and the tests may start failing with no apparent reason.

Quoted 5 lines

The concern from Darrick might be a problem. May I ask which fio commit brought in this issue, and which fio commit fixed it? If this issue be brought in and fixed within a fio release, it might be better. But if it crosses fio release, that might be bad, then we might be better to have this helper.

Hi Zorro, yes it does seem to cross version boundaries. The functionality was removed in fio v3.33 and added back in v3.38. I

confirmed this by running generic/1226 with both (for v3.33 run i commented out a few fio options that were added later but kept atomic=1):

Command: sudo perf record -e iomap:iomap_dio_rw_begin ./check generic/1226

perf script sample with fio v3.33:

fio 6626 [000] 777.668017: iomap:iomap_dio_rw_begin: <.sniip.> flags DIRECT|WRITE|AIO_RW dio_flags aio 1

perf script sample with fio v3.39:

fio 9830 [000] 895.042747: iomap:iomap_dio_rw_begin: <.snip> flags ATOMIC|DIRECT|WRITE|AIO_RW dio_flags aio 1

So as we can see, even though the test passes with atomic=1, fio is not sending the RWF_ATOMIC flag in the older version.

In which case I believe it should be okay to keep the helper, right?

Thanks,
Ojaswin

Quoted 12 lines

Thanks,
Zorro

Quoted 8 lines

Regards,
ojaswin

Quoted 5 lines

--D

Quoted 2 lines

Quoted 2 lines

Thanks,
Zorro

Quoted 6 lines

I think John has a bit more background but afaict, RWF_ATOMIC support was added (fio commit: d01612f3ae25) but then removed (commit:
a25ba6c64fe1) since the feature didn't make it to kernel in time. However the option seemed to be kept in place. Later, commit 40f1fc11d added the support back in a later version of fio.

Quoted 3 lines

So yes, I think there are some version where fio will accept atomic=1 but not act upon it and the tests may start failing with no apparent reason.

The concern from Darrick might be a problem. May I ask which fio commit brought in this issue, and which fio commit fixed it? If this issue be brought in and fixed within a fio release, it might be better. But if it crosses fio release, that might be bad, then we might be better to have this helper.

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Signed-off-by Ojaswin Mujoo <[email protected]>

---
  common/rc | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)

common/rc +0 -0

index 35a1c835..f45b9a38 100644

Implement atomic write support to help fuzz atomic writes with fsx.

Suggested-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

---
  ltp/fsx.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 110 insertions(+), 5 deletions(-)

ltp/fsx.c +20 -0

index 163b9453..1582f6d1 100644
@@ -40,6 +40,7 @@
  #include <liburing.h>
  #endif
  #include <sys/syscall.h>
+#include "statx.h"
 
  #ifndef MAP_FILE
  # define MAP_FILE 0

@@ -49,6 +50,10 @@
  #define RWF_DONTCACHE	0x80
  #endif
 
+#ifndef RWF_ATOMIC
+#define RWF_ATOMIC	0x40
+#endif
+
  #define NUMPRINTCOLUMNS 32	/* # columns of data to print on each line */
 
  /* Operation flags (bitmask) */

@@ -110,6 +115,7 @@ enum {
  	OP_READ_DONTCACHE,
  	OP_WRITE,
  	OP_WRITE_DONTCACHE,
+	OP_WRITE_ATOMIC,
  	OP_MAPREAD,
  	OP_MAPWRITE,
  	OP_MAX_LITE,

@@ -200,6 +206,11 @@ int	uring = 0;
  int	mark_nr = 0;
  int	dontcache_io = 1;
  int	hugepages = 0;                  /* -h flag */
+int	do_atomic_writes = 1;		/* -a flag disables */
+
+/* User for atomic writes */
+int awu_min = 0;
+int awu_max = 0;
 
  /* Stores info needed to periodically collapse hugepages */
  struct hugepages_collapse_info {

@@ -288,6 +299,7 @@ static const char *op_names[] = {
  	[OP_READ_DONTCACHE] = "read_dontcache",
  	[OP_WRITE] = "write",
  	[OP_WRITE_DONTCACHE] = "write_dontcache",
+	[OP_WRITE_ATOMIC] = "write_atomic",
  	[OP_MAPREAD] = "mapread",
  	[OP_MAPWRITE] = "mapwrite",
  	[OP_TRUNCATE] = "truncate",

@@ -422,6 +434,7 @@ logdump(void)
  				prt("\t***RRRR***");
  			break;
  		case OP_WRITE_DONTCACHE:
+		case OP_WRITE_ATOMIC:
  		case OP_WRITE:
  			prt("WRITE    0x%x thru 0x%x\t(0x%x bytes)",
  			    lp->args[0], lp->args[0] + lp->args[1] - 1,

@@ -1073,6 +1086,25 @@ update_file_size(unsigned offset, unsigned size)
  	file_size = offset + size;
  }
 
+static int is_power_of_2(unsigned n) {
+	return ((n & (n - 1)) == 0);
+}
+
+/*
+ * Round down n to nearest power of 2.
+ * If n is already a power of 2, return n;

ltp/fsx.c +16 -1

+
+		/* atomic writes need naturally aligned offsets */
+		offset -= offset % size;
+
+		/* Skip the write if we are crossing max filesize */
+		if ((offset + size) > maxfilelen) {
+			if (!quiet && testcalls > simulatedopcount)
+				prt("skipping atomic write past maxfilelen\n");
+			log4(OP_WRITE_ATOMIC, offset, size, FL_SKIPPED);
+			return;
+		}
+	}
  	if (size == 0) {
  		if (!quiet && testcalls > simulatedopcount && !o_direct)
  			prt("skipping zero size write\n");

@@ -1088,7 +1141,10 @@ dowrite(unsigned offset, unsigned size, int flags)
  		return;
  	}
 
-	log4(OP_WRITE, offset, size, FL_NONE);
+	if (flags & RWF_ATOMIC)
+		log4(OP_WRITE_ATOMIC, offset, size, FL_NONE);
+	else
+		log4(OP_WRITE, offset, size, FL_NONE);
 
  	gendata(original_buf, good_buf, offset, size);
  	if (offset + size > file_size) {

ltp/fsx.c +17 -0

  	iret = fsxwrite(fd, good_buf + offset, size, offset, flags);
  	if (iret != size) {
  		if (iret == -1)

@@ -1785,6 +1842,36 @@ do_dedupe_range(unsigned offset, unsigned length, unsigned dest)
  }
  #endif
 
+int test_atomic_writes(void) {
+	int ret;
+	struct statx stx;
+
+	if (o_direct != O_DIRECT) {
+		fprintf(stderr, "main: atomic writes need O_DIRECT (-Z), "
+				"disabling!\n");
+		return 0;
+	}
+
+	ret = xfstests_statx(AT_FDCWD, fname, 0, STATX_WRITE_ATOMIC, &stx);
+	if (ret < 0) {
+		fprintf(stderr, "main: Statx failed with %d."
+			" Failed to determine atomic write limits, "
+			" disabling!\n", ret);
+		return 0;
+	}

ltp/fsx.c +16 -1

+	return 0;
+}
+
  #ifdef HAVE_COPY_FILE_RANGE
  int
  test_copy_range(void)

@@ -2356,6 +2443,12 @@ have_op:
  			goto out;
  		}
  		break;
+	case OP_WRITE_ATOMIC:
+		if (!do_atomic_writes) {
+			log4(OP_WRITE_ATOMIC, offset, size, FL_SKIPPED);
+			goto out;
+		}
+		break;
  	}
 
  	switch (op) {

@@ -2385,6 +2478,11 @@ have_op:
  			dowrite(offset, size, 0);
  		break;
 
+	case OP_WRITE_ATOMIC:
+		TRIM_OFF_LEN(offset, size, maxfilelen);
+		dowrite(offset, size, RWF_ATOMIC);
+		break;
+
  	case OP_MAPREAD:
  		TRIM_OFF_LEN(offset, size, file_size);
  		domapread(offset, size);

@@ -2511,13 +2609,14 @@ void
  usage(void)
  {
  	fprintf(stdout, "usage: %s",
-		"fsx [-dfhknqxyzBEFHIJKLORWXZ0]\n\
+		"fsx [-adfhknqxyzBEFHIJKLORWXZ0]\n\
  	   [-b opnum] [-c Prob] [-g filldata] [-i logdev] [-j logid]\n\
  	   [-l flen] [-m start:end] [-o oplen] [-p progressinterval]\n\
  	   [-r readbdy] [-s style] [-t truncbdy] [-w writebdy]\n\
  	   [-A|-U] [-D startingop] [-N numops] [-P dirpath] [-S seed]\n\
  	   [--replay-ops=opsfile] [--record-ops[=opsfile]] [--duration=seconds]\n\
  	   ... fname\n\
+	-a: disable atomic writes\n\
  	-b opnum: beginning operation number (default 1)\n\
  	-c P: 1 in P chance of file close+open at each op (default infinity)\n\
  	-d: debug output for all operations\n\

ltp/fsx.c +3 -0

+			break;
  		case 'b':
  			simulatedopcount = getnum(optarg, &endp);
  			if (!quiet)

@@ -3475,6 +3578,8 @@ main(int argc, char **argv)
  		exchange_range_calls = test_exchange_range();
  	if (dontcache_io)
  		dontcache_io = test_dontcache_io();
+	if (do_atomic_writes)
+		do_atomic_writes = test_atomic_writes();
 
  	while (keep_running())
  		if (!test())

This adds atomic write test using fio based on it's crc check verifier. fio adds a crc header for each data block, which is verified later to ensure there is no data corruption or torn write.

This test essentially does a lot of parallel RWF_ATOMIC IO on a preallocated file to stress the write and end-io unwritten conversion code paths. The idea is to increase code coverage to ensure RWF_ATOMIC hasn't introduced any issues.

Avoid doing overlapping parallel atomic writes because it might give unexpected results. Use offset_increment=, size= fio options to achieve this behavior.

Co-developed-by Ritesh Harjani (IBM)<[email protected]>

Signed-off-by Ritesh Harjani (IBM)<[email protected]>

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

Signed-off-by Ojaswin Mujoo<[email protected]>

This test uses fio to first create a file with mixed mappings. Then it does atomic writes using aio dio with parallel jobs to the same file with mixed mappings. Finally, we perform a fio verify step to ensure there is no data corruption or torn write.

The aim is to stress the FS block allocation and extent handling logic to ensure it handles mixed mappings with RWF_ATOMIC correctly without tearing or losing data.

Avoid doing overlapping parallel atomic writes because it might give unexpected results. Use offset_increment=, size= fio options to achieve this behavior.

Co-developed-by Ritesh Harjani (IBM)<[email protected]>

Signed-off-by Ritesh Harjani (IBM)<[email protected]>

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

Signed-off-by Ojaswin Mujoo<[email protected]>

This adds various atomic write multi-fsblock stresst tests

with mixed mappings and O_SYNC, to ensure the data and metadata is atomically persisted even if there is a shutdown.

Suggested-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

---
  tests/generic/1228     | 137 +++++++++++++++++++++++++++++++++++++++++
  tests/generic/1228.out |   2 +
  2 files changed, 139 insertions(+)

create mode 100755 tests/generic/1228
create mode 100644 tests/generic/1228.out

tests/generic/1228 new file +112 -0

new file mode 100755
index 00000000..888599ce
@@ -0,0 +1,137 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 IBM Corporation. All Rights Reserved.
+#
+# FS QA Test 1228
+#
+# Atomic write multi-fsblock data integrity tests with mixed mappings
+# and O_SYNC
+#
+. ./common/preamble
+. ./common/atomicwrites
+_begin_fstest auto quick rw atomicwrites
+
+_require_scratch_write_atomic_multi_fsblock
+_require_atomic_write_test_commands
+_require_scratch_shutdown
+_require_xfs_io_command "truncate"
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount >> $seqres.full
+
+check_data_integrity() {
+	actual=$(_hexdump $testfile)
+	if [[ "$expected" != "$actual" ]]
+	then
+		echo "Integrity check failed"
+		echo "Integrity check failed" >> $seqres.full
+		echo "# Expected file contents:" >> $seqres.full
+		echo "$expected" >> $seqres.full
+		echo "# Actual file contents:" >> $seqres.full
+		echo "$actual" >> $seqres.full
+
+		_fail "Data integrity check failed. The atomic write was torn."
+	fi
+}
+
+prep_mixed_mapping() {
+	$XFS_IO_PROG -c "truncate 0" $testfile >> $seqres.full
+	local off=0
+	local mapping=""
+
+	local operations=("W" "H" "U")
+	local num_blocks=$((awu_max / blksz))
+	for ((i=0; i<num_blocks; i++)); do
+		local index=$((RANDOM % ${#operations[@]}))
+		local map="${operations[$index]}"
+		local mapping="${mapping}${map}"
+
+		case "$map" in
+			"W")
+				$XFS_IO_PROG -dc "pwrite -S 0x61 -b $blksz $off $blksz" $testfile > /dev/null
+				;;
+			"H")
+				# No operation needed for hole
+				;;
+			"U")
+				$XFS_IO_PROG -c "falloc $off $blksz" $testfile >> /dev/null
+				;;
+		esac
+		off=$((off + blksz))
+	done
+
+	echo "+ + Mixed mapping prep done. Full mapping pattern: $mapping" >> $seqres.full
+
+	sync $testfile
+}
+
+verify_atomic_write() {
+	test $bytes_written -eq $awu_max || _fail "atomic write len=$awu_max assertion failed"
+	check_data_integrity
+}
+
+mixed_mapping_test() {
+	prep_mixed_mapping
+
+	echo -"+ + Performing O_DSYNC atomic write from 0 to $awu_max" >> $seqres.full
+	if [[ "$1" == "shutdown" ]]
+	then
+		bytes_written=$($XFS_IO_PROG -x -dc \
+				"pwrite -DA -V1 -b $awu_max 0 $awu_max" \
+				-c "shutdown" $testfile | grep wrote | \
+				awk -F'[/ ]' '{print $2}')
+		_scratch_cycle_mount >>$seqres.full 2>&1 || _fail "remount failed"
+	else
+		bytes_written=$($XFS_IO_PROG -dc \
+				"pwrite -DA -V1 -b $awu_max 0 $awu_max" $testfile | \
+				grep wrote | awk -F'[/ ]' '{print $2}')
+	fi
+
+	verify_atomic_write
+}
+
+testfile=$SCRATCH_MNT/testfile
+touch $testfile
+
+awu_max=$(_get_atomic_write_unit_max $testfile)
+blksz=$(_get_block_size $SCRATCH_MNT)
+
+# Create an expected pattern to compare with
+$XFS_IO_PROG -tc "pwrite -b $awu_max 0 $awu_max" $testfile >> $seqres.full
+expected=$(_hexdump $testfile)
+echo "# Expected file contents:" >> $seqres.full
+echo "$expected" >> $seqres.full
+echo >> $seqres.full
+
+echo "# Test 1: Do O_DSYNC atomic write on random mixed mapping:" >> $seqres.full
+echo >> $seqres.full
+for ((iteration=1; iteration<=10; iteration++)); do
+	echo "=== Mixed Mapping Test Iteration $iteration ===" >> $seqres.full
+
+	echo "+ Testing without shutdown..." >> $seqres.full
+	mixed_mapping_test

tests/generic/1228 new file +15 -0

+
+
+echo >> $seqres.full
+echo "# Test 2: Do extending O_SYNC atomic writes: " >> $seqres.full
+bytes_written=$($XFS_IO_PROG -x -dstc "pwrite -A -V1 -b $awu_max 0 $awu_max" \
+		-c "shutdown" $testfile | grep wrote | awk -F'[/ ]' '{print $2}')
+_scratch_cycle_mount >>$seqres.full 2>&1 || _fail "remount failed"
+verify_atomic_write
+echo "# Test 2: Do extending O_SYNC atomic writes: OK" >> $seqres.full
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
+

tests/generic/1228.out new file +2 -0

new file mode 100644
index 00000000..1baffa91
@@ -0,0 +1,2 @@
+QA output created by 1228
+Silence is golden

Stress file with atomic writes to ensure we excercise codepaths

where we are mixing different FS operations with atomic writes

Suggested-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

---
  tests/generic/1229     | 68 ++++++++++++++++++++++++++++++++++++++++++
  tests/generic/1229.out |  2 ++
  2 files changed, 70 insertions(+)

create mode 100755 tests/generic/1229
create mode 100644 tests/generic/1229.out

tests/generic/1229 new file +24 -0

new file mode 100755
index 00000000..7fa57105
@@ -0,0 +1,68 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 IBM Corporation. All Rights Reserved.
+#
+# FS QA Test 1229
+#
+# fuzz fsx with atomic writes
+#
+. ./common/preamble
+. ./common/atomicwrites
+_begin_fstest rw auto quick atomicwrites
+
+_require_odirect
+_require_scratch_write_atomic
+
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount  >> $seqres.full 2>&1
+
+testfile=$SCRATCH_MNT/testfile
+touch $testfile
+
+awu_max=$(_get_atomic_write_unit_max $testfile)
+blksz=$(_get_block_size $SCRATCH_MNT)
+bsize=`$here/src/min_dio_alignment $SCRATCH_MNT $SCRATCH_DEV`

tests/generic/1229 new file +34 -0

+		# detect bigalloc to be on safer side.
+		if [ -z "$DUMPE2FS_PROG" ]; then
+			echo "dumpe2fs not found, disabling insert/collapse range" >> $seqres.full
+			FSX_AVOID+=" -I -C"
+			return
+		fi
+
+		$DUMPE2FS_PROG -h $dev 2>&1 | grep -q bigalloc && {
+			echo "fsx insert/collapse range not supported with bigalloc. Disabling.." >> $seqres.full
+			FSX_AVOID+=" -I -C"
+		}
+		;;
+	*)
+		;;
+	esac
+}
+
+# fsx usage:
+#
+# -N numops: total # operations to do
+# -l flen: the upper bound on file size
+# -o oplen: the upper bound on operation size (64k default)
+# -Z: O_DIRECT ()
+
+set_fsx_avoid
+_run_fsx_on_file $testfile -N 10000 -o $awu_max -A -l 500000 -r $bsize -w $bsize -Z $FSX_AVOID  >> $seqres.full
+if [[ "$?" != "0" ]]
+then
+	_fail "fsx returned error: $?"
+fi
+
+echo "Silence is golden"
+status=0
+exit

tests/generic/1229.out new file +2 -0

new file mode 100644
index 00000000..737d61c6
@@ -0,0 +1,2 @@
+QA output created by 1229
+Silence is golden

This test is intended to ensure that multi blocks atomic writes maintain atomic guarantees across sudden FS shutdowns.

The way we work is that we lay out a file with random mix of written, unwritten and hole extents. Then we start performing atomic writes sequentially on the file while we parallely shutdown the FS. Then we note the last offset where the atomic write happened just before shut down and then make sure blocks around it either have completely old data or completely new data, ie the write was not torn during shutdown.

We repeat the same with completely written, completely unwritten and completely empty file to ensure these cases are not torn either. Finally, we have a similar test for append atomic writes

Suggested-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

---
  tests/generic/1230     | 397 +++++++++++++++++++++++++++++++++++++++++
  tests/generic/1230.out |   2 +
  2 files changed, 399 insertions(+)

create mode 100755 tests/generic/1230
create mode 100644 tests/generic/1230.out

tests/generic/1230 new file +6 -0

new file mode 100755
index 00000000..cff5adc0
@@ -0,0 +1,397 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 IBM Corporation. All Rights Reserved.
+#
+# FS QA Test No. 1230
+#

tests/generic/1230 new file +5 -0

+
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount >> $seqres.full
+
+testfile=$SCRATCH_MNT/testfile

tests/generic/1230 new file +10 -0

+
+_cleanup() {
+	[ -n "$awloop_pid" ] && kill $awloop_pid &> /dev/null
+	wait
+}
+
+atomic_write_loop() {
+	local off=0
+	local size=$awu_max
+	for ((i=0; i<$((filesize / $size )); i++)); do

tests/generic/1230 new file +41 -0

+# 2. Sleep for 0.2s and shutdown the FS
+# 3. kill the atomic write process
+# 4. verify the writes were not torn
+#
+# We ideally want the shutdown to happen while an atomic write is ongoing
+# but this gets tricky since faster devices can actually finish the whole
+# atomic write loop before sleep 0.2s completes, resulting in the shutdown
+# happening after the write loop which is not what we want. A simple solution
+# to this is to increase $filesize so step 1 takes long enough but a big
+# $filesize leads to create_mixed_mappings() taking very long, which is not
+# ideal.
+#
+# Hence, use the dry_run function to figure out the rough device speed and set
+# $filesize accordingly.
+dry_run() {
+	echo >> $seqres.full
+	echo "# Estimating ideal filesize..." >> $seqres.full
+	atomic_write_loop &
+	awloop_pid=$!
+
+	local i=0
+	# Wait for atleast first write to be recorded or 10s
+	while [ ! -f "$tmp.aw" -a $i -le 50 ]; do i=$((i + 1)); sleep 0.2; done
+
+	if [[ $i -gt 50 ]]
+	then
+		_fail "atomic write process took too long to start"
+	fi
+
+	echo >> $seqres.full
+	echo "# Shutting down filesystem while write is running" >> $seqres.full
+	_scratch_shutdown
+
+	kill $awloop_pid 2>/dev/null  # the process might have finished already
+	wait $awloop_pid
+	unset $awloop_pid
+
+	bytes_written=$(tail -n 1 $tmp.aw | cut -d" " -f4)
+	echo "# Bytes written in 0.2s: $bytes_written" >> $seqres.full
+
+	filesize=$((bytes_written * 3))

tests/generic/1230 new file +5 -0

+	local file=$1
+	local size_bytes=$2
+
+	echo "# Filling file $file with alternate mappings till size $size_bytes" >> $seqres.full
+	#Fill the file with alternate written and unwritten blocks

tests/generic/1230 new file +40 -0

+			;;
+		    "U")
+			$XFS_IO_PROG -fc "falloc $off $blksz" $file >> /dev/null
+			;;
+		esac
+		off=$((off + blksz))
+	done
+
+	sync $file
+}
+
+populate_expected_data() {
+	# create a dummy file with expected old data for different cases
+	create_mixed_mappings $testfile.exp_old_mixed $awu_max
+	expected_data_old_mixed=$(od -An -t x1 -j 0 -N $awu_max $testfile.exp_old_mixed)
+
+	$XFS_IO_PROG -fc "falloc 0 $awu_max" $testfile.exp_old_zeroes >> $seqres.full
+	expected_data_old_zeroes=$(od -An -t x1 -j 0 -N $awu_max $testfile.exp_old_zeroes)
+
+	$XFS_IO_PROG -fc "pwrite -b $awu_max 0 $awu_max" $testfile.exp_old_mapped >> $seqres.full
+	expected_data_old_mapped=$(od -An -t x1 -j 0 -N $awu_max $testfile.exp_old_mapped)
+
+	# create a dummy file with expected new data
+	$XFS_IO_PROG -fc "pwrite -S 0x61 -b $awu_max 0 $awu_max" $testfile.exp_new >> $seqres.full
+	expected_data_new=$(od -An -t x1 -j 0 -N $awu_max $testfile.exp_new)
+}
+
+verify_data_blocks() {
+	local verify_start=$1
+	local verify_end=$2
+	local expected_data_old="$3"
+	local expected_data_new="$4"
+
+	echo >> $seqres.full
+	echo "# Checking data integrity from $verify_start to $verify_end" >> $seqres.full
+
+	# After an atomic write, for every chunk we ensure that the underlying
+	# data is either the old data or new data as writes shouldn't get torn.
+	local off=$verify_start
+	while [[ "$off" -lt "$verify_end" ]]

tests/generic/1230 new file +15 -0

+			echo
+			echo "Actual contents: "
+			echo "$actual_data"
+
+			_fail
+		fi
+		echo -n "Check at offset $off suceeded! " >> $seqres.full
+		if [[ "$actual_data" == "$expected_data_new" ]]
+		then
+			echo "matched new" >> $seqres.full
+		elif [[ "$actual_data" == "$expected_data_old" ]]
+		then
+			echo "matched old" >> $seqres.full
+		fi
+		off=$(( off + awu_max ))

tests/generic/1230 new file +6 -0

+	local i=0
+	# Wait for atleast first write to be recorded or 10s
+	while [ ! -f "$tmp.aw" -a $i -le 50 ]; do i=$((i + 1)); sleep 0.2; done
+
+	if [[ $i -gt 50 ]]
+	then

tests/generic/1230 new file +20 -0

+
+	last_offset=$(tail -n 1 $tmp.aw | cut -d" " -f4)
+	if [[ -z $last_offset ]]
+	then
+		last_offset=0
+	fi
+
+	echo >> $seqres.full
+	echo "# Last offset of atomic write: $last_offset" >> $seqres.full
+
+	rm $tmp.aw
+	sleep 0.5
+
+	_scratch_cycle_mount
+
+	# we want to verify all blocks around which the shutdown happended
+	verify_start=$(( last_offset - (awu_max * 5)))
+	if [[ $verify_start < 0 ]]
+	then
+		verify_start=0

tests/generic/1230 new file +4 -0

+test_data_integrity_mixed() {
+	$XFS_IO_PROG -fc "truncate 0" $testfile >> $seqres.full
+
+	echo >> $seqres.full

tests/generic/1230 new file +4 -0

+	$XFS_IO_PROG -fc "truncate 0" $testfile >> $seqres.full
+
+	echo >> $seqres.full
+	echo "# Creating testfile with fully written mapping" >> $seqres.full

tests/generic/1230 new file +112 -0

+	$XFS_IO_PROG -fc "truncate 0" $testfile >> $seqres.full
+
+	echo >> $seqres.full
+	echo "# Creating testfile with fully unwritten mappings" >> $seqres.full
+	$XFS_IO_PROG -c "falloc 0 $filesize" $testfile >> $seqres.full
+	sync $testfile
+
+	test_data_integrity
+
+	verify_data_blocks $verify_start $verify_end "$expected_data_old_zeroes" "$expected_data_new"
+}
+
+# test data integrity for file with no mappings
+test_data_integrity_hole() {
+	$XFS_IO_PROG -fc "truncate 0" $testfile >> $seqres.full
+
+	echo >> $seqres.full
+	echo "# Creating testfile with no mappings" >> $seqres.full
+	$XFS_IO_PROG -c "truncate $filesize" $testfile >> $seqres.full
+	sync $testfile
+
+	test_data_integrity
+
+	verify_data_blocks $verify_start $verify_end "$expected_data_old_zeroes" "$expected_data_new"
+}
+
+test_filesize_integrity() {
+	$XFS_IO_PROG -c "truncate 0" $testfile >> $seqres.full
+
+	echo >> $seqres.full
+	echo "# Performing extending atomic writes over file in background" >> $seqres.full
+	atomic_write_loop &
+	awloop_pid=$!
+
+	local i=0
+	# Wait for atleast first write to be recorded or 10s
+	while [ ! -f "$tmp.aw" -a $i -le 50 ]; do i=$((i + 1)); sleep 0.2; done
+
+	if [[ $i -gt 50 ]]
+	then
+		_fail "atomic write process took too long to start"
+	fi
+
+	echo >> $seqres.full
+	echo "# Shutting down filesystem while write is running" >> $seqres.full
+	_scratch_shutdown
+
+	kill $awloop_pid 2>/dev/null  # the process might have finished already
+	wait $awloop_pid
+	unset $awloop_pid
+
+	local last_offset=$(tail -n 1 $tmp.aw | cut -d" " -f4)
+	if [[ -z $last_offset ]]
+	then
+		last_offset=0
+	fi
+
+	echo >> $seqres.full
+	echo "# Last offset of atomic write: $last_offset" >> $seqres.full
+	rm $tmp.aw
+	sleep 0.5
+
+	_scratch_cycle_mount
+	local filesize=$(_get_filesize $testfile)
+	echo >> $seqres.full
+	echo "# Filesize after shutdown: $filesize" >> $seqres.full
+
+	# To confirm that the write went atomically, we check:
+	# 1. The last block should be a multiple of awu_max
+	# 2. The last block should be the completely new data
+
+	if (( $filesize % $awu_max ))
+	then
+		echo "Filesize after shutdown ($filesize) not a multiple of atomic write unit ($awu_max)"
+	fi
+
+	verify_start=$(( filesize - (awu_max * 5)))
+	if [[ $verify_start < 0 ]]
+	then
+		verify_start=0
+	fi
+
+	local verify_end=$filesize
+
+	# Here the blocks should always match new data hence, for simplicity of
+	# code, just corrupt the $expected_data_old buffer so it never matches
+	local expected_data_old="POISON"
+	verify_data_blocks $verify_start $verify_end "$expected_data_old" "$expected_data_new"
+}
+
+$XFS_IO_PROG -fc "truncate 0" $testfile >> $seqres.full
+
+dry_run
+
+echo >> $seqres.full
+echo "# Populating expected data buffers" >> $seqres.full
+populate_expected_data
+
+# Loop 20 times to shake out any races due to shutdown
+for ((iter=0; iter<20; iter++))
+do
+	echo >> $seqres.full
+	echo "------ Iteration $iter ------" >> $seqres.full
+
+	echo >> $seqres.full
+	echo "# Starting data integrity test for atomic writes over mixed mapping" >> $seqres.full
+	test_data_integrity_mixed
+
+	echo >> $seqres.full
+	echo "# Starting data integrity test for atomic writes over fully written mapping" >> $seqres.full
+	test_data_integrity_writ
+

tests/generic/1230 new file +6 -0

+	test_filesize_integrity
+done
+
+echo "Silence is golden"
+status=0
+exit

tests/generic/1230.out new file +2 -0

new file mode 100644
index 00000000..d01f54ea
@@ -0,0 +1,2 @@
+QA output created by 1230
+Silence is golden

From: "Ritesh Harjani (IBM)" <[email protected]>

This test does a lot of parallel RWF_ATOMIC IO on a preallocated file to stress the write and end-io unwritten conversion code paths. We brute force this for different blocksize and clustersizes and after each iteration we ensure the data was not torn or corrupted using fio crc verification.

Note that in this test we use overlapping atomic writes of same io size. Although serializing racing writes is not guaranteed for RWF_ATOMIC, NVMe and SCSI provide this guarantee as an inseparable feature to power-fail atomicity. Keeping the iosize as same also ensures that ext4 doesn't tear the write due to racing ioend unwritten conversion.

The value of this test is that we make sure the RWF_ATOMIC is handled correctly by ext4 as well as test that the block layer doesn't split or only generate multiple bios for an atomic write.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

Signed-off-by Ojaswin Mujoo <[email protected]>

From: "Ritesh Harjani (IBM)" <[email protected]>

This test does a parallel RWF_ATOMIC IO on a multiple truncated files in a small FS. The idea is to stress ext4 allocator to ensure we are able to handle low space scenarios correctly with atomic writes. We brute force this for different blocksize and clustersizes and after each iteration we ensure the data was not torn or corrupted using fio crc verification.

Note that in this test we use overlapping atomic writes of same io size. Although serializing racing writes is not guaranteed for RWF_ATOMIC, NVMe and SCSI provide this guarantee as an inseparable feature to power-fail atomicity. Keeping the iosize as same also ensures that ext4 doesn't tear the write due to racing ioend unwritten conversion.

The value of this test is that we make sure the RWF_ATOMIC is handled correctly by ext4 as well as test that the block layer doesn't split or only generate multiple bios for an atomic write.

Signed-off-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

---
  tests/ext4/062     | 203 +++++++++++++++++++++++++++++++++++++++++++++
  tests/ext4/062.out |   2 +
  2 files changed, 205 insertions(+)

create mode 100755 tests/ext4/062
create mode 100644 tests/ext4/062.out

tests/ext4/062 new file +82 -0

new file mode 100755
index 00000000..d48f69d3
@@ -0,0 +1,203 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 IBM Corporation. All Rights Reserved.
+#
+# FS QA Test 062
+#
+# This test does a parallel RWF_ATOMIC IO on a multiple truncated files in a
+# small FS. The idea is to stress ext4 allocator to ensure we are able to
+# handle low space scenarios correctly with atomic writes.. We brute force this
+# for all possible blocksize and clustersizes and after each iteration we
+# ensure the data was not torn or corrupted using fio crc verification.
+#
+# Note that in this test we use overlapping atomic writes of same io size.
+# Although serializing racing writes is not guaranteed for RWF_ATOMIC, NVMe and
+# SCSI provide this guarantee as an inseparable feature to power-fail
+# atomicity. Keeping the iosize as same also ensures that ext4 doesn't tear the
+# write due to racing ioend unwritten conversion.
+#
+# The value of this test is that we make sure the RWF_ATOMIC is handled
+# correctly by ext4 as well as test that the block layer doesn't split or only
+# generate multiple bios for an atomic write.
+#
+
+. ./common/preamble
+. ./common/atomicwrites
+
+_begin_fstest auto rw stress atomicwrites
+
+_require_scratch_write_atomic
+_require_aiodio
+_require_fio_version "3.38+"
+
+FSSIZE=$((360*1024*1024))
+FIO_LOAD=$(($(nproc) * LOAD_FACTOR))
+
+# Calculate bs as per bdev atomic write units.
+bdev_awu_min=$(_get_atomic_write_unit_min $SCRATCH_DEV)
+bdev_awu_max=$(_get_atomic_write_unit_max $SCRATCH_DEV)
+bs=$(_max 4096 "$bdev_awu_min")
+
+function create_fio_configs()
+{
+	local bsize=$1
+	create_fio_aw_config $bsize
+	create_fio_verify_config $bsize
+}
+
+function create_fio_verify_config()
+{
+	local bsize=$1
+cat >$fio_verify_config <<EOF
+	[global]
+	direct=1
+	ioengine=libaio
+	rw=read
+	bs=$bsize
+	fallocate=truncate
+	size=$((FSSIZE / 12))
+	iodepth=$FIO_LOAD
+	numjobs=$FIO_LOAD
+	group_reporting=1
+	atomic=1
+
+	verify_only=1
+	verify_state_save=0
+	verify=crc32c
+	verify_fatal=1
+	verify_write_sequence=0
+
+	[verify-job1]
+	filename=$SCRATCH_MNT/testfile-job1
+
+	[verify-job2]
+	filename=$SCRATCH_MNT/testfile-job2
+
+	[verify-job3]
+	filename=$SCRATCH_MNT/testfile-job3
+
+	[verify-job4]
+	filename=$SCRATCH_MNT/testfile-job4
+
+	[verify-job5]

Quoted 16 lines · Ojaswin Mujoo

The main motivation of adding this function on top of _require_fio is that there has been a case in fio where atomic= option was added but later it was changed to noop since kernel didn't yet have support for atomic writes. It was then again utilized to do atomic writes in a later version, once kernel got the support. Due to this there is a point in fio where _require_fio w/ atomic=1 will succeed even though it would not be doing atomic writes.

Hence, add an explicit helper to ensure tests to require specific versions of fio to work past such issues.

Signed-off-by Ojaswin Mujoo <[email protected]>

---
  common/rc | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)

common/rc _max() { · line 6002

Quoted 11 lines

common/rc +4 -0

index 35a1c835..f45b9a38 100644
@@ -5997,6 +5997,38 @@ _max() {
  	echo $ret
  }
+# Check the required fio version. Examples:
+#   _require_fio_version 3.38 (matches 3.38 only)
+#   _require_fio_version 3.38+ (matches 3.38 and above)
+#   _require_fio_version 3.38- (matches 3.38 and below)

This requires the user to know the version which corresponds to the feature. Is that how things are done for other such utilities and their versions vs features?

I was going to suggest exporting something like _require_fio_atomic_writes(), and _require_fio_atomic_writes() calls _require_fio_version() to check the version.

Thanks,
John

Quoted 3 lines

This requires the user to know the version which corresponds to the feature. Is that how things are done for other such utilities and their versions vs features?

Hi John,

So there are not many such helpers but the 2 I could see were used this way:

tests/btrfs/284:
_require_btrfs_send_version 2

tests/nfs/001:
_require_test_nfs_version 4

So I though of keeping it this way.

Quoted 6 lines · Ojaswin Mujoo

Implement atomic write support to help fuzz atomic writes with fsx.

Suggested-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

Generally this looks ok, but I do have some comments, so please check them:

Reviewed-by John Garry <[email protected]>

Quoted 73 lines

---
  ltp/fsx.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 110 insertions(+), 5 deletions(-)

ltp/fsx.c +24 -0

index 163b9453..1582f6d1 100644
@@ -40,6 +40,7 @@
  #include <liburing.h>
  #endif
  #include <sys/syscall.h>
+#include "statx.h"
  #ifndef MAP_FILE
  # define MAP_FILE 0

@@ -49,6 +50,10 @@
  #define RWF_DONTCACHE	0x80
  #endif
+#ifndef RWF_ATOMIC
+#define RWF_ATOMIC	0x40
+#endif
+
  #define NUMPRINTCOLUMNS 32	/* # columns of data to print on each line */
  /* Operation flags (bitmask) */

@@ -110,6 +115,7 @@ enum {
  	OP_READ_DONTCACHE,
  	OP_WRITE,
  	OP_WRITE_DONTCACHE,
+	OP_WRITE_ATOMIC,
  	OP_MAPREAD,
  	OP_MAPWRITE,
  	OP_MAX_LITE,

@@ -200,6 +206,11 @@ int	uring = 0;
  int	mark_nr = 0;
  int	dontcache_io = 1;
  int	hugepages = 0;                  /* -h flag */
+int	do_atomic_writes = 1;		/* -a flag disables */
+
+/* User for atomic writes */
+int awu_min = 0;
+int awu_max = 0;
  /* Stores info needed to periodically collapse hugepages */
  struct hugepages_collapse_info {

@@ -288,6 +299,7 @@ static const char *op_names[] = {
  	[OP_READ_DONTCACHE] = "read_dontcache",
  	[OP_WRITE] = "write",
  	[OP_WRITE_DONTCACHE] = "write_dontcache",
+	[OP_WRITE_ATOMIC] = "write_atomic",
  	[OP_MAPREAD] = "mapread",
  	[OP_MAPWRITE] = "mapwrite",
  	[OP_TRUNCATE] = "truncate",

@@ -422,6 +434,7 @@ logdump(void)
  				prt("\t***RRRR***");
  			break;
  		case OP_WRITE_DONTCACHE:
+		case OP_WRITE_ATOMIC:
  		case OP_WRITE:
  			prt("WRITE    0x%x thru 0x%x\t(0x%x bytes)",
  			    lp->args[0], lp->args[0] + lp->args[1] - 1,

@@ -1073,6 +1086,25 @@ update_file_size(unsigned offset, unsigned size)
  	file_size = offset + size;
  }
+static int is_power_of_2(unsigned n) {
+	return ((n & (n - 1)) == 0);
+}
+
+/*
+ * Round down n to nearest power of 2.
+ * If n is already a power of 2, return n;
+ */
+static int rounddown_pow_of_2(int n) {
+	int i = 0;
+

ltp/fsx.c update_file_size(unsigned offset, unsigned size) · line 1105

Quoted 6 lines

ltp/fsx.c +6 -0

+	if (is_power_of_2(n))
+		return n;
+
+	for (; (1 << i) < n; i++);
+
+	return 1 << (i - 1);

Is this the neatest way to do this?

ltp/fsx.c dowrite(unsigned offset, unsigned size, int flags) · line 1117

Quoted 12 lines

ltp/fsx.c +4 -0

+}
+
  void
  dowrite(unsigned offset, unsigned size, int flags)
  {

@@ -1081,6 +1113,27 @@ dowrite(unsigned offset, unsigned size, int flags)
  	offset -= offset % writebdy;
  	if (o_direct)
  		size -= size % writebdy;
+	if (flags & RWF_ATOMIC) {
+		/* atomic write len must be inbetween awu_min and awu_max */

in between

ltp/fsx.c dowrite(unsigned offset, unsigned size, int flags) · line 1124

Quoted 7 lines

ltp/fsx.c +7 -0

+		if (size < awu_min)
+			size = awu_min;
+		if (size > awu_max)
+			size = awu_max;
+
+		/* atomic writes need power-of-2 sizes */
+		size = rounddown_pow_of_2(size);

you could have:

if (size < awu_min)

	size = awu_min;
else if (size > awu_max)
	size = awu_max;
else
	size = rounddown_pow_of_2(size);

Quoted 27 lines

ltp/fsx.c +16 -1

+
+		/* atomic writes need naturally aligned offsets */
+		offset -= offset % size;
+
+		/* Skip the write if we are crossing max filesize */
+		if ((offset + size) > maxfilelen) {
+			if (!quiet && testcalls > simulatedopcount)
+				prt("skipping atomic write past maxfilelen\n");
+			log4(OP_WRITE_ATOMIC, offset, size, FL_SKIPPED);
+			return;
+		}
+	}
  	if (size == 0) {
  		if (!quiet && testcalls > simulatedopcount && !o_direct)
  			prt("skipping zero size write\n");

@@ -1088,7 +1141,10 @@ dowrite(unsigned offset, unsigned size, int flags)
  		return;
  	}
-	log4(OP_WRITE, offset, size, FL_NONE);
+	if (flags & RWF_ATOMIC)
+		log4(OP_WRITE_ATOMIC, offset, size, FL_NONE);
+	else
+		log4(OP_WRITE, offset, size, FL_NONE);
  	gendata(original_buf, good_buf, offset, size);
  	if (offset + size > file_size) {

ltp/fsx.c dowrite(unsigned offset, unsigned size, int flags) · line 1169

Quoted 8 lines

ltp/fsx.c +3 -2

@@ -1108,8 +1164,9 @@ dowrite(unsigned offset, unsigned size, int flags)
  		       (monitorstart == -1 ||
  			(offset + size > monitorstart &&
  			(monitorend == -1 || offset <= monitorend))))))
-		prt("%lld write\t0x%x thru\t0x%x\t(0x%x bytes)\tdontcache=%d\n", testcalls,
-		    offset, offset + size - 1, size, (flags & RWF_DONTCACHE) != 0);
+		prt("%lld write\t0x%x thru\t0x%x\t(0x%x bytes)\tdontcache=%d atomic_wr=%d\n", testcalls,
+		    offset, offset + size - 1, size, (flags & RWF_DONTCACHE) != 0,
+		    (flags & RWF_ATOMIC) != 0);

nit:

	!!(flags & RWF_ATOMIC)

I find that a bit neater, but I suppose you are following the example for RWF_DONTCACHE

Quoted 24 lines

ltp/fsx.c +17 -0

  	iret = fsxwrite(fd, good_buf + offset, size, offset, flags);
  	if (iret != size) {
  		if (iret == -1)

@@ -1785,6 +1842,36 @@ do_dedupe_range(unsigned offset, unsigned length, unsigned dest)
  }
  #endif
+int test_atomic_writes(void) {
+	int ret;
+	struct statx stx;
+
+	if (o_direct != O_DIRECT) {
+		fprintf(stderr, "main: atomic writes need O_DIRECT (-Z), "
+				"disabling!\n");
+		return 0;
+	}
+
+	ret = xfstests_statx(AT_FDCWD, fname, 0, STATX_WRITE_ATOMIC, &stx);
+	if (ret < 0) {
+		fprintf(stderr, "main: Statx failed with %d."
+			" Failed to determine atomic write limits, "
+			" disabling!\n", ret);
+		return 0;
+	}

ltp/fsx.c do_dedupe_range(unsigned offset, unsigned length, unsigned dest) · line 1870

Quoted 10 lines

ltp/fsx.c +10 -0

+
+	if (stx.stx_attributes & STATX_ATTR_WRITE_ATOMIC &&
+	    stx.stx_atomic_write_unit_min > 0) {
+		awu_min = stx.stx_atomic_write_unit_min;
+		awu_max = stx.stx_atomic_write_unit_max;
+		return 1;
+	}
+
+	fprintf(stderr, "main: IO Stack does not support "
+			"atomic writes, disabling!\n");

Do we really need to spread this over multiple lines?

Maybe that is the coding standard - I don't know.

Quoted 50 lines

ltp/fsx.c +16 -2

+	return 0;
+}
+
  #ifdef HAVE_COPY_FILE_RANGE
  int
  test_copy_range(void)

@@ -2356,6 +2443,12 @@ have_op:
  			goto out;
  		}
  		break;
+	case OP_WRITE_ATOMIC:
+		if (!do_atomic_writes) {
+			log4(OP_WRITE_ATOMIC, offset, size, FL_SKIPPED);
+			goto out;
+		}
+		break;
  	}
  	switch (op) {

@@ -2385,6 +2478,11 @@ have_op:
  			dowrite(offset, size, 0);
  		break;
+	case OP_WRITE_ATOMIC:
+		TRIM_OFF_LEN(offset, size, maxfilelen);
+		dowrite(offset, size, RWF_ATOMIC);
+		break;
+
  	case OP_MAPREAD:
  		TRIM_OFF_LEN(offset, size, file_size);
  		domapread(offset, size);

@@ -2511,13 +2609,14 @@ void
  usage(void)
  {
  	fprintf(stdout, "usage: %s",
-		"fsx [-dfhknqxyzBEFHIJKLORWXZ0]\n\
+		"fsx [-adfhknqxyzBEFHIJKLORWXZ0]\n\
  	   [-b opnum] [-c Prob] [-g filldata] [-i logdev] [-j logid]\n\
  	   [-l flen] [-m start:end] [-o oplen] [-p progressinterval]\n\
  	   [-r readbdy] [-s style] [-t truncbdy] [-w writebdy]\n\
  	   [-A|-U] [-D startingop] [-N numops] [-P dirpath] [-S seed]\n\
  	   [--replay-ops=opsfile] [--record-ops[=opsfile]] [--duration=seconds]\n\
  	   ... fname\n\
+	-a: disable atomic writes\n\
  	-b opnum: beginning operation number (default 1)\n\
  	-c P: 1 in P chance of file close+open at each op (default infinity)\n\
  	-d: debug output for all operations\n\

@@ -3059,9 +3158,13 @@ main(int argc, char **argv)
  	setvbuf(stdout, (char *)0, _IOLBF, 0); /* line buffered stdout */
  	while ((ch = getopt_long(argc, argv,
-				 "0b:c:de:fg:hi:j:kl:m:no:p:qr:s:t:uw:xyABD:EFJKHzCILN:OP:RS:UWXZ",

ltp/fsx.c main(int argc, char **argv) · line 3166

Quoted 6 lines

ltp/fsx.c +4 -0

+				 "0ab:c:de:fg:hi:j:kl:m:no:p:qr:s:t:uw:xyABD:EFJKHzCILN:OP:RS:UWXZ",
  				 longopts, NULL)) != EOF)
  		switch (ch) {
+		case 'a':
+			prt("main(): Atomic writes disabled\n");
+			do_atomic_writes = 0;

why an opt-out (and not opt-in)?

Quoted 13 lines

ltp/fsx.c +3 -0

+			break;
  		case 'b':
  			simulatedopcount = getnum(optarg, &endp);
  			if (!quiet)

@@ -3475,6 +3578,8 @@ main(int argc, char **argv)
  		exchange_range_calls = test_exchange_range();
  	if (dontcache_io)
  		dontcache_io = test_dontcache_io();
+	if (do_atomic_writes)
+		do_atomic_writes = test_atomic_writes();
  	while (keep_running())
  		if (!test())

Quoted 1 line · Ojaswin Mujoo

This adds various atomic write multi-fsblock stresst tests

stress

Quoted 6 lines

with mixed mappings and O_SYNC, to ensure the data and metadata is atomically persisted even if there is a shutdown.

Suggested-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

Regardless of any comments made (which are minor):

Reviewed-by John Garry <[email protected]>

Quoted 129 lines

---
  tests/generic/1228     | 137 +++++++++++++++++++++++++++++++++++++++++
  tests/generic/1228.out |   2 +
  2 files changed, 139 insertions(+)

create mode 100755 tests/generic/1228
create mode 100644 tests/generic/1228.out

tests/generic/1228 new file +116 -0

new file mode 100755
index 00000000..888599ce
@@ -0,0 +1,137 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 IBM Corporation. All Rights Reserved.
+#
+# FS QA Test 1228
+#
+# Atomic write multi-fsblock data integrity tests with mixed mappings
+# and O_SYNC
+#
+. ./common/preamble
+. ./common/atomicwrites
+_begin_fstest auto quick rw atomicwrites
+
+_require_scratch_write_atomic_multi_fsblock
+_require_atomic_write_test_commands
+_require_scratch_shutdown
+_require_xfs_io_command "truncate"
+
+_scratch_mkfs >> $seqres.full
+_scratch_mount >> $seqres.full
+
+check_data_integrity() {
+	actual=$(_hexdump $testfile)
+	if [[ "$expected" != "$actual" ]]
+	then
+		echo "Integrity check failed"
+		echo "Integrity check failed" >> $seqres.full
+		echo "# Expected file contents:" >> $seqres.full
+		echo "$expected" >> $seqres.full
+		echo "# Actual file contents:" >> $seqres.full
+		echo "$actual" >> $seqres.full
+
+		_fail "Data integrity check failed. The atomic write was torn."
+	fi
+}
+
+prep_mixed_mapping() {
+	$XFS_IO_PROG -c "truncate 0" $testfile >> $seqres.full
+	local off=0
+	local mapping=""
+
+	local operations=("W" "H" "U")
+	local num_blocks=$((awu_max / blksz))
+	for ((i=0; i<num_blocks; i++)); do
+		local index=$((RANDOM % ${#operations[@]}))
+		local map="${operations[$index]}"
+		local mapping="${mapping}${map}"
+
+		case "$map" in
+			"W")
+				$XFS_IO_PROG -dc "pwrite -S 0x61 -b $blksz $off $blksz" $testfile > /dev/null
+				;;
+			"H")
+				# No operation needed for hole
+				;;
+			"U")
+				$XFS_IO_PROG -c "falloc $off $blksz" $testfile >> /dev/null
+				;;
+		esac
+		off=$((off + blksz))
+	done
+
+	echo "+ + Mixed mapping prep done. Full mapping pattern: $mapping" >> $seqres.full
+
+	sync $testfile
+}
+
+verify_atomic_write() {
+	test $bytes_written -eq $awu_max || _fail "atomic write len=$awu_max assertion failed"
+	check_data_integrity
+}
+
+mixed_mapping_test() {
+	prep_mixed_mapping
+
+	echo -"+ + Performing O_DSYNC atomic write from 0 to $awu_max" >> $seqres.full
+	if [[ "$1" == "shutdown" ]]
+	then
+		bytes_written=$($XFS_IO_PROG -x -dc \
+				"pwrite -DA -V1 -b $awu_max 0 $awu_max" \
+				-c "shutdown" $testfile | grep wrote | \
+				awk -F'[/ ]' '{print $2}')
+		_scratch_cycle_mount >>$seqres.full 2>&1 || _fail "remount failed"
+	else
+		bytes_written=$($XFS_IO_PROG -dc \
+				"pwrite -DA -V1 -b $awu_max 0 $awu_max" $testfile | \
+				grep wrote | awk -F'[/ ]' '{print $2}')
+	fi
+
+	verify_atomic_write
+}
+
+testfile=$SCRATCH_MNT/testfile
+touch $testfile
+
+awu_max=$(_get_atomic_write_unit_max $testfile)
+blksz=$(_get_block_size $SCRATCH_MNT)
+
+# Create an expected pattern to compare with
+$XFS_IO_PROG -tc "pwrite -b $awu_max 0 $awu_max" $testfile >> $seqres.full
+expected=$(_hexdump $testfile)
+echo "# Expected file contents:" >> $seqres.full
+echo "$expected" >> $seqres.full
+echo >> $seqres.full
+
+echo "# Test 1: Do O_DSYNC atomic write on random mixed mapping:" >> $seqres.full
+echo >> $seqres.full
+for ((iteration=1; iteration<=10; iteration++)); do
+	echo "=== Mixed Mapping Test Iteration $iteration ===" >> $seqres.full
+
+	echo "+ Testing without shutdown..." >> $seqres.full
+	mixed_mapping_test
+	echo "Passed!" >> $seqres.full
+
+	echo "+ Testing with sudden shutdown..." >> $seqres.full
+	mixed_mapping_test "shutdown"

tests/generic/1228 line 122

Quoted 6 lines

tests/generic/1228 new file +6 -0

+	echo "Passed!" >> $seqres.full
+
+	echo "Iteration $iteration completed: OK" >> $seqres.full
+	echo >> $seqres.full
+done
+echo "# Test 1: Do O_SYNC atomic write on random mixed mapping (10 iterations): OK" >> $seqres.full

big nit: you should keep the loop count in a global variable

Quoted 25 lines

tests/generic/1228 new file +15 -0

+
+
+echo >> $seqres.full
+echo "# Test 2: Do extending O_SYNC atomic writes: " >> $seqres.full
+bytes_written=$($XFS_IO_PROG -x -dstc "pwrite -A -V1 -b $awu_max 0 $awu_max" \
+		-c "shutdown" $testfile | grep wrote | awk -F'[/ ]' '{print $2}')
+_scratch_cycle_mount >>$seqres.full 2>&1 || _fail "remount failed"
+verify_atomic_write
+echo "# Test 2: Do extending O_SYNC atomic writes: OK" >> $seqres.full
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
+

tests/generic/1228.out new file +2 -0

new file mode 100644
index 00000000..1baffa91
@@ -0,0 +1,2 @@
+QA output created by 1228
+Silence is golden

Quoted 14 lines · Ojaswin Mujoo

Quoted 3 lines

This requires the user to know the version which corresponds to the feature. Is that how things are done for other such utilities and their versions vs features?

Hi John,

So there are not many such helpers but the 2 I could see were used this way:

tests/btrfs/284:
_require_btrfs_send_version 2

tests/nfs/001:
_require_test_nfs_version 4

Quoted 1 line

So I though of keeping it this way.

What about the example of _require_xfs_io_command param, which checks if $param is supported?

We could have _require_fio_option atomics, which checks if a specific version is available which supports atomic? Or a more straightforward would be _require_fio_with_atomics.

Cheers

Quoted 1 line · Ojaswin Mujoo

Stress file with atomic writes to ensure we excercise codepaths

exercise

Quoted 5 lines

where we are mixing different FS operations with atomic writes

Suggested-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

Reviewed-by John Garry <[email protected]>

Quoted 10 lines · Ojaswin Mujoo

This test is intended to ensure that multi blocks atomic writes maintain atomic guarantees across sudden FS shutdowns.

The way we work is that we lay out a file with random mix of written, unwritten and hole extents. Then we start performing atomic writes sequentially on the file while we parallely shutdown the FS. Then we note the last offset where the atomic write happened just before shut down and then make sure blocks around it either have completely old data or completely new data, ie the write was not torn during shutdown.

Quoted 7 lines

We repeat the same with completely written, completely unwritten and completely empty file to ensure these cases are not torn either. Finally, we have a similar test for append atomic writes

Suggested-by Ritesh Harjani (IBM) <[email protected]>

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

Signed-off-by Ojaswin Mujoo <[email protected]>

Please check comments, below, thanks!

Quoted 23 lines

---
  tests/generic/1230     | 397 +++++++++++++++++++++++++++++++++++++++++
  tests/generic/1230.out |   2 +
  2 files changed, 399 insertions(+)

create mode 100755 tests/generic/1230
create mode 100644 tests/generic/1230.out

tests/generic/1230 new file +10 -0

new file mode 100755
index 00000000..cff5adc0
@@ -0,0 +1,397 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 IBM Corporation. All Rights Reserved.
+#
+# FS QA Test No. 1230
+#
+# Test multi block atomic writes with sudden FS shutdowns to ensure
+# the FS is not tearing the write operation
+. ./common/preamble
+. ./common/atomicwrites

tests/generic/1230 line 16

Quoted 6 lines

tests/generic/1230 new file +6 -0

+_begin_fstest auto atomicwrites
+
+_require_scratch_write_atomic_multi_fsblock
+_require_atomic_write_test_commands
+_require_scratch_shutdown
+_require_xfs_io_command "truncate"

is a similar fallocate test needed?

Quoted 10 lines

tests/generic/1230 new file +9 -0

+
+_scratch_mkfs >> $seqres.full 2>&1
+_scratch_mount >> $seqres.full
+
+testfile=$SCRATCH_MNT/testfile
+touch $testfile
+
+awu_max=$(_get_atomic_write_unit_max $testfile)
+blksz=$(_get_block_size $SCRATCH_MNT)

tests/generic/1230 line 31

Quoted 6 lines

tests/generic/1230 new file +6 -0

+echo "Awu max: $awu_max" >> $seqres.full
+
+num_blocks=$((awu_max / blksz))
+# keep initial value high for dry run. This will be
+# tweaked in dry_run() based on device write speed.
+filesize=$(( 10 * 1024 * 1024 * 1024 ))

could this cause some out-of-space issue? That's 10GB, right?

Quoted 15 lines

tests/generic/1230 new file +14 -0

+
+_cleanup() {
+	[ -n "$awloop_pid" ] && kill $awloop_pid &> /dev/null
+	wait
+}
+
+atomic_write_loop() {
+	local off=0
+	local size=$awu_max
+	for ((i=0; i<$((filesize / $size )); i++)); do
+		# Due to sudden shutdown this can produce errors so just
+		# redirect them to seqres.full
+		$XFS_IO_PROG -c "open -fsd $testfile" -c "pwrite -S 0x61 -DA -V1 -b $size $off $size" >> /dev/null 2>>$seqres.full
+		echo "Written to offset: $off" >> $tmp.aw

tests/generic/1230 line 51

Quoted 6 lines

tests/generic/1230 new file +6 -0

+		off=$((off + $size))
+	done
+}
+
+# This test has the following flow:
+# 1. Start doing sequential atomic writes in bg, upto $filesize

bg?

Quoted 46 lines

tests/generic/1230 new file +45 -0

+# 2. Sleep for 0.2s and shutdown the FS
+# 3. kill the atomic write process
+# 4. verify the writes were not torn
+#
+# We ideally want the shutdown to happen while an atomic write is ongoing
+# but this gets tricky since faster devices can actually finish the whole
+# atomic write loop before sleep 0.2s completes, resulting in the shutdown
+# happening after the write loop which is not what we want. A simple solution
+# to this is to increase $filesize so step 1 takes long enough but a big
+# $filesize leads to create_mixed_mappings() taking very long, which is not
+# ideal.
+#
+# Hence, use the dry_run function to figure out the rough device speed and set
+# $filesize accordingly.
+dry_run() {
+	echo >> $seqres.full
+	echo "# Estimating ideal filesize..." >> $seqres.full
+	atomic_write_loop &
+	awloop_pid=$!
+
+	local i=0
+	# Wait for atleast first write to be recorded or 10s
+	while [ ! -f "$tmp.aw" -a $i -le 50 ]; do i=$((i + 1)); sleep 0.2; done
+
+	if [[ $i -gt 50 ]]
+	then
+		_fail "atomic write process took too long to start"
+	fi
+
+	echo >> $seqres.full
+	echo "# Shutting down filesystem while write is running" >> $seqres.full
+	_scratch_shutdown
+
+	kill $awloop_pid 2>/dev/null  # the process might have finished already
+	wait $awloop_pid
+	unset $awloop_pid
+
+	bytes_written=$(tail -n 1 $tmp.aw | cut -d" " -f4)
+	echo "# Bytes written in 0.2s: $bytes_written" >> $seqres.full
+
+	filesize=$((bytes_written * 3))
+	echo "# Setting \$filesize=$filesize" >> $seqres.full
+
+	rm $tmp.aw
+	sleep 0.5

tests/generic/1230 line 102

Quoted 6 lines

tests/generic/1230 new file +6 -0

+
+	_scratch_cycle_mount
+
+}
+
+create_mixed_mappings() {

Is this same as patch 08/12?

Quoted 10 lines

tests/generic/1230 new file +9 -0

+	local file=$1
+	local size_bytes=$2
+
+	echo "# Filling file $file with alternate mappings till size $size_bytes" >> $seqres.full
+	#Fill the file with alternate written and unwritten blocks
+	local off=0
+	local operations=("W" "U")
+
+	for ((i=0; i<$((size_bytes / blksz )); i++)); do

tests/generic/1230 line 117

Quoted 6 lines

tests/generic/1230 new file +6 -0

+		index=$(($i % ${#operations[@]}))
+		map="${operations[$index]}"
+
+		case "$map" in
+		    "W")
+			$XFS_IO_PROG -fc "pwrite -b $blksz $off $blksz" $file  >> /dev/null

does this just write random data? I don't see any pattern being set.

Quoted 47 lines

tests/generic/1230 new file +46 -0

+			;;
+		    "U")
+			$XFS_IO_PROG -fc "falloc $off $blksz" $file >> /dev/null
+			;;
+		esac
+		off=$((off + blksz))
+	done
+
+	sync $file
+}
+
+populate_expected_data() {
+	# create a dummy file with expected old data for different cases
+	create_mixed_mappings $testfile.exp_old_mixed $awu_max
+	expected_data_old_mixed=$(od -An -t x1 -j 0 -N $awu_max $testfile.exp_old_mixed)
+
+	$XFS_IO_PROG -fc "falloc 0 $awu_max" $testfile.exp_old_zeroes >> $seqres.full
+	expected_data_old_zeroes=$(od -An -t x1 -j 0 -N $awu_max $testfile.exp_old_zeroes)
+
+	$XFS_IO_PROG -fc "pwrite -b $awu_max 0 $awu_max" $testfile.exp_old_mapped >> $seqres.full
+	expected_data_old_mapped=$(od -An -t x1 -j 0 -N $awu_max $testfile.exp_old_mapped)
+
+	# create a dummy file with expected new data
+	$XFS_IO_PROG -fc "pwrite -S 0x61 -b $awu_max 0 $awu_max" $testfile.exp_new >> $seqres.full
+	expected_data_new=$(od -An -t x1 -j 0 -N $awu_max $testfile.exp_new)
+}
+
+verify_data_blocks() {
+	local verify_start=$1
+	local verify_end=$2
+	local expected_data_old="$3"
+	local expected_data_new="$4"
+
+	echo >> $seqres.full
+	echo "# Checking data integrity from $verify_start to $verify_end" >> $seqres.full
+
+	# After an atomic write, for every chunk we ensure that the underlying
+	# data is either the old data or new data as writes shouldn't get torn.
+	local off=$verify_start
+	while [[ "$off" -lt "$verify_end" ]]
+	do
+		#actual_data=$(xxd -s $off -l $awu_max -p $testfile)
+		actual_data=$(od -An -t x1 -j $off -N $awu_max $testfile)
+		if [[ "$actual_data" != "$expected_data_new" ]] && [[ "$actual_data" != "$expected_data_old" ]]
+		then
+			echo "Checksum match failed at off: $off size: $awu_max"

tests/generic/1230 line 167

Quoted 4 lines

tests/generic/1230 new file +4 -0

+			echo "Expected contents: (Either of the 2 below):"
+			echo
+			echo "Expected old: "
+			echo "$expected_data_old"

it would be nice if this was deterministic - see comment in create_mixed_mappings

tests/generic/1230 line 170

Quoted 4 lines

tests/generic/1230 new file +3 -0

+			echo
+			echo "Expected new: "
+			echo "$expected_data_new"

nit: I am not sure what is meant by "expected". I would just have "new data". We don't know what to expect, as it could be old or new, right?

Quoted 16 lines

tests/generic/1230 new file +15 -0

+			echo
+			echo "Actual contents: "
+			echo "$actual_data"
+
+			_fail
+		fi
+		echo -n "Check at offset $off suceeded! " >> $seqres.full
+		if [[ "$actual_data" == "$expected_data_new" ]]
+		then
+			echo "matched new" >> $seqres.full
+		elif [[ "$actual_data" == "$expected_data_old" ]]
+		then
+			echo "matched old" >> $seqres.full
+		fi
+		off=$(( off + awu_max ))

tests/generic/1230 line 195

Quoted 10 lines

tests/generic/1230 new file +10 -0

+	done
+}
+
+# test data integrity for file by shutting down in between atomic writes
+test_data_integrity() {
+	echo >> $seqres.full
+	echo "# Writing atomically to file in background" >> $seqres.full
+	atomic_write_loop &
+	awloop_pid=$!
+

from here ...

Quoted 10 lines

tests/generic/1230 new file +10 -0

+	local i=0
+	# Wait for atleast first write to be recorded or 10s
+	while [ ! -f "$tmp.aw" -a $i -le 50 ]; do i=$((i + 1)); sleep 0.2; done
+
+	if [[ $i -gt 50 ]]
+	then
+		_fail "atomic write process took too long to start"
+	fi
+
+	echo >> $seqres.full

tests/generic/1230 line 87

Quoted 6 lines

tests/generic/1230 new file +6 -0

+	echo "# Shutting down filesystem while write is running" >> $seqres.full
+	_scratch_shutdown
+
+	kill $awloop_pid 2>/dev/null  # the process might have finished already
+	wait $awloop_pid
+	unset $awloop_pid

... to here looks similar in many functions. Can we factor it out?