From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752081AbbCZDbz (ORCPT ); Wed, 25 Mar 2015 23:31:55 -0400 Received: from mail-ie0-f176.google.com ([209.85.223.176]:33422 "EHLO mail-ie0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751227AbbCZDbw (ORCPT ); Wed, 25 Mar 2015 23:31:52 -0400 Date: Wed, 25 Mar 2015 20:31:49 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Johannes Weiner cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Linus Torvalds , Andrew Morton , Tetsuo Handa , Huang Ying , Andrea Arcangeli , Dave Chinner , Michal Hocko , "Theodore Ts'o" Subject: Re: [patch 03/12] mm: oom_kill: switch test-and-clear of known TIF_MEMDIE to clear In-Reply-To: <1427264236-17249-4-git-send-email-hannes@cmpxchg.org> Message-ID: References: <1427264236-17249-1-git-send-email-hannes@cmpxchg.org> <1427264236-17249-4-git-send-email-hannes@cmpxchg.org> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 25 Mar 2015, Johannes Weiner wrote: > exit_oom_victim() already knows that TIF_MEMDIE is set, and nobody > else can clear it concurrently. Use clear_thread_flag() directly. > > Signed-off-by: Johannes Weiner For the oom killer, that's true because of task_lock(): we always only set TIF_MEMDIE when there is a valid p->mm and it's cleared in the exit path after the unlock, acting as a barrier, when p->mm is set to NULL so it's no longer a valid victim. So that part is fine. The problem is the android low memory killer that does mark_tsk_oom_victim() without the protection of task_lock(), it's just rcu protected so the reference to the task itself is guaranteed to still be valid. I assume that's why Michal implemented it this way and added the comment to the lmk in commit 49550b605587 ("oom: add helpers for setting and clearing TIF_MEMDIE") to avoid TIF_MEMDIE entirely there. > --- > mm/oom_kill.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index b2f081fe4b1a..4b9547be9170 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -435,8 +435,7 @@ void mark_oom_victim(struct task_struct *tsk) > */ > void exit_oom_victim(void) > { > - if (!test_and_clear_thread_flag(TIF_MEMDIE)) > - return; > + clear_thread_flag(TIF_MEMDIE); > > down_read(&oom_sem); > /*