Re: [PATCH 01/18] mm/readahead: rework loop in page_cache_ra_unbounded()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-09-20 16:13, Hannes Reinecke wrote:
> On 9/20/23 13:56, 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?
> 
> Confused. Which condition?
> I'm not changing the condition at all, just changing the way how the 'i' index is calculated during
> the loop (ie getting rid of the weird decrement and increment on 'i' during the loop).
> Please clarify.
> 

I made a mistake of pointing out the wrong line in my reply. I was concerned about the increment to
the ractl->_index:

if (folio && !xa_is_value(folio)) {
....
  read_pages(ractl);
  ractl->_index += folio_nr_pages(folio); // This increment to the ractl->_index
...
}

But I think I was missing this point, as willy explained in his reply:

If there's a !uptodate folio in the page cache, <snip>. 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.

Plus your changes optimizes the code path for a large folio where we increment the index by 1 each
time instead of moving the index directly to the next folio in the page cache.

Please ignore the noise!



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux