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

[Yafang Shao <laoar.shao@gmail.com>]

On Tue, Aug 20, 2019 at 5:12 AM Roman Gushchin <guro@fb.com> wrote:
>
> 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?

memroy.low is different with memory.min, as the OOM killer will not be
invoked when it is reached.
If memory.low should be considered as well, we can use
mem_cgroup_protected() here to repclace task_under_memcg_protection()
here.

> 2) if the task is 200Gb large, does 10Mb memory protection make any
> difference? if so, why would we respect it?

Same with above, only consider it when the proctecion is enabled.

> 3) if it works for global OOMs, why not memcg-level OOMs?

memcg OOM is when the memory limit is reached and it can't find
something to relcaim in the memcg and have to kill processes in this
memcg.
That is different with global OOM, because the global OOM can chose
processes outside the memcg but the memcg OOM can't.

> 4) if the task is prioritized to be killed by OOM (via oom_score_adj),
> why even small memory.protection prevents it completely?

Would you pls. show me some examples that when we will set both
memory.min(meaning the porcesses in this memcg is very important) and
higher oom_score_adj(meaning the porcesses in this memcg is not
improtant at all) ?
Note that the memory.min don't know which processes is important,
while it only knows is if this process in this memcg.

> 5) if there are two tasks similar in size and both protected,
> should we prefer one with the smaller protection?
> etc.

Same with the answer in 1).

>
> 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.


I haven't taken a close look at the memcg-aware OOM killer, but even
with cgroup-aware OOM killer I think it still can't answer your
question 4).

> 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.
>

I will take a close look at it.  Thanks for your reference.

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


Thanks
Yafang