[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.10.1708081559001.54505@chino.kir.corp.google.com>
Date: Tue, 8 Aug 2017 16:06:38 -0700 (PDT)
From: David Rientjes <rientjes@...gle.com>
To: Roman Gushchin <guro@...com>
cc: Michal Hocko <mhocko@...nel.org>, linux-mm@...ck.org,
Vladimir Davydov <vdavydov.dev@...il.com>,
Johannes Weiner <hannes@...xchg.org>,
Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
Tejun Heo <tj@...nel.org>, kernel-team@...com,
cgroups@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [v4 2/4] mm, oom: cgroup-aware OOM killer
On Tue, 1 Aug 2017, Roman Gushchin wrote:
> > To the rest of the patch. I have to say I do not quite like how it is
> > implemented. I was hoping for something much simpler which would hook
> > into oom_evaluate_task. If a task belongs to a memcg with kill-all flag
> > then we would update the cumulative memcg badness (more specifically the
> > badness of the topmost parent with kill-all flag). Memcg will then
> > compete with existing self contained tasks (oom_badness will have to
> > tell whether points belong to a task or a memcg to allow the caller to
> > deal with it). But it shouldn't be much more complex than that.
>
> I'm not sure, it will be any simpler. Basically I'm doing the same:
> the difference is that you want to iterate over tasks and for each
> task traverse the memcg tree, update per-cgroup oom score and find
> the corresponding memcg(s) with the kill-all flag. I'm doing the opposite:
> traverse the cgroup tree, and for each leaf cgroup iterate over processes.
>
> Also, please note, that even without the kill-all flag the decision is made
> on per-cgroup level (except tasks in the root cgroup).
>
I think your implementation is preferred and is actually quite simple to
follow, and I would encourage you to follow through with it. It has a
similar implementation to what we have done for years to kill a process
from a leaf memcg.
I did notice that oom_kill_memcg_victim() calls directly into
__oom_kill_process(), however, so we lack the traditional oom killer
output that shows memcg usage and potential tasklist. I think we should
still be dumping this information to the kernel log so that we can see a
breakdown of charged memory.
Powered by blists - more mailing lists