From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752647AbbCZNBL (ORCPT ); Thu, 26 Mar 2015 09:01:11 -0400 Received: from cantor2.suse.de ([195.135.220.15]:44402 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751552AbbCZNBI (ORCPT ); Thu, 26 Mar 2015 09:01:08 -0400 Date: Thu, 26 Mar 2015 14:01:06 +0100 From: Michal Hocko 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 , "Theodore Ts'o" Subject: Re: [patch 04/12] mm: oom_kill: remove unnecessary locking in exit_oom_victim() Message-ID: <20150326130106.GG15257@dhcp22.suse.cz> References: <1427264236-17249-1-git-send-email-hannes@cmpxchg.org> <1427264236-17249-5-git-send-email-hannes@cmpxchg.org> <20150326125348.GF15257@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150326125348.GF15257@dhcp22.suse.cz> 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 Thu 26-03-15 13:53:48, Michal Hocko wrote: > On Wed 25-03-15 02:17:08, Johannes Weiner wrote: > > Disabling the OOM killer needs to exclude allocators from entering, > > not existing victims from exiting. > > The idea was that exit_oom_victim doesn't miss a waiter. > > exit_oom_victim is doing > atomic_dec_return(&oom_victims) && oom_killer_disabled) > > so there is a full (implicit) memory barrier befor oom_killer_disabled > check. The other part is trickier. oom_killer_disable does: > oom_killer_disabled = true; > up_write(&oom_sem); > > wait_event(oom_victims_wait, !atomic_read(&oom_victims)); > > up_write doesn't guarantee a full memory barrier AFAICS in > Documentation/memory-barriers.txt (although the generic and x86 > implementations seem to implement it as a full barrier) but wait_event > implies the full memory barrier (prepare_to_wait_event does spin > lock&unlock) before checking the condition in the slow path. This should > be sufficient and docummented... > > /* > * We do not need to hold oom_sem here because oom_killer_disable > * guarantees that oom_killer_disabled chage is visible before > * the waiter is put into sleep (prepare_to_wait_event) so > * we cannot miss a wake up. > */ > > in unmark_oom_victim() OK, I can see that the next patch removes oom_killer_disabled completely. The dependency won't be there and so the concerns about the memory barriers. Is there any reason why the ordering is done this way? It would sound more logical to me. -- Michal Hocko SUSE Labs