[PATCH v3 0/2] Initial NFS client support for RWF_DONTCACHE

[PATCH v3 0/2] Initial NFS client support for RWF_DONTCACHE

[Trond Myklebust]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

The following patch set attempts to add support for the RWF_DONTCACHE
flag in preadv2() and pwritev2() on NFS filesystems.

The main issue is allowing support on 2 stage writes (i.e. unstable
WRITE followed by a COMMIT) since those don't follow the current
assumption that the 'dropbehind' flag can be fulfilled as soon as the
writeback lock is dropped.

v2:
 - Make use of the new iocb parameter for nfs_write_begin()
v3:
 - Set/clear PG_DROPBEHIND on the head of the nfs_page group
 - Simplify helper folio_end_dropbehind

Trond Myklebust (2):
  filemap: Add a helper for filesystems implementing dropbehind
  NFS: Enable the RWF_DONTCACHE flag for the NFS client

 fs/nfs/file.c            |  6 ++----
 fs/nfs/nfs4file.c        |  1 +
 fs/nfs/write.c           | 20 +++++++++++++++++---
 include/linux/nfs_page.h |  1 +
 include/linux/pagemap.h  |  1 +
 mm/filemap.c             | 11 +++++++++++
 6 files changed, 33 insertions(+), 7 deletions(-)

-- 
2.50.1

[PATCH v3 1/2] filemap: Add a helper for filesystems implementing dropbehind

[Trond Myklebust]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

Add a helper to allow filesystems to attempt to free the 'dropbehind'
folio.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Link: https://lore.kernel.org/all/5588a06f6d5a2cf6746828e2d36e7ada668b1739.1745381692.git.trond.myklebust@hammerspace.com/
Reviewed-by: Mike Snitzer <snitzer@kernel.org>
---
 include/linux/pagemap.h |  1 +
 mm/filemap.c            | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 12a12dae727d..201b7c6f6441 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -1221,6 +1221,7 @@ void folio_wait_writeback(struct folio *folio);
 int folio_wait_writeback_killable(struct folio *folio);
 void end_page_writeback(struct page *page);
 void folio_end_writeback(struct folio *folio);
+void folio_end_dropbehind(struct folio *folio);
 void folio_wait_stable(struct folio *folio);
 void __folio_mark_dirty(struct folio *folio, struct address_space *, int warn);
 void folio_account_cleaned(struct folio *folio, struct bdi_writeback *wb);
diff --git a/mm/filemap.c b/mm/filemap.c
index 751838ef05e5..e35dd0f90850 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1666,6 +1666,17 @@ void folio_end_writeback(struct folio *folio)
 }
 EXPORT_SYMBOL(folio_end_writeback);
 
