Message-ID: <CAEvLuNbo=zf1aC9k7sitZgYPD=P1Awwne4mmUSRtJc0EF1xcAA@mail.gmail.com>
Date:   Fri, 20 Oct 2017 10:47:57 -0700
From:   Neha Agarwal <nehaagarwal@...gle.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Dan Williams <dan.j.williams@...el.com>,
        David Rientjes <rientjes@...gle.com>,
        Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Vlastimil Babka <vbabka@...e.cz>,
        Kemi Wang <kemi.wang@...el.com>,
        "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
        Shaohua Li <shli@...com>, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, cgroups@...r.kernel.org
Subject: Re: [RFC PATCH] mm, thp: make deferred_split_shrinker memcg-aware

On Fri, Oct 20, 2017 at 9:47 AM, Neha Agarwal <nehaagarwal@...gle.com> wrote:
> [Sorry for multiple emails, it wasn't in plain text before, thus resending.]
>
> On Fri, Oct 20, 2017 at 12:12 AM, Michal Hocko <mhocko@...nel.org> wrote:
>> On Thu 19-10-17 13:03:23, Neha Agarwal wrote:
>>> deferred_split_shrinker is NUMA aware. Making it memcg-aware if
>>> CONFIG_MEMCG is enabled to prevent shrinking memory of memcg(s) that are
>>> not under memory pressure. This change isolates memory pressure across
>>> memcgs from deferred_split_shrinker perspective, by not prematurely
>>> splitting huge pages for the memcg that is not under memory pressure.
>>
>> Why do we need this? THP pages are usually not shared between memcgs. Or
>> do you have a real world example where this is not the case? Your patch
>> is adding quite a lot of (and to be really honest very ugly) code so
>> there better should be a _very_ good reason to justify it. I haven't
>> looked very closely to the code, at least all those ifdefs in the code
>> are too ugly to live.
>> --
>> Michal Hocko
>> SUSE Labs
>
> Hi Michal,
>
> Let me try to pitch the motivation first:
> In the case of NUMA-aware shrinker, memory pressure may lead to
> splitting and freeing subpages within a THP, irrespective of whether
> the page belongs to the memcg that is under memory pressure. THP
> sharing between memcgs is not a pre-condition for above to happen.

I think I got confused here. The point I want to make is that when a
memcg is under memory pressure, only memcg-aware shrinkers are called.
However, a memcg with partially-mapped THPs (which can be split and
thus free up subpages) should be be able to split such THPs, to avoid
oom-kills under memory pressure. By making this shrinker memcg-aware,
we will be able to free up subpages by splitting partially-mapped THPs
under memory pressure.

>
> Let's consider two memcgs: memcg-A and memcg-B. Say memcg-A is under
> memory pressure that is hitting its limit. If this memory pressure
> invokes the shrinker (non-memcg-aware) and splits pages from memcg-B
> queued for deferred splits, then that won't reduce memcg-A's usage. It
> will reduce memcg-B's usage. Also, why should memcg-A's memory
> pressure reduce memcg-B's usage.
>
> By making this shrinker memcg-aware, we can invoke respective memcg
> shrinkers to handle the memory pressure. Furthermore, with this
> approach we can isolate the THPs of other memcg(s) (not under memory
> pressure) from premature splits. Isolation aids in reducing
> performance impact when we have several memcgs on the same machine.
>
> Regarding ifdef ugliness: I get your point and agree with you on that.
> I think I can do a better job at restricting the ugliness, will post
> another version.
>
> --
> Thanks,
> Neha Agarwal



-- 
Thanks,
Neha Agarwal

Powered by blists - more mailing lists