[PATCHSET 1/3] fstests: fixes for atomic writes tests

[PATCHSET 1/3] fstests: fixes for atomic writes tests

[Darrick J. Wong]

Hi all,

Here's a pile of bug fixes for the atomic writes tests.

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

This has been running on the djcloud for months with no problems.  Enjoy!
Comments and questions are, as always, welcome.

--D

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

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=atomic-writes
---
Commits in this patchset:
 * generic/211: don't run on an xfs that can't do inplace writes
 * generic/427: try to ensure there's some free space before we do the aio test
 * generic/767: require fallocate support
 * generic/767: only test the hardware atomic write unit
 * generic/767: allow on any atomic writes filesystem
 * xfs/838: actually force usage of the realtime device
 * common: fix _require_xfs_io_command pwrite -A for various blocksizes
---
 common/atomicwrites |    6 ++++++
 common/rc           |   14 +++++++++++---
 tests/generic/211   |    6 ++++++
 tests/generic/427   |    5 +++++
 tests/generic/767   |   19 +++++++++++++++----
 tests/xfs/838       |    1 +
 6 files changed, 44 insertions(+), 7 deletions(-)

[PATCH 2/7] generic/427: try to ensure there's some free space before we do the aio test

[Darrick J. Wong]

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

On a filesystem configured like this:
MKFS_OPTIONS="-m metadir=1,autofsck=1,uquota,gquota,pquota -d rtinherit=1 -r zoned=1"

This test fails like this:

--- a/tests/generic/427.out      2025-04-30 16:20:44.584246582 -0700
+++ b/tests/generic/427.out.bad        2025-07-14 10:47:07.605377287 -0700
@@ -1,2 +1,2 @@
 QA output created by 427
-Success, all done.
+pwrite: No space left on device

The pwrite failure comes from the aio-dio-eof-race.c program because the
filesystem ran out of space.  There are no speculative posteof
preallocations on a zoned filesystem, so let's skip this test on those
setups.

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


diff --git a/tests/generic/427 b/tests/generic/427
index bddfdb8714e9a7..a6b2571f563167 100755
--- a/tests/generic/427
+++ b/tests/generic/427
@@ -28,6 +28,11 @@ _require_no_compress
 _scratch_mkfs_sized $((256 * 1024 * 1024)) >>$seqres.full 2>&1
 _scratch_mount
 
+# Zoned filesystems don't support speculative preallocations
+if [ "$FSTYP" = "xfs" ]; then
+	_require_xfs_scratch_non_zoned
+fi
+
 # try to write more bytes than filesystem size to fill the filesystem,
 # then remove all these data. If we still can find these stale data in
 # a file' eofblock, then it's a bug

[PATCH 3/7] generic/767: require fallocate support

[Darrick J. Wong]

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

This test fails on filesystems that don't support fallocate, so screen
them out.

Cc: <fstests@vger.kernel.org> # v2025.07.13
Fixes: fa8694c823d853 ("generic: various atomic write tests with hardware and scsi_debug")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/generic/767 |    2 ++
 1 file changed, 2 insertions(+)


diff --git a/tests/generic/767 b/tests/generic/767
index 758222f4f8666b..31d599eacfd63b 100755
--- a/tests/generic/767
+++ b/tests/generic/767
@@ -23,6 +23,8 @@ _cleanup()
 _require_scsi_debug
 _require_scratch
 _require_block_device $SCRATCH_DEV
+_require_xfs_io_command "falloc"
+
 # Format something so that ./check doesn't freak out
 _scratch_mkfs >> $seqres.full
 

[PATCH 4/7] generic/767: only test the hardware atomic write unit

[Darrick J. Wong]

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

This test sets up scsi_debug so that we can test the fs and block layer
code for hardware-accelerated atomic writes (and not just a software
fallback).  However, the userspace ABI for detecting atomic write
geometry has changed since the start of development (to include said
software fallback) so we must add some extra code to find the real
hardware capabilities, and base the write sizes based on that.

This fixes a test failure with 32k blocksizes because the advertised
atomic_write_unit_max is 128M and fallocate quickly runs out of space.

While we're at it fix a stupid variable usage bug in the loop.

Cc: <fstests@vger.kernel.org> # v2025.07.13
Fixes: fa8694c823d853 ("generic: various atomic write tests with hardware and scsi_debug")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 common/atomicwrites |    6 ++++++
 tests/generic/767   |   16 +++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)


