diff options
author | Oleg Nesterov <oleg@redhat.com> | 2013-08-12 12:14:00 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2013-08-13 11:19:26 -0400 |
commit | e0acd0a68ec7dbf6b7a81a87a867ebd7ac9b76c4 (patch) | |
tree | 0421e55e2d74024f1ee1949ccdd4cd92765b2560 /include | |
parent | 584d88b2cd3b60507e708d2452651e4d3caa1b81 (diff) |
sched: fix the theoretical signal_wake_up() vs schedule() race
This is only theoretical, but after try_to_wake_up(p) was changed
to check p->state under p->pi_lock the code like
__set_current_state(TASK_INTERRUPTIBLE);
schedule();
can miss a signal. This is the special case of wait-for-condition,
it relies on try_to_wake_up/schedule interaction and thus it does
not need mb() between __set_current_state() and if(signal_pending).
However, this __set_current_state() can move into the critical
section protected by rq->lock, now that try_to_wake_up() takes
another lock we need to ensure that it can't be reordered with
"if (signal_pending(current))" check inside that section.
The patch is actually one-liner, it simply adds smp_wmb() before
spin_lock_irq(rq->lock). This is what try_to_wake_up() already
does by the same reason.
We turn this wmb() into the new helper, smp_mb__before_spinlock(),
for better documentation and to allow the architectures to change
the default implementation.
While at it, kill smp_mb__after_lock(), it has no callers.
Perhaps we can also add smp_mb__before/after_spinunlock() for
prepare_to_wait().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'include')
-rw-r--r-- | include/linux/spinlock.h | 14 |
1 files changed, 11 insertions, 3 deletions
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h index 7d537ced949a..75f34949d9ab 100644 --- a/include/linux/spinlock.h +++ b/include/linux/spinlock.h | |||
@@ -117,9 +117,17 @@ do { \ | |||
117 | #endif /*arch_spin_is_contended*/ | 117 | #endif /*arch_spin_is_contended*/ |
118 | #endif | 118 | #endif |
119 | 119 | ||
120 | /* The lock does not imply full memory barrier. */ | 120 | /* |
121 | #ifndef ARCH_HAS_SMP_MB_AFTER_LOCK | 121 | * Despite its name it doesn't necessarily has to be a full barrier. |
122 | static inline void smp_mb__after_lock(void) { smp_mb(); } | 122 | * It should only guarantee that a STORE before the critical section |
123 | * can not be reordered with a LOAD inside this section. | ||
124 | * spin_lock() is the one-way barrier, this LOAD can not escape out | ||
125 | * of the region. So the default implementation simply ensures that | ||
126 | * a STORE can not move into the critical section, smp_wmb() should | ||
127 | * serialize it with another STORE done by spin_lock(). | ||
128 | */ | ||
129 | #ifndef smp_mb__before_spinlock | ||
130 | #define smp_mb__before_spinlock() smp_wmb() | ||
123 | #endif | 131 | #endif |
124 | 132 | ||
125 | /** | 133 | /** |