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

Message-ID: <CALvZod5X3sLQT-We2VNCiAN9zi9MJvdk4fVGERVpw=GQGrGHEg@mail.gmail.com>
Date:   Fri, 10 Mar 2017 19:40:51 -0800
From:   Shakeel Butt <shakeelb@...gle.com>
To:     Yisheng Xie <ysxie@...mail.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

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)).

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