Re: [PATCH 2/2] file-posix: add statx(STATX_DIOALIGN) support

[Stefan Hajnoczi]

On Tue, Nov 01, 2022 at 08:32:30PM -0700, Eric Biggers wrote:
> On Tue, Nov 01, 2022 at 03:00:31PM -0400, Stefan Hajnoczi wrote:
> >      /* Let's try to use the logical blocksize for the alignment. */
> > -    if (probe_logical_blocksize(fd, &bs->bl.request_alignment) < 0) {
> > -        bs->bl.request_alignment = 0;
> > +    if (!bs->bl.request_alignment) {
> > +        if (probe_logical_blocksize(fd, &bs->bl.request_alignment) < 0) {
> > +            bs->bl.request_alignment = 0;
> > +        }
> >      }
> >  
> >  #ifdef __linux__
> > -    /*
> > -     * The XFS ioctl definitions are shipped in extra packages that might
> > -     * not always be available. Since we just need the XFS_IOC_DIOINFO 
> > ioctl
> > -     * here, we simply use our own definition instead:
> > -     */
> > -    struct xfs_dioattr {
> > -        uint32_t d_mem;
> > -        uint32_t d_miniosz;
> > -        uint32_t d_maxiosz;
> > -    } da;
> > -    if (ioctl(fd, _IOR('X', 30, struct xfs_dioattr), &da) >= 0) {
> > -        bs->bl.request_alignment = da.d_miniosz;
> > -        /* The kernel returns wrong information for d_mem */
> > -        /* s->buf_align = da.d_mem; */
> > +    if (!bs->bl.request_alignment) {
> 
> This patch changes the fallback code to make the request_alignment value from
> probe_logical_blocksize() override the value from XFS_IOC_DIOINFO.  Is that
> intentional?

Thanks for pointing out the bug. That was not intentional. Will fix.

> > +        if (ioctl(fd, _IOR('X', 30, struct xfs_dioattr), &da) >= 0) {
> > +            bs->bl.request_alignment = da.d_miniosz;
> > +            /* The kernel returns wrong information for d_mem */
> > +            /* s->buf_align = da.d_mem; */
> 
> Has this bug been reported to the XFS developers (linux-xfs@vger.kernel.org)?

Paolo: Do you remember if you reported this when you wrote commit
c25f53b06eba ("raw: Probe required direct I/O alignment")?

Stefan

signature.asc
Description: PGP signature