+/*
+ * Helper for filesystems that want to implement dropbehind, and that
+ * need to keep the folio around after folio_end_writeback, e.g. due to
+ * the need to first commit NFS stable writes.
+ */
+void folio_end_dropbehind(struct folio *folio)
+{
+	filemap_end_dropbehind_write(folio);
+}
+EXPORT_SYMBOL(folio_end_dropbehind);
+
 /**
  * __folio_lock - Get a lock on the folio, assuming we need to sleep to get it.
  * @folio: The folio to lock
-- 
2.50.1

[PATCH v3 2/2] NFS: Enable the RWF_DONTCACHE flag for the NFS client

[Trond Myklebust]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

While the NFS readahead code has no problems using the generic code to
manage the dropbehind behaviour enabled by RWF_DONTCACHE, the write code
needs to deal with the fact that NFS writeback uses a 2 step process
(UNSTABLE write followed by COMMIT).
This commit replaces the use of the folio dropbehind flag with a local
NFS request flag that triggers the dropbehind behaviour once the data
has been written to stable storage.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Link: https://lore.kernel.org/all/ec165b304a7b56d1fa4c6c2b1ad1c04d4dcbd3f6.1745381692.git.trond.myklebust@hammerspace.com/
Reviewed-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfs/file.c            |  6 ++----
 fs/nfs/nfs4file.c        |  1 +
 fs/nfs/write.c           | 20 +++++++++++++++++---
 include/linux/nfs_page.h |  1 +
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 86e36c630f09..f771bd92d61e 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -348,7 +348,6 @@ static int nfs_write_begin(const struct kiocb *iocb,
 			   loff_t pos, unsigned len, struct folio **foliop,
 			   void **fsdata)
 {
-	fgf_t fgp = FGP_WRITEBEGIN;
 	struct folio *folio;
 	struct file *file = iocb->ki_filp;
 	int once_thru = 0;
@@ -357,10 +356,8 @@ static int nfs_write_begin(const struct kiocb *iocb,
 	dfprintk(PAGECACHE, "NFS: write_begin(%pD2(%lu), %u@%lld)\n",
 		file, mapping->host->i_ino, len, (long long) pos);
 
-	fgp |= fgf_set_order(len);
 start:
-	folio = __filemap_get_folio(mapping, pos >> PAGE_SHIFT, fgp,
-				    mapping_gfp_mask(mapping));
+	folio = write_begin_get_folio(iocb, mapping, pos >> PAGE_SHIFT, len);
 	if (IS_ERR(folio))
 		return PTR_ERR(folio);
 	*foliop = folio;
@@ -915,5 +912,6 @@ const struct file_operations nfs_file_operations = {
 	.splice_write	= iter_file_splice_write,
 	.check_flags	= nfs_check_flags,
 	.setlease	= simple_nosetlease,
+	.fop_flags	= FOP_DONTCACHE,
 };
 EXPORT_SYMBOL_GPL(nfs_file_operations);
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 1d6b5f4230c9..654fda6f362c 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -454,4 +454,5 @@ const struct file_operations nfs4_file_operations = {
 #else
 	.llseek		= nfs_file_llseek,
 #endif
+	.fop_flags	= FOP_DONTCACHE,
 };
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 8b7c04737967..f0fd5162154f 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -349,8 +349,12 @@ static void nfs_folio_end_writeback(struct folio *folio)
 static void nfs_page_end_writeback(struct nfs_page *req)
 {
 	if (nfs_page_group_sync_on_bit(req, PG_WB_END)) {
+		struct folio *folio = nfs_page_to_folio(req);
+
+		if (folio_test_clear_dropbehind(folio))
+			set_bit(PG_DROPBEHIND, &req->wb_head->wb_flags);
 		nfs_unlock_request(req);
-		nfs_folio_end_writeback(nfs_page_to_folio(req));
+		nfs_folio_end_writeback(folio);
 	} else
 		nfs_unlock_request(req);
 }
@@ -787,8 +791,15 @@ static void nfs_inode_remove_request(struct nfs_page *req)
 			clear_bit(PG_MAPPED, &req->wb_head->wb_flags);
 		}
 		spin_unlock(&mapping->i_private_lock);
-	}
-	nfs_page_group_unlock(req);
+		nfs_page_group_unlock(req);
+
+		if (test_and_clear_bit(PG_DROPBEHIND,
+				       &req->wb_head->wb_flags)) {
+			folio_set_dropbehind(folio);
+			folio_end_dropbehind(folio);
+		}
+	} else
+		nfs_page_group_unlock(req);
 
 	if (test_and_clear_bit(PG_INODE_REF, &req->wb_flags)) {
 		atomic_long_dec(&nfsi->nrequests);
@@ -2068,6 +2079,7 @@ int nfs_wb_folio(struct inode *inode, struct folio *folio)
 		.range_start = range_start,
 		.range_end = range_start + len - 1,
 	};
+	bool dropbehind = folio_test_clear_dropbehind(folio);
 	int ret;
 
 	trace_nfs_writeback_folio(inode, range_start, len);
@@ -2088,6 +2100,8 @@ int nfs_wb_folio(struct inode *inode, struct folio *folio)
 			goto out_error;
 	}
 out_error:
+	if (dropbehind)
+		folio_set_dropbehind(folio);
 	trace_nfs_writeback_folio_done(inode, range_start, len, ret);
 	return ret;
 }
diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
index 9aed39abc94b..32ab7a590098 100644
--- a/include/linux/nfs_page.h
+++ b/include/linux/nfs_page.h
@@ -37,6 +37,7 @@ enum {
 	PG_REMOVE,		/* page group sync bit in write path */
 	PG_CONTENDED1,		/* Is someone waiting for a lock? */
 	PG_CONTENDED2,		/* Is someone waiting for a lock? */
+	PG_DROPBEHIND,		/* Implement RWF_DONTCACHE */
 };
 
 struct nfs_inode;
-- 
2.50.1

Re: [PATCH v3 1/2] filemap: Add a helper for filesystems implementing dropbehind

[Matthew Wilcox]

On Mon, Aug 18, 2025 at 07:39:49AM -0700, Trond Myklebust wrote:
> +void folio_end_dropbehind(struct folio *folio)
> +{
> +	filemap_end_dropbehind_write(folio);
> +}
> +EXPORT_SYMBOL(folio_end_dropbehind);

Why not just export filemap_end_dropbehind_write()?

