From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754691AbdIFNW7 (ORCPT ); Wed, 6 Sep 2017 09:22:59 -0400 Received: from mx2.suse.de ([195.135.220.15]:51590 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754214AbdIFNWy (ORCPT ); Wed, 6 Sep 2017 09:22:54 -0400 Date: Wed, 6 Sep 2017 15:22:49 +0200 From: Michal Hocko To: Roman Gushchin Cc: linux-mm@kvack.org, Vladimir Davydov , Johannes Weiner , Tetsuo Handa , David Rientjes , Andrew Morton , Tejun Heo , kernel-team@fb.com, cgroups@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [v7 2/5] mm, oom: cgroup-aware OOM killer Message-ID: <20170906132249.c2llo5zyrzgviqzc@dhcp22.suse.cz> References: <20170904142108.7165-1-guro@fb.com> <20170904142108.7165-3-guro@fb.com> <20170905145700.fd7jjd37xf4tb55h@dhcp22.suse.cz> <20170905202357.GA10535@castle.DHCP.thefacebook.com> <20170906083158.gvqx6pekrsy2ya47@dhcp22.suse.cz> <20170906125750.GB12904@castle> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170906125750.GB12904@castle> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 06-09-17 13:57:50, Roman Gushchin wrote: > On Wed, Sep 06, 2017 at 10:31:58AM +0200, Michal Hocko wrote: > > On Tue 05-09-17 21:23:57, Roman Gushchin wrote: > > > On Tue, Sep 05, 2017 at 04:57:00PM +0200, Michal Hocko wrote: > > [...] > > > > Hmm. The changelog says "By default, it will look for the biggest leaf > > > > cgroup, and kill the largest task inside." But you are accumulating > > > > oom_score up the hierarchy and so parents will have higher score than > > > > the layer of their children and the larger the sub-hierarchy the more > > > > biased it will become. Say you have > > > > root > > > > /\ > > > > / \ > > > > A D > > > > / \ > > > > B C > > > > > > > > B (5), C(15) thus A(20) and D(20). Unless I am missing something we are > > > > going to go down A path and then chose C even though D is the largest > > > > leaf group, right? > > > > > > You're right, changelog is not accurate, I'll fix it. > > > The behavior is correct, IMO. > > > > Please explain why. This is really a non-intuitive semantic. Why should > > larger hierarchies be punished more than shallow ones? I would > > completely agree if the whole hierarchy would be a killable entity (aka > > A would be kill-all). > > I think it's a reasonable and clear policy: we're looking for a memcg > with the smallest oom_priority and largest memory footprint recursively. But this can get really complex for non-trivial setups. Anything with deeper and larger hierarchies will get quite complex IMHO. Btw. do you have any specific usecase for the priority based oom killer? I remember David was asking for this because it _would_ be useful but you didn't have it initially. And I agree with that I am just not sure the semantic is thought through wery well. I am thinking whether it would be easier to push this further without priority thing for now and add it later with a clear example of the configuration and how it should work and a more thought through semantic. Would that sound acceptable? I believe the rest is quite useful to get merged on its own. > Then we reclaim some memory from it (by killing the biggest process > or all processes, depending on memcg preferences). > > In general, if there are two memcgs of equal importance (which is defined > by setting the oom_priority), we're choosing the largest, because there > are more chances that it contain a leaking process. The same is true > right now for processes. Yes except this is not the case as shown above. We can easily select a smaller leaf memcg just because it is in a larger hierarchy and that sounds very dubious to me. Even when all the priorities are the same. > I agree, that for size-based comparison we could use a different policy: > comparing leaf cgroups despite their level. But I don't see a clever > way to apply oom_priorities in this case. Comparing oom_priority > on each level is a simple and powerful policy, and it works well > for delegation. You are already shaping semantic around the implementation and that is a clear sign of problem. > > [...] > > > > I do not understand why do we have to handle root cgroup specially here. > > > > select_victim_memcg already iterates all memcgs in the oom hierarchy > > > > (including root) so if the root memcg is the largest one then we > > > > should simply consider it no? > > > > > > We don't have necessary stats for the root cgroup, so we can't calculate > > > it's oom_score. > > > > We used to charge pages to the root memcg as well so we might resurrect > > that idea. In any case this is something that could be hidden in > > memcg_oom_badness rather then special cased somewhere else. > > In theory I agree, but I do not see a good way to calculate root memcg > oom_score. Why cannot you emulate that by the largest task in the root? The same way you actually do in select_victim_root_cgroup_task now? -- Michal Hocko SUSE Labs