On Wed, Sep 20, 2023 at 01:56:43PM +0200, Pankaj Raghav wrote: > On Mon, Sep 18, 2023 at 01:04:53PM +0200, Hannes Reinecke wrote: > > if (folio && !xa_is_value(folio)) { > > @@ -239,8 +239,8 @@ void page_cache_ra_unbounded(struct readahead_control *ractl, > > * not worth getting one just for that. > > */ > > read_pages(ractl); > > - ractl->_index++; > > - i = ractl->_index + ractl->_nr_pages - index - 1; > > + ractl->_index += folio_nr_pages(folio); > > + i = ractl->_index + ractl->_nr_pages - index; > I am not entirely sure if this is correct. > > The above if condition only verifies if a folio is in the page cache but > doesn't tell if it is uptodate. But we are advancing the ractl->index > past this folio irrespective of that. > > Am I missing something? How readahead works? Readahead is for the optimistic case where nothing has gone wrong; we just don't have anything in the page cache yet. If there's a !uptodate folio in the page cache, there are two possibilities. The most likely is that we have two threads accessing this file at the same time. If so, we should stop and allow the other thread to do the readahead it has decided to do. The less likely scenario is that we had an error doing a previous readahead. If that happened, we should not try reading it again in readahead; we should let the thread retry the read when it actually tries to access the folio.