[00/12] Convert write_cache_pages() to an iterator

  Dave Howells doesn't like the indirect function call imposed by
write_cache_pages(), so refactor it into an iterator.  I took the
opportunity to add the ability to iterate a folio_batch without having
an external variable.

This is against next-20230623.  If you try to apply it on top of a tree
which doesn't include the pagevec removal series, IT WILL CRASH because
it won't reinitialise folio_batch->i and the iteration will index out
of bounds.

I have a feeling the 'done' parameter could have a better name, but I
can't think what it might be.

Matthew Wilcox (Oracle) (12):
  writeback: Factor out writeback_finish()
  writeback: Factor writeback_get_batch() out of write_cache_pages()
  writeback: Factor should_writeback_folio() out of write_cache_pages()
  writeback: Simplify the loops in write_cache_pages()
  pagevec: Add ability to iterate a queue
  writeback: Use the folio_batch queue iterator
  writeback: Factor writeback_iter_init() out of write_cache_pages()
  writeback: Factor writeback_get_folio() out of write_cache_pages()
  writeback: Factor writeback_iter_next() out of write_cache_pages()
  writeback: Add for_each_writeback_folio()
  iomap: Convert iomap_writepages() to use for_each_writeback_folio()
  writeback: Remove a use of write_cache_pages() from do_writepages()

 fs/iomap/buffered-io.c    |  14 +-
 include/linux/pagevec.h   |  18 +++
 include/linux/writeback.h |  22 ++-
 mm/page-writeback.c       | 310 +++++++++++++++++++++-----------------
 4 files changed, 216 insertions(+), 148 deletions(-)
  

On Mon, Jun 26, 2023 at 06:35:09PM +0100, Matthew Wilcox (Oracle) wrote:
> Dave Howells doesn't like the indirect function call imposed by
> write_cache_pages(), so refactor it into an iterator.  I took the
> opportunity to add the ability to iterate a folio_batch without having
> an external variable.
> 
> This is against next-20230623.  If you try to apply it on top of a tree
> which doesn't include the pagevec removal series, IT WILL CRASH because
> it won't reinitialise folio_batch->i and the iteration will index out
> of bounds.
> 
> I have a feeling the 'done' parameter could have a better name, but I
> can't think what it might be.

Heh, I actually have a local series with a minor subset of some of the
cleanups, but without the iterator conversion.
  

Do you have this on a branch somewhere?

David
  

On Tue, Jun 27, 2023 at 11:53:02AM +0100, David Howells wrote:
> Do you have this on a branch somewhere?

I just pushed it out to https://git.infradead.org/users/willy/pagecache.git/shortlog/refs/heads/writeback-iter

Running it through xfstests now.  This includes one of Christoph's
suggestions, a build fix for Linus's tree, and a bugfix I noticed last
night, so it's not quite the same as the emails that were sent out in
this thread.  I doubt it'll be what I send out for v2 either.

I'm looking at afs writeback now.
  

Matthew Wilcox <willy@infradead.org> wrote:

> I'm looking at afs writeback now.

:-)

>  fs/iomap/buffered-io.c    |  14 +-
>  include/linux/pagevec.h   |  18 +++
>  include/linux/writeback.h |  22 ++-
>  mm/page-writeback.c       | 310 +++++++++++++++++++++-----------------
>  4 files changed, 216 insertions(+), 148 deletions(-)

Documentation/mm/writeback.rst too please.

David
  

On Wed, Jun 28, 2023 at 09:03:10PM +0100, David Howells wrote:
> Matthew Wilcox <willy@infradead.org> wrote:
> 
> > I'm looking at afs writeback now.
> 
> :-)
> 
> >  fs/iomap/buffered-io.c    |  14 +-
> >  include/linux/pagevec.h   |  18 +++
> >  include/linux/writeback.h |  22 ++-
> >  mm/page-writeback.c       | 310 +++++++++++++++++++++-----------------
> >  4 files changed, 216 insertions(+), 148 deletions(-)
> 
> Documentation/mm/writeback.rst too please.

$ ls Documentation/mm/w*
ls: cannot access 'Documentation/mm/w*': No such file or directory

$ git grep writeback Documentation/mm
Documentation/mm/multigen_lru.rst:do not require TLB flushes; clean pages do not require writeback.
Documentation/mm/page_migration.rst:2. Ensure that writeback is complete.
Documentation/mm/page_migration.rst:15. Queued up writeback on the new page is triggered.
Documentation/mm/physical_memory.rst:``nr_writeback_throttled``
Documentation/mm/physical_memory.rst:  Number of pages written while reclaim is throttled waiting for writeback.

Or are you suggesting I write a new piece of kernel documentation?
If so, I respectfully decline.  I've updated the kernel-doc included
in Documentation/core-api/mm-api.rst and I think that's all I can
reasonably be asked to do.
  

Just curious where this did end up.  I have some changes that could use
or conflict with this depending on your view.  They aren't time critical
yt, but if we're going to the road in this series I'd appreciate if we'd
get it done rather sooner than later :)
  

On Wed, Jun 28, 2023 at 08:31:05PM +0100, Matthew Wilcox wrote:
> On Tue, Jun 27, 2023 at 11:53:02AM +0100, David Howells wrote:
> > Do you have this on a branch somewhere?
> 
> I just pushed it out to https://git.infradead.org/users/willy/pagecache.git/shortlog/refs/heads/writeback-iter
> 
> Running it through xfstests now.  This includes one of Christoph's
> suggestions, a build fix for Linus's tree, and a bugfix I noticed last
> night, so it's not quite the same as the emails that were sent out in
> this thread.  I doubt it'll be what I send out for v2 either.

So it turns out thіs version still applies fine and tests fine with
latest mainline.

I've put up a slight tweak here:

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/writeback-iter

this moves and documents the new fields in struct writeback_control
and drops the iomap patch for now as it has conflicts in the VFS tree
in this merge window.

Do you want me to send this version out, or do you want to take over or
is there a good reason not to progress with it?