linux-kernel - Re: [PATCH] vm_swappiness=0 should still try to avoid swapping anon memory

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]

Date:   Fri, 6 Aug 2021 21:37:13 -0400
From:   Nico Pache <npache@...hat.com>
To:     Shakeel Butt <shakeelb@...gle.com>
Cc:     Linux MM <linux-mm@...ck.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Johannes Weiner <hannes@...xchg.org>,
        Rafael Aquini <aquini@...hat.com>, longman@...hat.com
Subject: Re: [PATCH] vm_swappiness=0 should still try to avoid swapping anon
 memory


On 8/6/21 9:00 PM, Shakeel Butt wrote:
> On Fri, Aug 6, 2021 at 4:17 PM Nico Pache <npache@...hat.com> wrote:
>> Since commit b91ac374346b ("mm: vmscan: enforce inactive:active ratio at the
>> reclaim root") swappiness can start prematurely swapping anon memory.
>> This is due to the assumption that refaulting anon should always allow
>> the shrinker to target anon memory. Add a check for vm_swappiness being
>>> 0 before indiscriminately targeting Anon.
> Did you actually observe this behavior?
Yes, and I've successfully tested this patch. It does solve the issue.
>
>> Signed-off-by: Nico Pache <npache@...hat.com>
>> ---
>>  mm/vmscan.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 4620df62f0ff..8b932ff72e37 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -2909,8 +2909,8 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
>>
>>                 refaults = lruvec_page_state(target_lruvec,
>>                                 WORKINGSET_ACTIVATE_ANON);
>> -               if (refaults != target_lruvec->refaults[0] ||
>> -                       inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
>> +               if (vm_swappiness && (refaults != target_lruvec->refaults[0] ||
>> +                       inactive_is_low(target_lruvec, LRU_INACTIVE_ANON)))
> If you are really seeing the said behavior then why will this fix it.
> This is just about deactivating active anon LRU. I would rather look
> at get_scan_count() to check why swappiness = 0 is still letting the
> kernel to scan anon LRU. BTW in cgroup v1, the memcg can overwrite
> their swappiness which will be preferred over system vm_swappiness.
> Did you set system level swappiness or memcg one?

This fixes the issue because shrink_list() uses the may_deactivate field to determine if it should shrink the active list. This is not the only place that can cause the may_deactivate to deactivate anon, but it is the common path of kswapd/balance_pgdat. I can look into a get_scan_count() solution however this line is the ultimate cause of telling scan controller to go for anon so i figured this is the best spot ( stop the problem at the root, not all the way down in the call path). The get_scan_count balance can also be further modified after some shrinking occurs in shrink_lruvec. 

This is only the system level swappiness. As far as cgroups, I will also take a look into that to make sure we can generalize the solution for that as well. I dont think it should be too hard. 


Thanks for the review!

-- Nico


>>                         sc->may_deactivate |= DEACTIVATE_ANON;
>>                 else
>>                         sc->may_deactivate &= ~DEACTIVATE_ANON;
>> --
>> 2.31.1
>>