From: David Rientjes <rientjes@google.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Roman Gushchin <guro@fb.com>,
Andrew Morton <akpm@linux-foundation.org>,
Vladimir Davydov <vdavydov.dev@gmail.com>,
Johannes Weiner <hannes@cmpxchg.org>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Tejun Heo <tj@kernel.org>,
kernel-team@fb.com, cgroups@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Subject: Re: [PATCH v13 0/7] cgroup-aware OOM killer
Date: Tue, 16 Jan 2018 13:36:21 -0800 (PST) [thread overview]
Message-ID: <alpine.DEB.2.10.1801161323550.242486@chino.kir.corp.google.com> (raw)
In-Reply-To: <20180115115433.GA22473@dhcp22.suse.cz>
On Mon, 15 Jan 2018, Michal Hocko wrote:
> > No, this isn't how kernel features get introduced. We don't design a new
> > kernel feature with its own API for a highly specialized usecase and then
> > claim we'll fix the problems later. Users will work around the
> > constraints of the new feature, if possible, and then we can end up
> > breaking them later. Or, we can pollute the mem cgroup v2 filesystem with
> > even more tunables to cover up for mistakes in earlier designs.
>
> This is a blatant misinterpretation of the proposed changes. I haven't
> heard _any_ single argument against the proposed user interface except
> for complaints for missing tunables. This is not how the kernel
> development works and should work. The usecase was clearly described and
> far from limited to a single workload or company.
>
The complaint about the user interface is that it is not extensible, as my
next line states. This doesn't need to be opted into with a mount option
locking the entire system into a single oom policy. That, itself, is the
result of a poor design. What is needed is a way for users to define an
oom policy that is generally useful, not something that is locked in for
the whole system. We don't need several different cgroup mount options
only for mem cgroup oom policies. We also don't need random
memory.groupoom files being added to the mem cgroup v2 filesystem only for
one or two particular policies and being no-ops otherwise. It can easily
be specified as part of the policy itself. My suggestion adds two new
files to the mem cgroup v2 filesystem and no mount option, and allows any
policy to be added later that only uses these two files. I see you've
ignored all of that in this email, so perhaps reading it would be
worthwhile so that you can provide constructive feedback.
> > The key point to all three of my objections: extensibility.
>
> And it has been argued that further _features_ can be added on top. I am
> absolutely fed up discussing those things again and again without any
> progress. You simply keep _ignoring_ counter arguments and that is a
> major PITA to be honest with you. You are basically blocking a useful
> feature because it doesn't solve your particular workload. This is
> simply not acceptable in the kernel development.
>
As the thread says, this has nothing to do with my own particular
workload, it has to do with three obvious shortcomings in the design that
the user has no control over. We can't add features on top if the
heuristic itself changes as a result of the proposal, it needs to be
introduced in an extensible way so that additional changes can be made
later, if necessary, while still working around the very obvious problems
with this current implementation. My suggestion is that we introduce a
way to define the oom policy once so that we don't have to change it later
and are left with needless mount options or mem cgroup v2 files that
become no-ops with the suggested design. I hope that you will read the
proposal for that extensible interface and comment on it about any
concerns that you have, because that feedback would generally be useful.
> > Both you and Michal have acknowledged blantently obvious shortcomings in
> > the design.
>
> What you call blatant shortcomings we do not see affecting any
> _existing_ workloads. If they turn out to be real issues then we can fix
> them without deprecating any user APIs added by this patchset.
>
There are existing workloads that use mem cgroup subcontainers purely for
tracking charging and vmscan stats, which results in this logic being
evaded. It's a real issue, and a perfectly acceptable usecase for mem
cgroup. It's a result of the entire oom policy either being opted into or
opted out of for the entire system and impossible for the user to
configure or avoid. That can be done better by enabling the oom policy
only for a subtree, as I've suggested, but you've ignored. It would also
render both the mount option and the additional file in the mem cgroup v2
filesystem added by this patchset to be no-ops.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2018-01-16 21:36 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-30 15:28 Roman Gushchin
2017-11-30 15:28 ` [PATCH v13 1/7] mm, oom: refactor the oom_kill_process() function Roman Gushchin
2017-11-30 15:28 ` [PATCH v13 2/7] mm: implement mem_cgroup_scan_tasks() for the root memory cgroup Roman Gushchin
2017-11-30 15:28 ` [PATCH v13 3/7] mm, oom: cgroup-aware OOM killer Roman Gushchin
2017-12-01 8:35 ` Michal Hocko
2017-12-07 1:24 ` Andrew Morton
2017-12-07 13:39 ` Roman Gushchin
2017-11-30 15:28 ` [PATCH v13 4/7] mm, oom: introduce memory.oom_group Roman Gushchin
2017-11-30 15:28 ` [PATCH v13 5/7] mm, oom: add cgroup v2 mount option for cgroup-aware OOM killer Roman Gushchin
2017-12-01 8:41 ` Michal Hocko
2017-12-01 13:15 ` Roman Gushchin
2017-12-01 13:31 ` Michal Hocko
2017-12-01 17:00 ` Roman Gushchin
2017-11-30 15:28 ` [PATCH v13 6/7] mm, oom, docs: describe the " Roman Gushchin
2017-12-01 8:41 ` Michal Hocko
2017-12-01 17:01 ` Roman Gushchin
2017-12-01 17:13 ` Michal Hocko
2017-11-30 15:28 ` [PATCH v13 7/7] cgroup: list groupoom in cgroup features Roman Gushchin
2017-11-30 20:39 ` [PATCH v13 0/7] cgroup-aware OOM killer Andrew Morton
2018-01-10 0:57 ` David Rientjes
2018-01-10 13:11 ` Roman Gushchin
2018-01-10 19:33 ` Andrew Morton
2018-01-11 9:08 ` Michal Hocko
2018-01-11 13:18 ` Roman Gushchin
2018-01-12 22:03 ` David Rientjes
2018-01-15 11:54 ` Michal Hocko
2018-01-16 21:36 ` David Rientjes [this message]
2018-01-16 22:09 ` Michal Hocko
2018-01-11 21:57 ` David Rientjes
2018-01-13 17:14 ` Johannes Weiner
2018-01-14 23:44 ` David Rientjes
2018-01-15 16:25 ` Johannes Weiner
2018-01-16 21:21 ` David Rientjes
2018-01-10 20:50 ` David Rientjes
2017-12-01 9:14 ` [PATCH] mm, oom: simplify alloc_pages_before_oomkill handling Michal Hocko
2017-12-01 13:26 ` Tetsuo Handa
2017-12-01 13:32 ` Roman Gushchin
2017-12-01 13:54 ` Michal Hocko
2018-06-05 11:47 ` [PATCH v13 0/7] cgroup-aware OOM killer Michal Hocko
2018-06-05 12:13 ` Michal Hocko
2018-07-13 21:59 ` David Rientjes
2018-07-14 1:55 ` Tetsuo Handa
2018-07-16 21:13 ` Tetsuo Handa
2018-07-16 22:09 ` Roman Gushchin
2018-07-17 0:55 ` Tetsuo Handa
2018-07-31 14:14 ` Tetsuo Handa
2018-08-01 16:37 ` Roman Gushchin
2018-08-01 22:01 ` Tetsuo Handa
2018-08-01 22:55 ` Roman Gushchin
2018-07-16 9:36 ` Michal Hocko
2018-07-17 3:59 ` David Rientjes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.10.1801161323550.242486@chino.kir.corp.google.com \
--to=rientjes@google.com \
--cc=akpm@linux-foundation.org \
--cc=cgroups@vger.kernel.org \
--cc=guro@fb.com \
--cc=hannes@cmpxchg.org \
--cc=kernel-team@fb.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=tj@kernel.org \
--cc=vdavydov.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox