lkml.org 
[lkml]   [2019]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 028/187] memcg: killed threads should not invoke memcg OOM killer
    Date
    4.19-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    [ Upstream commit 7775face207922ea62a4e96b9cd45abfdc7b9840 ]

    If a memory cgroup contains a single process with many threads
    (including different process group sharing the mm) then it is possible
    to trigger a race when the oom killer complains that there are no oom
    elible tasks and complain into the log which is both annoying and
    confusing because there is no actual problem. The race looks as
    follows:

    P1 oom_reaper P2
    try_charge try_charge
    mem_cgroup_out_of_memory
    mutex_lock(oom_lock)
    out_of_memory
    oom_kill_process(P1,P2)
    wake_oom_reaper
    mutex_unlock(oom_lock)
    oom_reap_task
    mutex_lock(oom_lock)
    select_bad_process # no victim

    The problem is more visible with many threads.

    Fix this by checking for fatal_signal_pending from
    mem_cgroup_out_of_memory when the oom_lock is already held.

    The oom bypass is safe because we do the same early in the try_charge
    path already. The situation migh have changed in the mean time. It
    should be safe to check for fatal_signal_pending and tsk_is_oom_victim
    but for a better code readability abstract the current charge bypass
    condition into should_force_charge and reuse it from that path. "

    Link: http://lkml.kernel.org/r/01370f70-e1f6-ebe4-b95e-0df21a0bc15e@i-love.sakura.ne.jp
    Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Acked-by: Michal Hocko <mhocko@suse.com>
    Acked-by: Johannes Weiner <hannes@cmpxchg.org>
    Cc: David Rientjes <rientjes@google.com>
    Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    mm/memcontrol.c | 19 ++++++++++++++-----
    1 file changed, 14 insertions(+), 5 deletions(-)

    diff --git a/mm/memcontrol.c b/mm/memcontrol.c
    index 9518aefd8cbb..7c712c4565e6 100644
    --- a/mm/memcontrol.c
    +++ b/mm/memcontrol.c
    @@ -248,6 +248,12 @@ enum res_type {
    iter != NULL; \
    iter = mem_cgroup_iter(NULL, iter, NULL))

    +static inline bool should_force_charge(void)
    +{
    + return tsk_is_oom_victim(current) || fatal_signal_pending(current) ||
    + (current->flags & PF_EXITING);
    +}
    +
    /* Some nice accessors for the vmpressure. */
    struct vmpressure *memcg_to_vmpressure(struct mem_cgroup *memcg)
    {
    @@ -1382,8 +1388,13 @@ static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
    };
    bool ret;

    - mutex_lock(&oom_lock);
    - ret = out_of_memory(&oc);
    + if (mutex_lock_killable(&oom_lock))
    + return true;
    + /*
    + * A few threads which were not waiting at mutex_lock_killable() can
    + * fail to bail out. Therefore, check again after holding oom_lock.
    + */
    + ret = should_force_charge() || out_of_memory(&oc);
    mutex_unlock(&oom_lock);
    return ret;
    }
    @@ -2200,9 +2211,7 @@ retry:
    * bypass the last charges so that they can exit quickly and
    * free their memory.
    */
    - if (unlikely(tsk_is_oom_victim(current) ||
    - fatal_signal_pending(current) ||
    - current->flags & PF_EXITING))
    + if (unlikely(should_force_charge()))
    goto force;

    /*
    --
    2.19.1


    \
     
     \ /
      Last update: 2019-04-04 11:44    [W:5.151 / U:0.360 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site