diff --git a/common/atomicwrites b/common/atomicwrites
index 95d545a67cadda..33526399d2e980 100644
--- a/common/atomicwrites
+++ b/common/atomicwrites
@@ -18,6 +18,12 @@ _get_atomic_write_unit_max()
         grep -w atomic_write_unit_max | grep -o '[0-9]\+'
 }
 
+_get_atomic_write_unit_max_opt()
+{
+	$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $1 | \
+        grep -w atomic_write_unit_max_opt | grep -o '[0-9]\+'
+}
+
 _get_atomic_write_segments_max()
 {
 	$XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $1 | \
diff --git a/tests/generic/767 b/tests/generic/767
index 31d599eacfd63b..161fef03825db4 100755
--- a/tests/generic/767
+++ b/tests/generic/767
@@ -61,8 +61,18 @@ $XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $testfile >> $seqres.full
 sector_size=$(blockdev --getss $SCRATCH_DEV)
 min_awu=$(_get_atomic_write_unit_min $testfile)
 max_awu=$(_get_atomic_write_unit_max $testfile)
+opt_awu=$(_get_atomic_write_unit_max_opt $testfile)
 
-$XFS_IO_PROG -f -c "falloc 0 $((max_awu * 2))" -c fsync $testfile
+echo "min:$min_awu max:$max_awu opt:$opt_awu" >> $seqres.full
+
+# We want to test hardware support, so use that if detected
+if [ -n "$opt_awu" ] && [ "$opt_awu" != "0" ]; then
+	write_size="$opt_awu"
+else
+	write_size="$max_awu"
+fi
+
+$XFS_IO_PROG -f -c "falloc 0 $((write_size * 2))" -c fsync $testfile
 
 # try outside the advertised sizes
 echo "two EINVAL for unsupported sizes"
@@ -73,8 +83,8 @@ _simple_atomic_write $max_i $max_i $testfile -d
 
 # try all of the advertised sizes
 echo "all should work"
-for ((i = min_awu; i <= max_awu; i *= 2)); do
-	$XFS_IO_PROG -f -c "falloc 0 $((max_awu * 2))" -c fsync $testfile
+for ((i = min_awu; i <= write_size; i *= 2)); do
+	$XFS_IO_PROG -f -c "falloc 0 $((write_size * 2))" -c fsync $testfile
 	_test_atomic_file_writes $i $testfile
 done
 

[PATCH 5/7] generic/767: allow on any atomic writes filesystem

[Darrick J. Wong]

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

This test now works correctly for any atomic writes geometry since we
control all the parameters.  Remove this restriction so that we can get
minimal testing on ext4 and fuse2fs.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/generic/767 |    1 -
 1 file changed, 1 deletion(-)


diff --git a/tests/generic/767 b/tests/generic/767
index 161fef03825db4..558aab1e6acf34 100755
--- a/tests/generic/767
+++ b/tests/generic/767
@@ -36,7 +36,6 @@ export SCRATCH_DEV=$dev
 unset USE_EXTERNAL
 
 _require_scratch_write_atomic
-_require_scratch_write_atomic_multi_fsblock
 
 # Check that xfs_io supports the commands needed to run this test
 # Note: _require_xfs_io_command is not used here because we want to

[PATCH 6/7] xfs/838: actually force usage of the realtime device

[Darrick J. Wong]

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

Actually force the filesystem to create testfile as a realtime file so
that the test does anything useful.

Cc: <fstests@vger.kernel.org> # v2025.07.13
Fixes: 7ca990e22e0d61 ("xfs: more multi-block atomic writes tests")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 tests/xfs/838 |    1 +
 1 file changed, 1 insertion(+)


diff --git a/tests/xfs/838 b/tests/xfs/838
index 73e91530b53a67..84e6a750eb07ab 100755
--- a/tests/xfs/838
+++ b/tests/xfs/838
@@ -23,6 +23,7 @@ $XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $SCRATCH_RTDEV >> $seqres.full
 
 _scratch_mkfs >> $seqres.full
 _scratch_mount
+_xfs_force_bdev realtime $SCRATCH_MNT
 
 testfile=$SCRATCH_MNT/testfile
 touch $testfile

[PATCH 7/7] common: fix _require_xfs_io_command pwrite -A for various blocksizes

[Darrick J. Wong]

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

In this predicate, we should test an atomic write of the minimum
supported size, not just 4k.  This fixes a problem where none of the
atomic write tests actually run on a 32k-fsblock xfs because you can't
do a sub-fsblock atomic write.

Cc: <fstests@vger.kernel.org> # v2025.04.13
Fixes: d90ee3b6496346 ("generic: add a test for atomic writes")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 common/rc |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)


