Add more tests for multi fs block atomic writes

Changes:

- (1/13) new patch with _min and _max helpers
- (2/13) remove setup_fs_options and add fsx specifc helper
- (4/13) skip atomic write instead of falling back to normal write (fsx)
- (4/13) make atomic write default on instead of default off (fsx)
- (5,6/13) refactor and cleanup fio tests
- (7/13) refactored common code
- (8/13) dont ignore mmap writes for fsx with atomic writes
- (9/13) use od instead of xxd. handle cleanup of bg threads in _cleanup()
- (10-13/13) minor refactors
- change all tests use _fail for better consistency
- use higher tests numbers for easier merging

Link to rfc:https://urldefense.com/v3/__https://lore.kernel.org/fstests/ [email protected]/__;!!ACWV5N9M2RV99hQ! J14tJ2AlGzQmxs408RVEDMmyt2ePPukro0IrPiuxMGEaq1wFgSfj91M8FQypI- MM18b01NTxCQl8GLPSKwWNBQ$

PS: I'm on vacation till next week so there might be a delay in response

Ojaswin Mujoo (9):
common/rc: Add _min() and _max() helpers
common/rc: Fix fsx for ext4 with bigalloc
common/rc: Add a helper to run fsx on a given file ltp/fsx.c: Add atomic writes support to fsx
generic/1228: Add atomic write multi-fsblock O_[D]SYNC tests generic/1229: Stress fsx with atomic writes enabled generic/1230: Add sudden shutdown tests for multi block atomic writes ext4/063: Atomic write test for extent split across leaf nodes ext4/064: Add atomic write tests for journal credit calculation

tests/generic/1226 new file +5 -0

+blocksize=$(_min "$blocksize" "65536")
+
+fio_config=$tmp.fio
+fio_out=$tmp.fio.out
+

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

This tests 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. This forces the filesystem allocator to allocate extents over mixed mapping regions to stress FS block allocators.

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

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

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]>

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

common/rc _require_hugepage_fsx() · line 5122

Quoted 11 lines · Ojaswin Mujoo

This is achieved by defining a new function _set_default_fsx_avoid called via run_fsx helper. This can be used to selectively disable fsx options based on the configuration.

common/rc +7 -0

+_set_default_fsx_avoid() {
+       case "$FSTYP" in
+       "ext4")
+               if [[ "$MKFS_OPTIONS" =~ bigalloc ]]; then
+                       export FSX_AVOID+=" -I -C"
+               fi
+               ;;

This assumes that MKFS_OPTIONS reflects the file system features enabled on the file system. That's true by definition for the scratch fs, but it's not necessarily true for the test fs. At least in theory, someone could run fstests without a scrach device, and then MKFS_OPTION?S might not be set.

I'm not sure that we care; is this something that we make assumptions in other tests?

- Ted

Quoted 19 lines · Theodore Ts'o

common/rc _require_hugepage_fsx() · line 5122

Quoted 11 lines · Ojaswin Mujoo

This is achieved by defining a new function _set_default_fsx_avoid called via run_fsx helper. This can be used to selectively disable fsx options based on the configuration.

common/rc +7 -0

+_set_default_fsx_avoid() {
+       case "$FSTYP" in
+       "ext4")
+               if [[ "$MKFS_OPTIONS" =~ bigalloc ]]; then
+                       export FSX_AVOID+=" -I -C"
+               fi
+               ;;

This assumes that MKFS_OPTIONS reflects the file system features enabled on the file system. That's true by definition for the scratch fs, but it's not necessarily true for the test fs. At least in theory, someone could run fstests without a scrach device, and then MKFS_OPTION?S might not be set.

Quoted 2 lines

I'm not sure that we care; is this something that we make assumptions in other tests?

/me suspects it ought to be checking dumpe2fs -h output.

--D

Quoted 4 lines

- Ted

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 6 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]>

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

this seems to work ok for xfs, as I get data verify errors when I remove the -A arg to xfs_io when doing the atomic writes.

But I see this (always):

(quoted diff) +1 -0

@@ -1,2 +1,83 @@
 QA output created by 1230
+/home/opc/xfstests-dev/tests/generic/1230: line 13:

_require_scratch_write_atomic_multi_fsblock: command not found

+/home/opc/xfstests-dev/tests/generic/1230: line 149: kill: (535173) - No such process

any idea (apart from _require_scratch_write_atomic_multi_fsblock)?

tests/generic/1226 line 26

