[Qemu-devel] Re: [PATCH][RFT] Fix the regression with SPARC emulation

[Anthony Liguori <aliguori@us.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

Anthony Liguori wrote:
> The attached patch should be the proper fix for the sparc performance 
> regression.  Unfortunately, this means abandoning signalfd() in favor 
> of the signal handler/pipe trick but I don't see another solution to 
> the problem.
>
> Please test and let me know if this works for all of your cases (it 
> did for me with -clock unix and -clock dynticks).

Err, sorry, here is the proper patch.  git did not do what I expected it 
to do.

Regards,

Anthony Liguori

> Regards,
>
> Anthony Liguori


[-- Attachment #2: sigio-fix.patch --]
[-- Type: text/x-patch, Size: 10326 bytes --]

commit d3e13a91224d167652288ba98de15658a233d60e
Author: Anthony Liguori <anthony@squirrel.(none)>
Date:   Tue Oct 7 17:21:38 2008 -0500

    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 <aliguori@us.ibm.com>

diff --git a/compatfd.c b/compatfd.c
deleted file mode 100644
index cc5ced3..0000000
--- a/compatfd.c
+++ /dev/null
@@ -1,127 +0,0 @@
-/*
- * signalfd/eventfd compatibility
- *
- * Copyright IBM, Corp. 2008
- *
- * Authors:
- *  Anthony Liguori   <aliguori@us.ibm.com>
- *
- * 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);
-}
diff --git a/compatfd.h b/compatfd.h
deleted file mode 100644
index 55a111a..0000000
--- a/compatfd.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * signalfd/eventfd compatibility
- *
- * Copyright IBM, Corp. 2008
- *
- * Authors:
- *  Anthony Liguori   <aliguori@us.ibm.com>
- *
- * 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
diff --git a/Makefile b/Makefile
index 35061a4..36b36cd 100644
--- a/Makefile
+++ b/Makefile
@@ -59,10 +59,6 @@ else
 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
diff --git a/Makefile.target b/Makefile.target
index c2016d1..f6ec8ef 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -481,10 +481,6 @@ else
 OBJS+=block-raw-posix.o
 endif
 
-ifdef CONFIG_AIO
-OBJS+=compatfd.o
-endif
-
 LIBS+=-lz
 ifdef CONFIG_ALSA
 LIBS += -lasound
diff --git a/block-raw-posix.c b/block-raw-posix.c
index c0404fa..83a358c 100644
--- a/block-raw-posix.c
+++ b/block-raw-posix.c
@@ -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 RawAIOCB {
 
 typedef struct PosixAioState
 {
-    int fd;
+    int rfd, wfd;
     RawAIOCB *first_aio;
 } PosixAioState;
 
@@ -494,30 +493,17 @@ static void posix_aio_read(void *opaque)
     PosixAioState *s = opaque;
     RawAIOCB *acb, **pacb;
     int ret;
-    size_t offset;
-    union {
-        struct qemu_signalfd_siginfo siginfo;
-        char buf[128];
-    } sig;
-
-    /* try to read from signalfd, don't freak out if we can't read anything */
-    offset = 0;
-    while (offset < 128) {
-        ssize_t len;
-
-        len = read(s->fd, sig.buf + offset, 128 - offset);
+    ssize_t len;
+
+    do {
+        char byte;
+
+        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;
-        }
-
-        offset += len;
-    }
+        if (len == -1 && errno == EAGAIN)
+            break;
+    } while (len == -1);
 
     for(;;) {
         pacb = &s->first_aio;
@@ -565,10 +551,22 @@ static int posix_aio_flush(void *opaque)
 
 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,21 +575,23 @@ static int posix_aio_init(void)
     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];
+
+    fcntl(s->wfd, F_SETFL, O_NONBLOCK);
 
-    qemu_aio_set_fd_handler(s->fd, posix_aio_read, NULL, posix_aio_flush, s);
+    qemu_aio_set_fd_handler(s->rfd, posix_aio_read, NULL, posix_aio_flush, s);
 
 #if defined(__linux__)
     {
diff --git a/configure b/configure
index a224e6a..2a0d180 100755
--- a/configure
+++ b/configure
@@ -112,8 +112,6 @@ aio="yes"
 nptl="yes"
 mixemu="no"
 bluez="yes"
-signalfd="no"
-eventfd="no"
 
 # OS specific
 targetos=`uname -s`
@@ -919,33 +917,6 @@ EOF
   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
@@ -1280,12 +1251,6 @@ if test "$aio" = "yes" ; then
   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
diff --git a/qemu-common.h b/qemu-common.h
index 0731223..cc95fe6 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -139,4 +139,7 @@ struct pcmcia_card_s;
 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
diff --git a/qemu-tool.c b/qemu-tool.c
index 63e2056..87cc294 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -26,6 +26,10 @@ struct QEMUBH
     void *opaque;
 };
 
+void qemu_service_io(void)
+{
+}
+
 void term_printf(const char *fmt, ...)
 {
 }
diff --git a/vl.c b/vl.c
index 439d317..5e72199 100644
--- a/vl.c
+++ b/vl.c
@@ -7482,6 +7482,19 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
     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) */