linux-kernel - Re: [PATCH RFC] mm/vmscan: donot retry shrink zones when memcg is disabled

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:   Sat, 11 Mar 2017 17:41:28 +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
Subject: Re: [PATCH RFC] mm/vmscan: donot retry shrink zones when memcg is
 disabled

Hi Shakeel,

Thanks for reviewing.

On 03/11/2017 11:40 AM, Shakeel Butt wrote:
> On Fri, Mar 10, 2017 at 6:19 PM, 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, if CONFIG_MEMCG=n, 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 and made no progress.
>>
>> Signed-off-by: Yisheng Xie <xieyisheng1@...wei.com>
>> ---
>>  mm/vmscan.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index bc8031e..b03ccc1 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -2808,7 +2808,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>>                 return 1;
>>
>>         /* Untapped cgroup reserves?  Don't OOM, retry. */
>> -       if (!sc->may_thrash) {
>> +       if (!sc->may_thrash && IS_ENABLED(CONFIG_MEMCG)) {
> In my opinion it should be even more restrictive (restricting
> cgroup_disabled=memory boot option and cgroup legacy hierarchy). So,
> instead of IS_ENABLED(CONFIG_MEMCG), the check should be something
> like (cgroup_subsys_enabled(memory_cgrp_subsys) &&
> cgroup_subsys_on_dfl(memory_cgrp_subsys)).
Right，  I will send another version soon.

Thanks
Yisheng Xie.

>>                 sc->priority = initial_priority;
>>                 sc->may_thrash = 1;
>>                 goto retry;
>> --
>> 1.9.1
>>
>>
>>