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@xxxxxxxxxxxxxxxxxx/ > > 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.
