Re: [PATCH 1/3] nfsd: avoid using DONTCACHE for misaligned DIO's buffered IO fallback

Mike Snitzer
2025-11-04T17:35:17Z

On Tue, Nov 04, 2025 at 12:23:02PM -0500, Chuck Lever wrote:
> On 11/4/25 11:42 AM, Mike Snitzer wrote:
> > Also, use buffered IO (without DONTCACHE) if READ is less than 32K.
> > But do use DONTCACHE if an entire WRITE is misaligned, this preserves
> > intent of NFSD_IO_DIRECT.
>
> These two changes need to be separate patches.

They are linked, otherwise if READ uses DONTCACHE for the small IO
it'll kill any benefit to RMW.

Unless I'm misunderstanding which two changes you're referring to?

The "But do use DONTCACHE if an entire WRITE is misaligned" just
amounts to a comment tweak in nfsd_direct_write (last hunk below)

> > The misaligned ends of a misaligned DIO WRITE will use buffered IO
> > (without DONTCACHE) but the middle DIO-aligned segment with use direct
> > IO. This provides ideal performance for streaming misaligned DIO
> > (e.g. IO500's IOR_HARD) because buffered IO is used to benefit RMW.
> >
> > On one capable testbed, this commit improved IOR_HARD WRITE
> > performance from 0.3433GB/s to 1.26GB/s.
> >
> > Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
> > ---
> > fs/nfsd/vfs.c | 28 +++++++++++++++++++++++-----
> > 1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 701dd261c252..9403ec8bb2da 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -104,6 +104,7 @@ nfserrno (int errno)
> > { nfserr_perm, -ENOKEY },
> > { nfserr_no_grace, -ENOGRACE},
> > { nfserr_io, -EBADMSG },
> > + { nfserr_eagain, -ENOTBLK },
> > };
> > int i;
> >
> > @@ -1099,13 +1100,18 @@ nfsd_direct_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > size_t len;
> >
> > init_sync_kiocb(&kiocb, nf->nf_file);
> > - kiocb.ki_flags |= IOCB_DIRECT;
> >
> > /* Read a properly-aligned region of bytes into rq_bvec */
> > dio_start = round_down(offset, nf->nf_dio_read_offset_align);
> > dio_end = round_up((u64)offset + *count, nf->nf_dio_read_offset_align);
> >
> > + /* Don't use expanded DIO READ for IO less than 32K */
> > + if ((*count < (32 << 10)) &&
> > + (((offset - dio_start) > 0) || ((dio_end - (offset + *count)) > 0)))
> > + return nfserrno(-ENOTBLK); /* fallback to buffered */
>
> Why not just return a specific nfserr code here? No need to go through
> nfserrno.

Could, I just tethered it to ENOTBLK given the history of such things
elsewhere for direct to buffered fallback. But yes, could just as
easily simply return nfserr_eagain (or some other better suggestion).

> > +
> > kiocb.ki_pos = dio_start;
> > + kiocb.ki_flags |= IOCB_DIRECT;
> >
> > v = 0;
> > total = dio_end - dio_start;
> > @@ -1184,10 +1190,13 @@ __be32 nfsd_iter_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > break;
> > case NFSD_IO_DIRECT:
> > /* When dio_read_offset_align is zero, dio is not supported */
> > - if (nf->nf_dio_read_offset_align && !rqstp->rq_res.page_len)
> > - return nfsd_direct_read(rqstp, fhp, nf, offset,
> > + if (nf->nf_dio_read_offset_align && !rqstp->rq_res.page_len) {
> > + __be32 nfserr = nfsd_direct_read(rqstp, fhp, nf, offset,
> > count, eof);
> > - fallthrough;
> > + if (nfserr != nfserr_eagain)
> > + return nfserr;
> > + }
> > + break; /* fallback to buffered */
> > case NFSD_IO_DONTCACHE:
> > if (file->f_op->fop_flags & FOP_DONTCACHE)
> > kiocb.ki_flags = IOCB_DONTCACHE;
> > @@ -1347,6 +1356,15 @@ nfsd_write_dio_iters_init(struct bio_vec *bvec, unsigned int nvecs,
> > ++args->nsegs;
> > }
> >
> > + /*
> > + * Don't use IOCB_DONTCACHE if misaligned DIO WRITE (args->nsegs > 1),
> > + * because it compromises unaligned segments' RMW IO being able to
> > + * benefit from buffered IO (especially important for streaming
> > + * misaligned DIO WRITE performance).
> > + */
> > + if (args->nsegs > 1 && (args->flags_buffered & IOCB_DONTCACHE))
> > + args->flags_buffered &= ~IOCB_DONTCACHE;
> > +
> > return;
> >
> > no_dio:
> > @@ -1400,7 +1418,7 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >
> > /*
> > * IOCB_DONTCACHE preserves the intent of NFSD_IO_DIRECT when
> > - * writing unaligned segments or handling fallback I/O.
> > + * falling back to buffered IO if entire WRITE is unaligned.
> > */
> > args.flags_buffered = kiocb->ki_flags;
> > if (args.nf->nf_file->f_op->fop_flags & FOP_DONTCACHE)
>
>
> --
> Chuck Lever