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

Darrick J. Wong
2025-01-08T17:25:37Z

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