[PATCH 03/19] writeback: rework the loop termination condition in write_cache_pages

Christoph Hellwig
2024-01-25T08:58:23Z

Rework the way we deal with the cleanup after the writepage call.

First handle the magic AOP_WRITEPAGE_ACTIVATE separately from real error
returns to get it out of the way of the actual error handling path.
Then merge the code to set ret for integrity vs non-integrity writeback.
For non-integrity writeback the loop is terminated on the first error, so
ret will never be non-zero. Then use a single block to check for
non-integrity writewack to consolidate the cases where it returns for
either an error or running off the end of nr_to_write.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Acked-by: Dave Chinner <dchinner@redhat.com>
---
mm/page-writeback.c | 62 +++++++++++++++++++++------------------------
1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0c1e9c016bc48f..259c37bc34d2a7 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2474,43 +2474,39 @@ int write_cache_pages(struct address_space *mapping,
error = writepage(folio, wbc, data);
nr = folio_nr_pages(folio);
wbc->nr_to_write -= nr;
- if (unlikely(error)) {
- /*
- * Handle errors according to the type of
- * writeback. There's no need to continue for
- * background writeback. Just push done_index
- * past this page so media errors won't choke
- * writeout for the entire file. For integrity
- * writeback, we must process the entire dirty
- * set regardless of errors because the fs may
- * still have state to clear for each page. In
- * that case we continue processing and return
- * the first error.
- */
- if (error == AOP_WRITEPAGE_ACTIVATE) {
- folio_unlock(folio);
- error = 0;
- } else if (wbc->sync_mode != WB_SYNC_ALL) {
- ret = error;
- done_index = folio->index + nr;
- done = 1;
- break;
- }
- if (!ret)
- ret = error;
+
+ /*
+ * Handle the legacy AOP_WRITEPAGE_ACTIVATE magic return
+ * value. Eventually all instances should just unlock
+ * the folio themselves and return 0;
+ */
+ if (error == AOP_WRITEPAGE_ACTIVATE) {
+ folio_unlock(folio);
+ error = 0;
}

/*
- * We stop writing back only if we are not doing
- * integrity sync. In case of integrity sync we have to
- * keep going until we have written all the pages
- * we tagged for writeback prior to entering this loop.
+ * For integrity sync we have to keep going until we
+ * have written all the folios we tagged for writeback
+ * prior to entering this loop, even if we run past
+ * wbc->nr_to_write or encounter errors. This is
+ * because the file system may still have state to clear
+ * for each folio. We'll eventually return the first
+ * error encountered.
+ *
+ * For background writeback just push done_index past
+ * this folio so that we can just restart where we left
+ * off and media errors won't choke writeout for the
+ * entire file.
*/
- done_index = folio->index + nr;
- if (wbc->nr_to_write <= 0 &&
- wbc->sync_mode == WB_SYNC_NONE) {
- done = 1;
- break;
+ if (error && !ret)
+ ret = error;
+ if (wbc->sync_mode == WB_SYNC_NONE) {
+ if (ret || wbc->nr_to_write <= 0) {
+ done_index = folio->index + nr;
+ done = 1;
+ break;
+ }
}
}
folio_batch_release(&fbatch);
--
2.39.2