Re: [PATCH v11 2/3] NFSD: Implement NFSD_IO_DIRECT for NFS WRITE

[Mike Snitzer <snitzer@kernel.org>]

On Thu, Nov 13, 2025 at 12:13:09AM -0800, Christoph Hellwig wrote:
> On Wed, Nov 12, 2025 at 06:14:15PM -0500, Mike Snitzer wrote:
> > > -	 * Check if direct I/O is feasible for this write request.
> > > -	 * If alignments are not available, the write is too small,
> > > -	 * or no alignment can be found, fall back to buffered I/O.
> > > +	 * If the file system doesn't advertise any alignment requirements,
> > > +	 * don't try to issue direct I/O.  Fall back to uncached buffered
> > > +	 * I/O if possible because we'll assume it is not block based and
> > > +	 * doesn't need read-modify-write cycles.
> > >  	 */
> > 
> > Not sure what is implied by "not block based" in this comment.
> 
> e.g. re-exporting another network file system or tmpfs.
> 
> > > +	/*
> > > +	 * If the I/O is smaller than the larger of the memory and logical
> > > +	 * offset alignment, it is like to require read-modify-write cycles.
> > > +	 * Issue cached buffered I/O.
> > > +	 */
> > 
> > Nit: s/like/likely/ typo.
> 
> Thanks.
> 
> > > +	if (!middle ||
> > > +	    ((prefix || suffix) && middle < PAGE_SIZE * 2))
> > >  		goto no_dio;
> > >  
> > >  	if (prefix)
> > 
> > Expressing this threshold in terms of PAGE_SIZE might be a
> > problem for other archs like ARM (where using 64K the norm for
> > performance).
> 
> True.
> 
> > The "Note:" portion of the above comment might do well to address the
> > goal being to avoid using cached buffered for higher order
> > allocations. You or Chuck may have solid ideas for refining wording.
> 
> Honestly I'm not worried about higher order allocations at all.  Yes,
> they used to be very painful in the past, but if we get them here
> it means the file system supports large folios, which means we're going
> to churn through a lot of them anyway.
> 
> > But so you're aware Jon Flynn (who kindly does the heavy lifting of
> > performance testing for me) agrees with you: larger will likely be
> > beneficial but it'll be hardware dependent.  So I'm going to expose a
> > knob for Jon to try various values so we can see.
> 
> That does sound useful, thanks!
> 
> > Overall this patch looks good.  Would welcome seeing you submit a
> > formal patch.  In parallel I'll work with Jon to get this tested on
> > Hammerspace's performance cluster to confirm all looks good.
> 
> I have no problem fixing up the nits and writing a commit message,
> but I have no time to properly test this.  I'm too busy with my
> own projects and just stealing some time to help with whiteboard
> coding here :)  I'd also be perfectly fine with whoever actually
> brings this across taking full credit.

Think it best to be attributed to you so if you have time to polish
the nits and put a header on it that'd establish a base that can be
tweaked (by Chuck) if needed.

You don't need to worry about testing, we'll get that covered for you
and report back.