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

[Mike Snitzer <snitzer@kernel.org>]

On Fri, Oct 24, 2025 at 10:43:06AM -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Christoph says:
> > > +	if (file->f_op->fop_flags & FOP_DONTCACHE)
> > > +		kiocb->ki_flags |= IOCB_DONTCACHE;
> > IOCB_DONTCACHE isn't defined for IOCB_DIRECT.  So this should
> > move into a branch just for buffered I/O.
> 
> Instead, let's set up separate ki_flags for buffered I/O and for
> direct I/O requests. Then we don't have to set a jumble of flag
> bits in a single flags field.
> 
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/vfs.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index b0e4105e0075..b7b9f8cf0452 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1262,6 +1262,8 @@ struct nfsd_write_dio_seg {
>  
>  struct nfsd_write_dio_args {
>  	struct nfsd_file		*nf;
> +	int				flags_buffered;
> +	int				flags_direct;
>  	struct nfsd_write_dio_seg	segment[3];
>  };
>  
> @@ -1379,11 +1381,11 @@ nfsd_issue_write_dio(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		if (segment->len == 0)
>  			continue;
>  		if (segment->use_dio) {
> -			kiocb->ki_flags |= IOCB_DIRECT;
> +			kiocb->ki_flags = args->flags_direct;
>  			trace_nfsd_write_direct(rqstp, fhp, kiocb->ki_pos,
>  						segment->len);
>  		} else
> -			kiocb->ki_flags &= ~IOCB_DIRECT;
> +			kiocb->ki_flags = args->flags_buffered;
>  		host_err = vfs_iocb_iter_write(file, kiocb, &segment->iter);
>  		if (host_err < 0)
>  			return host_err;
> @@ -1405,30 +1407,27 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	struct nfsd_write_dio_args args;
>  
>  	/*
> -	 * Check if IOCB_DONTCACHE can be used when issuing buffered IO;
> -	 * if so, set it to preserve intent of NFSD_IO_DIRECT (it will
> -	 * be ignored for any DIO issued here).
> +	 * IOCB_DONTCACHE preserves the intent of NFSD_IO_DIRECT when
> +	 * writing unaligned segments or handling fallback I/O.
>  	 */
> +	args.flags_buffered = kiocb->ki_flags | IOCB_SYNC | IOCB_DSYNC;
>  	if (file->f_op->fop_flags & FOP_DONTCACHE)
> -		kiocb->ki_flags |= IOCB_DONTCACHE;
> +		args.flags_buffered |= IOCB_DONTCACHE;
>  
>  	/*
> -	 * 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")

Otherwise, everything else looks good, thanks.

Reviewed-by: Mike Snitzer <snitzer@kernel.org>