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 S1753457AbbIGRJj (ORCPT <rfc822;w@1wt.eu>);
	Mon, 7 Sep 2015 13:09:39 -0400
Received: from mx1.redhat.com ([209.132.183.28]:55979 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751331AbbIGRJh (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Mon, 7 Sep 2015 13:09:37 -0400
Date: Mon, 7 Sep 2015 19:06:52 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        Michal Hocko <mhocko@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        David Howells <dhowells@redhat.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Jonathan Corbet <corbet@lwn.net>
Subject: Re: wake_up_process implied memory barrier clarification
Message-ID: <20150907170652.GA32459@redhat.com>
References: <20150829092514.GA3240@fixme-laptop.cn.ibm.com> <20150829142707.GA19263@redhat.com> <20150831003719.GC924@fixme-laptop.cn.ibm.com> <20150831183335.GA26333@redhat.com> <20150831203739.GX4029@linux.vnet.ibm.com> <20150901034014.GD1071@fixme-laptop.cn.ibm.com> <20150901095923.GB31368@redhat.com> <20150901145024.GA8007@fixme-laptop.cn.ibm.com> <20150901163923.GA20055@redhat.com> <20150902011027.GB8007@fixme-laptop.cn.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150902011027.GB8007@fixme-laptop.cn.ibm.com>
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

Sorry for delay,

On 09/02, Boqun Feng wrote:
>
> On Tue, Sep 01, 2015 at 06:39:23PM +0200, Oleg Nesterov wrote:
> > On 09/01, Boqun Feng wrote:
> > >
> > > On Tue, Sep 01, 2015 at 11:59:23AM +0200, Oleg Nesterov wrote:
> > > >
> > > > And just in case, wake_up() differs in a sense that it doesn't even need
> > > > that STORE-LOAD barrier in try_to_wake_up(), we can rely on
> > > > wait_queue_head_t->lock. Assuming that wake_up() pairs with the "normal"
> > > > wait_event()-like code.
> >
> > Looks like, you have missed this part of my previous email. See below.
>
> I guess I need to think through this, though I haven't found any problem
> in wake_up() if we remove the STORE-LOAD barrier in try_to_wake_up().
> And I know that in wake_up(), try_to_wake_up() will be called with
> holding wait_queue_head_t->lock, however, only part of wait_event()
> holds the same lock, I can't figure out why the barrier is not needed
> because of the lock..

This is very simple. __wait_event() does

	for (;;) {
		prepare_to_wait_event(WQ, ...);	// takes WQ->lock

		if (CONDITION)
			break;

		schedule();
	}

and we have

	CONDITION = 1;
	wake_up(WQ);				// takes WQ->lock

on another side.

Suppose that __wait_event() wins and takes WQ->lock first. It can block
then. In this case wake_up() must see the result of set_current_state()
and list_add() when it takes the same lock, otherwise spin_lock() would
be simply buggy. So it will wake the waiter up.

At the same time, if __wait_event() takes this lock after wake_up(), it
can not miss CONDITION = 1. It must see it after it takes the lock, and
of course after it drops the lock too.

So I am not sure I understand your concerns in this case...

Oleg.