On Tue, Jan 30, 2024 at 03:22:05PM +0100, Christoph Hellwig wrote:
> And now for real:
Another slight variant of this would be to move the nr_to_write || err
check for WB_SYNC_NONE out of the branch. This adds extra tests for
the initial iteration, but unindents a huge comment, and moves it closer
to the other branch that it also describes. The downside at least to
me is that the normal non-termination path is not the straight line
through function. Thoughs?
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 973f57ad9ee548..ff6e73453aa8c4 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2461,24 +2461,6 @@ struct folio *writeback_iter(struct address_space *mapping,
wbc->nr_to_write -= folio_nr_pages(folio);
if (*error && !wbc->err)
wbc->err = *error;
-
- /*
- * For integrity sync we have to keep going until we have
- * written all the folios we tagged for writeback prior to
- * entering the writeback 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.
- */
- if (wbc->sync_mode == WB_SYNC_NONE &&
- (wbc->err || wbc->nr_to_write <= 0))
- goto finish;
} else {
if (wbc->range_cyclic)
wbc->index = mapping->writeback_index; /* prev offset */
@@ -2491,17 +2473,20 @@ struct folio *writeback_iter(struct address_space *mapping,
wbc->err = 0;
}
- folio = writeback_get_folio(mapping, wbc);
- if (!folio)
- goto finish;
- return folio;
-
-finish:
- folio_batch_release(&wbc->fbatch);
-
/*
+ * For integrity sync we have to keep going until we have written all
+ * the folios we tagged for writeback prior to entering the writeback
+ * 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.
+ *
* For range cyclic writeback we need to remember where we stopped so
- * that we can continue there next time we are called. If we hit the
+ * that we can continue there next time we are called. If we hit the
* last page and there is more work to be done, wrap back to the start
* of the file.
*
@@ -2509,14 +2494,21 @@ struct folio *writeback_iter(struct address_space *mapping,
* of the file if we are called again, which can only happen due to
* -ENOMEM from the file system.
*/
- if (wbc->range_cyclic) {
- WARN_ON_ONCE(wbc->sync_mode != WB_SYNC_NONE);
- if (wbc->err || wbc->nr_to_write <= 0)
+ if (wbc->sync_mode == WB_SYNC_NONE &&
+ (wbc->err || wbc->nr_to_write <= 0)) {
+ if (wbc->range_cyclic)
mapping->writeback_index =
folio->index + folio_nr_pages(folio);
- else
+ } else {
+ folio = writeback_get_folio(mapping, wbc);
+ if (folio)
+ return folio;
+
+ if (wbc->range_cyclic)
mapping->writeback_index = 0;
}
+
+ folio_batch_release(&wbc->fbatch);
*error = wbc->err;
return NULL;
}