diff --git a/common/rc b/common/rc
index 96578d152dafb9..177e7748f4bb89 100644
--- a/common/rc
+++ b/common/rc
@@ -3027,16 +3027,24 @@ _require_xfs_io_command()
 	"pwrite")
 		# -N (RWF_NOWAIT) only works with direct vectored I/O writes
 		local pwrite_opts=" "
+		local write_size="4k"
 		if [ "$param" == "-N" ]; then
 			opts+=" -d"
-			pwrite_opts+="-V 1 -b 4k"
+			pwrite_opts+="-V 1 -b $write_size"
 		fi
 		if [ "$param" == "-A" ]; then
 			opts+=" -d"
-			pwrite_opts+="-V 1 -b 4k"
+			# try to write the minimum supported atomic write size
+			write_size="$($XFS_IO_PROG -f -c "statx -r -m $STATX_WRITE_ATOMIC" $testfile 2>/dev/null | \
+				grep atomic_write_unit_min | \
+				grep -o '[0-9]\+')"
+			if [ -z "$write_size" ] || [ "$write_size" = "0" ]; then
+				write_size="0 --not-supported"
+			fi
+			pwrite_opts+="-V 1 -b $write_size"
 		fi
 		testio=`$XFS_IO_PROG -f $opts -c \
-		        "pwrite $pwrite_opts $param 0 4k" $testfile 2>&1`
+		        "pwrite $pwrite_opts $param 0 $write_size" $testfile 2>&1`
 		param_checked="$pwrite_opts $param"
 		;;
 	"scrub"|"repair")

Re: [PATCH 2/7] generic/427: try to ensure there's some free space before we do the aio test

[Christoph Hellwig]

On Tue, Jul 29, 2025 at 01:08:46PM -0700, Darrick J. Wong wrote:
> The pwrite failure comes from the aio-dio-eof-race.c program because the
> filesystem ran out of space.  There are no speculative posteof
> preallocations on a zoned filesystem, so let's skip this test on those
> setups.

Did it run out of space because it is overwriting and we need a new
allocation (I've not actually seen this fail in my zoned testing,
that's why I'm asking)?  If so it really should be using the new
_require_inplace_writes Filipe just sent to the list.

If now we need to figure out what this depends on instead of adding
random xfs-specific hacks to common code.

Re: [PATCH 3/7] generic/767: require fallocate support

[Christoph Hellwig]

On Tue, Jul 29, 2025 at 01:09:01PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This test fails on filesystems that don't support fallocate, so screen
> them out.

Looks good, I was just going to send this as well..

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

Re: [PATCH 4/7] generic/767: only test the hardware atomic write unit

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 6/7] xfs/838: actually force usage of the realtime device

[Christoph Hellwig]

Looks good:

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

Re: [PATCHSET 1/3] fstests: fixes for atomic writes tests

[Zorro Lang]

On Tue, Jul 29, 2025 at 01:08:28PM -0700, Darrick J. Wong wrote:
> Hi all,
> 
> Here's a pile of bug fixes for the atomic writes tests.
> 
> If you're going to start using this code, I strongly recommend pulling
> from my git trees, which are linked below.
> 
> This has been running on the djcloud for months with no problems.  Enjoy!
> Comments and questions are, as always, welcome.
> 
> --D
> 
> xfsprogs git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=atomic-writes
> 
> fstests git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=atomic-writes
> ---
> Commits in this patchset:
>  * generic/211: don't run on an xfs that can't do inplace writes
>  * generic/427: try to ensure there's some free space before we do the aio test
>  * generic/767: require fallocate support
>  * generic/767: only test the hardware atomic write unit
>  * generic/767: allow on any atomic writes filesystem
>  * xfs/838: actually force usage of the realtime device
>  * common: fix _require_xfs_io_command pwrite -A for various blocksizes
> ---

Do I miss something? I didn't find [PATCH 1/7] in this patch thread :)

>  common/atomicwrites |    6 ++++++
>  common/rc           |   14 +++++++++++---
>  tests/generic/211   |    6 ++++++
>  tests/generic/427   |    5 +++++
>  tests/generic/767   |   19 +++++++++++++++----
>  tests/xfs/838       |    1 +
>  6 files changed, 44 insertions(+), 7 deletions(-)
> 

Re: [PATCH 7/7] common: fix _require_xfs_io_command pwrite -A for various blocksizes

[Zorro Lang]

On Tue, Jul 29, 2025 at 01:10:04PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> In this predicate, we should test an atomic write of the minimum
> supported size, not just 4k.  This fixes a problem where none of the
> atomic write tests actually run on a 32k-fsblock xfs because you can't
> do a sub-fsblock atomic write.
> 
> Cc: <fstests@vger.kernel.org> # v2025.04.13
> Fixes: d90ee3b6496346 ("generic: add a test for atomic writes")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  common/rc |   14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/common/rc b/common/rc
> index 96578d152dafb9..177e7748f4bb89 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3027,16 +3027,24 @@ _require_xfs_io_command()
>  	"pwrite")
>  		# -N (RWF_NOWAIT) only works with direct vectored I/O writes
>  		local pwrite_opts=" "
> +		local write_size="4k"
>  		if [ "$param" == "-N" ]; then
>  			opts+=" -d"
> -			pwrite_opts+="-V 1 -b 4k"
> +			pwrite_opts+="-V 1 -b $write_size"
>  		fi
>  		if [ "$param" == "-A" ]; then
>  			opts+=" -d"
> -			pwrite_opts+="-V 1 -b 4k"
> +			# try to write the minimum supported atomic write size
> +			write_size="$($XFS_IO_PROG -f -c "statx -r -m $STATX_WRITE_ATOMIC" $testfile 2>/dev/null | \
> +				grep atomic_write_unit_min | \
> +				grep -o '[0-9]\+')"
> +			if [ -z "$write_size" ] || [ "$write_size" = "0" ]; then
> +				write_size="0 --not-supported"
                                              ^^^^^^^^^^^^^^^

What is this "--not-supported" for? If write_size="0 --not-supported", will we get...


> +			fi
> +			pwrite_opts+="-V 1 -b $write_size"
>  		fi
>  		testio=`$XFS_IO_PROG -f $opts -c \
> -		        "pwrite $pwrite_opts $param 0 4k" $testfile 2>&1`
> +		        "pwrite $pwrite_opts $param 0 $write_size" $testfile 2>&1`

