From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754776AbbH0NOt (ORCPT ); Thu, 27 Aug 2015 09:14:49 -0400 Received: from mail-wi0-f182.google.com ([209.85.212.182]:34868 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752792AbbH0NOq (ORCPT ); Thu, 27 Aug 2015 09:14:46 -0400 Date: Thu, 27 Aug 2015 15:14:44 +0200 From: Michal Hocko To: Peter Zijlstra Cc: LKML , Oleg Nesterov , David Howells , Linus Torvalds Subject: Re: wake_up_process implied memory barrier clarification Message-ID: <20150827131444.GE27052@dhcp22.suse.cz> References: <20150827122727.GC27052@dhcp22.suse.cz> <20150827124334.GY16853@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150827124334.GY16853@twins.programming.kicks-ass.net> 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 27-08-15 14:43:34, Peter Zijlstra wrote: > On Thu, Aug 27, 2015 at 02:27:27PM +0200, Michal Hocko wrote: > > Hi, > > I have just stumbled over the comment above wake_up_process which > > claims: > > " > > * It may be assumed that this function implies a write memory barrier before > > * changing the task state if and only if any tasks are woken up. > > " > > > > but try_to_wake_up does smp_mb__before_spinlock and did smp_wmb > > since 04e2f1741d235 unconditionally. The comment was added when the > > smp_wmb was in place already so I am wondering whether the comment is > > wrong/misleading. > > > > Could somebody clarify please? > > Its true for wake_up(), since that bails early if the waitqueue list is > empty. > > I suspect there was no exception made for wake_up_process() to simplify > the rules. Thanks for the confirmation. Shouldn't we rather change the documentation because this is clearly misleading and confusing. --- >>From b70d9a384cfd018e686c0aca06e830f564a34dd9 Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Thu, 27 Aug 2015 15:10:55 +0200 Subject: [PATCH] sched: Clarigy wake_up_process memory barrier semantic wake_up_process unlike other wake up primitives based on __wake_up implies the write memory barrier unconditionally because it relies on try_to_wake_up directly. Clarify this in the function comment and memory-barriers.txt because the current doc is quite misleading. Signed-off-by: Michal Hocko --- Documentation/memory-barriers.txt | 3 +++ kernel/sched/core.c | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt index 13feb697271f..c4f180caf0ff 100644 --- a/Documentation/memory-barriers.txt +++ b/Documentation/memory-barriers.txt @@ -2031,6 +2031,9 @@ something up. The barrier occurs before the task state is cleared, and so sits STORE current->state LOAD event_indicated +Please note that wake_up_process is an exception here because it implies +the write memory barrier unconditionally. + To repeat, this write memory barrier is present if and only if something is actually awakened. To see this, consider the following sequence of events, where X and Y are both initially zero: diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 78b4bad10081..39583b76ad2c 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1967,8 +1967,7 @@ static void try_to_wake_up_local(struct task_struct *p) * * Return: 1 if the process was woken up, 0 if it was already running. * - * It may be assumed that this function implies a write memory barrier before - * changing the task state if and only if any tasks are woken up. + * It may be assumed that this function implies a write memory barrier. */ int wake_up_process(struct task_struct *p) { -- 2.5.0 -- Michal Hocko SUSE Labs