linux-kernel - Re: [PATCH] It may not be assumed that wake_up(), finish

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090424174301.GB6754@linux.vnet.ibm.com>
Date:	Fri, 24 Apr 2009 10:43:01 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	David Howells <dhowells@...hat.com>, Ingo Molnar <mingo@...e.hu>,
	torvalds@...l.org, Andrew Morton <akpm@...ux-foundation.org>,
	serue@...ibm.com, viro@...iv.linux.org.uk,
	Nick Piggin <nickpiggin@...oo.com.au>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] It may not be assumed that wake_up(), finish_wait()
	and co. imply a memory barrier

On Fri, Apr 24, 2009 at 07:08:30PM +0200, Oleg Nesterov wrote:
> On 04/24, Paul E. McKenney wrote:
> >
> > One question, assuming that this documentation intends to guide the
> > reader on where to put the locking and/or memory-barrier primitives...
> >
> > Suppose we have the following sequence of events:
> >
> > 1.	The waiter does "set_current_state(TASK_UNINTERRUPTIBLE);".
> > 	This implies a full memory barrier.
> >
> > 2.	The awakener updates some shared state.
> >
> > 3.	The awakener does "event_indicated = 1;".
> >
> > 4.	The waiter does "if (event_indicated)", and, finding that
> > 	the event has in fact been indicated, does "break".
> >
> > 5.	The waiter accesses the shared state set in #2 above.
> >
> > 6.	Some time later, the awakener does "wake_up(&event_wait_queue);"
> > 	This does not awaken anyone, so no memory barrier.
> >
> > Because there is no memory barrier between #2 and #3, reordering by
> > either the compiler or the CPU might cause the awakener to update the
> > event_indicated flag in #3 -before- completing its update of shared
> > state in #2.  Less likely (but still possible) optimizations might
> > cause the waiter to access the shared state in #5 before checking
> > the event_indicated flag in #4.
> 
> Do you mean something like
> 
> 	awakener:
> 
> 		DATA = value;
> 		DATA_IS_READY = true;
> 		wake_up(wq);
> 
> 
> 	waiter:
> 
> 		set_current_state(UNINTERRUPTIBLE);
> 		if (DATA_IS_READY)
> 			do_something(DATA);
> 
> ?
> 
> Imho, the code above is just buggy and should be ignored by documentation ;)
> 
> Or do I miss your point?

I was hoping that this sort of code might be actively discouraged by the
documentation.  ;-)

							Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/