"pwrite -V 1 -b  0 --not-supported 0 0 --not-supported" at here?

Thanks,
Zorro

>  		param_checked="$pwrite_opts $param"
>  		;;
>  	"scrub"|"repair")
> 

Re: [PATCH 5/7] generic/767: allow on any atomic writes filesystem

[Zorro Lang]

On Tue, Jul 29, 2025 at 01:09:32PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This test now works correctly for any atomic writes geometry since we
> control all the parameters.  Remove this restriction so that we can get
> minimal testing on ext4 and fuse2fs.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---

Makes sense to me,

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/generic/767 |    1 -
>  1 file changed, 1 deletion(-)
> 
> 
> diff --git a/tests/generic/767 b/tests/generic/767
> index 161fef03825db4..558aab1e6acf34 100755
> --- a/tests/generic/767
> +++ b/tests/generic/767
> @@ -36,7 +36,6 @@ export SCRATCH_DEV=$dev
>  unset USE_EXTERNAL
>  
>  _require_scratch_write_atomic
> -_require_scratch_write_atomic_multi_fsblock
>  
>  # Check that xfs_io supports the commands needed to run this test
>  # Note: _require_xfs_io_command is not used here because we want to
> 

Re: [PATCH 3/7] generic/767: require fallocate support

[John Garry]

On 29/07/2025 21:09, Darrick J. Wong wrote:
> From: Darrick J. Wong<djwong@kernel.org>
> 
> This test fails on filesystems that don't support fallocate, so screen
> them out.
> 
> Cc:<fstests@vger.kernel.org> # v2025.07.13
> Fixes: fa8694c823d853 ("generic: various atomic write tests with hardware and scsi_debug")
> Signed-off-by: "Darrick J. Wong"<djwong@kernel.org>
> ---

Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH 4/7] generic/767: only test the hardware atomic write unit

[John Garry]

On 29/07/2025 21:09, Darrick J. Wong wrote:
> From: Darrick J. Wong<djwong@kernel.org>
> 
> This test sets up scsi_debug so that we can test the fs and block layer
> code for hardware-accelerated atomic writes (and not just a software
> fallback).  However, the userspace ABI for detecting atomic write
> geometry has changed since the start of development (to include said
> software fallback) so we must add some extra code to find the real
> hardware capabilities, and base the write sizes based on that.
> 
> This fixes a test failure with 32k blocksizes because the advertised
> atomic_write_unit_max is 128M and fallocate quickly runs out of space.
> 
> While we're at it fix a stupid variable usage bug in the loop.
> 
> Cc:<fstests@vger.kernel.org> # v2025.07.13
> Fixes: fa8694c823d853 ("generic: various atomic write tests with hardware and scsi_debug")
> Signed-off-by: "Darrick J. Wong"<djwong@kernel.org>

Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH 5/7] generic/767: allow on any atomic writes filesystem

