Messages in this thread | | | Date | Fri, 24 Apr 2009 10:43:01 -0700 | From | "Paul E. McKenney" <> | 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
| |