Re: [PATCH 2/5] min_dio_alignment: add a -r option to query read alignment

[Zorro Lang <zlang@kernel.org>]

On Thu, Sep 03, 2026 at 07:34:09AM +0200, Christoph Hellwig wrote:
> On Thu, Sep 03, 2026 at 12:26:26AM +0800, Zorro Lang wrote:
> > On Mon, Aug 31, 2026 at 09:51:05AM +0300, Christoph Hellwig wrote:
> > > Add an option to query the STATX_DIO_READ_ALIGN if provided.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  src/min_dio_alignment.c | 44 ++++++++++++++++++++++++++++++-----------
> > >  1 file changed, 33 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/src/min_dio_alignment.c b/src/min_dio_alignment.c
> > > index 866fba1546c4..143cee684ab4 100644
> > > --- a/src/min_dio_alignment.c
> > > +++ b/src/min_dio_alignment.c
> > > @@ -1,17 +1,19 @@
> > >  // SPDX-License-Identifier: GPL-2.0
> > >  /*
> > > - * Copyright (c) 2024 Christoph Hellwig
> > > + * Copyright (c) 2024,2026 Christoph Hellwig
> > >   */
> > >  #include <fcntl.h>
> > > +#include <stdbool.h>
> > >  #include <stdio.h>
> > >  #include <stdlib.h>
> > >  #include <unistd.h>
> > > +#include <getopt.h>
> > >  #include <sys/mount.h>
> > >  #include <sys/ioctl.h>
> > >  #include <sys/stat.h>
> > >  #include "statx.h"
> > >  
> > > -static int min_dio_alignment(const char *mntpnt, const char *devname)
> > > +static int min_dio_alignment(const char *mntpnt, const char *devname, bool read)
> > >  {
> > >  	struct statx stx = { };
> > >  	struct stat st;
> > > @@ -27,9 +29,16 @@ static int min_dio_alignment(const char *mntpnt, const char *devname)
> > >  	 */
> > >  	fd = open(mntpnt, O_TMPFILE | O_RDWR | O_EXCL, 0600);
> > >  	if (fd >= 0 &&
> > > -	    xfstests_statx(fd, "", AT_EMPTY_PATH, STATX_DIOALIGN, &stx) == 0 &&
> > > -	    (stx.stx_mask & STATX_DIOALIGN) && stx.stx_dio_offset_align != 0)
> > > -		return stx.stx_dio_offset_align;
> > > +	    xfstests_statx(fd, "", AT_EMPTY_PATH, STATX_DIOALIGN, &stx) == 0) {
> > 
> > 
> > Hmm... shouldn't STATX_DIO_READ_ALIGN be passed in the request mask as well?
> > 
> >   xfstests_statx(fd, "", AT_EMPTY_PATH,
> >                  STATX_DIOALIGN|STATX_DIO_READ_ALIGN, &stx)
> 
> Yeah, strictly speaking we should.  Currently this is only supported
> for XFS, and XFS automatically adds STATX_DIO_READ_ALIGN when
> STATX_DIOALIGN is requested, so it works.  But more by chance than
> design.

I couldn't find any mention in the statx doc that STATX_DIO_READ_ALIGN is
added by default, explicitly specifying it seems more aligned with how
the mask parameter is intended to work. Would you like me to fold this in
when merging? or did you omit it due to other concerns?

This is the only open point left for the patchset, once we reach consensus,
I'm good to merge it :)

Thanks,
Zorro

> 
>