[John Garry]

On 29/07/2025 21:09, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This test now works correctly for any atomic writes geometry since we
> control all the parameters.  Remove this restriction so that we can get
> minimal testing on ext4 and fuse2fs.

The test comment reads "Validate multi-fsblock atomic write support with 
simulated hardware support". Is that still true?

On another point, in checking generic/765, there still does seem to be 
enough exclusive tests in 765 to make that test still have value.

> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
 > ---

Reviewed-by: John Garry <john.g.garry@oracle.com>

>   tests/generic/767 |    1 -
>   1 file changed, 1 deletion(-)
> 
> 
> diff --git a/tests/generic/767 b/tests/generic/767
> index 161fef03825db4..558aab1e6acf34 100755
> --- a/tests/generic/767
> +++ b/tests/generic/767
> @@ -36,7 +36,6 @@ export SCRATCH_DEV=$dev
>   unset USE_EXTERNAL
>   
>   _require_scratch_write_atomic
> -_require_scratch_write_atomic_multi_fsblock
>   
>   # Check that xfs_io supports the commands needed to run this test
>   # Note: _require_xfs_io_command is not used here because we want to
> 
> 

Re: [PATCH 6/7] xfs/838: actually force usage of the realtime device

[John Garry]

On 29/07/2025 21:09, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Actually force the filesystem to create testfile as a realtime file so
> that the test does anything useful.
> 
> Cc: <fstests@vger.kernel.org> # v2025.07.13
> Fixes: 7ca990e22e0d61 ("xfs: more multi-block atomic writes tests")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>   tests/xfs/838 |    1 +
>   1 file changed, 1 insertion(+)
> 
> 
> diff --git a/tests/xfs/838 b/tests/xfs/838
> index 73e91530b53a67..84e6a750eb07ab 100755
> --- a/tests/xfs/838
> +++ b/tests/xfs/838
> @@ -23,6 +23,7 @@ $XFS_IO_PROG -c "statx -r -m $STATX_WRITE_ATOMIC" $SCRATCH_RTDEV >> $seqres.full
>   
>   _scratch_mkfs >> $seqres.full
>   _scratch_mount
> +_xfs_force_bdev realtime $SCRATCH_MNT

I'm struggling to follow this test setup.

How are we ensuring that reflink is available for the rt paths?

Thanks,
John

>   
>   testfile=$SCRATCH_MNT/testfile
>   touch $testfile
> 
> 

Re: [PATCH 7/7] common: fix _require_xfs_io_command pwrite -A for various blocksizes

[John Garry]

On 29/07/2025 21:10, Darrick J. Wong wrote:
> From: Darrick J. Wong<djwong@kernel.org>
> 
> In this predicate, we should test an atomic write of the minimum
> supported size, not just 4k.  This fixes a problem where none of the
> atomic write tests actually run on a 32k-fsblock xfs because you can't
> do a sub-fsblock atomic write.
> 
> Cc:<fstests@vger.kernel.org> # v2025.04.13
> Fixes: d90ee3b6496346 ("generic: add a test for atomic writes")
> Signed-off-by: "Darrick J. Wong"<djwong@kernel.org>

Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH 2/7] generic/427: try to ensure there's some free space before we do the aio test

[Darrick J. Wong]

On Wed, Jul 30, 2025 at 07:18:48AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 29, 2025 at 01:08:46PM -0700, Darrick J. Wong wrote:
> > The pwrite failure comes from the aio-dio-eof-race.c program because the
> > filesystem ran out of space.  There are no speculative posteof
> > preallocations on a zoned filesystem, so let's skip this test on those
> > setups.
> 
> Did it run out of space because it is overwriting and we need a new
> allocation (I've not actually seen this fail in my zoned testing,
> that's why I'm asking)?  If so it really should be using the new
> _require_inplace_writes Filipe just sent to the list.

I took a deeper look into what's going on here, and I think the
intermittent ENOSPC failures are caused by:

1. First we write to every byte in the 256M zoned rt device so that
   0x55 gets written to the disk.
2. Then we delete the huge file we created.
3. The zoned garbage collector doesn't run.
4. aio-dio-eof-race starts up and initiates an aiodio at pos 0.
5. xfs_file_dio_write_zoned calls xfs_zoned_write_space_reserve
6. xfs_zoned_space_reserve tries to decrement 64k from XC_FREE_RTEXTENTS
   but gets ENOSPC.
