Re: [PATCH v2] memcg, oom: check memcg margin for parallel oom

[David Rientjes <rientjes@google.com>]

On Thu, 16 Jul 2020, Michal Hocko wrote:

> > It's not possible to present data because we've had such a check for years 
> > in our fleet so I can't say that it has prevented X unnecessary oom kills 
> > compared to doing the check prior to calling out_of_memory().  I'm hoping 
> > that can be understood.
> > 
> > Since Yafang is facing the same issue, and there is no significant 
> > downside to doing the mem_cgroup_margin() check prior to 
> > oom_kill_process() (or checking task_will_free_mem(current)), and it's 
> > acknowledged that it *can* prevent unnecessary oom killing, which is a 
> > very good thing, I'd like to understand why such resistance to it.
> 
> Because exactly this kind of arguments has led to quite some "should be
> fine" heuristics which kicked back: do not kill exiting task, sacrifice
> child instead of a victim just to name few. All of them make some sense
> from a glance but they can serious kick back as the experience has
> thought us.
> 
> Really, I do not see what is so hard to understand that each heuristic,
> especially those to subtle areas like oom definitely is, needs data to
> justify them. We are running this for years is really not an argument.
> Sure arguing that your workload leads to x amount of false positives
> and just shifting the check to later saves y amount of them sounds like
> a relevant argument to me.
> 

Deferring the go/no-go decision on the oom kill to the very last moment 
doesn't seem like a heuristic, I think it's an inherent responsibility of 
the kernel to do whatever necessary to prevent a userspace process from 
being oom killed (and the way to solve Yafang's issue that we had solved 
years ago).  That can be done by closing the window as much as possible 
(including within out_of_memory()) to reduce the likelihood of unnecessary 
oom killing.  It's intuitive and seems rather trivial.

I would understand an argument against such an approach if it added 
elaborate complexity, but this isn't doing so.  If the decision was 
already made in oom_kill_process(), I don't think anybody would advocate 
for moving it below out_of_memory() to its current state.  We aren't 
losing anything here, we are only preventing unnecessary oom killing that 
has caused issues for Yafang as well as us.

Any solution that does a mem_cgroup_margin() check before out_of_memory() 
in the memcg path is closing that window a little bit, but I think we can 
do better.