Use signalfd() to work around signal/select race

This patch introduces signalfd() to work around the signal/select race in
checking for AIO completions.  For platforms that don't support signalfd(), we
emulate it with threads.

There was a long discussion about this approach.  I don't believe there are any
fundamental problems with this approach and I believe eliminating the use of
signals is a good thing.

I've tested Windows and Linux using Windows and Linux guests.  I've also checked
for disk IO performance regressions.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>



git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@5187 c046a42c-6fe2-441c-8c8c-71466251a162

Makefile

@@ -177,7 +177,7 @@ QEMU_IMG_BLOCK_OBJS = $(BLOCK_OBJS)
 ifdef CONFIG_WIN32
 QEMU_IMG_BLOCK_OBJS += qemu-img-block-raw-win32.o
 else
-QEMU_IMG_BLOCK_OBJS += nbd.o qemu-img-block-raw-posix.o
+QEMU_IMG_BLOCK_OBJS += nbd.o qemu-img-block-raw-posix.o compatfd.o
 endif
 
 ######################################################################
@@ -195,7 +195,7 @@ qemu-nbd-%.o: %.c
 	$(CC) $(CFLAGS) $(CPPFLAGS) -DQEMU_NBD -c -o $@ $<
 
 qemu-nbd$(EXESUF):  qemu-nbd.o qemu-nbd-nbd.o qemu-img-block.o \
-		    osdep.o qemu-nbd-block-raw-posix.o $(BLOCK_OBJS)
+		    osdep.o qemu-nbd-block-raw-posix.o compatfd.o $(BLOCK_OBJS)
 	$(CC) $(LDFLAGS) -o $@ $^ -lz $(LIBS)
 
 # dyngen host tool

Makefile.target

@@ -476,7 +476,7 @@ OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o net-checksum.o
 ifdef CONFIG_WIN32
 OBJS+=block-raw-win32.o
 else
-OBJS+=block-raw-posix.o
+OBJS+=block-raw-posix.o compatfd.o
 endif
 
 LIBS+=-lz

block-raw-posix.c

@@ -25,8 +25,10 @@
 #if !defined(QEMU_IMG) && !defined(QEMU_NBD)
 #include "qemu-timer.h"
 #include "exec-all.h"
+#include "qemu-char.h"
 #endif
 #include "block_int.h"
+#include "compatfd.h"
 #include <assert.h>
 #ifdef CONFIG_AIO
 #include <aio.h>
@@ -438,52 +440,12 @@ typedef struct RawAIOCB {
     int ret;
 } RawAIOCB;
 
+static int aio_sig_fd = -1;
 static int aio_sig_num = SIGUSR2;
 static RawAIOCB *first_aio; /* AIO issued */
 static int aio_initialized = 0;
 
-static void aio_signal_handler(int signum)
+static void qemu_aio_poll(void *opaque)
-{
-#if !defined(QEMU_IMG) && !defined(QEMU_NBD)
-    CPUState *env = cpu_single_env;
-    if (env) {
-        /* stop the currently executing cpu because a timer occured */
-        cpu_interrupt(env, CPU_INTERRUPT_EXIT);
-#ifdef USE_KQEMU
-        if (env->kqemu_enabled) {
-            kqemu_cpu_interrupt(env);
-        }
-#endif
-    }
-#endif
-}
-
-void qemu_aio_init(void)
-{
-    struct sigaction act;
-
-    aio_initialized = 1;
-
-    sigfillset(&act.sa_mask);
-    act.sa_flags = 0; /* do not restart syscalls to interrupt select() */
-    act.sa_handler = aio_signal_handler;
-    sigaction(aio_sig_num, &act, NULL);
-
-#if defined(__GLIBC__) && defined(__linux__)
-    {
-        /* XXX: aio thread exit seems to hang on RedHat 9 and this init
-           seems to fix the problem. */
-        struct aioinit ai;
-        memset(&ai, 0, sizeof(ai));
-        ai.aio_threads = 1;
-        ai.aio_num = 1;
-        ai.aio_idle_time = 365 * 100000;
-        aio_init(&ai);
-    }
-#endif
-}
-
-void qemu_aio_poll(void)
 {
     RawAIOCB *acb, **pacb;
     int ret;
@@ -524,49 +486,66 @@ void qemu_aio_poll(void)
  the_end: ;
 }
 