7. We didn't pass XFS_ZR_GREEDY, so we error out.

If I make the test sleep until I see zonegc do some work before starting
aio-dio-eof-race, the problem goes away.  I'm not sure what the proper
solution is, but maybe it's adding a wake_up to the gc process and
waiting for it?

diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
index 1313c55b8cbe51..dfd0384f8e3931 100644
--- a/fs/xfs/xfs_zone_space_resv.c
+++ b/fs/xfs/xfs_zone_space_resv.c
@@ -223,15 +223,25 @@ xfs_zoned_space_reserve(
        unsigned int                    flags,
        struct xfs_zone_alloc_ctx       *ac)
 {
+       int                             tries = 5;
        int                             error;
 
        ASSERT(ac->reserved_blocks == 0);
        ASSERT(ac->open_zone == NULL);
 
+again:
        error = xfs_dec_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb,
                        flags & XFS_ZR_RESERVED);
        if (error == -ENOSPC && (flags & XFS_ZR_GREEDY) && count_fsb > 1)
                error = xfs_zoned_reserve_extents_greedy(mp, &count_fsb, flags);
+       if (error == -ENOSPC && !(flags & XFS_ZR_GREEDY) && --tries) {
+               struct xfs_zone_info    *zi = mp->m_zone_info;
+
+               xfs_err(mp, "OI ZONEGC %d", tries);
+               wake_up_process(zi->zi_gc_thread);
+               udelay(100);
+               goto again;
+       }
        if (error)
                return error;
 
This fugly patch makes the test failures go away.  On my system we
rarely go below "OI ZONEGC 2" after 100x runs.

> If now we need to figure out what this depends on instead of adding
> random xfs-specific hacks to common code.

<nod> I saw the "this tests speculative posteof preallocations" and
thought that didn't sound like an interesting test on a zoned fs. ;)

--D

Re: [PATCH 2/7] generic/427: try to ensure there's some free space before we do the aio test

[Christoph Hellwig]

On Tue, Aug 12, 2025 at 11:54:59AM -0700, Darrick J. Wong wrote:
> On Wed, Jul 30, 2025 at 07:18:48AM -0700, Christoph Hellwig wrote:
> > On Tue, Jul 29, 2025 at 01:08:46PM -0700, Darrick J. Wong wrote:
> > > The pwrite failure comes from the aio-dio-eof-race.c program because the
> > > filesystem ran out of space.  There are no speculative posteof
> > > preallocations on a zoned filesystem, so let's skip this test on those
> > > setups.
> > 
> > Did it run out of space because it is overwriting and we need a new
> > allocation (I've not actually seen this fail in my zoned testing,
> > that's why I'm asking)?  If so it really should be using the new
> > _require_inplace_writes Filipe just sent to the list.
> 
> I took a deeper look into what's going on here, and I think the
> intermittent ENOSPC failures are caused by:
> 
> 1. First we write to every byte in the 256M zoned rt device so that
>    0x55 gets written to the disk.
> 2. Then we delete the huge file we created.
> 3. The zoned garbage collector doesn't run.
> 4. aio-dio-eof-race starts up and initiates an aiodio at pos 0.
> 5. xfs_file_dio_write_zoned calls xfs_zoned_write_space_reserve
> 6. xfs_zoned_space_reserve tries to decrement 64k from XC_FREE_RTEXTENTS
>    but gets ENOSPC.
> 7. We didn't pass XFS_ZR_GREEDY, so we error out.
> 
> If I make the test sleep until I see zonegc do some work before starting
> aio-dio-eof-race, the problem goes away.  I'm not sure what the proper
> solution is, but maybe it's adding a wake_up to the gc process and
> waiting for it?

Isn't the problem here that zonegc only even sees the freed block
after inodegc did run?  i.e. after 2 the inode hasn't been truncated
yet, and thus the blocks haven't been marked as free.

Re: [PATCH 2/7] generic/427: try to ensure there's some free space before we do the aio test

[Darrick J. Wong]

