On Thu, Jan 25, 2024 at 09:57:45AM +0100, Christoph Hellwig wrote:
> From: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
>
> Instead of having a 'done' variable that controls the nested loops,
> have a writeback_finish() that can be returned directly. This involves
> keeping more things in writeback_control, but it's just moving stuff
> allocated on the stack to being allocated slightly earlier on the stack.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> [hch: heavily rebased, reordered and commented struct writeback_control]
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
> Acked-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> include/linux/writeback.h | 6 +++
> mm/page-writeback.c | 79 ++++++++++++++++++++-------------------
> 2 files changed, 47 insertions(+), 38 deletions(-)
>
...
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 437745a511c634..fcd90a176d806c 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
...
> @@ -2419,22 +2437,23 @@ int write_cache_pages(struct address_space *mapping,
> } else {
> tag = PAGECACHE_TAG_DIRTY;
> }
> - done_index = index;
> - while (!done && (index <= end)) {
> +
> + folio_batch_init(&wbc->fbatch);
> + wbc->err = 0;
> +
> + while (index <= end) {
> int i;
>
> nr_folios = filemap_get_folios_tag(mapping, &index, end,
> - tag, &fbatch);
> + tag, &wbc->fbatch);
>
> if (nr_folios == 0)
> break;
>
> for (i = 0; i < nr_folios; i++) {
> - struct folio *folio = fbatch.folios[i];
> + struct folio *folio = wbc->fbatch.folios[i];
> unsigned long nr;
>
> - done_index = folio->index;
> -
> folio_lock(folio);
>
> /*
> @@ -2481,6 +2500,9 @@ int write_cache_pages(struct address_space *mapping,
> folio_unlock(folio);
> error = 0;
> }
> +
JFYI: whitespace damage on the above line.
> + if (error && !wbc->err)
> + wbc->err = error;
>
Also what happened to the return of the above "first error encountered"
for the WB_SYNC_ALL case? Is that not needed for some reason (and so the
comment just below might require an update)?
Brian
> /*
> * For integrity sync we have to keep going until we
> @@ -2496,38 +2518,19 @@ int write_cache_pages(struct address_space *mapping,
> * off and media errors won't choke writeout for the
> * entire file.
> */
> - 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;
> - }
> + if (wbc->sync_mode == WB_SYNC_NONE &&
> + (wbc->err || wbc->nr_to_write <= 0)) {
> + writeback_finish(mapping, wbc,
> + folio->index + nr);
> + return error;
> }
> }
> - folio_batch_release(&fbatch);
> + folio_batch_release(&wbc->fbatch);
> cond_resched();
> }
>
> - /*
> - * 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
> - * last page and there is more work to be done, wrap back to the start
> - * of the file.
> - *
> - * For non-cyclic writeback we always start looking up at the beginning
> - * of the file if we are called again, which can only happen due to
> - * -ENOMEM from the file system.
> - */
> - if (wbc->range_cyclic) {
> - if (done)
> - mapping->writeback_index = done_index;
> - else
> - mapping->writeback_index = 0;
> - }
> -
> - return ret;
> + writeback_finish(mapping, wbc, 0);
> + return 0;
> }
> EXPORT_SYMBOL(write_cache_pages);
>
> --
> 2.39.2
>
>
