On 5/25/18 2:32 AM, Christoph Hellwig wrote: > On Thu, May 24, 2018 at 10:02:51AM -0600, Jens Axboe wrote: >> The only caller of ->readpages() is from read-ahead, yet we don't >> submit IO flagged with REQ_RAHEAD. This means we don't see it in >> blktrace, for instance, which is a shame. We already make assumptions >> about ->readpages() just being for read-ahead in the mpage >> implementation, using readahead_gfp_mask(mapping) as out GFP mask of >> choice. >> >> This small series fixes up mpage_readpages() to submit with >> REQ_RAHEAD, which takes care of file systems using mpage_readpages(). >> The last two fixup ext4 and btrfs. > > What are the benefits? Any setup where this buys us anything? Any The first benefit is getting the tracing right. Before you could not see which parts where readahead in blktrace, and which parts were not. This now works fine. The second motivation is fixing an issue around big read ahead windows where a severely bogged down box takes forever to exit a task that was killed, because it's going through tons of IOs on an oversubscribed disk. We see this at FB. 4MB window, and IOs being largely non-sequential. 1000 requests on a rotating drive that takes seconds to do each one. This requires fixing on top, which can be pretty easily done as long as we acknowledge that ->readpages() is strictly for read-ahead. > setup where this actually regressed because drivers/hardware are > doing stupid things with REQ_RAHEAD? I'd sure hope not, we are using REQ_RAHEAD in various meta data related bits. But the main read-ahead was not. -- Jens Axboe