+void qemu_aio_init(void)
+{
+    sigset_t mask;
+
+    aio_initialized = 1;
+
+    /* Make sure to block AIO signal */
+    sigemptyset(&mask);
+    sigaddset(&mask, aio_sig_num);
+    sigprocmask(SIG_BLOCK, &mask, NULL);
+    
+    aio_sig_fd = qemu_signalfd(&mask);
+#if !defined(QEMU_IMG) && !defined(QEMU_NBD)
+    qemu_set_fd_handler2(aio_sig_fd, NULL, qemu_aio_poll, NULL, NULL);
+#endif
+
+#if defined(__GLIBC__) && defined(__linux__)
+    {
+        /* XXX: aio thread exit seems to hang on RedHat 9 and this init
+           seems to fix the problem. */
+        struct aioinit ai;
+        memset(&ai, 0, sizeof(ai));
+        ai.aio_threads = 1;
+        ai.aio_num = 1;
+        ai.aio_idle_time = 365 * 100000;
+        aio_init(&ai);
+    }
+#endif
+}
+
 /* Wait for all IO requests to complete.  */
 void qemu_aio_flush(void)
 {
-    qemu_aio_wait_start();
+    qemu_aio_poll(NULL);
-    qemu_aio_poll();
     while (first_aio) {
         qemu_aio_wait();
     }
-    qemu_aio_wait_end();
-}
-
-/* wait until at least one AIO was handled */
-static sigset_t wait_oset;
-
-void qemu_aio_wait_start(void)
-{
-    sigset_t set;
-
-    if (!aio_initialized)
-        qemu_aio_init();
-    sigemptyset(&set);
-    sigaddset(&set, aio_sig_num);
-    sigprocmask(SIG_BLOCK, &set, &wait_oset);
 }
 
 void qemu_aio_wait(void)
 {
-    sigset_t set;
+    int ret;
-    int nb_sigs;
 
 #if !defined(QEMU_IMG) && !defined(QEMU_NBD)
     if (qemu_bh_poll())
         return;
 #endif
-    sigemptyset(&set);
-    sigaddset(&set, aio_sig_num);
-    sigwait(&set, &nb_sigs);
-    qemu_aio_poll();
-}
 
-void qemu_aio_wait_end(void)
+    do {
-{
+        fd_set rdfds;
-    sigprocmask(SIG_SETMASK, &wait_oset, NULL);
+
+        FD_ZERO(&rdfds);
+        FD_SET(aio_sig_fd, &rdfds);
+
+        ret = select(aio_sig_fd + 1, &rdfds, NULL, NULL, NULL);
+        if (ret == -1 && errno == EINTR)
+            continue;
+    } while (ret == 0);
+
+    qemu_aio_poll(NULL);
 }
 
 static RawAIOCB *raw_aio_setup(BlockDriverState *bs,
@@ -704,18 +683,10 @@ void qemu_aio_init(void)
 {
 }
 
-void qemu_aio_poll(void)
-{
-}
-
 void qemu_aio_flush(void)
 {
 }
 
-void qemu_aio_wait_start(void)
-{
-}
-
 void qemu_aio_wait(void)
 {
 #if !defined(QEMU_IMG) && !defined(QEMU_NBD)
@@ -723,10 +694,6 @@ void qemu_aio_wait(void)
 #endif
 }
 
-void qemu_aio_wait_end(void)
-{
-}
-
 #endif /* CONFIG_AIO */
 
 static void raw_close(BlockDriverState *bs)

block-raw-win32.c

@@ -350,18 +350,10 @@ void qemu_aio_init(void)
 {
 }
 
-void qemu_aio_poll(void)
-{
-}
-
 void qemu_aio_flush(void)
 {
 }
 
-void qemu_aio_wait_start(void)
-{
-}
-
 void qemu_aio_wait(void)
 {
 #ifndef QEMU_IMG
@@ -369,10 +361,6 @@ void qemu_aio_wait(void)
 #endif
 }
 
-void qemu_aio_wait_end(void)
-{
-}
-
 BlockDriver bdrv_raw = {
     "raw",
     sizeof(BDRVRawState),

block.c

@@ -1280,17 +1280,15 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
     BlockDriverAIOCB *acb;
 
     async_ret = NOT_DONE;
-    qemu_aio_wait_start();
     acb = bdrv_aio_read(bs, sector_num, buf, nb_sectors,
                         bdrv_rw_em_cb, &async_ret);
-    if (acb == NULL) {
+    if (acb == NULL)
-        qemu_aio_wait_end();
         return -1;
-    }
+
     while (async_ret == NOT_DONE) {
         qemu_aio_wait();
     }
-    qemu_aio_wait_end();
+
     return async_ret;
 }
 
@@ -1301,17 +1299,13 @@ static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
     BlockDriverAIOCB *acb;
 
     async_ret = NOT_DONE;
-    qemu_aio_wait_start();
     acb = bdrv_aio_write(bs, sector_num, buf, nb_sectors,
                          bdrv_rw_em_cb, &async_ret);
-    if (acb == NULL) {
+    if (acb == NULL)
-        qemu_aio_wait_end();
         return -1;
-    }
     while (async_ret == NOT_DONE) {
         qemu_aio_wait();
     }
-    qemu_aio_wait_end();
     return async_ret;
 }
 

block.h

@@ -90,11 +90,8 @@ BlockDriverAIOCB *bdrv_aio_write(BlockDriverState *bs, int64_t sector_num,
 void bdrv_aio_cancel(BlockDriverAIOCB *acb);
 
 void qemu_aio_init(void);
-void qemu_aio_poll(void);
 void qemu_aio_flush(void);
-void qemu_aio_wait_start(void);
 void qemu_aio_wait(void);
-void qemu_aio_wait_end(void);
 
 int qemu_key_check(BlockDriverState *bs, const char *name);
 

vl.c

@@ -7482,7 +7482,6 @@ void main_loop_wait(int timeout)
         slirp_select_poll(&rfds, &wfds, &xfds);
     }
 #endif
-    qemu_aio_poll();
 
     if (vm_running) {
         if (likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)))