Skip to content

Commit 7775fac

Browse files
Tetsuo Handatorvalds
authored andcommitted
memcg: killed threads should not invoke memcg OOM killer
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/[email protected] Signed-off-by: Tetsuo Handa <[email protected]> Acked-by: Michal Hocko <[email protected]> Acked-by: Johannes Weiner <[email protected]> Cc: David Rientjes <[email protected]> Cc: Kirill Tkhai <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 23a7052 commit 7775fac

File tree

1 file changed

+14
-5
lines changed

1 file changed

+14
-5
lines changed

mm/memcontrol.c

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,12 @@ enum res_type {
248248
iter != NULL; \
249249
iter = mem_cgroup_iter(NULL, iter, NULL))
250250

251+
static inline bool should_force_charge(void)
252+
{
253+
return tsk_is_oom_victim(current) || fatal_signal_pending(current) ||
254+
(current->flags & PF_EXITING);
255+
}
256+
251257
/* Some nice accessors for the vmpressure. */
252258
struct vmpressure *memcg_to_vmpressure(struct mem_cgroup *memcg)
253259
{
@@ -1389,8 +1395,13 @@ static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
13891395
};
13901396
bool ret;
13911397

1392-
mutex_lock(&oom_lock);
1393-
ret = out_of_memory(&oc);
1398+
if (mutex_lock_killable(&oom_lock))
1399+
return true;
1400+
/*
1401+
* A few threads which were not waiting at mutex_lock_killable() can
1402+
* fail to bail out. Therefore, check again after holding oom_lock.
1403+
*/
1404+
ret = should_force_charge() || out_of_memory(&oc);
13941405
mutex_unlock(&oom_lock);
13951406
return ret;
13961407
}
@@ -2209,9 +2220,7 @@ static int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask,
22092220
* bypass the last charges so that they can exit quickly and
22102221
* free their memory.
22112222
*/
2212-
if (unlikely(tsk_is_oom_victim(current) ||
2213-
fatal_signal_pending(current) ||
2214-
current->flags & PF_EXITING))
2223+
if (unlikely(should_force_charge()))
22152224
goto force;
22162225

22172226
/*

0 commit comments

Comments
 (0)