writeback cleanups v2

writeback cleanups v2

[Christoph Hellwig]

Hi all,

this series has a bunch of cosmetic cleanups for the NFS folio writeback
code.

Changes since v1:
 - don't mess up code placement in one patch only to fix it up later

Diffstat:
 write.c |   54 +++++++++++++++++++-----------------------------------
 1 file changed, 19 insertions(+), 35 deletions(-)

[PATCH 1/4] nfs: fold nfs_page_async_flush into nfs_do_writepage

[Christoph Hellwig]

Fold nfs_page_async_flush into its only caller to clean up the code a
bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/write.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 23df8b214474..148e773c3665 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -632,13 +632,14 @@ static void nfs_write_error(struct nfs_page *req, int error)
  * Find an associated nfs write request, and prepare to flush it out
  * May return an error if the user signalled nfs_wait_on_request().
  */
-static int nfs_page_async_flush(struct folio *folio,
-				struct writeback_control *wbc,
-				struct nfs_pageio_descriptor *pgio)
+static int nfs_do_writepage(struct folio *folio, struct writeback_control *wbc,
+		struct nfs_pageio_descriptor *pgio)
 {
 	struct nfs_page *req;
 	int ret = 0;
 
+	nfs_pageio_cond_complete(pgio, folio->index);
+
 	req = nfs_lock_and_join_requests(folio);
 	if (!req)
 		goto out;
@@ -677,13 +678,6 @@ static int nfs_page_async_flush(struct folio *folio,
 	return 0;
 }
 
-static int nfs_do_writepage(struct folio *folio, struct writeback_control *wbc,
-			    struct nfs_pageio_descriptor *pgio)
-{
-	nfs_pageio_cond_complete(pgio, folio->index);
-	return nfs_page_async_flush(folio, wbc, pgio);
-}
-
 /*
  * Write an mmapped page to the server.
  */
-- 
2.47.2

[PATCH 2/4] nfs: don't return AOP_WRITEPAGE_ACTIVATE from nfs_do_writepage

[Christoph Hellwig]

nfs_do_writepage is a successful return that requires the caller to
unlock the folio.  Using it here requires special casing both in
nfs_do_writepage and nfs_writepages_callback and leaves a land mine in
nfs_wb_folio in case it ever set the flag.  Remove it and just
unconditionally unlock in nfs_writepages_callback.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/write.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 148e773c3665..4e1d57b63a85 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -663,8 +663,6 @@ static int nfs_do_writepage(struct folio *folio, struct writeback_control *wbc,
 		 */
 		if (nfs_error_is_fatal_on_server(ret))
 			goto out_launder;
-		if (wbc->sync_mode == WB_SYNC_NONE)
-			ret = AOP_WRITEPAGE_ACTIVATE;
 		folio_redirty_for_writepage(wbc, folio);
 		nfs_redirty_request(req);
 		pgio->pg_error = 0;
@@ -703,8 +701,7 @@ static int nfs_writepages_callback(struct folio *folio,
 	int ret;
 
 	ret = nfs_do_writepage(folio, wbc, data);
-	if (ret != AOP_WRITEPAGE_ACTIVATE)
-		folio_unlock(folio);
+	folio_unlock(folio);
 	return ret;
 }
 
-- 
2.47.2

[PATCH 3/4] nfs: refactor nfs_do_writepage

[Christoph Hellwig]

Use early returns wherever possible to simplify the code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/write.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 4e1d57b63a85..68c5dc061abe 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -636,16 +636,15 @@ static int nfs_do_writepage(struct folio *folio, struct writeback_control *wbc,
 		struct nfs_pageio_descriptor *pgio)
 {
 	struct nfs_page *req;
-	int ret = 0;
+	int ret;
 
 	nfs_pageio_cond_complete(pgio, folio->index);
 
 	req = nfs_lock_and_join_requests(folio);
 	if (!req)
-		goto out;
-	ret = PTR_ERR(req);
+		return 0;
 	if (IS_ERR(req))
-		goto out;
+		return PTR_ERR(req);
 
 	nfs_folio_set_writeback(folio);
 	WARN_ON_ONCE(test_bit(PG_CLEAN, &req->wb_flags));
@@ -655,7 +654,6 @@ static int nfs_do_writepage(struct folio *folio, struct writeback_control *wbc,
 	if (nfs_error_is_fatal_on_server(ret))
 		goto out_launder;
 
-	ret = 0;
 	if (!nfs_pageio_add_request(pgio, req)) {
 		ret = pgio->pg_error;
 		/*
@@ -666,11 +664,12 @@ static int nfs_do_writepage(struct folio *folio, struct writeback_control *wbc,
 		folio_redirty_for_writepage(wbc, folio);
 		nfs_redirty_request(req);
 		pgio->pg_error = 0;
-	} else
-		nfs_add_stats(folio->mapping->host,
-			      NFSIOS_WRITEPAGES, 1);
-out:
-	return ret;
+		return ret;
+	}
+
+	nfs_add_stats(folio->mapping->host, NFSIOS_WRITEPAGES, 1);
+	return 0;
+
 out_launder:
 	nfs_write_error(req, ret);
 	return 0;
-- 
2.47.2

[PATCH 4/4] nfs: use writeback_iter directly

[Christoph Hellwig]

Stop using write_cache_pages and use writeback_iter directly.  This
removes an indirect call per written folio and makes the code easier
to follow.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/write.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 68c5dc061abe..374fc6b34c79 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -694,16 +694,6 @@ static int nfs_writepage_locked(struct folio *folio,
 	return err;
 }
 
-static int nfs_writepages_callback(struct folio *folio,
-				   struct writeback_control *wbc, void *data)
-{
-	int ret;
-
-	ret = nfs_do_writepage(folio, wbc, data);
-	folio_unlock(folio);
-	return ret;
-}
-
 static void nfs_io_completion_commit(void *inode)
 {
 	nfs_commit_inode(inode, 0);
@@ -739,11 +729,15 @@ int nfs_writepages(struct address_space *mapping, struct writeback_control *wbc)
 	}
 
 	do {
+		struct folio *folio = NULL;
+
 		nfs_pageio_init_write(&pgio, inode, priority, false,
 				      &nfs_async_write_completion_ops);
 		pgio.pg_io_completion = ioc;
-		err = write_cache_pages(mapping, wbc, nfs_writepages_callback,
-					&pgio);
+		while ((folio = writeback_iter(mapping, wbc, folio, &err))) {
+			err = nfs_do_writepage(folio, wbc, &pgio);
+			folio_unlock(folio);
+		}
 		pgio.pg_error = 0;
 		nfs_pageio_complete(&pgio);
 		if (err == -EAGAIN && mntflags & NFS_MOUNT_SOFTERR)
-- 
2.47.2

Re: writeback cleanups v2

[Christoph Hellwig]

On Wed, May 07, 2025 at 06:48:50AM +0200, Christoph Hellwig wrote:
> Hi all,
> 
> this series has a bunch of cosmetic cleanups for the NFS folio writeback
> code.

Is this going to get picked up somehere?