linux-kernel - Re: wake_up_process implied memory barrier clarification

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150908002245.GA16157@fixme-laptop.cn.ibm.com>
Date:	Tue, 8 Sep 2015 08:22:45 +0800
From:	Boqun Feng <boqun.feng@...il.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Michal Hocko <mhocko@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	David Howells <dhowells@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jonathan Corbet <corbet@....net>
Subject: Re: wake_up_process implied memory barrier clarification

Hi Oleg,

On Mon, Sep 07, 2015 at 07:06:52PM +0200, Oleg Nesterov wrote:
> 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.
> 

Yes, you're right! I wasn't aware that in prepare_to_wait_event(),
set_current_state() is called with the WQ->lock.

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

It's my mistake. Thank you for your explanation ;-)

Regards,
Boqun

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)