| Message ID | 20231218153553.807799-15-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/17] writeback: fix done_index when hitting the wbc->nr_to_write | expand |
On Mon 18-12-23 16:35:50, Christoph Hellwig wrote: > From: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > Pull the post-processing of the writepage_t callback into a > separate function. That means changing writeback_finish() to > return NULL, and writeback_get_next() to call writeback_finish() > when we naturally run out of folios. The part about writeback_finish() does not seem to be true anymore. Otherwise feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > mm/page-writeback.c | 85 ++++++++++++++++++++++++--------------------- > 1 file changed, 45 insertions(+), 40 deletions(-) > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 9d37dd5e58ffb6..0771f19950081f 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2449,8 +2449,10 @@ static struct folio *writeback_get_folio(struct address_space *mapping, > > for (;;) { > folio = writeback_get_next(mapping, wbc); > - if (!folio) > + if (!folio) { > + writeback_finish(mapping, wbc, 0); > return NULL; > + } > folio_lock(folio); > if (likely(should_writeback_folio(mapping, wbc, folio))) > break; > @@ -2477,6 +2479,46 @@ static struct folio *writeback_iter_init(struct address_space *mapping, > return writeback_get_folio(mapping, wbc); > } > > +static struct folio *writeback_iter_next(struct address_space *mapping, > + struct writeback_control *wbc, struct folio *folio, int error) > +{ > + unsigned long nr = folio_nr_pages(folio); > + > + wbc->nr_to_write -= nr; > + > + /* > + * 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; > + } > + > + 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)) { > + writeback_finish(mapping, wbc, folio->index + nr); > + return NULL; > + } > + > + return writeback_get_folio(mapping, wbc); > +} > + > /** > * write_cache_pages - walk the list of dirty pages of the given address space and write all of them. > * @mapping: address space structure to write > @@ -2517,47 +2559,10 @@ int write_cache_pages(struct address_space *mapping, > > for (folio = writeback_iter_init(mapping, wbc); > folio; > - folio = writeback_get_folio(mapping, wbc)) { > - unsigned long nr; > - > + folio = writeback_iter_next(mapping, wbc, folio, error)) > error = writepage(folio, wbc, data); > - nr = folio_nr_pages(folio); > - wbc->nr_to_write -= nr; > - > - /* > - * 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; > - } > - > - 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 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. > - */ > - if (wbc->sync_mode == WB_SYNC_NONE && > - (wbc->err || wbc->nr_to_write <= 0)) { > - writeback_finish(mapping, wbc, folio->index + nr); > - return error; > - } > - } > - > - writeback_finish(mapping, wbc, 0); > - return 0; > + return wbc->err; > } > EXPORT_SYMBOL(write_cache_pages); > > -- > 2.39.2 >
diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 9d37dd5e58ffb6..0771f19950081f 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2449,8 +2449,10 @@ static struct folio *writeback_get_folio(struct address_space *mapping, for (;;) { folio = writeback_get_next(mapping, wbc); - if (!folio) + if (!folio) { + writeback_finish(mapping, wbc, 0); return NULL; + } folio_lock(folio); if (likely(should_writeback_folio(mapping, wbc, folio))) break; @@ -2477,6 +2479,46 @@ static struct folio *writeback_iter_init(struct address_space *mapping, return writeback_get_folio(mapping, wbc); } +static struct folio *writeback_iter_next(struct address_space *mapping, + struct writeback_control *wbc, struct folio *folio, int error) +{ + unsigned long nr = folio_nr_pages(folio); + + wbc->nr_to_write -= nr; + + /* + * 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; + } + + 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)) { + writeback_finish(mapping, wbc, folio->index + nr); + return NULL; + } + + return writeback_get_folio(mapping, wbc); +} + /** * write_cache_pages - walk the list of dirty pages of the given address space and write all of them. * @mapping: address space structure to write @@ -2517,47 +2559,10 @@ int write_cache_pages(struct address_space *mapping, for (folio = writeback_iter_init(mapping, wbc); folio; - folio = writeback_get_folio(mapping, wbc)) { - unsigned long nr; - + folio = writeback_iter_next(mapping, wbc, folio, error)) error = writepage(folio, wbc, data); - nr = folio_nr_pages(folio); - wbc->nr_to_write -= nr; - - /* - * 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; - } - - 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 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. - */ - if (wbc->sync_mode == WB_SYNC_NONE && - (wbc->err || wbc->nr_to_write <= 0)) { - writeback_finish(mapping, wbc, folio->index + nr); - return error; - } - } - - writeback_finish(mapping, wbc, 0); - return 0; + return wbc->err; } EXPORT_SYMBOL(write_cache_pages);