Re: [PATCH v3 2/2] NFS: Enable the RWF_DONTCACHE flag for the NFS client

[Matthew Wilcox]

On Mon, Aug 18, 2025 at 07:39:50AM -0700, Trond Myklebust wrote:
> @@ -349,8 +349,12 @@ static void nfs_folio_end_writeback(struct folio *folio)
>  static void nfs_page_end_writeback(struct nfs_page *req)
>  {
>  	if (nfs_page_group_sync_on_bit(req, PG_WB_END)) {
> +		struct folio *folio = nfs_page_to_folio(req);
> +
> +		if (folio_test_clear_dropbehind(folio))
> +			set_bit(PG_DROPBEHIND, &req->wb_head->wb_flags);
>  		nfs_unlock_request(req);
> @@ -787,8 +791,15 @@ static void nfs_inode_remove_request(struct nfs_page *req)
>  			clear_bit(PG_MAPPED, &req->wb_head->wb_flags);
>  		}
>  		spin_unlock(&mapping->i_private_lock);
> -	}
> -	nfs_page_group_unlock(req);
> +		nfs_page_group_unlock(req);
> +
> +		if (test_and_clear_bit(PG_DROPBEHIND,
> +				       &req->wb_head->wb_flags)) {
> +			folio_set_dropbehind(folio);
> +			folio_end_dropbehind(folio);
> +		}

I don't think this technique is "safe".  By clearing the flag early,
the page cache can't see that a folio that was created by dropbehind
has now been reused and should have its dropbehind flag cleared.  So we
might see pages dropped from the cache that really should not be.

Re: [PATCH v3 2/2] NFS: Enable the RWF_DONTCACHE flag for the NFS client

[Trond Myklebust]

On Mon, 2025-08-18 at 16:21 +0100, Matthew Wilcox wrote:
> On Mon, Aug 18, 2025 at 07:39:50AM -0700, Trond Myklebust wrote:
> > @@ -349,8 +349,12 @@ static void nfs_folio_end_writeback(struct
> > folio *folio)
> >  static void nfs_page_end_writeback(struct nfs_page *req)
> >  {
> >  	if (nfs_page_group_sync_on_bit(req, PG_WB_END)) {
> > +		struct folio *folio = nfs_page_to_folio(req);
> > +
> > +		if (folio_test_clear_dropbehind(folio))
> > +			set_bit(PG_DROPBEHIND, &req->wb_head-
> > >wb_flags);
> >  		nfs_unlock_request(req);
> > @@ -787,8 +791,15 @@ static void nfs_inode_remove_request(struct
> > nfs_page *req)
> >  			clear_bit(PG_MAPPED, &req->wb_head-
> > >wb_flags);
> >  		}
> >  		spin_unlock(&mapping->i_private_lock);
> > -	}
> > -	nfs_page_group_unlock(req);
> > +		nfs_page_group_unlock(req);
> > +
> > +		if (test_and_clear_bit(PG_DROPBEHIND,
> > +				       &req->wb_head->wb_flags)) {
> > +			folio_set_dropbehind(folio);
> > +			folio_end_dropbehind(folio);
> > +		}
> 
> I don't think this technique is "safe".  By clearing the flag early,
> the page cache can't see that a folio that was created by dropbehind
> has now been reused and should have its dropbehind flag cleared.  So
> we
> might see pages dropped from the cache that really should not be.

The only alternative would be to add back in a helper in mm/filemap.c
that does the normal folio_end_writeback() routine, but ignores the
dropbehind flag. (folio_end_writeback_ignore_dropbehind()?)

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [PATCH v3 2/2] NFS: Enable the RWF_DONTCACHE flag for the NFS client

[Matthew Wilcox]

On Mon, Aug 18, 2025 at 08:56:31AM -0700, Trond Myklebust wrote:
> On Mon, 2025-08-18 at 16:21 +0100, Matthew Wilcox wrote:
> > I don't think this technique is "safe".  By clearing the flag early,
> > the page cache can't see that a folio that was created by dropbehind
> > has now been reused and should have its dropbehind flag cleared.  So
> > we
> > might see pages dropped from the cache that really should not be.
> 
> The only alternative would be to add back in a helper in mm/filemap.c
> that does the normal folio_end_writeback() routine, but ignores the
> dropbehind flag. (folio_end_writeback_ignore_dropbehind()?)

Can you remind me why we clear the writeback flag as soon as the WRITE
completes instead of leaving it set until the COMMIT completes?

Re: [PATCH v3 1/2] filemap: Add a helper for filesystems implementing dropbehind

