Re: mm: don't read i_size of inode unless we need it

[Jens Axboe <axboe@kernel.dk>]

On 10/26/21 1:11 PM, Chris Mason wrote:
> 
>> On Oct 26, 2021, at 2:15 PM, Jens Axboe <axboe@kernel.dk> wrote:
>>
>> We always go through i_size_read(), and we rarely end up needing it. Push
>> the read to down where we need to check it, which avoids it for most
>> cases.
>>
>> It looks like we can even remove this check entirely, which might be
>> worth pursuing. But at least this takes it out of the hot path.
>>
>> Acked-by: Chris Mason <clm@fb.com>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> ---
>>
>> I came across this and wrote the patch the other day, then Pavel pointed
>> me at his original posting of a very similar patch back in August.
>> Discussed it with Chris, and it sure _seems_ like this would be fine.
>>
>> In an attempt to move the original discussion forward, here's this
>> posting.
>>
> 
> I had the same concerns Dave Chinner did, but I think the i_size check
> inside generic_file_read_iter() is dead code at this point.  Checking
> ki_pos against i_size was added for Btrfs:
> 
> commit 66f998f611897319b555364cefd5d6e88a205866
> Author: Josef Bacik <josef@redhat.com>
> Date:   Sun May 23 11:00:54 2010 -0400
> 
>     fs: allow short direct-io reads to be completed via buffered IO
> 
> And we’ve switched to btrfs_file_read_iter(), which does the check the
> same way PavelJens have done it here.
> 
> I don’t think checking i_size before or after O_DIRECT makes the race
> fundamentally different.  We might return a short read at different
> times than we did before, but we won’t be returning stale/incorrect
> data.

Andrew, can you queue this one up in the mm branch?

-- 
Jens Axboe