On Thu 27-08-15 14:43:34, Peter Zijlstra wrote:
> On Thu, Aug 27, 2015 at 02:27:27PM +0200, Michal Hocko wrote:
> > Hi,
> > I have just stumbled over the comment above wake_up_process which
> > claims:
> > "
> > * It may be assumed that this function implies a write memory barrier
> > before
> > * changing the task state if and only if any tasks are woken up.
> > "
> >
> > but try_to_wake_up does smp_mb__before_spinlock and did smp_wmb
> > since 04e2f1741d235 unconditionally. The comment was added when the
> > smp_wmb was in place already so I am wondering whether the comment is
> > wrong/misleading.
> >
> > Could somebody clarify please?
>
> Its true for wake_up(), since that bails early if the waitqueue list is
> empty.
>
> I suspect there was no exception made for wake_up_process() to simplify
> the rules.
Thanks for the confirmation. Shouldn't we rather change the
documentation because this is clearly misleading and confusing.
---
>From b70d9a384cfd018e686c0aca06e830f564a34dd9 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mho...@suse.com>
Date: Thu, 27 Aug 2015 15:10:55 +0200
Subject: [PATCH] sched: Clarigy wake_up_process memory barrier semantic
wake_up_process unlike other wake up primitives based on __wake_up
implies the write memory barrier unconditionally because it relies
on try_to_wake_up directly.
Clarify this in the function comment and memory-barriers.txt because the
current doc is quite misleading.
Signed-off-by: Michal Hocko <mho...@suse.com>
---
Documentation/memory-barriers.txt | 3 +++
kernel/sched/core.c | 3 +--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/memory-barriers.txt
b/Documentation/memory-barriers.txt
index 13feb697271f..c4f180caf0ff 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -2031,6 +2031,9 @@ something up. The barrier occurs before the task state
is cleared, and so sits
<general barrier> STORE current->state
LOAD event_indicated
+Please note that wake_up_process is an exception here because it implies
+the write memory barrier unconditionally.
+
To repeat, this write memory barrier is present if and only if something
is actually awakened. To see this, consider the following sequence of
events, where X and Y are both initially zero:
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 78b4bad10081..39583b76ad2c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1967,8 +1967,7 @@ static void try_to_wake_up_local(struct task_struct *p)
*
* Return: 1 if the process was woken up, 0 if it was already running.
*
- * It may be assumed that this function implies a write memory barrier before
- * changing the task state if and only if any tasks are woken up.
+ * It may be assumed that this function implies a write memory barrier.
*/
int wake_up_process(struct task_struct *p)
{
--
2.5.0
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
