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

David Howells
2009-04-22T13:38:24Z

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)
{