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 -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html