linux-kernel - Re: [PATCH v2 RFC] mm/vmscan: more restrictive condition for retry in do_try_to_free

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:   Sun, 12 Mar 2017 18:15:05 +0800
From:   Yisheng Xie <ysxie@...mail.com>
To:     Shakeel Butt <shakeelb@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Johannes Weiner <hannes@...xchg.org>,
        Mel Gorman <mgorman@...e.de>, Vlastimil Babka <vbabka@...e.cz>,
        Michal Hocko <mhocko@...e.com>, riel@...hat.com,
        Linux MM <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>, xieyisheng1@...wei.com,
        guohanjun@...wei.com, Xishi Qiu <qiuxishi@...wei.com>
Subject: Re: [PATCH v2 RFC] mm/vmscan: more restrictive condition for retry in
 do_try_to_free_pages

hi, Shakeel,

On 03/12/2017 01:52 AM, Shakeel Butt wrote:
> On Sat, Mar 11, 2017 at 5:51 AM, Yisheng Xie <ysxie@...mail.com> wrote:
>> From: Yisheng Xie <xieyisheng1@...wei.com>
>>
>> When we enter do_try_to_free_pages, the may_thrash is always clear, and
>> it will retry shrink zones to tap cgroup's reserves memory by setting
>> may_thrash when the former shrink_zones reclaim nothing.
>>
>> However, when memcg is disabled or on legacy hierarchy, it should not do
>> this useless retry at all, for we do not have any cgroup's reserves
>> memory to tap, and we have already done hard work but made no progress.
>>
>> To avoid this time costly and useless retrying, add a stub function
>> may_thrash and return true when memcg is disabled or on legacy
>> hierarchy.
>>
>> Signed-off-by: Yisheng Xie <xieyisheng1@...wei.com>
>> Suggested-by: Shakeel Butt <shakeelb@...gle.com>
>> ---
>>
>>                 return 1;
>>
>>         /* Untapped cgroup reserves?  Don't OOM, retry. */
>> -       if (!sc->may_thrash) {
>> +       if (!may_thrash(sc)) {
> Thanks Yisheng. The name of the function may_thrash() is confusing in
> the sense that it is returning exactly the opposite of what its name
> implies. 
Right.

> How about reversing the condition of may_thrash() function
> and change the scan_control's field may_thrash to thrashed?
hmm, maybe I can change the may_thrash() function to mem_cgroup_thrashed().
For, if change the scan_control's may_thrash to thrashed, it may also looks
confusing in shrink_node, and it will be like:
                         if (mem_cgroup_low(root, memcg)) {
                                 if (!sc->thrashed) -----> looks confuse here?
                                         continue;
                                 mem_cgroup_events(memcg, MEMCG_LOW, 1);
                        }

Thanks
Yisheng Xie
	@