On Tue, May 13, 2014 at 01:53:13PM +0100, Mel Gorman wrote: > On Tue, May 13, 2014 at 10:45:50AM +0100, Mel Gorman wrote: > > void unlock_page(struct page *page) > > { > > + wait_queue_head_t *wqh = clear_page_waiters(page); > > + > > VM_BUG_ON_PAGE(!PageLocked(page), page); > > + > > + /* > > + * No additional barrier needed due to clear_bit_unlock barriering all updates > > + * before waking waiters > > + */ > > clear_bit_unlock(PG_locked, &page->flags); > > - smp_mb__after_clear_bit(); > > - wake_up_page(page, PG_locked); > > This is wrong. The smp_mb__after_clear_bit() is still required to ensure > that the cleared bit is visible before the wakeup on all architectures. wakeup implies a mb, and I just noticed that our Documentation is 'obsolete' and only mentions it implies a wmb. Also, if you're going to use smp_mb__after_atomic() you can use clear_bit() and not use clear_bit_unlock(). --- Subject: doc: Update wakeup barrier documentation As per commit e0acd0a68ec7 ("sched: fix the theoretical signal_wake_up() vs schedule() race") both wakeup and schedule now imply a full barrier. Furthermore, the barrier is unconditional when calling try_to_wake_up() and has been for a fair while. Cc: Oleg Nesterov <oleg@xxxxxxxxxx> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> Cc: David Howells <dhowells@xxxxxxxxxx> Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> Signed-off-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx> --- Documentation/memory-barriers.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt index 46412bded104..dae5158c2382 100644 --- a/Documentation/memory-barriers.txt +++ b/Documentation/memory-barriers.txt @@ -1881,9 +1881,9 @@ The whole sequence above is available in various canned forms, all of which event_indicated = 1; wake_up_process(event_daemon); -A write memory barrier is implied by wake_up() and co. if and only if they wake -something up. The barrier occurs before the task state is cleared, and so sits -between the STORE to indicate the event and the STORE to set TASK_RUNNING: +A full memory barrier is implied by wake_up() and co. The barrier occurs +before the task state is cleared, and so sits between the STORE to indicate +the event and the STORE to set TASK_RUNNING: CPU 1 CPU 2 =============================== =============================== -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html