Re: [PATCH v4 1/4] NFSD: avoid using iov_iter_is_aligned() in nfsd_iter_read()

[Mike Snitzer <snitzer@kernel.org>]

On Wed, Aug 06, 2025 at 09:18:51AM -0400, Chuck Lever wrote:
> On 8/5/25 2:44 PM, Mike Snitzer wrote:
> > From: Mike Snitzer <snitzer@hammerspace.com>
> > 
> > Check the bvec is DIO-aligned while creating it, saves CPU cycles by
> > avoiding iterating the bvec elements a second time using
> > iov_iter_is_aligned().
> > 
> > This prepares for Keith Busch's near-term removal of the
> > iov_iter_is_aligned() interface.  This fixes cel/nfsd-testing commit
> > 5d78ac1e674b4 ("NFSD: issue READs using O_DIRECT even if IO is
> > misaligned") and it should be folded into that commit so that NFSD
> > doesn't require iov_iter_is_aligned() while it is being removed
> > upstream in parallel.
> > 
> > Fixes: cel/nfsd-testing 5d78ac1e674b4 ("NFSD: issue READs using O_DIRECT even if IO is misaligned")
> > Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
> > ---
> >  fs/nfsd/vfs.c | 29 +++++++++++++++--------------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 46189020172fb..e1751d3715264 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -1226,7 +1226,10 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  			 */
> >  			offset = read_dio.start;
> >  			in_count = read_dio.end - offset;
> > -			kiocb.ki_flags = IOCB_DIRECT;
> > +			/* Verify ondisk DIO alignment, memory addrs checked below */
> > +			if (likely(((offset | in_count) &
> > +				    (nf->nf_dio_read_offset_align - 1)) == 0))
> > +				kiocb.ki_flags = IOCB_DIRECT;
> >  		}
> >  	} else if (nfsd_io_cache_read == NFSD_IO_DONTCACHE)
> >  		kiocb.ki_flags = IOCB_DONTCACHE;
> > @@ -1236,16 +1239,24 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  	v = 0;
> >  	total = in_count;
> >  	if (read_dio.start_extra) {
> > -		bvec_set_page(&rqstp->rq_bvec[v++], read_dio.start_extra_page,
> > +		bvec_set_page(&rqstp->rq_bvec[v], read_dio.start_extra_page,
> >  			      read_dio.start_extra, PAGE_SIZE - read_dio.start_extra);
> > +		if (unlikely((kiocb.ki_flags & IOCB_DIRECT) &&
> > +			     rqstp->rq_bvec[v].bv_offset & (nf->nf_dio_mem_align - 1)))
> > +			kiocb.ki_flags &= ~IOCB_DIRECT;
> >  		total -= read_dio.start_extra;
> > +		v++;
> >  	}
> >  	while (total) {
> >  		len = min_t(size_t, total, PAGE_SIZE - base);
> > -		bvec_set_page(&rqstp->rq_bvec[v++], *(rqstp->rq_next_page++),
> > -			      len, base);
> > +		bvec_set_page(&rqstp->rq_bvec[v], *(rqstp->rq_next_page++), len, base);
> > +		/* No need to verify memory is DIO-aligned since bv_offset is 0 */
> > +		if (unlikely((kiocb.ki_flags & IOCB_DIRECT) && base &&
> > +			     (base & (nf->nf_dio_mem_align - 1))))
> > +			kiocb.ki_flags &= ~IOCB_DIRECT;
> >  		total -= len;
> >  		base = 0;
> > +		v++;
> >  	}
> >  	if (WARN_ONCE(v > rqstp->rq_maxpages,
> >  		      "%s: v=%lu exceeds rqstp->rq_maxpages=%lu\n", __func__,
> > @@ -1256,16 +1267,6 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  	if (!host_err) {
> >  		trace_nfsd_read_vector(rqstp, fhp, offset, in_count);
> >  		iov_iter_bvec(&iter, ITER_DEST, rqstp->rq_bvec, v, in_count);
> > -
> > -		/* Double check nfsd_analyze_read_dio's DIO-aligned result */
> > -		if (unlikely((kiocb.ki_flags & IOCB_DIRECT) &&
> > -			     !iov_iter_is_aligned(&iter,
> > -				nf->nf_dio_mem_align - 1,
> > -				nf->nf_dio_read_offset_align - 1))) {
> > -			/* Fallback to buffered IO */
> > -			kiocb.ki_flags &= ~IOCB_DIRECT;
> > -		}
> > -
> >  		host_err = vfs_iocb_iter_read(file, &kiocb, &iter);
> >  	}
> >  
> 
> Hi Mike,
> 
> In cases where the SQUASHME patch is this large, I usually drop the
> patch (or series) in nfsd-testing and ask the contributor to rebase and
> repost. This gets the new version of the patch properly archived on
> lore, for one thing.

Yeah, make sense, I missed that iov_iter_is_aligned() was used early
on in the series too, so I'll fixup further back.
 
> Before reposting, please do run checkpatch.pl on the series.

Will do, will also ensure bisect safe and that sparse is happy.

Thanks,
Mike