Re: [RFC PATCH v2 0/7] cgroup-aware OOM killer

[Michal Hocko <mhocko@kernel.org>]

On Thu 01-06-17 19:35:08, Roman Gushchin wrote:
> This patchset makes the OOM killer cgroup-aware.
> 
> Patches 1-3 are simple refactorings of the OOM killer code,
> required to reuse the code in the memory controller.
> Patches 4 & 5 are introducing new memcg settings:
> oom_kill_all_tasks and oom_score_adj.
> Patch 6 introduces the cgroup-aware OOM killer.
> Patch 7 is docs update.

I have only had a look at the cumulative diff (sorry I've been quite
busy throughout the week) and here are my high level comments. I can see
few rather serious issues which will need to be resolved before this
can move on.
- the first problem is a pre-existing one but it will get more urgent
  with the fact that more tasks will be killed with your approach. OOM
  victims are allowed to consume memory reserves without any bound. The
  current throttling is quite arguable and it relies on the fact that we
  try to limit the number of tasks to have this access to reserves.
  Theoretically, though, a heavily multithread application can deplete the
  reserves completely even now. With more processes being killed this
  will get much more likely. Johannes and me have already posted patches
  to address that. The last patch was
  http://lkml.kernel.org/r/1472723464-22866-2-git-send-email-mhocko@kernel.org
- I do not see any explicit lockout mechanism to prevent from too eager oom
  invocation while the previous oom killed memcg is still not torn down
  completely. We use tsk_is_oom_victim check in oom_evaluate_task for
  that purpose. You seem to rely on the fact that such a memcg would be
  still the largest one, right? I am not really sure this is sufficient.
- You seem to completely ignore per task oom_score_adj and override it
  by the memcg value. This makes some sense but it can lead to an
  unexpected behavior when somebody relies on the original behavior.
  E.g. a workload that would corrupt data when killed unexpectedly and
  so it is protected by OOM_SCORE_ADJ_MIN. Now this assumption will
  break when running inside a container. I do not have a good answer
  what is the desirable behavior and maybe there is no universal answer.
  Maybe you just do not to kill those tasks? But then you have to be
  careful when selecting a memcg victim. Hairy...
- While we are at it oom_score_adj has turned out to be quite unusable
  for a sensible oom prioritization from my experience. Practically
  it reduced to disable/enforce the task for selection. The scale is
  quite small as well. There certainly is a need for prioritization
  and maybe a completely different api would be better. Maybe a simple
  priority in (0, infinity) range will be better. Priority could be used
  either as the only criterion or as a tie breaker when consumption of
  more memcgs is too close (this could be implemented for each strategy
  in a different way if we go modules way)
- oom_kill_all_tasks should be hierarchical and consistent within a
  hierarchy. Or maybe it should be applicable to memcgs with tasks (leaf
  cgroups). Although selecting a memcg higher in the hierarchy kill all
  tasks in that hierarchy makes some sense as well IMHO. Say you
  delegate a hierarchy to an unprivileged user and still want to contain
  that user.

I have likely forgot some points but the above ones should be the most
important ones I guess.
-- 
Michal Hocko
SUSE Labs

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