Re: [PATCH 1/6] generic/765: fix a few issues

[John Garry <john.g.garry@oracle.com>]

On 15/05/2025 15:54, Darrick J. Wong wrote:
> On Thu, May 15, 2025 at 09:16:12AM +0100, John Garry wrote:
>> On 14/05/2025 16:38, Darrick J. Wong wrote:
>>>>> --- a/common/rc
>>>>> +++ b/common/rc
>>>>> @@ -2989,7 +2989,7 @@ _require_xfs_io_command()
>>>>>     		fi
>>>>>     		if [ "$param" == "-A" ]; then
>>>>>     			opts+=" -d"
>>>>> -			pwrite_opts+="-D -V 1 -b 4k"
>>>>> +			pwrite_opts+="-d -V 1 -b 4k"
>>>> according to the documentation for -b, 4096 is the default (so I don't think
>>>> that we need to set it explicitly). But is that flag even relevant to
>>>> pwritev2?
>>> The documentation is wrong -- on XFS the default is the fs blocksize.
>>> Everywhere else is 4k.
>>
>> Right, I see that in init_cvtnum()
>>
>> However, from checking write_buffer(), we seem to split writes on this
>> blocksize - that does not seem proper in this instance.
>>
>> Should we really be doing something like:
>>
>> xfs_io -d -C "pwrite -b $SIZE -V 1 -A -D 0 $SIZE" file
> 
> In _require_xfs_io_command?  That only writes the first 4k of a file, so
> matching buffer size is ok.

right, I missed that. The usage in _require_xfs_io_command looks ok.

> 
> Are you asking if _require_xfs_io_command should seek out the filesystem
> block size, and use that for the buffer and write size arguments instead
> of hardcoding 4k?  For atomic writes, maybe it should be doing this,
> since the fs blocksize could be 64k.
> 

I was just a bit thrown by how we need to specify -b $size with -A to 
actually write $size atomically.

Thanks,
John