Linux NFS development
 help / color / mirror / Atom feed
From: Chuck Lever <cel@kernel.org>
To: NeilBrown <neil@brown.name>, Jeff Layton <jlayton@kernel.org>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: <linux-nfs@vger.kernel.org>, Christoph Hellwig <hch@lst.de>,
	Chuck Lever <chuck.lever@oracle.com>
Subject: [PATCH v8 11/12] NFSD: Handle kiocb->ki_flags correctly
Date: Mon, 27 Oct 2025 11:46:29 -0400	[thread overview]
Message-ID: <20251027154630.1774-12-cel@kernel.org> (raw)
In-Reply-To: <20251027154630.1774-1-cel@kernel.org>

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.

and

> > Promoting all NFSD_IO_DIRECT writes to FILE_SYNC was my idea,
> > based on the assumption that IOCB_DIRECT writes to local file
> > systems left nothing to be done by a later commit. My assumption
> > is based on the behavior of O_DIRECT on NFS files.
> >
> > If that assumption is not true, then I agree there is no
> > technical reason to promote NFSD_IO_DIRECT writes to FILE_SYNC,
> > and I can remove that built-in assumption for v8 of this series.
>
> It is not true, or rather only true for a tiny subset of use cases
> (which NFS can't even query a head of time).

So, observe the existing setting of ki_flags rather than forcing
persistence unconditionally, and ensure that DONTCACHE is not set
for IOCB_DIRECT writes.

Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/vfs.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index be0688f2ab3d..3c78b3aeea4b 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1261,6 +1261,8 @@ struct nfsd_write_dio_seg {
 
 struct nfsd_write_dio_args {
 	struct nfsd_file		*nf;
+	int				flags_buffered;
+	int				flags_direct;
 	unsigned int			nsegs;
 	struct nfsd_write_dio_seg	segment[3];
 };
@@ -1396,33 +1398,25 @@ nfsd_buffered_write(struct svc_rqst *rqstp, struct file *file,
 }
 
 static int
-nfsd_issue_dio_write(struct svc_rqst *rqstp, struct svc_fh *fhp, u32 *stable_how,
-		     struct kiocb *kiocb, unsigned int nvecs, unsigned long *cnt,
-		     struct nfsd_write_dio_args *args)
+nfsd_issue_dio_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
+		     struct kiocb *kiocb, unsigned int nvecs,
+		     unsigned long *cnt, struct nfsd_write_dio_args *args)
 {
 	struct file *file = args->nf->nf_file;
 	ssize_t host_err;
 	unsigned int i;
 
-	/*
-	 * Any buffered IO issued here will be misaligned, use
-	 * sync IO to ensure it has completed before returning.
-	 * Also update @stable_how to avoid need for COMMIT.
-	 */
-	kiocb->ki_flags |= (IOCB_DSYNC|IOCB_SYNC);
-	*stable_how = NFS_FILE_SYNC;
-
 	nfsd_write_dio_iters_init(rqstp->rq_bvec, nvecs, kiocb->ki_pos,
 				  *cnt, args);
 
 	*cnt = 0;
 	for (i = 0; i < args->nsegs; i++) {
 		if (args->segment[i].use_dio) {
-			kiocb->ki_flags |= IOCB_DIRECT;
+			kiocb->ki_flags = args->flags_direct;
 			trace_nfsd_write_direct(rqstp, fhp, kiocb->ki_pos,
 						args->segment[i].iter.count);
 		} else
-			kiocb->ki_flags &= ~IOCB_DIRECT;
+			kiocb->ki_flags = args->flags_buffered;
 
 		host_err = vfs_iocb_iter_write(file, kiocb,
 					       &args->segment[i].iter);
@@ -1446,15 +1440,16 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	args.nf = nf;
 
 	/*
-	 * 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;
 	if (args.nf->nf_file->f_op->fop_flags & FOP_DONTCACHE)
-		kiocb->ki_flags |= IOCB_DONTCACHE;
+		args.flags_buffered |= IOCB_DONTCACHE;
 
-	return nfsd_issue_dio_write(rqstp, fhp, stable_how, kiocb, nvecs,
-				    cnt, &args);
+	args.flags_direct = kiocb->ki_flags | IOCB_DIRECT;
+
+	return nfsd_issue_dio_write(rqstp, fhp, kiocb, nvecs, cnt, &args);
 }
 
 /**
-- 
2.51.0


  parent reply	other threads:[~2025-10-27 15:46 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-27 15:46 [PATCH v8 00/12] NFSD: Implement NFSD_IO_DIRECT for NFS WRITE Chuck Lever
2025-10-27 15:46 ` [PATCH v8 01/12] NFSD: Make FILE_SYNC WRITEs comply with spec Chuck Lever
2025-10-27 15:46 ` [PATCH v8 02/12] NFSD: Enable return of an updated stable_how to NFS clients Chuck Lever
2025-10-27 15:46 ` [PATCH v8 03/12] NFSD: Implement NFSD_IO_DIRECT for NFS WRITE Chuck Lever
2025-10-27 15:46 ` [PATCH v8 04/12] NFSD: Remove specific error handling Chuck Lever
2025-10-27 15:46 ` [PATCH v8 05/12] NFSD: Remove alignment size checking Chuck Lever
2025-10-27 15:46 ` [PATCH v8 06/12] NFSD: Clean up struct nfsd_write_dio Chuck Lever
2025-10-27 15:46 ` [PATCH v8 07/12] NFSD: Introduce struct nfsd_write_dio_seg Chuck Lever
2025-10-27 15:46 ` [PATCH v8 08/12] NFSD: Simplify nfsd_iov_iter_aligned_bvec() Chuck Lever
2025-10-30 15:00   ` Jeff Layton
2025-10-31 13:16   ` Christoph Hellwig
2025-10-27 15:46 ` [PATCH v8 09/12] NFSD: Handle both offset and memory alignment for direct I/O Chuck Lever
2025-10-30 19:52   ` Jeff Layton
2025-10-30 19:55     ` Chuck Lever
2025-10-31  9:13     ` Christoph Hellwig
2025-10-31 13:19   ` Christoph Hellwig
2025-10-31 13:21     ` Chuck Lever
2025-10-31 13:23       ` Christoph Hellwig
2025-10-31 16:07       ` Mike Snitzer
2025-10-27 15:46 ` [PATCH v8 10/12] NFSD: Combine direct I/O feasibility check with iterator setup Chuck Lever
2025-10-30 19:59   ` Jeff Layton
2025-10-31 13:20   ` Christoph Hellwig
2025-10-27 15:46 ` Chuck Lever [this message]
2025-10-30 20:01   ` [PATCH v8 11/12] NFSD: Handle kiocb->ki_flags correctly Jeff Layton
2025-10-31 13:21   ` Christoph Hellwig
2025-10-27 15:46 ` [PATCH v8 12/12] NFSD: Refactor nfsd_vfs_write Chuck Lever
2025-10-30 20:02   ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251027154630.1774-12-cel@kernel.org \
    --to=cel@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=dai.ngo@oracle.com \
    --cc=hch@lst.de \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox