[Qemu-devel] [5451] Fix IO performance regression in sparc

[Anthony Liguori]

Revision: 5451
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5451
Author:   aliguori
Date:     2008-10-08 19:50:24 +0000 (Wed, 08 Oct 2008)

Log Message:
-----------
Fix IO performance regression in sparc

Replace signalfd with signal handler/pipe.  There is no way to interrupt
the CPU execution loop when a file descriptor becomes readable.  This
results in a large performance regression in sparc emulation during
bootup.
   
This patch switches us to signal handler/pipe which was originally
suggested by Ian Jackson.  The signal handler lets us interrupt the
CPU emulation loop while the write to a pipe lets us avoid the
select/signal race condition.
    
Signed-off-by: Anthony Liguori <address@hidden>

Modified Paths:
--------------
    trunk/Makefile
    trunk/Makefile.target
    trunk/block-raw-posix.c
    trunk/configure
    trunk/qemu-common.h
    trunk/qemu-tool.c
    trunk/vl.c

Removed Paths:
-------------
    trunk/compatfd.c
    trunk/compatfd.h

Modified: trunk/Makefile
===================================================================
--- trunk/Makefile      2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/Makefile      2008-10-08 19:50:24 UTC (rev 5451)
@@ -59,10 +59,6 @@
 BLOCK_OBJS += block-raw-posix.o
 endif
 
-ifdef CONFIG_AIO
-BLOCK_OBJS += compatfd.o
-endif
-
 ######################################################################
 # libqemu_common.a: Target independent part of system emulation. The
 # long term path is to suppress *all* target specific code in case of

Modified: trunk/Makefile.target
===================================================================
--- trunk/Makefile.target       2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/Makefile.target       2008-10-08 19:50:24 UTC (rev 5451)
@@ -481,10 +481,6 @@
 OBJS+=block-raw-posix.o
 endif
 
-ifdef CONFIG_AIO
-OBJS+=compatfd.o
-endif
-
 LIBS+=-lz
 ifdef CONFIG_ALSA
 LIBS += -lasound

Modified: trunk/block-raw-posix.c
===================================================================
--- trunk/block-raw-posix.c     2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/block-raw-posix.c     2008-10-08 19:50:24 UTC (rev 5451)
@@ -25,7 +25,6 @@
 #include "qemu-timer.h"
 #include "qemu-char.h"
 #include "block_int.h"
-#include "compatfd.h"
 #include <assert.h>
 #ifdef CONFIG_AIO
 #include <aio.h>
@@ -453,7 +452,7 @@
 
 typedef struct PosixAioState
 {
-    int fd;
+    int rfd, wfd;
     RawAIOCB *first_aio;
 } PosixAioState;
 
@@ -494,31 +493,18 @@
     PosixAioState *s = opaque;
     RawAIOCB *acb, **pacb;
     int ret;
-    size_t offset;
-    union {
-        struct qemu_signalfd_siginfo siginfo;
-        char buf[128];
-    } sig;
+    ssize_t len;
 
