Re: [PATCH 5/5] xfs: report larger dio alignment for COW inodes

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jan 08, 2025 at 10:11:27AM +0000, John Garry wrote:
> On 08/01/2025 08:55, Christoph Hellwig wrote:
> > For I/O to reflinked blocks we always need to write an entire new file
> > system block, and the code enforces the file system block alignment for
> > the entire file if it has any reflinked blocks.  Mirror the larger
> > value reported in the statx in the dio_offset_align in the xfs-specific
> > XFS_IOC_DIOINFO ioctl for the same reason.
> > 
> > Don't bother adding a new field for the read alignment to this legacy
> > ioctl as all new users should use statx instead.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >   fs/xfs/xfs_ioctl.c | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 0789c18aaa18..20f3cf5391c6 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -1204,7 +1204,16 @@ xfs_file_ioctl(
> >   		struct xfs_buftarg	*target = xfs_inode_buftarg(ip);
> >   		struct dioattr		da;
> > -		da.d_mem =  da.d_miniosz = target->bt_logical_sectorsize;
> > +		da.d_mem = target->bt_logical_sectorsize;
> > +
> > +		/*
> > +		 * See xfs_report_dioalign() why report a potential larger than
> > +		 * sector sizevalue here for COW inodes.
> 
> nit: sizevalue

The sentence reads oddly to me; how about:

"See xfs_report_dioalign() for an explanation about why we report a
value larger than the sector size for COW inodes."?

The code change looks ok though.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> > +		 */
> > +		if (xfs_is_cow_inode(ip))
> > +			da.d_miniosz = xfs_inode_alloc_unitsize(ip);
> > +		else
> > +			da.d_miniosz = target->bt_logical_sectorsize;
> >   		da.d_maxiosz = INT_MAX & ~(da.d_miniosz - 1);
> >   		if (copy_to_user(arg, &da, sizeof(da)))
> 
>