linux-kernel - Re: [linus:master] [page cache] 9425c591e0: vm-scalability.throughput -20.0% regression

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 27 Jun 2023 12:38:59 +0800
From:   Yin Fengwei <fengwei.yin@...el.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>,
        kernel test robot <oliver.sang@...el.com>
CC:     <oe-lkp@...ts.linux.dev>, <lkp@...el.com>,
        <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ackerley Tng <ackerleytng@...gle.com>,
        Erdem Aktas <erdemaktas@...gle.com>,
        Matthew Wilcox <willy@...radead.org>,
        Muchun Song <songmuchun@...edance.com>,
        "Sidhartha Kumar" <sidhartha.kumar@...cle.com>,
        Vishal Annapurve <vannapurve@...gle.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        <linux-fsdevel@...r.kernel.org>, <ying.huang@...el.com>,
        <feng.tang@...el.com>
Subject: Re: [linus:master] [page cache] 9425c591e0: vm-scalability.throughput
 -20.0% regression



On 6/26/23 17:05, Yin, Fengwei wrote:
> Hi Mike,
> 
>> On 06/21/23 15:19, kernel test robot wrote:
>   <snip> 
>> I suspected this change could impact page_cache_next/prev_miss users, but had
>> no idea how much.
>>
>> Unless someone sees something wrong in 9425c591e06a, the best approach
>> might be to revert and then add a simple interface to check for 'folio at
>> a given index in the cache' as suggested by Ackerley Tng.
>> https://lore.kernel.org/linux-mm/98624c2f481966492b4eb8272aef747790229b73.1683069252.git.ackerleytng@google.com/
> 
> Some findings in my side.
> 1. You patch impact the folio order for file readahead. I collect the histogram of
>    order parameter to filemap_alloc_folio() call w/o your patch:
> 
>    With your patch:
>      page order    : count     distribution
>         0          : 892073   |                                        |
>         1          : 0        |                                        |
>         2          : 65120457 |****************************************|
>         3          : 32914005 |********************                    |
>         4          : 33020991 |********************                    |
> 
>    Without your patch:
>      page order    : count     distribution
>         0          : 3417288  |****                                    |
>         1          : 0        |                                        |
>         2          : 877012   |*                                       |
>         3          : 288      |                                        |
>         4          : 5607522  |*******                                 |
>         5          : 29974228 |****************************************|
> 
>    We could see the order 5 dominate the filemap folio without your patch. With your
>    patch, order 2,3,4 are most used for filemap folio.
> 
> 2. My understanding is your patch is correct and shouldn't be reverted. I made
>    a small change based on your patch. The performance regression is gone.
> 
> diff --git a/mm/readahead.c b/mm/readahead.c
> index 47afbca1d122..cca333f9b560 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -610,7 +610,7 @@ static void ondemand_readahead(struct readahead_control *ractl,
>                 pgoff_t start;
> 
>                 rcu_read_lock();
> -               start = page_cache_next_miss(ractl->mapping, index + 1,
> +               start = page_cache_next_miss(ractl->mapping, index,
>                                 max_pages);
>                 rcu_read_unlock();
> 
>    And the filemap folio order is restored also:
>      page order    : count     distribution
>         0          : 3357622  |****                                    |
>         1          : 0        |                                        |
>         2          : 861726   |*                                       |
>         3          : 285      |                                        |
>         4          : 4511637  |*****                                   |
>         5          : 30505713 |****************************************|
> 
>    I still didn't figure out why this simple change can restore the performance.
>    And why index + 1 was used. Will check more.

The thing is the ra initialization after page_cache_next_miss() in function
ondemand_readahead():
  ra->start = start; (start is index + max_pages + 1 + 1 after your patch)
  ra->size = start - index;

And +1 will be accumulated to ra->start and breaks the filemap folio order.


Regards
Yin, Fengwei

> 
> 
> Regards
> Yin, Fengwei