On Tue, Aug 12, 2025 at 10:15:55PM -0700, Christoph Hellwig wrote:
> On Tue, Aug 12, 2025 at 11:54:59AM -0700, Darrick J. Wong wrote:
> > On Wed, Jul 30, 2025 at 07:18:48AM -0700, Christoph Hellwig wrote:
> > > On Tue, Jul 29, 2025 at 01:08:46PM -0700, Darrick J. Wong wrote:
> > > > The pwrite failure comes from the aio-dio-eof-race.c program because the
> > > > filesystem ran out of space.  There are no speculative posteof
> > > > preallocations on a zoned filesystem, so let's skip this test on those
> > > > setups.
> > > 
> > > Did it run out of space because it is overwriting and we need a new
> > > allocation (I've not actually seen this fail in my zoned testing,
> > > that's why I'm asking)?  If so it really should be using the new
> > > _require_inplace_writes Filipe just sent to the list.
> > 
> > I took a deeper look into what's going on here, and I think the
> > intermittent ENOSPC failures are caused by:
> > 
> > 1. First we write to every byte in the 256M zoned rt device so that
> >    0x55 gets written to the disk.
> > 2. Then we delete the huge file we created.
> > 3. The zoned garbage collector doesn't run.
> > 4. aio-dio-eof-race starts up and initiates an aiodio at pos 0.
> > 5. xfs_file_dio_write_zoned calls xfs_zoned_write_space_reserve
> > 6. xfs_zoned_space_reserve tries to decrement 64k from XC_FREE_RTEXTENTS
> >    but gets ENOSPC.
> > 7. We didn't pass XFS_ZR_GREEDY, so we error out.
> > 
> > If I make the test sleep until I see zonegc do some work before starting
> > aio-dio-eof-race, the problem goes away.  I'm not sure what the proper
> > solution is, but maybe it's adding a wake_up to the gc process and
> > waiting for it?
> 
> Isn't the problem here that zonegc only even sees the freed block
> after inodegc did run?  i.e. after 2 the inode hasn't been truncated
> yet, and thus the blocks haven't been marked as free.

Yeah... for the other ENOSPC-on-write paths, we kick inodegc, so maybe
xfs_zoned_space_reserve (or its caller, more likely) ought to do that
too?

--D

Re: [PATCH 2/7] generic/427: try to ensure there's some free space before we do the aio test

[Christoph Hellwig]

On Tue, Aug 12, 2025 at 11:14:52PM -0700, Darrick J. Wong wrote:
> Yeah... for the other ENOSPC-on-write paths, we kick inodegc, so maybe
> xfs_zoned_space_reserve (or its caller, more likely) ought to do that
> too?

Can you give this a spin?  Still running testing here, but so far
nothing blew up.

diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
index 1313c55b8cbe..9cd38716fd25 100644
--- a/fs/xfs/xfs_zone_space_resv.c
+++ b/fs/xfs/xfs_zone_space_resv.c
@@ -10,6 +10,7 @@
 #include "xfs_mount.h"
 #include "xfs_inode.h"
 #include "xfs_rtbitmap.h"
+#include "xfs_icache.h"
 #include "xfs_zone_alloc.h"
 #include "xfs_zone_priv.h"
 #include "xfs_zones.h"
@@ -230,6 +231,11 @@ xfs_zoned_space_reserve(
 
 	error = xfs_dec_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb,
 			flags & XFS_ZR_RESERVED);
+	if (error == -ENOSPC && !(flags & XFS_ZR_NOWAIT)) {
+		xfs_inodegc_flush(mp);
+		error = xfs_dec_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb,
+				flags & XFS_ZR_RESERVED);
+	}
 	if (error == -ENOSPC && (flags & XFS_ZR_GREEDY) && count_fsb > 1)
 		error = xfs_zoned_reserve_extents_greedy(mp, &count_fsb, flags);
 	if (error)

Re: [PATCH 2/7] generic/427: try to ensure there's some free space before we do the aio test

[Darrick J. Wong]

On Tue, Aug 12, 2025 at 11:24:11PM -0700, Christoph Hellwig wrote:
> On Tue, Aug 12, 2025 at 11:14:52PM -0700, Darrick J. Wong wrote:
> > Yeah... for the other ENOSPC-on-write paths, we kick inodegc, so maybe
> > xfs_zoned_space_reserve (or its caller, more likely) ought to do that
> > too?
> 
> Can you give this a spin?  Still running testing here, but so far
> nothing blew up.

Running inodegc_flush() once doesn't fix it, but doesn't hurt either.

--D

