Re: [PATCH v2] mm, memcg: skip killing processes under memcg protection at first scan

[Roman Gushchin <guro@fb.com>]

On Sun, Aug 18, 2019 at 09:18:06PM -0400, Yafang Shao wrote:
> In the current memory.min design, the system is going to do OOM instead
> of reclaiming the reclaimable pages protected by memory.min if the
> system is lack of free memory. While under this condition, the OOM
> killer may kill the processes in the memcg protected by memory.min.
> This behavior is very weird.
> In order to make it more reasonable, I make some changes in the OOM
> killer. In this patch, the OOM killer will do two-round scan. It will
> skip the processes under memcg protection at the first scan, and if it
> can't kill any processes it will rescan all the processes.
> 
> Regarding the overhead this change may takes, I don't think it will be a
> problem because this only happens under system  memory pressure and
> the OOM killer can't find any proper victims which are not under memcg
> protection.

Hi Yafang!

The idea makes sense at the first glance, but actually I'm worried
about mixing per-memcg and per-process characteristics.
Actually, it raises many questions:
1) if we do respect memory.min, why not memory.low too?
2) if the task is 200Gb large, does 10Mb memory protection make any
difference? if so, why would we respect it?
3) if it works for global OOMs, why not memcg-level OOMs?
4) if the task is prioritized to be killed by OOM (via oom_score_adj),
why even small memory.protection prevents it completely?
5) if there are two tasks similar in size and both protected,
should we prefer one with the smaller protection?
etc.

Actually, I think that it makes more sense to build a completely
cgroup-aware OOM killer, which will select the OOM victim scanning
the memcg tree, not individual tasks. And then it can easily respect
memory.low/min in a reasonable way.
But I failed to reach the upstream consensus on how it should work.
You can search for "memcg-aware OOM killer" in the lkml archive,
there was a ton of discussions and many many patchset versions.


The code itself can be simplified a bit too, but I think it's
not that important now.

Thanks!