Quoted 7 lines

tests/generic/1226 new file +6 -0

+touch "$SCRATCH_MNT/f1"
+awu_min_write=$(_get_atomic_write_unit_min "$SCRATCH_MNT/f1")
+awu_max_write=$(_get_atomic_write_unit_max "$SCRATCH_MNT/f1")
+blocksize=$(_max "$awu_min_write" "$((awu_max_write/2))")
+
+# XFS can have high awu_max_write due to software fallback. Cap it at 64k

This test fails on xfs due for this reason. software fallback -based atomic writes have no serialization against reads or other writes.

Can you cap at atomic write unit max opt? That will mean that we get serialization from atomic write HW support.

BTW, to repeat what I said before, it can also fail for atomic write bios using HW support, as reads may be split.

See this sample output:

generic/1226 68s ... - output mismatch (see /home/opc/xfstests-dev/results//generic/1226.out.bad)

    --- tests/generic/1226.out  2025-06-27 11:26:58.411121674 +0000
    +++ /home/opc/xfstests-dev/results//generic/1226.out.bad 2025-06-27

14:03:08.933811064 +0000
@@ -1,2 +1,104 @@
QA output created by 1226
+crc32c: verify failed at file /home/opc/mnt/scratch/test-file offset
16457728, length 8192 (requested block: offset=16457728, length=8192, flags=84)
+ Expected CRC: 712de900
+ Received CRC: f872a18d
+crc32c: verify failed at file /home/opc/mnt/scratch/test-file offset
88104960, length 8192 (requested block: offset=88104960, length=8192, flags=84)
+ Expected CRC: 4aaaf009
+ Received CRC: 2a5770cf
...
(Run 'diff -u /home/opc/xfstests-dev/tests/generic/1226.out
/home/opc/xfstests-dev/results//generic/1226.out.bad' to see the entire diff)
Ran: generic/1226
Failures: generic/1226
Failed 1 of 1 tests

[root@jgarry-ol9new xfstests-dev]# cat /sys/block/sdi/queue/max_sectors_kb 4
[root@jgarry-ol9new xfstests-dev]#

However, nobody should fiddle with max_sectors_kb, so the test still has value.

Quoted 5 lines

tests/generic/1226 new file +5 -0

+blocksize=$(_min "$blocksize" "65536")
+
+fio_config=$tmp.fio
+fio_out=$tmp.fio.out
+

Quoted 10 lines · John Garry

tests/generic/1226 line 26

Quoted 7 lines

tests/generic/1226 new file +6 -0

+touch "$SCRATCH_MNT/f1"
+awu_min_write=$(_get_atomic_write_unit_min "$SCRATCH_MNT/f1")
+awu_max_write=$(_get_atomic_write_unit_max "$SCRATCH_MNT/f1")
+blocksize=$(_max "$awu_min_write" "$((awu_max_write/2))")
+
+# XFS can have high awu_max_write due to software fallback. Cap it at 64k

This test fails on xfs due for this reason. software fallback -based atomic writes have no serialization against reads or other writes.

Okay so Im still not understanding why serialization is affecting anything here? Lets assume the IO never breaks,

from what I understand
in fio code, we do the write and then once it is complete we try to read it back and verify it.

Why serialization matters here, because even if device reorders our read before the atomic write, we still fetch an older $blocksize chunk which should have the data and the header.

The data crc should match the
header because it would have gone atomically. What am I missing?

(Or do you mean that max_sectors_kb is not big enough to support reads after software atomic writes and thats the issue?)

Quoted 4 lines

Can you cap at atomic write unit max opt? That will mean that we get serialization from atomic write HW support.

Quoted 2 lines

BTW, to repeat what I said before, it can also fail for atomic write bios using HW support, as reads may be split.

This one I can understand, the fio's read is split causing a short read and seems like fio is not requeing short read properly for the verify step(?) so it tries to verify against short read itself.

Quoted 20 lines · Darrick J. Wong

Quoted 19 lines · Theodore Ts'o

common/rc _require_hugepage_fsx() · line 5122

Quoted 11 lines · Ojaswin Mujoo

This is achieved by defining a new function _set_default_fsx_avoid called via run_fsx helper. This can be used to selectively disable fsx options based on the configuration.

common/rc +7 -0

+_set_default_fsx_avoid() {
+       case "$FSTYP" in
+       "ext4")
+               if [[ "$MKFS_OPTIONS" =~ bigalloc ]]; then
+                       export FSX_AVOID+=" -I -C"
+               fi
+               ;;

This assumes that MKFS_OPTIONS reflects the file system features enabled on the file system. That's true by definition for the scratch fs, but it's not necessarily true for the test fs. At least in theory, someone could run fstests without a scrach device, and then MKFS_OPTION?S might not be set.

Quoted 6 lines

Quoted 2 lines

I'm not sure that we care; is this something that we make assumptions in other tests?

/me suspects it ought to be checking dumpe2fs -h output.

--D

Hey Ted, Darrick,

I agree that MKFS_OPTIONS is not ideal because it won't reflect the TEST_DEV options. Most of the existing users of run_fsx helper actually use the TEST_DEV instead so Darrick's suggestion of using dumpe2fs makes sense.

I can use that approach in v3 if that sounds okay to everyone.

Regards,
ojaswin

Quoted 4 lines

Quoted 4 lines

- Ted

Quoted 12 lines · Ojaswin Mujoo

Quoted 10 lines · John Garry

tests/generic/1226 line 26

Quoted 7 lines

tests/generic/1226 new file +6 -0

+touch "$SCRATCH_MNT/f1"
+awu_min_write=$(_get_atomic_write_unit_min "$SCRATCH_MNT/f1")
+awu_max_write=$(_get_atomic_write_unit_max "$SCRATCH_MNT/f1")
+blocksize=$(_max "$awu_min_write" "$((awu_max_write/2))")
+
+# XFS can have high awu_max_write due to software fallback. Cap it at 64k

This test fails on xfs due for this reason. software fallback -based atomic writes have no serialization against reads or other writes.

Quoted 2 lines

Okay so Im still not understanding why serialization is affecting anything here? Lets assume the IO never breaks,

That's a false assumption.

An atomic write should never be split into multiple requests for REQ_ATOMIC set. However, regular reads and writes may be split in the block stack for any reason. We can easily force a split in regular reads and writes by setting max_sectors_kb to a value less than the read/write size.

Check blk_mq_submit_bio() -> __bio_split_to_limits() -> bio_split_rw()

Quoted 7 lines

from what I understand
in fio code, we do the write and then once it is complete we try to read it back and verify it.

Why serialization matters here, because even if device reorders our read before the atomic write, we still fetch an older $blocksize chunk which should have the data and the header.

Yes, we fetch $blocksize, but it may be as multiple read requests, so we can have:

partial read $blocksize request, atomic write request, partial read $blocksize request

Quoted 11 lines

The data crc should match the
header because it would have gone atomically. What am I missing?

(Or do you mean that max_sectors_kb is not big enough to support reads after software atomic writes and thats the issue?)

Quoted 4 lines

Can you cap at atomic write unit max opt? That will mean that we get serialization from atomic write HW support.

Quoted 6 lines

Quoted 2 lines

BTW, to repeat what I said before, it can also fail for atomic write bios using HW support, as reads may be split.

This one I can understand, the fio's read is split causing a short read and seems like fio is not requeing short read properly for the verify step(?) so it tries to verify against short read itself.

this is not an fio problem, as above.

I would like to repeat that the test should be ok to use, as long as we don't try to use an atomic write size > HW support (and we also have max_sectors_kb > $blocksize, which it would normally be).

Thanks,
John

Quoted 8 lines

Quoted 3 lines

Why serialization matters here, because even if device reorders our read before the atomic write, we still fetch an older $blocksize chunk which should have the data and the header.

Yes, we fetch $blocksize, but it may be as multiple read requests, so we can have:

partial read $blocksize request, atomic write request, partial read $blocksize request

Ahh now I get the picture. Since software atomic writes might have a value way higher than the max_sectors that the bio may split at, this causes the reads to split which is the issue happnes, as you mentioned.

Quoted 15 lines

Quoted 15 lines

The data crc should match the
header because it would have gone atomically. What am I missing?

(Or do you mean that max_sectors_kb is not big enough to support reads after software atomic writes and thats the issue?)

Quoted 5 lines

Can you cap at atomic write unit max opt? That will mean that we get serialization from atomic write HW support.

BTW, to repeat what I said before, it can also fail for atomic write bios using HW support, as reads may be split.

This one I can understand, the fio's read is split causing a short read and seems like fio is not requeing short read properly for the verify step(?) so it tries to verify against short read itself.

Quoted 1 line

this is not an fio problem, as above.

Yep got it, its the block layer splitting the reads.

So as a solution, do you think it is okay to cap $blocksize to max_sectors_kb or would you suggest it would be better to just stick to awu max opt?

Quoted 6 lines · Ojaswin Mujoo

Quoted 6 lines

Quoted 3 lines

Why serialization matters here, because even if device reorders our read before the atomic write, we still fetch an older $blocksize chunk which should have the data and the header.

Yes, we fetch $blocksize, but it may be as multiple read requests, so we can have:

Quoted 5 lines

Quoted 2 lines

partial read $blocksize request, atomic write request, partial read $blocksize request

Ahh now I get the picture. Since software atomic writes might have a value way higher than the max_sectors that the bio may split at, this causes the reads to split which is the issue happnes, as you mentioned.

This issue also exists for atomic writes based on REQ_ATOMIC.

Quoted 17 lines

Quoted 17 lines

Quoted 8 lines

The data crc should match the
header because it would have gone atomically. What am I missing?

(Or do you mean that max_sectors_kb is not big enough to support reads after software atomic writes and thats the issue?)

Quoted 8 lines

Quoted 5 lines

Can you cap at atomic write unit max opt? That will mean that we get serialization from atomic write HW support.

BTW, to repeat what I said before, it can also fail for atomic write bios using HW support, as reads may be split.

This one I can understand, the fio's read is split causing a short read and seems like fio is not requeing short read properly for the verify step(?) so it tries to verify against short read itself.

this is not an fio problem, as above.

Quoted 5 lines

Yep got it, its the block layer splitting the reads.

So as a solution, do you think it is okay to cap $blocksize to max_sectors_kb or would you suggest it would be better to just stick to awu max opt?

I tend to think that it is better to cap at awu max opt. If zero, then don't do the test.

What awu max opt is reported for ext4 (for awu max > FS block size)? Is min(bigalloc size, bdev awu max)?

Thanks,
John

Quoted 7 lines · John Garry

Quoted 6 lines · Ojaswin Mujoo

Quoted 6 lines

Quoted 3 lines

Why serialization matters here, because even if device reorders our read before the atomic write, we still fetch an older $blocksize chunk which should have the data and the header.

Yes, we fetch $blocksize, but it may be as multiple read requests, so we can have:

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

partial read $blocksize request, atomic write request, partial read $blocksize request

Ahh now I get the picture. Since software atomic writes might have a value way higher than the max_sectors that the bio may split at, this causes the reads to split which is the issue happnes, as you mentioned.

This issue also exists for atomic writes based on REQ_ATOMIC.

Yep got it now, thanks.

Quoted 23 lines

Quoted 22 lines

Quoted 11 lines

Quoted 11 lines

The data crc should match the
header because it would have gone atomically. What am I missing?

(Or do you mean that max_sectors_kb is not big enough to support reads after software atomic writes and thats the issue?)

Quoted 3 lines

Can you cap at atomic write unit max opt? That will mean that we get serialization from atomic write HW support.

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

BTW, to repeat what I said before, it can also fail for atomic write bios using HW support, as reads may be split.

This one I can understand, the fio's read is split causing a short read and seems like fio is not requeing short read properly for the verify step(?) so it tries to verify against short read itself.

this is not an fio problem, as above.

Yep got it, its the block layer splitting the reads.

So as a solution, do you think it is okay to cap $blocksize to max_sectors_kb or would you suggest it would be better to just stick to awu max opt?

Quoted 2 lines

I tend to think that it is better to cap at awu max opt. If zero, then don't do the test.

Hmm yep this makes sense but seems like we don't expose it yet in xfs_io. Will need to add that support as well.

Quoted 4 lines

What awu max opt is reported for ext4 (for awu max > FS block size)? Is min(bigalloc size, bdev awu max)?

For awu_max_opt is returned as 0 for ext4, since 0 indicates there is no such slow fallback. From commit

5d894321c49e fs: add atomic write unit max opt to statx

...
When zero, it means that there is no such performance boundary.

	...

Regards,
ojaswin

Quoted 4 lines

Thanks,
John

Quoted 7 lines · John Garry

Quoted 6 lines · Ojaswin Mujoo

Quoted 6 lines

Quoted 3 lines

Why serialization matters here, because even if device reorders our read before the atomic write, we still fetch an older $blocksize chunk which should have the data and the header.

Yes, we fetch $blocksize, but it may be as multiple read requests, so we can have:

Quoted 7 lines

Quoted 5 lines

Quoted 2 lines

partial read $blocksize request, atomic write request, partial read $blocksize request

Ahh now I get the picture. Since software atomic writes might have a value way higher than the max_sectors that the bio may split at, this causes the reads to split which is the issue happnes, as you mentioned.

This issue also exists for atomic writes based on REQ_ATOMIC.

Yep got it now, thanks.

Quoted 23 lines

Quoted 22 lines

Quoted 11 lines

Quoted 11 lines

The data crc should match the
header because it would have gone atomically. What am I missing?

(Or do you mean that max_sectors_kb is not big enough to support reads after software atomic writes and thats the issue?)

Quoted 3 lines

Can you cap at atomic write unit max opt? That will mean that we get serialization from atomic write HW support.

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

BTW, to repeat what I said before, it can also fail for atomic write bios using HW support, as reads may be split.

This one I can understand, the fio's read is split causing a short read and seems like fio is not requeing short read properly for the verify step(?) so it tries to verify against short read itself.

this is not an fio problem, as above.

Yep got it, its the block layer splitting the reads.

So as a solution, do you think it is okay to cap $blocksize to max_sectors_kb or would you suggest it would be better to just stick to awu max opt?

Quoted 2 lines

I tend to think that it is better to cap at awu max opt. If zero, then don't do the test.

Hmm yep this makes sense but seems like we don't expose it yet in xfs_io. Will need to add that support as well.

Quoted 4 lines

What awu max opt is reported for ext4 (for awu max > FS block size)? Is min(bigalloc size, bdev awu max)?

For awu_max_opt is returned as 0 for ext4, since 0 indicates there is no such slow fallback. From commit

5d894321c49e fs: add atomic write unit max opt to statx

   ...
   When zero, it means that there is no such performance boundary.
	...

Quoted 2 lines

Hmm yep this makes sense but seems like we don't expose it yet in xfs_io. Will need to add that support as well.

Okay so thinking about it a bit more, Im not sure if capping to awu_max_opt is the right thing to do.

There is no guarantee in kernel that awu_max_opt or even awu_max would be smaller than lim->max_sectors so why to derive our io size based on that.

Further, handling awu_max vs awu_max_opt and their different interpretations for XFS vs EXT4 etc are adding uneccessary complexity.

The simplest way seems to be to just limit the $blocksize to something like $(_min 16KB awu_max) and hope that 16KB is small enough to not be split during reads. We anyways have other tests like generic/1230 that can be used to test larger atomic write sizes

Thoughts?

Quoted 8 lines · Ojaswin Mujoo

Quoted 2 lines

Hmm yep this makes sense but seems like we don't expose it yet in xfs_io. Will need to add that support as well.

Okay so thinking about it a bit more, Im not sure if capping to awu_max_opt is the right thing to do.

There is no guarantee in kernel that awu_max_opt or even awu_max would be smaller than lim->max_sectors so why to derive our io size based on that.

Absolutely. There is no guarantee. But generally it would not be less than lim->max_sectors. But lim->max_sectors kernel default is 1280KB, which is quite large. However, there are other reasons why splits could be broken. I am just giving the simple example in lim->max_sectors, as that is configurable from userspace (so useful for experimenting).

Quoted 3 lines

Further, handling awu_max vs awu_max_opt and their different interpretations for XFS vs EXT4 etc are adding uneccessary complexity.

You could assume awu max opt == awu max for ext4, as we no there are no software-based atomic writes. That could be simpler than my previous suggestion, which involves a kernel change.

Quoted 8 lines

The simplest way seems to be to just limit the $blocksize to something like $(_min 16KB awu_max) and hope that 16KB is small enough to not be split during reads. We anyways have other tests like generic/1230 that can be used to test larger atomic write sizes

Thoughts?

As I said at the start, we never had guarantees of serialization of reads and atomic writes.

However I still think that this is a useful test. It's just it is theoretically possible to give false positives.

You could get the test to read max_sectors_kb, and check whether it is greater than the bs. Again, more complexity.

As an alternative, maybe it's better to maintain this test out-of-tree. I'm not sure.

Thanks,
John

Quoted 5 lines

Quoted 2 lines

Further, handling awu_max vs awu_max_opt and their different interpretations for XFS vs EXT4 etc are adding uneccessary complexity.

You could assume awu max opt == awu max for ext4, as we no there are no software-based atomic writes. That could be simpler than my previous suggestion, which involves a kernel change.

I want to avoid adding something like

iosize = FSTYP == "ext4" ? awu_max : awu_max_opt

in the test iteslf, because it is a bit difficult to remember changing this in case things change in the kernel later, eg if ext4 gets a fallback.

Im not sure if returning non-zero for ext4 is also correct as per the documented behavior from 5d894321c49e613.

When zero, it means that there is no such performance boundary.

Anyways, I think there might be a simpler solution...

Quoted 9 lines

Quoted 6 lines

The simplest way seems to be to just limit the $blocksize to something like $(_min 16KB awu_max) and hope that 16KB is small enough to not be split during reads. We anyways have other tests like generic/1230 that can be used to test larger atomic write sizes

Thoughts?

As I said at the start, we never had guarantees of serialization of reads and atomic writes.

Quoted 5 lines

However I still think that this is a useful test. It's just it is theoretically possible to give false positives.

You could get the test to read max_sectors_kb, and check whether it is greater than the bs. Again, more complexity.

I think this might work. So i can set the iosize as 16kb which is already low enought. Incase the max_sectors is < than 16kb then we bail out. (but then maybe there is a very small chance of read split still)

OR another approach is to do the verify at the very end when all threads are done writing so the reads don't race. The tradeoff is that this will reduce the effictiveness of the test though to some extent.

Quoted 3 lines

As an alternative, maybe it's better to maintain this test out-of-tree. I'm not sure.

That'll be a hassle for everyone :p Does any of the above 2 approaches seem acceptable for upstreaming this test?

Quoted 5 lines · Ojaswin Mujoo

Quoted 5 lines

Quoted 2 lines

Further, handling awu_max vs awu_max_opt and their different interpretations for XFS vs EXT4 etc are adding uneccessary complexity.

You could assume awu max opt == awu max for ext4, as we no there are no software-based atomic writes. That could be simpler than my previous suggestion, which involves a kernel change.

Quoted 7 lines

I want to avoid adding something like

iosize = FSTYP == "ext4" ? awu_max : awu_max_opt

in the test iteslf, because it is a bit difficult to remember changing this in case things change in the kernel later, eg if ext4 gets a fallback.

You could do the statx call on the mounted bdev, and see if atomics are supported there. If not, skip the test.

But even for xfs when HW support is available, we can still use software fallback when HW support is not possible, i.e. misaligned. So, in that regard, we really should only test this on ext4.

Quoted 17 lines

Im not sure if returning non-zero for ext4 is also correct as per the documented behavior from 5d894321c49e613.

When zero, it means that there is no such performance boundary.

Anyways, I think there might be a simpler solution...

Quoted 9 lines

Quoted 6 lines

The simplest way seems to be to just limit the $blocksize to something like $(_min 16KB awu_max) and hope that 16KB is small enough to not be split during reads. We anyways have other tests like generic/1230 that can be used to test larger atomic write sizes

Thoughts?

As I said at the start, we never had guarantees of serialization of reads and atomic writes.

Quoted 8 lines

Quoted 5 lines

However I still think that this is a useful test. It's just it is theoretically possible to give false positives.

You could get the test to read max_sectors_kb, and check whether it is greater than the bs. Again, more complexity.

I think this might work. So i can set the iosize as 16kb which is already low enought. Incase the max_sectors is < than 16kb then we bail out. (but then maybe there is a very small chance of read split still)

Sure, that seems fine.

Quoted 1 line

Quoted 3 lines

OR another approach is to do the verify at the very end when all threads are done writing so the reads don't race. The tradeoff is that this will reduce the effictiveness of the test though to some extent.

Yeah, right. We would want this test to prove that atomic writes are not getting split by the block stack, so racing reads can help prove that.

Quoted 6 lines

Quoted 4 lines

As an alternative, maybe it's better to maintain this test out-of-tree. I'm not sure.

That'll be a hassle for everyone :p Does any of the above 2 approaches seem acceptable for upstreaming this test?

Sure, I think ether are fine, but I am still a bit hesitant to even have this test for xfs (and not just ext4).

Thanks,
John

Quoted 6 lines

You could do the statx call on the mounted bdev, and see if atomics are supported there. If not, skip the test.

But even for xfs when HW support is available, we can still use software fallback when HW support is not possible, i.e. misaligned. So, in that regard, we really should only test this on ext4.

Hmm, okay so I'm a bit confused now. What is different in ext4 vs xfs. Why does the software fallback for xfs matter in this case?

If we limit
io to 16KB hoping that reads don't break, then either ways the fio verify should work. And if reads break then both ext4 and xfs suffer irrespective of hardware or software fallback. (We also saw that hardware atomic writes also suffer with same issue when max_sectors is low)

Quoted 25 lines

Quoted 15 lines

Im not sure if returning non-zero for ext4 is also correct as per the documented behavior from 5d894321c49e613.

When zero, it means that there is no such performance boundary.

Anyways, I think there might be a simpler solution...

Quoted 9 lines

Quoted 6 lines

The simplest way seems to be to just limit the $blocksize to something like $(_min 16KB awu_max) and hope that 16KB is small enough to not be split during reads. We anyways have other tests like generic/1230 that can be used to test larger atomic write sizes

Thoughts?

As I said at the start, we never had guarantees of serialization of reads and atomic writes.

Quoted 8 lines

Quoted 5 lines

However I still think that this is a useful test. It's just it is theoretically possible to give false positives.

You could get the test to read max_sectors_kb, and check whether it is greater than the bs. Again, more complexity.

I think this might work. So i can set the iosize as 16kb which is already low enought. Incase the max_sectors is < than 16kb then we bail out. (but then maybe there is a very small chance of read split still)

Sure, that seems fine.

Quoted 5 lines

Quoted 3 lines

OR another approach is to do the verify at the very end when all threads are done writing so the reads don't race. The tradeoff is that this will reduce the effictiveness of the test though to some extent.

Yeah, right. We would want this test to prove that atomic writes are not getting split by the block stack, so racing reads can help prove that.

Yeahh, but now i'm leaning towards this approach of keeping verify at the very end instead of parallel.

Yes, it is a bit less effective but
then having a data integrity test that can theoretically return false positives is not good, it'll always keep me guessing about any failure. Having verify at end will not have false failures for sure. Even though it comes at a cost of less coverage.

Maybe we can also get some other prespectives on this. @Zorro, @Darrick, can you please give your thoughts on this?

As a sidenote, with this issue of fio's verify read splitting, it seems like the parallel verify feature in fio itself is flawed for non-atomic writes as well since the read can always split and race with the writes.

Not sure what the solution is here, maybe time for non splittable atomic writes (:p)

Quoted 8 lines · Ojaswin Mujoo

Quoted 6 lines

You could do the statx call on the mounted bdev, and see if atomics are supported there. If not, skip the test.

But even for xfs when HW support is available, we can still use software fallback when HW support is not possible, i.e. misaligned. So, in that regard, we really should only test this on ext4.

Hmm, okay so I'm a bit confused now. What is different in ext4 vs xfs. Why does the software fallback for xfs matter in this case?

For xfs, software fallback-based atomic writes are not even serialized against other software fallback-based atomic writes. They can trample on one another. That is a reason I say that this test should only be for HW-based atomic writes.

But even if the bs <= HW limit, on xfs may rarely still use atomic write software fallback if HW-based atomic write is not possible, e.g. misaligned, multiple extents, etc. I will emphasis that this would be rare, but it is still possible.

For ext4, since alignment is always guaranteed, and HW-based atomics will only ever be used.

Quoted 35 lines

If we limit
io to 16KB hoping that reads don't break, then either ways the fio verify should work. And if reads break then both ext4 and xfs suffer irrespective of hardware or software fallback. (We also saw that hardware atomic writes also suffer with same issue when max_sectors is low)

Quoted 28 lines

Quoted 15 lines

Im not sure if returning non-zero for ext4 is also correct as per the documented behavior from 5d894321c49e613.

When zero, it means that there is no such performance boundary.

Anyways, I think there might be a simpler solution...

Quoted 9 lines

Quoted 6 lines

The simplest way seems to be to just limit the $blocksize to something like $(_min 16KB awu_max) and hope that 16KB is small enough to not be split during reads. We anyways have other tests like generic/1230 that can be used to test larger atomic write sizes

Thoughts?

As I said at the start, we never had guarantees of serialization of reads and atomic writes.

Quoted 8 lines

Quoted 5 lines

However I still think that this is a useful test. It's just it is theoretically possible to give false positives.

You could get the test to read max_sectors_kb, and check whether it is greater than the bs. Again, more complexity.

I think this might work. So i can set the iosize as 16kb which is already low enought. Incase the max_sectors is < than 16kb then we bail out. (but then maybe there is a very small chance of read split still)

Sure, that seems fine.

Quoted 3 lines

OR another approach is to do the verify at the very end when all threads are done writing so the reads don't race. The tradeoff is that this will reduce the effictiveness of the test though to some extent.

Quoted 4 lines

Quoted 2 lines

Yeah, right. We would want this test to prove that atomic writes are not getting split by the block stack, so racing reads can help prove that.

Yeahh, but now i'm leaning towards this approach of keeping verify at the very end instead of parallel.

I don't think that fio supports such a mode. Or does it?

Quoted 7 lines

Yes, it is a bit less effective but
then having a data integrity test that can theoretically return false positives is not good, it'll always keep me guessing about any failure. Having verify at end will not have false failures for sure. Even though it comes at a cost of less coverage.

Quoted 6 lines

Maybe we can also get some other prespectives on this. @Zorro, @Darrick, can you please give your thoughts on this?

As a sidenote, with this issue of fio's verify read splitting, it seems like the parallel verify feature in fio itself is flawed for non-atomic writes as well since the read can always split and race with the writes.

fio does warn that multiple read and writers in verify mode is unsafe.

Quoted 4 lines

Not sure what the solution is here, maybe time for non splittable atomic writes (:p)

I just think that fio in verify mode for ext4 is ok, as long as max_sectors_kb is large.

Quoted 1 line

I don't think that fio supports such a mode. Or does it?

Yes we can achieve it via combination of write job with verify=crc,do_verify=0 followed by verify job with verify=crc,verify_only=1.

I'm testing this right now to
see if it can survive max_sectors_kb = 4 (which i think it should since there are no races now). If everything is fine I'll use this approach for the fio tests in the next revision.

Quoted 6 lines

Quoted 6 lines

Yes, it is a bit less effective but
then having a data integrity test that can theoretically return false positives is not good, it'll always keep me guessing about any failure. Having verify at end will not have false failures for sure. Even though it comes at a cost of less coverage.

Quoted 7 lines

Quoted 6 lines

Maybe we can also get some other prespectives on this. @Zorro, @Darrick, can you please give your thoughts on this?

As a sidenote, with this issue of fio's verify read splitting, it seems like the parallel verify feature in fio itself is flawed for non-atomic writes as well since the read can always split and race with the writes.

fio does warn that multiple read and writers in verify mode is unsafe.

Right.

Quoted 2 lines

Quoted 2 lines

Not sure what the solution is here, maybe time for non splittable atomic writes (:p)

Quoted 2 lines

I just think that fio in verify mode for ext4 is ok, as long as max_sectors_kb is large.

Yes but i'd rather have the write followed by verify version to avoid false positives in case the read bio does end up splitting for whatever reason.

Thanks for catching this issue in the test!

Quoted 3 lines · Ojaswin Mujoo

Quoted 1 line

I don't think that fio supports such a mode. Or does it?

Yes we can achieve it via combination of write job with verify=crc,do_verify=0 followed by verify job with verify=crc,verify_only=1.

So does verify=crc,do_verify=0 really omit the verify and only do write? If so, seems good.

Quoted 5 lines

I'm testing this right now to
see if it can survive max_sectors_kb = 4 (which i think it should since there are no races now). If everything is fine I'll use this approach for the fio tests in the next revision.

understood

Quoted 19 lines

Quoted 6 lines

Quoted 6 lines

Yes, it is a bit less effective but
then having a data integrity test that can theoretically return false positives is not good, it'll always keep me guessing about any failure. Having verify at end will not have false failures for sure. Even though it comes at a cost of less coverage.

Quoted 7 lines

Quoted 6 lines

Maybe we can also get some other prespectives on this. @Zorro, @Darrick, can you please give your thoughts on this?

As a sidenote, with this issue of fio's verify read splitting, it seems like the parallel verify feature in fio itself is flawed for non-atomic writes as well since the read can always split and race with the writes.

fio does warn that multiple read and writers in verify mode is unsafe.

Right.

Quoted 4 lines

Quoted 2 lines

Not sure what the solution is here, maybe time for non splittable atomic writes (:p)

I just think that fio in verify mode for ext4 is ok, as long as max_sectors_kb is large.

Quoted 5 lines

Yes but i'd rather have the write followed by verify version to avoid false positives in case the read bio does end up splitting for whatever reason.

Thanks for catching this issue in the test!

So, if you want to test this way then you need to omit xfs or ensure that the bs <= awu max opt for xfs and hope for no problems.

Thanks,
John