From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752534AbbJBMLr (ORCPT ); Fri, 2 Oct 2015 08:11:47 -0400 Received: from mail-wi0-f177.google.com ([209.85.212.177]:35392 "EHLO mail-wi0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752464AbbJBMLo (ORCPT ); Fri, 2 Oct 2015 08:11:44 -0400 Date: Fri, 2 Oct 2015 14:11:42 +0200 From: Michal Hocko To: Tetsuo Handa Cc: oleg@redhat.com, akpm@linux-foundation.org, rientjes@google.com, kwalker@redhat.com, skozina@redhat.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH -mm v2 1/3] mm/oom_kill: remove the wrong fatal_signal_pending() check in oom_kill_process() Message-ID: <20151002121142.GD2927@dhcp22.suse.cz> References: <20151001150010.GB6781@redhat.com> <20151001152739.GJ24077@dhcp22.suse.cz> <20151001154115.GA10342@redhat.com> <20151001161916.GK24077@dhcp22.suse.cz> <20151001175319.GA16313@redhat.com> <201510022032.IFC65105.JFtMOQOVSHFLOF@I-love.SAKURA.ne.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201510022032.IFC65105.JFtMOQOVSHFLOF@I-love.SAKURA.ne.jp> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 02-10-15 20:32:41, Tetsuo Handa wrote: > Oleg Nesterov wrote: > > On 10/01, Michal Hocko wrote: > > > > > > zap_process will add SIGKILL to all threads but the > > > current which will go on without being killed and if this is not a > > > thread group leader then we would miss it. > > > > Yes. And note that de_thread() does the same. Speaking of oom-killer > > this is mostly fine, the execing thread is going to release its old > > ->mm and it has already passed the copy_strings() stage which can use > > a lot more memory. > > So, we have the same wrong fatal_signal_pending() check in out_of_memory() > > /* > * If current has a pending SIGKILL or is exiting, then automatically > * select it. The goal is to allow it to allocate so that it may > * quickly exit and free its memory. > * > * But don't select if current has already released its mm and cleared > * TIF_MEMDIE flag at exit_mm(), otherwise an OOM livelock may occur. > */ > if (current->mm && > (fatal_signal_pending(current) || task_will_free_mem(current))) { > mark_oom_victim(current); > return true; > } > > because it is possible that T starts the coredump, T sends SIGKILL to P, > P calls out_of_memory() on GFP_FS allocation, P misses to set SIGKILL on T? So what? P will get an access to memory reserves to move on with the allocation. This has nothing to do with other thread. If the current thread (P) doesn't release any memory we would get to regular oom killer path and eventually send the signal. This is a simple heuristic to prevent from unnecessary killing. If it doesn't help we should still be able to kill something. If you really want to prevent from an unlikely case where the current is the only task in the OOM path and TIF_MEMDIE didn't help then disable the heuristic if the current already has TIF_MEMDIE set. > Since T sends SIGKILL to all clone(CLONE_VM) tasks upon coredump, P needs > to do It does that only to all threads in the _same_ thread group AFAIU. > > rcu_read_lock(); > for_each_process(p) { > if (!process_shares_mm(p, current->mm)) > continue; > if (unlikely(p->flags & PF_KTHREAD)) > continue; > if (p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) > continue; > > do_send_sig_info(SIGKILL, SEND_SIG_FORCED, p, true); > } > rcu_read_unlock(); > > after mark_oom_victim(current) in case T is not in the same thread group? What does this have to do with coredumping? Have you fallen into the same confusion trap I did yesterday? > If yes, what happens if some task failed to receive SIGKILL due to > p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN condition? > Will we hit mm->mmap_sem livelock? I am not sure which livelock you mean here (exit_mm not being able to proceed because some of the task is holding mmap_sem for write and looping in the allocator)? Anyway, having tasks sharing mm but having incompatible OOM_SCORE_ADJ_MIN is basically a misconfiguration IMHO. I wouldn't lose sleep over it to be honest. And yes if one process is pinning the address space then our chances. -- Michal Hocko SUSE Labs