Re: [PATCH v10 2/7] nfs/localio: avoid issuing misaligned IO using O_DIRECT

[Mike Snitzer <snitzer@kernel.org>]

On Thu, Sep 18, 2025 at 01:15:10PM -0400, Anna Schumaker wrote:
> Hi Mike,
> 
> On 9/17/25 2:18 PM, Mike Snitzer wrote:
> > Add nfsd_file_dio_alignment and use it to avoid issuing misaligned IO
> > using O_DIRECT. Any misaligned DIO falls back to using buffered IO.
> > 
> > Because misaligned DIO is now handled safely, remove the nfs modparam
> > 'localio_O_DIRECT_semantics' that was added to require users opt-in to
> > the requirement that all O_DIRECT be properly DIO-aligned.
> > 
> > Also, introduce nfs_iov_iter_aligned_bvec() which is a variant of
> > iov_iter_aligned_bvec() that also verifies the offset associated with
> > an iov_iter is DIO-aligned.  NOTE: in a parallel effort,
> > iov_iter_aligned_bvec() is being removed along with
> > iov_iter_is_aligned().
> > 
> > Lastly, add pr_info_ratelimited if underlying filesystem returns
> > -EINVAL because it was made to try O_DIRECT for IO that is not
> > DIO-aligned (shouldn't happen, so its best to be louder if it does).
> > 
> > Fixes: 3feec68563d ("nfs/localio: add direct IO enablement with sync and async IO support")
> > Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> > ---
> >  fs/nfs/localio.c           | 69 ++++++++++++++++++++++++++++++++------
> >  fs/nfsd/localio.c          | 11 ++++++
> >  include/linux/nfslocalio.h |  2 ++
> >  3 files changed, 72 insertions(+), 10 deletions(-)
> > 
> > diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> > index 42ea50d42c995..380407c41822c 100644
> > --- a/fs/nfs/localio.c
> > +++ b/fs/nfs/localio.c

<snip>

> > @@ -406,6 +441,13 @@ nfs_local_read_done(struct nfs_local_kiocb *iocb, long status)
> >  	struct nfs_pgio_header *hdr = iocb->hdr;
> >  	struct file *filp = iocb->kiocb.ki_filp;
> >  
> > +	if (iocb->kiocb.ki_flags & IOCB_DIRECT) {
> > +		if (status == -EINVAL) {
> > +			/* Underlying FS will return -EINVAL if misaligned DIO is attempted. */
> > +			pr_info_ratelimited("nfs: Unexpected direct I/O read alignment failure\n");
> > +		}
> > +	}
> > +
> >  	nfs_local_pgio_done(hdr, status);
> >  
> >  	/*
> > @@ -598,6 +640,13 @@ nfs_local_write_done(struct nfs_local_kiocb *iocb, long status)
> >  
> >  	dprintk("%s: wrote %ld bytes.\n", __func__, status > 0 ? status : 0);
> >  
> > +	if (iocb->kiocb.ki_flags & IOCB_DIRECT) {
> > +		if (status == -EINVAL) {
> > +			/* Underlying FS will return -EINVAL if misaligned DIO is attempted. */
> > +			pr_info_ratelimited("nfs: Unexpected direct I/O write alignment failure\n");
> > +		}
> > +	}
> > +
> >  	/* Handle short writes as if they are ENOSPC */
> >  	if (status > 0 && status < hdr->args.count) {
> >  		hdr->mds_offset += status;

I'm just noticing these 2 hunks above. I revised them to remove 1
level of nesting but it seems that wasn't folded in early (in this
patch 2) but instead was left until patch 5 to cleanup.

You can leave patch 2 and 5 like they are, just mentioning this
because fixing up patch 2 to reflect how patch 5 left things for these
two hunks would reduce patch 5's changes/churn.

I can also carry that change through and post a v11 -- will wait to do
so until you've been able to get through the rest of the series in
case there are other things that need fixing.

> > diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
> > index 269fa9391dc46..be710d809a3ba 100644
> > --- a/fs/nfsd/localio.c
> > +++ b/fs/nfsd/localio.c
> 
> I'll need an acked-by from Chuck or Jeff for the NFSD portions of this patch.
>
> Thanks,
> Anna

Absolutely, I should've cc'd them (I've done so now).

Thanks,
Mike