> diff --git a/fs/xfs/xfs_zone_space_resv.c b/fs/xfs/xfs_zone_space_resv.c
> index 1313c55b8cbe..9cd38716fd25 100644
> --- a/fs/xfs/xfs_zone_space_resv.c
> +++ b/fs/xfs/xfs_zone_space_resv.c
> @@ -10,6 +10,7 @@
>  #include "xfs_mount.h"
>  #include "xfs_inode.h"
>  #include "xfs_rtbitmap.h"
> +#include "xfs_icache.h"
>  #include "xfs_zone_alloc.h"
>  #include "xfs_zone_priv.h"
>  #include "xfs_zones.h"
> @@ -230,6 +231,11 @@ xfs_zoned_space_reserve(
>  
>  	error = xfs_dec_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb,
>  			flags & XFS_ZR_RESERVED);
> +	if (error == -ENOSPC && !(flags & XFS_ZR_NOWAIT)) {
> +		xfs_inodegc_flush(mp);
> +		error = xfs_dec_freecounter(mp, XC_FREE_RTEXTENTS, count_fsb,
> +				flags & XFS_ZR_RESERVED);
> +	}
>  	if (error == -ENOSPC && (flags & XFS_ZR_GREEDY) && count_fsb > 1)
>  		error = xfs_zoned_reserve_extents_greedy(mp, &count_fsb, flags);
>  	if (error)
> 

Re: [PATCH 2/7] generic/427: try to ensure there's some free space before we do the aio test

[Christoph Hellwig]

On Thu, Aug 14, 2025 at 03:16:23PM -0700, Darrick J. Wong wrote:
> On Tue, Aug 12, 2025 at 11:24:11PM -0700, Christoph Hellwig wrote:
> > On Tue, Aug 12, 2025 at 11:14:52PM -0700, Darrick J. Wong wrote:
> > > Yeah... for the other ENOSPC-on-write paths, we kick inodegc, so maybe
> > > xfs_zoned_space_reserve (or its caller, more likely) ought to do that
> > > too?
> > 
> > Can you give this a spin?  Still running testing here, but so far
> > nothing blew up.
> 
> Running inodegc_flush() once doesn't fix it, but doesn't hurt either.

Weird.  I can't think of anything else that would hide the available
space in this case.

Re: [PATCH 7/7] common: fix _require_xfs_io_command pwrite -A for various blocksizes

[Darrick J. Wong]

On Sat, Aug 02, 2025 at 02:53:15AM +0800, Zorro Lang wrote:
> On Tue, Jul 29, 2025 at 01:10:04PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > In this predicate, we should test an atomic write of the minimum
> > supported size, not just 4k.  This fixes a problem where none of the
> > atomic write tests actually run on a 32k-fsblock xfs because you can't
> > do a sub-fsblock atomic write.
> > 
> > Cc: <fstests@vger.kernel.org> # v2025.04.13
> > Fixes: d90ee3b6496346 ("generic: add a test for atomic writes")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  common/rc |   14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/common/rc b/common/rc
> > index 96578d152dafb9..177e7748f4bb89 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -3027,16 +3027,24 @@ _require_xfs_io_command()
> >  	"pwrite")
> >  		# -N (RWF_NOWAIT) only works with direct vectored I/O writes
> >  		local pwrite_opts=" "
> > +		local write_size="4k"
> >  		if [ "$param" == "-N" ]; then
> >  			opts+=" -d"
> > -			pwrite_opts+="-V 1 -b 4k"
> > +			pwrite_opts+="-V 1 -b $write_size"
> >  		fi
> >  		if [ "$param" == "-A" ]; then
> >  			opts+=" -d"
> > -			pwrite_opts+="-V 1 -b 4k"
> > +			# try to write the minimum supported atomic write size
> > +			write_size="$($XFS_IO_PROG -f -c "statx -r -m $STATX_WRITE_ATOMIC" $testfile 2>/dev/null | \
> > +				grep atomic_write_unit_min | \
> > +				grep -o '[0-9]\+')"
> > +			if [ -z "$write_size" ] || [ "$write_size" = "0" ]; then
> > +				write_size="0 --not-supported"
>                                               ^^^^^^^^^^^^^^^
> 
> What is this "--not-supported" for? If write_size="0 --not-supported", will we get...
> 
> 
> > +			fi
> > +			pwrite_opts+="-V 1 -b $write_size"
> >  		fi
> >  		testio=`$XFS_IO_PROG -f $opts -c \
> > -		        "pwrite $pwrite_opts $param 0 4k" $testfile 2>&1`
> > +		        "pwrite $pwrite_opts $param 0 $write_size" $testfile 2>&1`
> 
> "pwrite -V 1 -b  0 --not-supported 0 0 --not-supported" at here?

Yes, this is correct.  "--not-supported" is not a valid flag to the
pwrite subcommand.  I'm injecting it here intentionally when we don't
detect any atomic write capability so that the _require checks will
behave as if xfs_io doesn't support the -A flag, and _notrun the test.

--D

> Thanks,
> Zorro
> 
> >  		param_checked="$pwrite_opts $param"
> >  		;;
> >  	"scrub"|"repair")
> > 
> 
>