On Thu, Feb 20, 2020 at 07:57:27AM -0800, Christoph Hellwig wrote: > On Thu, Feb 20, 2020 at 07:54:52AM -0800, Matthew Wilcox wrote: > > On Thu, Feb 20, 2020 at 07:46:58AM -0800, Christoph Hellwig wrote: > > > On Thu, Feb 20, 2020 at 05:48:49AM -0800, Matthew Wilcox wrote: > > > > btrfs: Convert from readpages to readahead > > > > > > > > Implement the new readahead method in btrfs. Add a readahead_page_batch() > > > > to optimise fetching a batch of pages at once. > > > > > > Shouldn't this readahead_page_batch heper go into a separate patch so > > > that it clearly stands out? > > > > I'll move it into 'Put readahead pages in cache earlier' for v8 (the > > same patch where we add readahead_page()) > > One argument for keeping it in a patch of its own is that btrfs appears > to be the only user, and Goldwyn has a WIP conversion of btrfs to iomap, > so it might go away pretty soon and we could just revert the commit. > > But this starts to get into really minor details, so I'll shut up now :) So looking at this again I have another comment and a question. First I think the implicit ARRAY_SIZE in readahead_page_batch is highly dangerous, as it will do the wrong thing when passing a pointer or function argument. Second I wonder іf it would be worth to also switch to a batched operation in iomap if the xarray overhead is high enough. That should be pretty trivial, but we don't really need to do it in this series.
