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

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 10/25/25 03:46, Darrick J. Wong wrote:
> On Fri, Oct 24, 2025 at 02:48:00PM +0530, Nirjhar Roy (IBM) wrote:
>> On Wed, 2025-10-15 at 09:38 -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 1b78cd0c358bb9..dcae5bc33b19ce 100644
>>> --- a/common/rc
>>> +++ b/common/rc
>>> @@ -3030,16 +3030,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"
>> Nit: We can still keep this to 4k (or any random size and not necessarily a size = fsblocksize),
>> right?
> Well, yes, the default will still be 4k on an old kernel that doesn't
> support STATX_WRITE_ATOMIC.  For kernels that do support that flag,
> write_size will now be whatever the filesystem claims is the minimum
> write unit.

Okay.

--NR

>
>>>   		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`
>> This looks good to me:
>>
>> Reviewed-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> Thanks!
>
> --D
>
>>>   		param_checked="$pwrite_opts $param"
>>>   		;;
>>>   	"scrub"|"repair")
>>>
>>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore