Re: [PATCH] fs, mm: account filp and names caches to kmemcg

From: Greg Thelen
Date: Thu Oct 26 2017 - 15:56:58 EST


Michal Hocko wrote:
> Greg Thelen wrote:
> > So a force charge fallback might be a needed even with oom killer successful
> > invocations. Or we'll need to teach out_of_memory() to return three values
> > (e.g. NO_VICTIM, NEW_VICTIM, PENDING_VICTIM) and try_charge() can loop on
> > NEW_VICTIM.
>
> No we, really want to wait for the oom victim to do its job. The only thing we
> should be worried about is when out_of_memory doesn't invoke the reaper. There
> is only one case like that AFAIK - GFP_NOFS request. I have to think about
> this case some more. We currently fail in that case the request.

Nod, but I think only wait a short time (more below). The
schedule_timeout_killable(1) in out_of_memory() seems ok to me. I don't
think there's a problem overcharging a little bit to expedite oom
killing.

Johannes Weiner wrote:
> True. I was assuming we'd retry MEM_CGROUP_RECLAIM_RETRIES times at a maximum,
> even if the OOM killer indicates a kill has been issued. What you propose
> makes sense too.

Sounds good.

It looks like the oom reaper will wait 1 second
(MAX_OOM_REAP_RETRIES*HZ/10) before giving up and setting MMF_OOM_SKIP,
which enables the oom killer to select another victim. Repeated
try_charge() => out_of_memory() calls will return true while there's a
pending victim. After the first call, out_of_memory() doesn't appear to
sleep. So I assume try_charge() would quickly burn through
MEM_CGROUP_RECLAIM_RETRIES (5) attempts before resorting to
overcharging. IMO, this is fine because:
1) it's possible the victim wants locks held by try_charge caller. So
waiting for the oom reaper to timeout and out_of_memory to select
additional victims would kill more than required.
2) waiting 1 sec to detect a livelock between try_charge() and pending
oom victim seems unfortunate.