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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <xr93y3nxbs3j.fsf@gthelen.svl.corp.google.com>
Date:   Thu, 26 Oct 2017 12:56:48 -0700
From:   Greg Thelen <gthelen@...gle.com>
To:     Johannes Weiner <hannes@...xchg.org>
Cc:     Michal Hocko <mhocko@...nel.org>,
        Shakeel Butt <shakeelb@...gle.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux MM <linux-mm@...ck.org>, linux-fsdevel@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] fs, mm: account filp and names caches to kmemcg

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.