On 13.03.19 г. 19:27 ч., David Sterba wrote: > On Mon, Mar 11, 2019 at 09:55:38AM +0200, Nikolay Borisov wrote: >> Currently extent_readpages (called from btrfs_redpages) will always call >> __extent_readpages which tries to create contiguous range of pages and >> call __do_contiguous_readpages when such contiguous range is created. >> >> It turns out this is unnecessary due to the fact that generic VFS code >> always calls filesystem's ->readpages callback (btrfs_readpages in >> this case) with already contiguous pages. Armed with this knowledge it's >> possible to simplify extent_readpages by eliminating the call to >> __extent_readpages and directly calling contiguous_readpages. The only >> edge case that needs to be handled is when add_to_page_cache_lru >> fails. This is easy as all that is needed is to submit whatever is the >> number of pages successfully added to the lru. > > I'd like to have more details why this is correct. Submitting what we > have so far seems ok, the reasons why add_to_page_cache_lru are unclear > and go back to xarray. I'm having a hard time parsing the sentence after the comma, could you rephrase that? > > Possible error is EEXSIT, here it's clear that we don't need to read the Not only EEXIST, the code can also fail due to memory pressure from mem_cgroup_try_charge in __add_to_page_cache_locked. In fact this code was triggered by some of the xfstests I can't remember which one hence I added the if (nr) check. > pages (already there). A sequence of such pages will make it wildly hop > from 1st for cycle iteration, to the if (nr) check and back. > > Previously this looped still in the for-cycle, which was easier to > follow but otherwise not due to __extent_readpages. So I think it's an > improvement in the end, though a few comments would be good there. > > Reviewed-by: David Sterba <dsterba@xxxxxxxx> > <snip>
