On Wed, Jul 10, 2024 at 07:52:06PM -0700, Darrick J. Wong wrote:
> On Thu, Jul 11, 2024 at 10:36:37AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > As of v6.0, the DIO memory buffer alignment is no longer aligned to
> > the logical sector size of the underlying block device. There is now
> > a specific DMA alignment parameter that memory buffers should be
> > aligned to. statx(STATX_DIOALIGN) gets this right, but
> > XFS_IOC_DIOINFO does not - it still uses the older fixed alignment
> > defined by the block device logical sector size.
> >
> > This was found because the s390 DASD driver increased DMA alignment
> > to PAGE_SIZE in commit bc792884b76f ("s390/dasd: Establish DMA
> > alignment") and DIO aligned to logical sector sizes have started
> > failing on kernels with that commit. Fixing the "userspace fails
> > because device alignment constraints increased" issue is not XFS's
> > problem, but we really should be reporting the correct device memory
> > alignment in XFS_IOC_DIOINFO.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_ioctl.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index f0117188f302..71eba4849e03 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -1368,7 +1368,8 @@ 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 = bdev_dma_alignment(target->bt_bdev);
>
> bdev_dma_alignment returns a mask, so I think you want to add one here?
Ah, yes, good eyes, I forgot to refresh the patch. I'll send an
updated version to the list.
> Though at this point, perhaps DIOINFO should query the STATX_DIOALIGN
> information so xfs doesn't have to maintain this anymore?
We open code the STATX_DIOALIGN stuff in xfs_vn_getattr() ourselves
- there's no point using statx to query information we supply statx
with in the first place.
> (Or just make a helper that statx and DIOINFO can both call?)
If we grow more internal users, then maybe?
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
