From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753359AbbH0S31 (ORCPT <rfc822;w@1wt.eu>);
	Thu, 27 Aug 2015 14:29:27 -0400
Received: from mx1.redhat.com ([209.132.183.28]:56552 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752009AbbH0S30 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Thu, 27 Aug 2015 14:29:26 -0400
Date: Thu, 27 Aug 2015 20:26:54 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>, LKML <linux-kernel@vger.kernel.org>,
        David Howells <dhowells@redhat.com>,
        Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: wake_up_process implied memory barrier clarification
Message-ID: <20150827182654.GA12191@redhat.com>
References: <20150827122727.GC27052@dhcp22.suse.cz> <20150827124334.GY16853@twins.programming.kicks-ass.net> <20150827131444.GE27052@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150827131444.GE27052@dhcp22.suse.cz>
User-Agent: Mutt/1.5.18 (2008-05-17)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 08/27, Michal Hocko wrote:
>
> --- 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
>  	    <general barrier>		  STORE current->state
>  	LOAD event_indicated
>
> +Please note that wake_up_process is an exception here because it implies
> +the write memory barrier unconditionally.
> +

I simply can't understand (can't even parse) this part of memory-barriers.txt.

> --- 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.
>   */

I won't argue, technically this is correct of course. And I agree that
the old comment is misleading.



But the new comment looks as if it is fine to avoid wmb() if you do
wake_up_process(). Say,

	void w(void)
	{
		A = 1;
		wake_up_process(something_unrelated);
		// we know that it implies wmb().
		B = 1;
	}

	void r(void)
	{
		int a, b;

		b = B;
		rmb();
		a = A;

		BUG_ON(b && !a);
	}

Perhaps this part of the comment should be simply removed, the unconditional
wmb() in ttwu() is just implementation detail. And note that initially the
documented behaviour of smp_mb__before_spinlock() was only the STORE - LOAD
serialization. Then people noticed that it actually does wmb() and started
to rely on this fact.

To me, this comment should just explain that this function implies a barrier
but only in a sense that you do not need another one after CONDITION = T and
before wake_up_process().

Oleg.