Re: [PATCH v7 14/14] NFSD: Initialize separate ki_flags

[Mike Snitzer <snitzer@kernel.org>]

On Mon, Oct 27, 2025 at 06:50:03AM -0400, Jeff Layton wrote:
> On Mon, 2025-10-27 at 01:15 -0700, Christoph Hellwig wrote:
> > On Fri, Oct 24, 2025 at 07:56:59PM -0400, Mike Snitzer wrote:
> > > Really, even ignoring all the quirkiness of this: that O_DIRECT can
> > > fallback to buffered, and we need IOCB_DSYNC|IOCB_SYNC for our use of
> > > buffered IO when NFSD_IO_DIRECT configured to ensure data has hit
> > > stable storage -- that's enough justification.  Bit circular but
> > > compelling to prove the need.. albeit wordy and a lot to unpack.
> > 
> > You always need IOCB_DSYNC for data to hit stable storage, both for
> > buffered and direct I/O.  You need IOCB_SYNC in addition to also sync
> > out the timestamps, which I think we now agree we need.  I still don't
> > understand why using direct I/O implies that we want NFS stable writes
> > and not two-stage writes, though.
> 
> That's certainly a possibility too. Consider the case where we have a
> WRITE with unaligned parts at both ends. This set so far just does the
> ends as synchronous I/Os.
> 
> We could do the end bits as non-synchronous writes, and follow up with
> a vfs_fsync_range() call before returning NFS_FILE_SYNC.
> 
> We could also just return NFS_FILE_UNSTABLE and let the client follow
> up with a commit when the write is unaligned. That may be the most
> efficient scheme if you have a client streaming unaligned writes to the
> server without gaps.

Yes, that is how I handled streaming unaligned writes last I needed to
benchmark it (IO500's IOR_HARD benchmark uses IO size of 47008 bytes).

Use buffered IO for misaligned WRITE's head/tail.  In addition, I had
the heuristic to always used buffered IO for any READ less than 32K
(the misaligned head and tail included).  This allows to leverage page
cache for RMW needed to service the unaligned streaming WRITE.

> My feeling is that if you're doing a lot of unaligned I/Os, you're
> probably better off not enabling DIO support and just doing regular
> buffered (or DONTCACHE) I/Os.

DONTCACHE will not do well for RMW because it'll immediately drop IO
it just read.

But will be testing this further...

> That said, we don't really know either way (which is why all of this is
> behind debugfs switch instead of a more permanent interface).

Yeap.
 
> > You also need IOCB_DSYNC for direct I/O to hit the media if you want
> > to return NFS_FILE_SYNC.  But I still don't understand why we want or
> > need to return NFS_FILE_SYNC to start with.
> 
> NFS_FILE_SYNC is not required here, but it's better if we can return
> that. If the server returns NFS_FILE_SYNC there is no need for the
> client to follow up with a COMMIT.

Yes, which is why I'm confused by Chuck wanting to do away with
NFSD_IO_DIRECT setting NFS_FILE_SYNC "for now".  Not heard compelling
reason, but "it is what it is". ;)

Were we not all concerned about safety first (especially of mixing
buffered and DIO) and performance a secondary concern?  Using
IOCB_DSYNC|IOCB_SYNC for all WRITEs and returning NFS_FILE_SYNC is
really safe right?

And we already showed that doing so really isn't slow.