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

[Mike Snitzer <snitzer@kernel.org>]

On Fri, Oct 24, 2025 at 03:34:00PM -0400, Chuck Lever wrote:
> On 10/24/25 2:13 PM, Mike Snitzer wrote:
> >>  	/*
> >> -	 * IOCB_SYNC + IOCB_DIRECT requests that iter_write should persist
> >> -	 * both written data and dirty time stamps.
> >> -	 *
> >> -	 * When falling back to buffered I/O or handling the unaligned
> >> -	 * first and last segments, the data and time stamps must be
> >> -	 * durable before nfsd_vfs_write() returns to its caller, matching
> >> -	 * the behavior of direct I/O.
> >> +	 * IOCB_SYNC + IOCB_DIRECT requests that iter_write should
> >> +	 * persist both written data and dirty time stamps.
> >>  	 */
> >> -	kiocb->ki_flags |= IOCB_SYNC | IOCB_DSYNC;
> >> +	args.flags_direct = kiocb->ki_flags | IOCB_SYNC | IOCB_DIRECT;
> > AFAIK we still need: IOCB_DIRECT | IOCB_DSYNC | IOCB_SYNC
> > 
> > IOCB_DIRECT | IOCB_DSYNC was recently put under a microscope relative
> > to XFS performance and the resulting improvement was merged for 6.18
> > with commit c91d38b57f ("xfs: rework datasync tracking and execution")
> 
> This looks like an xfs-specific fix. I'm reluctant to apply a fix for
> a specific file system implementation in what's supposed to be generic
> code.
> 
> If (IOCB_DIRECT | IOCB_DSYNC | IOCB_SYNC) /is/ correct for all file
> systems, then it needs an explanatory code comment, which I'm not yet
> qualified to write. I don't see any textual material in previous
> incarnations of this code that might help get me started.

The XFS specific performance improvement isn't the point.  The point
is that applications (like I think DB2 is what started all this with
Jan Kara and the XFS filesystem) results in the use of
O_DIRECT+O_DSYNC.

It is a clear reality that other filesystems are catering to
O_DIRECT+O_DSYNC. And given our findings with Christoph that buffered
IO needs O_DSYNC+O_SYNC, I'd rather we not expose ourselves to not
having O_DSYNC set.

Particularly because any filesystem NFSD is writing to _can_ also
fallback to using buffered IO if O_DIRECT set (NFSD is doing exactly
that). Which we _know_ (from Christoph) that having O_DSYNC set is
important when we fallback to using buffered IO (like we do for the
misaligned head and/or tail).

Please let's not make the same mistake twice.