aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJeff Dike <jdike@addtoit.com>2006-11-03 01:07:22 -0500
committerLinus Torvalds <torvalds@g5.osdl.org>2006-11-03 15:27:58 -0500
commit53b173327d283b9bdbfb0c3b6de6f0eb197819d6 (patch)
tree0b41508295ec6a7a826ec36f72f77433da460e57
parentd2c89a4284ea4ecfba77c6f2d7d6f96d52e801e5 (diff)
[PATCH] uml: fix I/O hang
Fix a UML hang in which everything would just stop until some I/O happened - a ping, someone whacking the keyboard - at which point everything would start up again as though nothing had happened. The cause was gcc reordering some code which absolutely needed to be executed in the order in the source. When unblock_signals switches signals from off to on, it needs to see if any interrupts had happened in the critical section. The interrupt handlers check signals_enabled - if it is zero, then the handler adds a bit to the "pending" bitmask and returns. unblock_signals checks this mask to see if any signals need to be delivered. The crucial part is this: signals_enabled = 1; save_pending = pending; if(save_pending == 0) return; pending = 0; In order to avoid an interrupt arriving between reading pending and setting it to zero, in which case, the record of the interrupt would be erased, signals are enabled. What happened was that gcc reordered this so that 'save_pending = pending' came before 'signals_enabled = 1', creating a one-instruction window within which an interrupt could arrive, set its bit in pending, and have it be immediately erased. When the I/O workload is purely disk-based, the loss of a block device interrupt stops the entire I/O system because the next block request will wait for the current one to finish. Thus the system hangs until something else causes some I/O to arrive, such as a network packet or console input. The fix to this particular problem is a memory barrier between enabling signals and reading the pending signal mask. An xchg would also probably work. Looking over this code for similar problems led me to do a few more things: - make signals_enabled and pending volatile so that they don't get cached in registers - add an mb() to the return paths of block_signals and unblock_signals so that the modification of signals_enabled doesn't get shuffled into the caller in the event that these are inlined in the future. Signed-off-by: Jeff Dike <jdike@addtoit.com> Cc: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
-rw-r--r--arch/um/include/sysdep-i386/barrier.h9
-rw-r--r--arch/um/include/sysdep-x86_64/barrier.h7
-rw-r--r--arch/um/os-Linux/signal.c31
3 files changed, 44 insertions, 3 deletions
diff --git a/arch/um/include/sysdep-i386/barrier.h b/arch/um/include/sysdep-i386/barrier.h
new file mode 100644
index 000000000000..b58d52c5b2f4
--- /dev/null
+++ b/arch/um/include/sysdep-i386/barrier.h
@@ -0,0 +1,9 @@
1#ifndef __SYSDEP_I386_BARRIER_H
2#define __SYSDEP_I386_BARRIER_H
3
4/* Copied from include/asm-i386 for use by userspace. i386 has the option
5 * of using mfence, but I'm just using this, which works everywhere, for now.
6 */
7#define mb() asm volatile("lock; addl $0,0(%esp)")
8
9#endif
diff --git a/arch/um/include/sysdep-x86_64/barrier.h b/arch/um/include/sysdep-x86_64/barrier.h
new file mode 100644
index 000000000000..7b610befdc8f
--- /dev/null
+++ b/arch/um/include/sysdep-x86_64/barrier.h
@@ -0,0 +1,7 @@
1#ifndef __SYSDEP_X86_64_BARRIER_H
2#define __SYSDEP_X86_64_BARRIER_H
3
4/* Copied from include/asm-x86_64 for use by userspace. */
5#define mb() asm volatile("mfence":::"memory")
6
7#endif
diff --git a/arch/um/os-Linux/signal.c b/arch/um/os-Linux/signal.c
index 6b81739279d1..b897e8592d77 100644
--- a/arch/um/os-Linux/signal.c
+++ b/arch/um/os-Linux/signal.c
@@ -15,6 +15,7 @@
15#include "user.h" 15#include "user.h"
16#include "signal_kern.h" 16#include "signal_kern.h"
17#include "sysdep/sigcontext.h" 17#include "sysdep/sigcontext.h"
18#include "sysdep/barrier.h"
18#include "sigcontext.h" 19#include "sigcontext.h"
19#include "mode.h" 20#include "mode.h"
20#include "os.h" 21#include "os.h"
@@ -34,8 +35,12 @@
34#define SIGALRM_BIT 2 35#define SIGALRM_BIT 2
35#define SIGALRM_MASK (1 << SIGALRM_BIT) 36#define SIGALRM_MASK (1 << SIGALRM_BIT)
36 37
37static int signals_enabled = 1; 38/* These are used by both the signal handlers and
38static int pending = 0; 39 * block/unblock_signals. I don't want modifications cached in a
40 * register - they must go straight to memory.
41 */
42static volatile int signals_enabled = 1;
43static volatile int pending = 0;
39 44
40void sig_handler(int sig, struct sigcontext *sc) 45void sig_handler(int sig, struct sigcontext *sc)
41{ 46{
@@ -152,6 +157,12 @@ int change_sig(int signal, int on)
152void block_signals(void) 157void block_signals(void)
153{ 158{
154 signals_enabled = 0; 159 signals_enabled = 0;
160 /* This must return with signals disabled, so this barrier
161 * ensures that writes are flushed out before the return.
162 * This might matter if gcc figures out how to inline this and
163 * decides to shuffle this code into the caller.
164 */
165 mb();
155} 166}
156 167
157void unblock_signals(void) 168void unblock_signals(void)
@@ -171,9 +182,23 @@ void unblock_signals(void)
171 */ 182 */
172 signals_enabled = 1; 183 signals_enabled = 1;
173 184
185 /* Setting signals_enabled and reading pending must
186 * happen in this order.
187 */
188 mb();
189
174 save_pending = pending; 190 save_pending = pending;
175 if(save_pending == 0) 191 if(save_pending == 0){
192 /* This must return with signals enabled, so
193 * this barrier ensures that writes are
194 * flushed out before the return. This might
195 * matter if gcc figures out how to inline
196 * this (unlikely, given its size) and decides
197 * to shuffle this code into the caller.
198 */
199 mb();
176 return; 200 return;
201 }
177 202
178 pending = 0; 203 pending = 0;
179 204