On Mon, Jun 26, 2023 at 09:34:17PM -0700, Christoph Hellwig wrote:
> > + for (;;) {
> > + folio = writeback_get_next(mapping, wbc);
> > + if (!folio)
> > + return NULL;
> > + wbc->done_index = folio->index;
> > +
> > + folio_lock(folio);
> > + if (likely(should_writeback_folio(mapping, wbc, folio)))
> > + break;
> > + folio_unlock(folio);
> > + }
> > +
> > + trace_wbc_writepage(wbc, inode_to_bdi(mapping->host));
> > + return folio;
>
> Same minor nitpick, why not:
>
>
> while ((folio = writeback_get_next(mapping, wbc)) {
> wbc->done_index = folio->index;
>
> folio_lock(folio);
> if (likely(should_writeback_folio(mapping, wbc, folio))) {
> trace_wbc_writepage(wbc, inode_to_bdi(mapping->host));
> break;
> }
> folio_unlock(folio);
> }
>
> return folio;
>
> ?
Because we end up having to call writeback_finish() somewhere, and it's
neater to do it here than in either writeback_get_next() or both callers
of writeback_get_folio().
