Re: [RFC PATCH] mm, thp: make deferred_split_shrinker memcg-aware

[Neha Agarwal]

On Fri, Oct 20, 2017 at 9:47 AM, Neha Agarwal <nehaagarwal@xxxxxxxxxx> 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@xxxxxxxxxx> 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