[Trond Myklebust]

On Mon, 2025-08-18 at 15:51 +0100, Matthew Wilcox wrote:
> On Mon, Aug 18, 2025 at 07:39:49AM -0700, Trond Myklebust wrote:
> > +void folio_end_dropbehind(struct folio *folio)
> > +{
> > +	filemap_end_dropbehind_write(folio);
> > +}
> > +EXPORT_SYMBOL(folio_end_dropbehind);
> 
> Why not just export filemap_end_dropbehind_write()?

It seemed more appropriate to use the 'folio' prefix when exporting, so
that it is symmetric with folio_end_writeback().

I'm perfectly fine with changing that if people disagree.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [PATCH v3 1/2] filemap: Add a helper for filesystems implementing dropbehind

[Matthew Wilcox]

On Mon, Aug 18, 2025 at 08:59:41AM -0700, Trond Myklebust wrote:
> On Mon, 2025-08-18 at 15:51 +0100, Matthew Wilcox wrote:
> > On Mon, Aug 18, 2025 at 07:39:49AM -0700, Trond Myklebust wrote:
> > > +void folio_end_dropbehind(struct folio *folio)
> > > +{
> > > +	filemap_end_dropbehind_write(folio);
> > > +}
> > > +EXPORT_SYMBOL(folio_end_dropbehind);
> > 
> > Why not just export filemap_end_dropbehind_write()?
> 
> It seemed more appropriate to use the 'folio' prefix when exporting, so
> that it is symmetric with folio_end_writeback().
> 
> I'm perfectly fine with changing that if people disagree.

I don't mind renaming filemap_end_dropbehind_write() to
folio_end_dropbehind() as a solution!

Re: [PATCH v3 2/2] NFS: Enable the RWF_DONTCACHE flag for the NFS client

[Trond Myklebust]

On Mon, 2025-08-18 at 16:58 +0100, Matthew Wilcox wrote:
> On Mon, Aug 18, 2025 at 08:56:31AM -0700, Trond Myklebust wrote:
> > On Mon, 2025-08-18 at 16:21 +0100, Matthew Wilcox wrote:
> > > I don't think this technique is "safe".  By clearing the flag
> > > early,
> > > the page cache can't see that a folio that was created by
> > > dropbehind
> > > has now been reused and should have its dropbehind flag cleared. 
> > > So
> > > we
> > > might see pages dropped from the cache that really should not be.
> > 
> > The only alternative would be to add back in a helper in
> > mm/filemap.c
> > that does the normal folio_end_writeback() routine, but ignores the
> > dropbehind flag. (folio_end_writeback_ignore_dropbehind()?)
> 
> Can you remind me why we clear the writeback flag as soon as the
> WRITE
> completes instead of leaving it set until the COMMIT completes?

It's about reducing latency.

An unstable WRITE is typically a quick operation because it only
requires the server to cache the data.

COMMIT requires persistence, and so it is typically slower.
Furthermore, the intention of COMMIT is to also allow the batching of
writeback on the server, so that disk wakeup and seeks are minimised.
While that is probably much less of a concern with modern SSDs vs older
hard drives, the NFS client design has to cater to both.

So by clearing the writeback flag after the WRITE, we allow operations
that want to further modify a specific folio to proceed without having
to wait for persistence of the entire batch.

More importantly, it also speeds up stat() calls, since we can retrieve
the updated mtime+ctime values from the server as soon as the WRITEs
are complete, without having to wait for COMMIT.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [PATCH v3 2/2] NFS: Enable the RWF_DONTCACHE flag for the NFS client

[Matthew Wilcox]

On Mon, Aug 18, 2025 at 09:49:47AM -0700, Trond Myklebust wrote:
> > Can you remind me why we clear the writeback flag as soon as the
> > WRITE
> > completes instead of leaving it set until the COMMIT completes?
> 
> It's about reducing latency.
> 
> An unstable WRITE is typically a quick operation because it only
> requires the server to cache the data.
> 
> COMMIT requires persistence, and so it is typically slower.
> Furthermore, the intention of COMMIT is to also allow the batching of
> writeback on the server, so that disk wakeup and seeks are minimised.
> While that is probably much less of a concern with modern SSDs vs older
> hard drives, the NFS client design has to cater to both.
> 
> So by clearing the writeback flag after the WRITE, we allow operations
> that want to further modify a specific folio to proceed without having
> to wait for persistence of the entire batch.

Ah; NFS sets SB_I_STABLE_WRITES.  Unfortunately, 6c17260ca4ae is
a little light on details about why.