linux-kernel - Re: [PATCH 1/1] mm: add swapiness= arg to memory.reclaim

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: <abc73ea8-f172-422e-bc58-7424e47636b8@vivo.com>
Date:   Fri, 1 Dec 2023 10:13:50 +0800
From:   Huan Yang <11133793@...o.com>
To:     Yosry Ahmed <yosryahmed@...gle.com>
Cc:     Dan Schatzberg <schatzberg.dan@...il.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Roman Gushchin <roman.gushchin@...ux.dev>,
        Huan Yang <link@...o.com>, linux-kernel@...r.kernel.org,
        cgroups@...r.kernel.org, linux-mm@...ck.org,
        Michal Hocko <mhocko@...nel.org>,
        Shakeel Butt <shakeelb@...gle.com>,
        Muchun Song <muchun.song@...ux.dev>,
        Andrew Morton <akpm@...ux-foundation.org>,
        David Hildenbrand <david@...hat.com>,
        Matthew Wilcox <willy@...radead.org>,
        Huang Ying <ying.huang@...el.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        Peter Xu <peterx@...hat.com>,
        "Vishal Moola (Oracle)" <vishal.moola@...il.com>,
        Yue Zhao <findns94@...il.com>, Hugh Dickins <hughd@...gle.com>
Subject: Re: [PATCH 1/1] mm: add swapiness= arg to memory.reclaim


在 2023/12/1 10:05, Yosry Ahmed 写道:
>> @@ -2327,7 +2330,8 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
>>          struct pglist_data *pgdat = lruvec_pgdat(lruvec);
>>          struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>>          unsigned long anon_cost, file_cost, total_cost;
>> -       int swappiness = mem_cgroup_swappiness(memcg);
>> +       int swappiness = sc->swappiness ?
>> +               *sc->swappiness : mem_cgroup_swappiness(memcg);
>>
>> Should we use "unlikely" here to indicate that sc->swappiness is an unexpected behavior?
>> Due to current use case only apply in proactive reclaim.
> On a system that is not under memory pressure, the rate of proactive
> reclaim could be higher than reactive reclaim. We should only use
> likely/unlikely when it's obvious a scenario will happen most of the
> time. I don't believe that's the case here.
Not all vendors will use proactive interfaces, and reactive reclaim are 
a normal
system behavior. In this regard, I think it is appropriate to add 
"unlikely".
>
>>          u64 fraction[ANON_AND_FILE];
>>          u64 denominator = 0;    /* gcc */
>>          enum scan_balance scan_balance;
>> @@ -2608,6 +2612,9 @@ static int get_swappiness(struct lruvec *lruvec, struct scan_control *sc)
>>              mem_cgroup_get_nr_swap_pages(memcg) < MIN_LRU_BATCH)
>>                  return 0;
>>
>> +       if (sc->swappiness)
>> +               return *sc->swappiness;
>>
>> Also there.
>>
>> +
>>          return mem_cgroup_swappiness(memcg);
>>   }
>>
>> @@ -6433,7 +6440,8 @@ unsigned long mem_cgroup_shrink_node(struct mem_cgroup *memcg,
>>   unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
>>                                             unsigned long nr_pages,
>>                                             gfp_t gfp_mask,
>> -                                          unsigned int reclaim_options)
>> +                                          unsigned int reclaim_options,
>> +                                          int *swappiness)
>>   {
>>          unsigned long nr_reclaimed;
>>          unsigned int noreclaim_flag;
>> @@ -6448,6 +6456,7 @@ unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
>>                  .may_unmap = 1,
>>                  .may_swap = !!(reclaim_options & MEMCG_RECLAIM_MAY_SWAP),
>>                  .proactive = !!(reclaim_options & MEMCG_RECLAIM_PROACTIVE),
>> +               .swappiness = swappiness,
>>          };
>>          /*
>>           * Traverse the ZONELIST_FALLBACK zonelist of the current node to put
>> --
>> 2.34.1
>>
>> My previous patch attempted to ensure fully deterministic semantics under extreme swappiness.
>> For example, when swappiness is set to 200, only anonymous pages will be reclaimed.
>> Due to code in MGLRU isolate_folios will try scan anon if no scanned, will try other type.(We do not want
>> it to attempt this behavior.)
>> How do you think about extreme swappiness scenarios?
> I think having different semantics between swappiness passed to
> proactive reclaim and global swappiness can be confusing. If it's
> needed to have a swappiness value that says "anon only no matter
> what", perhaps we should introduce such a new value and make it
> supported by both global and proactive reclaim swappiness? We could
> support writing "max" or something similar instead of a special value
> to mean that.

Yes, use other hint more suitable for this scenario.

However, from this patch, it seems that this feature is not supported.
Do you have a demand for this scenario?

>
> Writing such value to global swappiness may cause problems and
> premature OOMs IIUC, but that would be misconfiguration. If we think
> that's dangerous, we can introduce this new value but make it valid
> only for proactive reclaim for now.