Re: [PATCH 3/3] fs: Change the type of filler_t

From: Matthew Wilcox <willy@infradead.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-fsdevel@vger.kernel.org, cluster-devel@redhat.com,
	linux-mtd@lists.infradead.org, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 3/3] fs: Change the type of filler_t
Date: Sun, 8 May 2022 20:06:37 +0100	[thread overview]
Message-ID: <YngUvbIyPA2gsslF@casper.infradead.org> (raw)
In-Reply-To: <YnE60WTzSzxt9OxY@infradead.org>

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.