linux-kernel - Re: [patch v5] mm: lru_cache_disable: replace work queue synchronization with synchronize

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <0ce05ce7-1a00-82df-f37a-bf7f9e216504@MichaelLarabel.com>
Date:   Sat, 28 May 2022 19:48:06 -0500
From:   Michael Larabel <Michael@...haelLarabel.com>
To:     Andrew Morton <akpm@...ux-foundation.org>,
        Marcelo Tosatti <mtosatti@...hat.com>
Cc:     Borislav Petkov <bp@...en8.de>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, Minchan Kim <minchan@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Nicolas Saenz Julienne <nsaenzju@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        "Paul E. McKenney" <paulmck@...nel.org>
Subject: Re: [patch v5] mm: lru_cache_disable: replace work queue
 synchronization with synchronize_rcu

On 5/28/22 17:54, Michael Larabel wrote:
> On 5/28/22 16:18, Andrew Morton wrote:
>> On Thu, 28 Apr 2022 15:00:11 -0300 Marcelo Tosatti 
>> <mtosatti@...hat.com> wrote:
>>
>>> On Thu, Mar 31, 2022 at 03:52:45PM +0200, Borislav Petkov wrote:
>>>> On Thu, Mar 10, 2022 at 10:22:12AM -0300, Marcelo Tosatti wrote:
>>> ...
>>>
>>>> Someone pointed me at this:
>>>>
>>>> https://www.phoronix.com/scan.php?page=news_item&px=Linux-518-Stress-NUMA-Goes-Boom 
>>>>
>>>>
>>>> which says this one causes a performance regression with stress-ng's
>>>> NUMA test...
>>> Michael,
>>>
>>> This is probably do_migrate_pages that is taking too long due to
>>> synchronize_rcu().
>>>
>>> Switching to synchronize_rcu_expedited() should probably fix it...
>>> Can you give it a try, please?
>> I guess not.
>>
>> Is anyone else able to demonstrate a stress-ng performance regression
>> due to ff042f4a9b0508?  And if so, are they able to try Marcelo's
>> one-liner?
>
>
> Apologies I don't believe I got the email previously (or if it ended 
> up in spam or otherwise overlooked) so just noticed this thread now...
>
> I have the system around and will work on verifying it can reproduce 
> still and can then test the patch, should be able to get it tomorrow.
>
> Thanks and sorry about the delay.
>
> Michael
>
>

Had a chance to look at it today still. I was able to reproduce the 
regression still on that 5950X system going from v5.17 to v5.18 (using 
newer stress-ng benchmark and other system changes since the prior 
tests). Confirmed it also still showed slower as of today's Git.

I can confirm with Marcelo's patch below that the stress-ng NUMA 
performance is back to the v5.17 level of performance (actually, faster) 
and certainly not like what I was seeing on v5.18 or Git to this point.

So all seems to be good with that one-liner for the stress-ng NUMA test 
case. All the system details and results for those interested is 
documented @ https://openbenchmarking.org/result/2205284-PTS-NUMAREGR17 
but basically amounts to:

     Stress-NG 0.14
     Test: NUMA
     Bogo Ops/s > Higher Is Better
     v5.17: 412.88
     v5.18: 49.33
     20220528 Git: 49.66
     20220528 Git + sched-rcu-exped patch: 468.81

Apologies again about the delay / not seeing the email thread earlier.

Thanks,

Michael


Tested-by: Michael Larabel <Michael@...haelLarabel.com>



>
>>
>>> diff --git a/mm/swap.c b/mm/swap.c
>>> index bceff0cb559c..04a8bbf9817a 100644
>>> --- a/mm/swap.c
>>> +++ b/mm/swap.c
>>> @@ -879,7 +879,7 @@ void lru_cache_disable(void)
>>>        * lru_disable_count = 0 will have exited the critical
>>>        * section when synchronize_rcu() returns.
>>>        */
>>> -    synchronize_rcu();
>>> +    synchronize_rcu_expedited();
>>>   #ifdef CONFIG_SMP
>>>       __lru_add_drain_all(true);
>>>   #else
>>>
>>>