[PATCH] Document that wake_up(), complete() and co. imply a full memory barrier

[David Howells <dhowells@redhat.com>]

Oleg Nesterov <oleg@redhat.com> wrote:

> > That's an interesting question.  Should wake_up() imply a barrier of any
> > sort, I wonder.  Well, __wake_up() does impose a barrier as it uses a
> > spinlock, but I wonder if that's sufficient.
> 
> wake_up() does imply the barrier. Note the smp_wmb() in try_to_wake_up().
> And in fact this wmb() implies mb(), because spin_lock() itself is STORE,
> and the futher LOADs can't leak up before spin_lock().
> 
> But afaics, this doesn't matter? prepare_to_wait() sets task->state under
> wait_queue_head_t->lock and wake_up() takes this look too, so we can't miss
> the event.
> 
> Or I completely misunderstood the issue...

The problem is not what wake_up() and co. do, it's what you are allowed to
assume that they do.

However, I think you're right, and that we can assume they imply a full memory
barrier.  To this end, I've attached a patch to document this.

David
---
From: David Howells <dhowells@redhat.com>
Subject: [PATCH] Document that wake_up(), complete() and co. imply a full memory barrier

Add to the memory barriers document to note that wake_up(), complete() and
co. all imply a full memory barrier.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 Documentation/memory-barriers.txt |    4 ++++
 kernel/sched.c                    |   10 ++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)


diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index f5b7127..2c8062c 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1224,6 +1224,10 @@ Other functions that imply barriers:
 
  (*) schedule() and similar imply full memory barriers.
 
+ (*) wake_up(), try_to_wake_up() and co. imply a full memory barrier.
+
+ (*) complete() and co. imply a full memory barrier.
+
 
 =================================
 INTER-CPU LOCKING BARRIER EFFECTS
diff --git a/kernel/sched.c b/kernel/sched.c
index b902e58..faccaa0 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2337,6 +2337,8 @@ static int sched_balance_self(int cpu, int flag)
  * runnable without the overhead of this.
  *
  * returns failure only if the task is already active.
+ *
+ * It may be assumed that this function implies a full memory barrier.
  */
 static int try_to_wake_up(struct task_struct *p, unsigned int state, int sync)
 {
@@ -5241,6 +5243,8 @@ void __wake_up_common(wait_queue_head_t *q, unsigned int mode,
  * @mode: which threads
  * @nr_exclusive: how many wake-one or wake-many threads to wake up
  * @key: is directly passed to the wakeup function
+ *
+ * It may be assumed that this function implies a full memory barrier.
  */
 void __wake_up(wait_queue_head_t *q, unsigned int mode,
 			int nr_exclusive, void *key)
@@ -5279,6 +5283,8 @@ void __wake_up_locked_key(wait_queue_head_t *q, unsigned int mode, void *key)
  * with each other. This can prevent needless bouncing between CPUs.
  *
  * On UP it can prevent extra preemption.
+ *
+ * It may be assumed that this function implies a full memory barrier.
  */
 void __wake_up_sync_key(wait_queue_head_t *q, unsigned int mode,
 			int nr_exclusive, void *key)
@@ -5315,6 +5321,8 @@ EXPORT_SYMBOL_GPL(__wake_up_sync);	/* For internal use only */
  * awakened in the same order in which they were queued.
  *
  * See also complete_all(), wait_for_completion() and related routines.
+ *
+ * It may be assumed that this function implies a full memory barrier.
  */
 void complete(struct completion *x)
 {
@@ -5332,6 +5340,8 @@ EXPORT_SYMBOL(complete);
  * @x:  holds the state of this particular completion
  *
  * This will wake up all threads waiting on this particular completion event.
+ *
+ * It may be assumed that this function implies a full memory barrier.
  */
 void complete_all(struct completion *x)
 {