On 10/26/21 1:11 PM, Chris Mason wrote: > >> On Oct 26, 2021, at 2:15 PM, Jens Axboe <axboe@xxxxxxxxx> 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@xxxxxx> >> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> >> >> --- >> >> 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@xxxxxxxxxx> > 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