-    /* try to read from signalfd, don't freak out if we can't read anything */
-    offset = 0;
-    while (offset < 128) {
-        ssize_t len;
+    do {
+        char byte;
 
-        len = read(s->fd, sig.buf + offset, 128 - offset);
+        len = read(s->rfd, &byte, 1);
         if (len == -1 && errno == EINTR)
             continue;
-        if (len == -1 && errno == EAGAIN) {
-            /* there is no natural reason for this to happen,
-             * so we'll spin hard until we get everything just
-             * to be on the safe side. */
-            if (offset > 0)
-                continue;
-        }
+        if (len == -1 && errno == EAGAIN)
+            break;
+    } while (len == -1);
 
-        offset += len;
-    }
-
     for(;;) {
         pacb = &s->first_aio;
         for(;;) {
@@ -565,10 +551,22 @@
 
 static PosixAioState *posix_aio_state;
 
+static void aio_signal_handler(int signum)
+{
+    if (posix_aio_state) {
+        char byte = 0;
+
+        write(posix_aio_state->wfd, &byte, sizeof(byte));
+    }
+
+    qemu_service_io();
+}
+
 static int posix_aio_init(void)
 {
-    sigset_t mask;
+    struct sigaction act;
     PosixAioState *s;
+    int fds[2];
   
     if (posix_aio_state)
         return 0;
@@ -577,22 +575,24 @@
     if (s == NULL)
         return -ENOMEM;
 
-    /* Make sure to block AIO signal */
-    sigemptyset(&mask);
-    sigaddset(&mask, SIGUSR2);
-    sigprocmask(SIG_BLOCK, &mask, NULL);
-    
+    sigfillset(&act.sa_mask);
+    act.sa_flags = 0; /* do not restart syscalls to interrupt select() */
+    act.sa_handler = aio_signal_handler;
+    sigaction(SIGUSR2, &act, NULL);
+
     s->first_aio = NULL;
-    s->fd = qemu_signalfd(&mask);
-    if (s->fd == -1) {
-        fprintf(stderr, "failed to create signalfd\n");
+    if (pipe(fds) == -1) {
+        fprintf(stderr, "failed to create pipe\n");
         return -errno;
     }
 
-    fcntl(s->fd, F_SETFL, O_NONBLOCK);
+    s->rfd = fds[0];
+    s->wfd = fds[1];
 
-    qemu_aio_set_fd_handler(s->fd, posix_aio_read, NULL, posix_aio_flush, s);
+    fcntl(s->wfd, F_SETFL, O_NONBLOCK);
 
+    qemu_aio_set_fd_handler(s->rfd, posix_aio_read, NULL, posix_aio_flush, s);
+
 #if defined(__linux__)
     {
         struct aioinit ai;

Deleted: trunk/compatfd.c
===================================================================
--- trunk/compatfd.c    2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/compatfd.c    2008-10-08 19:50:24 UTC (rev 5451)
@@ -1,127 +0,0 @@
-/*
- * signalfd/eventfd compatibility
- *
- * Copyright IBM, Corp. 2008
- *
- * Authors:
- *  Anthony Liguori   <address@hidden>
- *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
- *
- */
-
-#include "qemu-common.h"
-#include "compatfd.h"
-
-#include <sys/syscall.h>
-#include <pthread.h>
-
-struct sigfd_compat_info
-{
-    sigset_t mask;
-    int fd;
-};
-
-static void *sigwait_compat(void *opaque)
-{
-    struct sigfd_compat_info *info = opaque;
-    int err;
-    sigset_t all;
-
-    sigfillset(&all);
-    sigprocmask(SIG_BLOCK, &all, NULL);
-
-    do {
-        siginfo_t siginfo;
-
-        err = sigwaitinfo(&info->mask, &siginfo);
-        if (err == -1 && errno == EINTR) {
-            err = 0;
-            continue;
-        }
-
-        if (err > 0) {
-            char buffer[128];
-            size_t offset = 0;
-
-            memcpy(buffer, &err, sizeof(err));
-            while (offset < sizeof(buffer)) {
-                ssize_t len;
-
-                len = write(info->fd, buffer + offset,
-                            sizeof(buffer) - offset);
-                if (len == -1 && errno == EINTR)
-                    continue;
-
-                if (len <= 0) {
-                    err = -1;
-                    break;
-                }
-
-                offset += len;
-            }
-        }
-    } while (err >= 0);
-
-    return NULL;
-}
-
-static int qemu_signalfd_compat(const sigset_t *mask)
-{
-    pthread_attr_t attr;
-    pthread_t tid;
-    struct sigfd_compat_info *info;
-    int fds[2];
-
-    info = malloc(sizeof(*info));
-    if (info == NULL) {
-        errno = ENOMEM;
-        return -1;
-    }
-
-    if (pipe(fds) == -1) {
-        free(info);
-        return -1;
-    }
-
-    memcpy(&info->mask, mask, sizeof(*mask));
-    info->fd = fds[1];
-
-    pthread_attr_init(&attr);
-    pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
-
-    pthread_create(&tid, &attr, sigwait_compat, info);
-
-    pthread_attr_destroy(&attr);
-
-    return fds[0];
-}
-
-int qemu_signalfd(const sigset_t *mask)
-{
-#if defined(CONFIG_signalfd)
-    int ret;
-
-    ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
-    if (ret != -1)
-        return ret;
-#endif
-
-    return qemu_signalfd_compat(mask);
-}
-
-int qemu_eventfd(int *fds)
-{
-#if defined(CONFIG_eventfd)
-    int ret;
-
-    ret = syscall(SYS_eventfd, 0);
-    if (ret >= 0) {
-        fds[0] = fds[1] = ret;
-        return 0;
-    }
-#endif
-
-    return pipe(fds);
-}

Deleted: trunk/compatfd.h
===================================================================
--- trunk/compatfd.h    2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/compatfd.h    2008-10-08 19:50:24 UTC (rev 5451)
@@ -1,28 +0,0 @@
-/*
- * signalfd/eventfd compatibility
- *
- * Copyright IBM, Corp. 2008
- *
- * Authors:
- *  Anthony Liguori   <address@hidden>
- *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
- *
- */
-
-#ifndef QEMU_COMPATFD_H
-#define QEMU_COMPATFD_H
-
-#include <signal.h>
-
-struct qemu_signalfd_siginfo {
-    uint32_t ssi_signo;
-    uint8_t pad[124];
-};
-
-int qemu_signalfd(const sigset_t *mask);
-
-int qemu_eventfd(int *fds);
-
-#endif

Modified: trunk/configure
===================================================================
--- trunk/configure     2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/configure     2008-10-08 19:50:24 UTC (rev 5451)
@@ -113,8 +113,6 @@
 nptl="yes"
 mixemu="no"
 bluez="yes"
-signalfd="no"
-eventfd="no"
 
 # OS specific
 targetos=`uname -s`
@@ -930,33 +928,6 @@
   fi
 fi
 
-##########################################
-# signalfd probe
-cat > $TMPC << EOF
-#define _GNU_SOURCE
-#include <unistd.h>
-#include <sys/syscall.h>
-#include <signal.h>
-int main(void) { return syscall(SYS_signalfd, -1, NULL, _NSIG / 8); }
-EOF
-
-if $cc $ARCH_CFLAGS -o $TMPE $TMPC 2> /dev/null ; then
-  signalfd=yes
-fi
-
-##########################################
-# eventfd probe
-cat > $TMPC << EOF
-#define _GNU_SOURCE
-#include <unistd.h>
-#include <sys/syscall.h>
-int main(void) { return syscall(SYS_eventfd, 0); }
-EOF
-
-if $cc $ARCH_CFLAGS -o $TMPE $TMPC 2> /dev/null ; then
-  eventfd=yes
-fi
-
 # Check if tools are available to build documentation.
 if [ -x "`which texi2html 2>/dev/null`" ] && \
    [ -x "`which pod2man 2>/dev/null`" ]; then
@@ -1297,12 +1268,6 @@
   echo "#define CONFIG_AIO 1" >> $config_h
   echo "CONFIG_AIO=yes" >> $config_mak
 fi
-if test "$signalfd" = "yes" ; then
-  echo "#define CONFIG_signalfd 1" >> $config_h
-fi
-if test "$eventfd" = "yes" ; then
-  echo "#define CONFIG_eventfd 1" >> $config_h
-fi
 
 # XXX: suppress that
 if [ "$bsd" = "yes" ] ; then

Modified: trunk/qemu-common.h
===================================================================
--- trunk/qemu-common.h 2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/qemu-common.h 2008-10-08 19:50:24 UTC (rev 5451)
@@ -139,4 +139,7 @@
 void cpu_save(QEMUFile *f, void *opaque);
 int cpu_load(QEMUFile *f, void *opaque, int version_id);
 
+/* Force QEMU to stop what it's doing and service IO */
+void qemu_service_io(void);
+
 #endif

Modified: trunk/qemu-tool.c
===================================================================
--- trunk/qemu-tool.c   2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/qemu-tool.c   2008-10-08 19:50:24 UTC (rev 5451)
@@ -26,6 +26,10 @@
     void *opaque;
 };
 
+void qemu_service_io(void)
+{
+}
+
 void term_printf(const char *fmt, ...)
 {
 }

Modified: trunk/vl.c
===================================================================
--- trunk/vl.c  2008-10-08 14:39:08 UTC (rev 5450)
+++ trunk/vl.c  2008-10-08 19:50:24 UTC (rev 5451)
@@ -7475,6 +7475,19 @@
     return 0;
 }
 
+void qemu_service_io(void)
+{
+    CPUState *env = cpu_single_env;
+    if (env) {
+        cpu_interrupt(env, CPU_INTERRUPT_EXIT);
+#ifdef USE_KQEMU
+        if (env->kqemu_enabled) {
+            kqemu_cpu_interrupt(env);
+        }
+#endif
+    }
+}
+
 /***********************************************************/
 /* bottom halves (can be seen as timers which expire ASAP) */