[Qemu-devel] [5187] Use signalfd() to work around signal/select race

[Anthony Liguori]

Revision: 5187
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5187
Author:   aliguori
Date:     2008-09-10 15:45:19 +0000 (Wed, 10 Sep 2008)

Log Message:
-----------
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 <address@hidden>

Modified Paths:
--------------
    trunk/Makefile
    trunk/Makefile.target
    trunk/block-raw-posix.c
    trunk/block-raw-win32.c
    trunk/block.c
    trunk/block.h
    trunk/vl.c

Modified: trunk/Makefile
===================================================================
--- trunk/Makefile      2008-09-10 15:23:19 UTC (rev 5186)
+++ trunk/Makefile      2008-09-10 15:45:19 UTC (rev 5187)
@@ -177,7 +177,7 @@
 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 @@
        $(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

Modified: trunk/Makefile.target
===================================================================
--- trunk/Makefile.target       2008-09-10 15:23:19 UTC (rev 5186)
+++ trunk/Makefile.target       2008-09-10 15:45:19 UTC (rev 5187)
@@ -476,7 +476,7 @@
 ifdef CONFIG_WIN32
 OBJS+=block-raw-win32.o
 else
-OBJS+=block-raw-posix.o
+OBJS+=block-raw-posix.o compatfd.o
 endif
 
 LIBS+=-lz

Modified: trunk/block-raw-posix.c
===================================================================
--- trunk/block-raw-posix.c     2008-09-10 15:23:19 UTC (rev 5186)
+++ trunk/block-raw-posix.c     2008-09-10 15:45:19 UTC (rev 5187)
@@ -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,53 +440,13 @@
     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 @@
  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();
+    qemu_aio_poll(NULL);
     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 nb_sigs;
+    int ret;
 
 #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)
-{
-    sigprocmask(SIG_SETMASK, &wait_oset, NULL);
+    do {
+        fd_set rdfds;
+
+        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_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 @@
 #endif
 }
 
-void qemu_aio_wait_end(void)
-{
-}
-
 #endif /* CONFIG_AIO */
 
 static void raw_close(BlockDriverState *bs)

Modified: trunk/block-raw-win32.c
===================================================================
--- trunk/block-raw-win32.c     2008-09-10 15:23:19 UTC (rev 5186)
+++ trunk/block-raw-win32.c     2008-09-10 15:45:19 UTC (rev 5187)
@@ -350,18 +350,10 @@
 {
 }
 
-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 @@
 #endif
 }
 
-void qemu_aio_wait_end(void)
-{
-}
-
 BlockDriver bdrv_raw = {
     "raw",
     sizeof(BDRVRawState),

Modified: trunk/block.c
===================================================================
--- trunk/block.c       2008-09-10 15:23:19 UTC (rev 5186)
+++ trunk/block.c       2008-09-10 15:45:19 UTC (rev 5187)
@@ -1280,17 +1280,15 @@
     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) {
-        qemu_aio_wait_end();
+    if (acb == NULL)
         return -1;
-    }
+
     while (async_ret == NOT_DONE) {
         qemu_aio_wait();
     }
-    qemu_aio_wait_end();
+
     return async_ret;
 }
 
@@ -1301,17 +1299,13 @@
     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) {
-        qemu_aio_wait_end();
+    if (acb == NULL)
         return -1;
-    }
     while (async_ret == NOT_DONE) {
         qemu_aio_wait();
     }
-    qemu_aio_wait_end();
     return async_ret;
 }
 

Modified: trunk/block.h
===================================================================
--- trunk/block.h       2008-09-10 15:23:19 UTC (rev 5186)
+++ trunk/block.h       2008-09-10 15:45:19 UTC (rev 5187)
@@ -90,11 +90,8 @@
 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);
 

Modified: trunk/vl.c
===================================================================
--- trunk/vl.c  2008-09-10 15:23:19 UTC (rev 5186)
+++ trunk/vl.c  2008-09-10 15:45:19 UTC (rev 5187)
@@ -7482,7 +7482,6 @@
         slirp_select_poll(&rfds, &wfds, &xfds);
     }
 #endif
-    qemu_aio_poll();
 
     if (vm_running) {
         if (likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)))