On Tue, May 03, 2022 at 07:23:13AM -0700, Christoph Hellwig wrote: > > @@ -3504,9 +3504,9 @@ static struct folio *do_read_cache_folio(struct address_space *mapping, > > > > filler: > > if (filler) > > - err = filler(data, &folio->page); > > + err = filler(file, folio); > > else > > - err = mapping->a_ops->read_folio(data, folio); > > + err = mapping->a_ops->read_folio(file, folio); > > Wouldn't it just make sense to just pass mapping->a_ops->read_folio as > the filler here from the callers that currently pass NULL? Thanks for the review. We're heading in the same direction; my plan was to (in some subsequent merge window) convert all callers of read_cache_page() to use read_mapping_folio() (by setting ->read_folio) and then get rid of filler_t altogether. Maybe there are some filesystems that can't do that, eg they need a different ->read_folio() when called from read_cache_folio() to the one they need when called for filemap_read(), but I bet they can all be worked around.