On Mon, Aug 18, 2025 at 07:13:43AM +0200, Christoph Hellwig wrote:
> Use the direct I/O alignment reporting from ->getattr instead of
> reimplementing it. This exposes the relaxation of the memory
> alignment in the XFS_IOC_DIOINFO info and ensure the information will
> stay in sync. Note that randholes.c in xfstests has a bug where it
> incorrectly fails when the required memory alignment is smaller than the
> pointer size. Round up the reported value as there is a fair chance that
> this code got copied into various applications.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_ioctl.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index e1051a530a50..21ae68896caa 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1209,21 +1209,24 @@ xfs_file_ioctl(
> current->comm);
> return -ENOTTY;
> case XFS_IOC_DIOINFO: {
> - struct xfs_buftarg *target = xfs_inode_buftarg(ip);
> + struct kstat st;
> struct dioattr da;
>
> - da.d_mem = target->bt_logical_sectorsize;
> + error = vfs_getattr(&filp->f_path, &st, STATX_DIOALIGN, 0);
> + if (error)
> + return error;
>
> /*
> - * See xfs_report_dioalign() for an explanation about why this
> - * reports a value larger than the sector size for COW inodes.
> + * The randholes tool in xfstests expects the alignment to not
> + * be smaller than the size of a pointer for whatever reason.
> + *
Do we need to keep this comment that tied to an userspace tool? It just
looks weird to have a comment about alignment constraints changes for a single
tool.
The issue with randholes is that it uses posix_memalign, and the pointer
size constraint comes from that.
I couldn't find any details on why this is required, but I'm assuming
it's to keep posix_memalign architecture/implementation independent?!
So, perhaps instead of being 'randholes' specific, it should specify to
be posix compliant or because posix requires this way?
Otherwise it looks good to me
Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> + * Align the report value to that so that the dword (4 byte)
> + * alignment supported by many storage devices doesn't trip it
> + * up.
> */
> - if (xfs_is_cow_inode(ip))
> - da.d_miniosz = xfs_inode_alloc_unitsize(ip);
> - else
> - da.d_miniosz = target->bt_logical_sectorsize;
> + da.d_mem = roundup(st.dio_mem_align, sizeof(void *));
> + da.d_miniosz = st.dio_offset_align;
> da.d_maxiosz = INT_MAX & ~(da.d_miniosz - 1);
> -
> if (copy_to_user(arg, &da, sizeof(da)))
> return -EFAULT;
> return 0;
> --
> 2.47.2
>
