Re: Upcoming: fscache rewrite

Matthew Wilcox
2020-07-30T12:16:43Z

On Thu, Jul 30, 2020 at 12:51:16PM +0100, David Howells wrote:
> (3) Al has objections to the ITER_MAPPING iov_iter type that I added
>
> https://lore.kernel.org/linux-fsdevel/20200719014436.GG2786714@ZenIV.linux.org.uk/
>
> but note that iov_iter_for_each_range() is not actually used by anything.
>
> However, Willy likes it and would prefer to make it ITER_XARRAY instead
> as he might be able to use it in other places, though there's an issue
> where I'm calling find_get_pages_contig() which takes a mapping (though
> all it does is then get the xarray out of it).

I suspect you don't need to call find_get_pages_contig(). If you look
at __readahead_batch() in pagemap.h, it does basically what you want
(other than being wrapped up inside the readahead iterator). You require
the pages already be pinned in the xarray, so there's no need for the
page_cache_get_speculative() dance that find_get_pages_contig) does,
nor the check for xa_is_value().

My main concern with your patchset is that it introduces a new page flag
to sleep on which basically means "I am writing this page to the fscache".
I don't understand why you need it; you've elevated the refcount on
the pages so they're not going to get reused for another purpose.
All it does (as far as I can tell) is make a task calling truncate()
wait for the page to finish being written to cache, which isn't actually
necessary.

Overall, I do like the patch series! It's a big improvement over what we
currently have and will make it easier to finish the readpages->readahead
conversion.