public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Use signalfd() in io-thread
@ 2008-05-04 20:20 Anthony Liguori
  2008-05-04 20:20 ` [PATCH 2/3] Interrupt io thread in qemu_set_fd_handler2 Anthony Liguori
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Anthony Liguori @ 2008-05-04 20:20 UTC (permalink / raw)
  To: kvm-devel; +Cc: Anthony Liguori, Marcelo Tosatti, Avi Kivity

This patch reworks the IO thread to use signalfd() instead of sigtimedwait().
This will eliminate the need to use SIGIO everywhere.  In this version of the
patch, we use signalfd() when it's available.  When it isn't available, we
create a separate thread and use sigwaitinfo() to simulate signalfd().

We cannot handle thread-specific signals with signalfd() emulation so also
replace SIGUSR1 notifications to the io-thread with an eventfd.  Since eventfd
isn't always available, use pipe() to emulate eventfd.

I've tested Windows and Linux guests with SMP without seeing an obvious
regressions.

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

diff --git a/qemu/Makefile.target b/qemu/Makefile.target
index 2316c92..db6912e 100644
--- a/qemu/Makefile.target
+++ b/qemu/Makefile.target
@@ -203,7 +203,7 @@ CPPFLAGS+=-I$(SRC_PATH)/tcg/sparc
 endif
 
 ifeq ($(USE_KVM), 1)
-LIBOBJS+=qemu-kvm.o
+LIBOBJS+=qemu-kvm.o kvm-compatfd.o
 endif
 ifdef CONFIG_SOFTFLOAT
 LIBOBJS+=fpu/softfloat.o
diff --git a/qemu/kvm-compatfd.c b/qemu/kvm-compatfd.c
new file mode 100644
index 0000000..3c2be28
--- /dev/null
+++ b/qemu/kvm-compatfd.c
@@ -0,0 +1,127 @@
+/*
+ * 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 "qemu-kvm.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;
+
+    sigprocmask(SIG_BLOCK, &info->mask, NULL);
+
+    do {
+	siginfo_t siginfo;
+
+	kvm_sleep_begin();
+	err = sigwaitinfo(&info->mask, &siginfo);
+	kvm_sleep_end();
+
+	if (err == -1 && errno == EINTR)
+	    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 kvm_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 kvm_signalfd(const sigset_t *mask)
+{
+#if defined(SYS_signalfd)
+    int ret;
+
+    ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
+    if (!(ret == -1 && errno == ENOSYS))
+	return ret;
+#endif
+
+    return kvm_signalfd_compat(mask);
+}
+
+int kvm_eventfd(int *fds)
+{
+#if defined(SYS_eventfd)
+    int ret;
+
+    ret = syscall(SYS_eventfd, 0);
+    if (ret >= 0) {
+	fds[0] = fds[1] = ret;
+	return 0;
+    } else if (!(ret == -1 && errno == ENOSYS))
+	return ret;
+#endif
+
+    return pipe(fds);
+}
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 9a9bf59..e16b261 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -12,6 +12,9 @@ int kvm_allowed = 1;
 int kvm_irqchip = 1;
 int kvm_pit = 1;
 
+#include "qemu-common.h"
+#include "console.h"
+
 #include <string.h>
 #include "hw/hw.h"
 #include "sysemu.h"
@@ -38,14 +41,6 @@ __thread struct vcpu_info *vcpu;
 
 static int qemu_system_ready;
 
-struct qemu_kvm_signal_table {
-    sigset_t sigset;
-    sigset_t negsigset;
-};
-
-static struct qemu_kvm_signal_table io_signal_table;
-static struct qemu_kvm_signal_table vcpu_signal_table;
-
 #define SIG_IPI (SIGRTMIN+4)
 
 struct vcpu_info {
@@ -61,6 +56,7 @@ struct vcpu_info {
 } vcpu_info[256];
 
 pthread_t io_thread;
+static int io_thread_fd = -1;
 
 static inline unsigned long kvm_get_thread_id(void)
 {
@@ -169,37 +165,23 @@ static int has_work(CPUState *env)
     return kvm_arch_has_work(env);
 }
 
-static int kvm_process_signal(int si_signo)
-{
-    struct sigaction sa;
-
-    switch (si_signo) {
-    case SIGUSR2:
-        pthread_cond_signal(&qemu_aio_cond);
-        break;
-    case SIGALRM:
-    case SIGIO:
-        sigaction(si_signo, NULL, &sa);
-        sa.sa_handler(si_signo);
-        break;
-    }
-
-    return 1;
-}
-
-static int kvm_eat_signal(struct qemu_kvm_signal_table *waitset, CPUState *env,
-                          int timeout)
+static int kvm_eat_signal(CPUState *env, int timeout)
 {
     struct timespec ts;
     int r, e, ret = 0;
     siginfo_t siginfo;
+    sigset_t waitset;
 
     ts.tv_sec = timeout / 1000;
     ts.tv_nsec = (timeout % 1000) * 1000000;
-    r = sigtimedwait(&waitset->sigset, &siginfo, &ts);
+    sigemptyset(&waitset);
+    sigaddset(&waitset, SIG_IPI);
+
+    r = sigtimedwait(&waitset, &siginfo, &ts);
     if (r == -1 && (errno == EAGAIN || errno == EINTR) && !timeout)
 	return 0;
     e = errno;
+
     pthread_mutex_lock(&qemu_mutex);
     if (env && vcpu)
         cpu_single_env = vcpu->env;
@@ -208,12 +190,12 @@ static int kvm_eat_signal(struct qemu_kvm_signal_table *waitset, CPUState *env,
 	exit(1);
     }
     if (r != -1)
-        ret = kvm_process_signal(siginfo.si_signo);
+	ret = 1;
 
     if (env && vcpu_info[env->cpu_index].stop) {
 	vcpu_info[env->cpu_index].stop = 0;
 	vcpu_info[env->cpu_index].stopped = 1;
-	pthread_kill(io_thread, SIGUSR1);
+	qemu_kvm_notify_work();
     }
     pthread_mutex_unlock(&qemu_mutex);
 
@@ -224,14 +206,13 @@ static int kvm_eat_signal(struct qemu_kvm_signal_table *waitset, CPUState *env,
 static void kvm_eat_signals(CPUState *env, int timeout)
 {
     int r = 0;
-    struct qemu_kvm_signal_table *waitset = &vcpu_signal_table;
 
-    while (kvm_eat_signal(waitset, env, 0))
+    while (kvm_eat_signal(env, 0))
 	r = 1;
     if (!r && timeout) {
-	r = kvm_eat_signal(waitset, env, timeout);
+	r = kvm_eat_signal(env, timeout);
 	if (r)
-	    while (kvm_eat_signal(waitset, env, 0))
+	    while (kvm_eat_signal(env, 0))
 		;
     }
 }
@@ -264,9 +245,7 @@ static void pause_all_threads(void)
 	pthread_kill(vcpu_info[i].thread, SIG_IPI);
     }
     while (!all_threads_paused()) {
-	pthread_mutex_unlock(&qemu_mutex);
-	kvm_eat_signal(&io_signal_table, NULL, 1000);
-	pthread_mutex_lock(&qemu_mutex);
+	main_loop_wait(1000);
 	cpu_single_env = NULL;
     }
 }
@@ -307,6 +286,12 @@ static void setup_kernel_sigmask(CPUState *env)
 {
     sigset_t set;
 
+    sigemptyset(&set);
+    sigaddset(&set, SIGUSR2);
+    sigaddset(&set, SIGIO);
+    sigaddset(&set, SIGALRM);
+    sigprocmask(SIG_BLOCK, &set, NULL);
+
     sigprocmask(SIG_BLOCK, NULL, &set);
     sigdelset(&set, SIG_IPI);
     
@@ -343,7 +328,7 @@ static int kvm_main_loop_cpu(CPUState *env)
     cpu_single_env = env;
     while (1) {
 	while (!has_work(env))
-	    kvm_main_loop_wait(env, 10);
+	    kvm_main_loop_wait(env, 1000);
 	if (env->interrupt_request & CPU_INTERRUPT_HARD)
 	    env->hflags &= ~HF_HALTED_MASK;
 	if (!kvm_irqchip_in_kernel(kvm_context) && info->sipi_needed)
@@ -391,18 +376,6 @@ static void *ap_main_loop(void *_env)
     return NULL;
 }
 
-static void qemu_kvm_init_signal_table(struct qemu_kvm_signal_table *sigtab)
-{
-    sigemptyset(&sigtab->sigset);
-    sigfillset(&sigtab->negsigset);
-}
-
-static void kvm_add_signal(struct qemu_kvm_signal_table *sigtab, int signum)
-{
-    sigaddset(&sigtab->sigset, signum);
-    sigdelset(&sigtab->negsigset, signum);
-}
-
 void kvm_init_new_ap(int cpu, CPUState *env)
 {
     pthread_create(&vcpu_info[cpu].thread, NULL, ap_main_loop, env);
@@ -411,28 +384,12 @@ void kvm_init_new_ap(int cpu, CPUState *env)
 	pthread_cond_wait(&qemu_vcpu_cond, &qemu_mutex);
 }
 
-static void qemu_kvm_init_signal_tables(void)
-{
-    qemu_kvm_init_signal_table(&io_signal_table);
-    qemu_kvm_init_signal_table(&vcpu_signal_table);
-
-    kvm_add_signal(&io_signal_table, SIGIO);
-    kvm_add_signal(&io_signal_table, SIGALRM);
-    kvm_add_signal(&io_signal_table, SIGUSR1);
-    kvm_add_signal(&io_signal_table, SIGUSR2);
-
-    kvm_add_signal(&vcpu_signal_table, SIG_IPI);
-
-    sigprocmask(SIG_BLOCK, &io_signal_table.sigset, NULL);
-}
-
 int kvm_init_ap(void)
 {
 #ifdef TARGET_I386
     kvm_tpr_opt_setup();
 #endif
     qemu_add_vm_change_state_handler(kvm_vm_state_change_handler, NULL);
-    qemu_kvm_init_signal_tables();
 
     signal(SIG_IPI, sig_ipi_handler);
     return 0;
@@ -440,29 +397,152 @@ int kvm_init_ap(void)
 
 void qemu_kvm_notify_work(void)
 {
-    if (io_thread)
-        pthread_kill(io_thread, SIGUSR1);
+    uint64_t value = 1;
+    char buffer[8];
+    size_t offset = 0;
+
+    if (io_thread_fd == -1)
+	return;
+
+    memcpy(buffer, &value, sizeof(value));
+
+    while (offset < 8) {
+	ssize_t len;
+
+	len = write(io_thread_fd, buffer + offset, 8 - offset);
+	if (len == -1 && errno == EINTR)
+	    continue;
+
+	if (len <= 0)
+	    break;
+
+	offset += len;
+    }
+
+    if (offset != 8)
+	fprintf(stderr, "failed to notify io thread\n");
+}
+
+static int received_signal;
+
+/* QEMU relies on periodically breaking out of select via EINTR to poll for IO
+   and timer signals.  Since we're now using a file descriptor to handle
+   signals, select() won't be interrupted by a signal.  We need to forcefully
+   break the select() loop when a signal is received hence
+   kvm_check_received_signal(). */
+
+int kvm_check_received_signal(void)
+{
+    if (received_signal) {
+	received_signal = 0;
+	return 1;
+    }
+
+    return 0;
 }
 
-/*
- * The IO thread has all signals that inform machine events
- * blocked (io_signal_table), so it won't get interrupted
- * while processing in main_loop_wait().
+/* If we have signalfd, we mask out the signals we want to handle and then
+ * use signalfd to listen for them.  We rely on whatever the current signal
+ * handler is to dispatch the signals when we receive them.
  */
 
+static void sigfd_handler(void *opaque)
+{
+    int fd = (unsigned long)opaque;
+    struct signalfd_siginfo info;
+    struct sigaction action;
+    ssize_t len;
+
+    while (1) {
+	do {
+	    len = read(fd, &info, sizeof(info));
+	} while (len == -1 && errno == EINTR);
+
+	if (len == -1 && errno == EAGAIN)
+	    break;
+
+	if (len != sizeof(info)) {
+	    printf("read from sigfd returned %ld: %m\n", len);
+	    return;
+	}
+
+	sigaction(info.ssi_signo, NULL, &action);
+	if (action.sa_handler)
+	    action.sa_handler(info.ssi_signo);
+
+	if (info.ssi_signo == SIGUSR2) {
+	    pthread_cond_signal(&qemu_aio_cond); 
+	}
+    }
+
+    received_signal = 1;
+}
+
+/* Used to break IO thread out of select */
+static void io_thread_wakeup(void *opaque)
+{
+    int fd = (unsigned long)opaque;
+    char buffer[8];
+    size_t offset = 0;
+
+    while (offset < 8) {
+	ssize_t len;
+
+	len = read(fd, buffer + offset, 8 - offset);
+	if (len == -1 && errno == EINTR)
+	    continue;
+
+	if (len <= 0)
+	    break;
+
+	offset += len;
+    }
+
+    received_signal = 1;
+}
+
 int kvm_main_loop(void)
 {
+    int fds[2];
+    sigset_t mask;
+    int sigfd;
+
     io_thread = pthread_self();
     qemu_system_ready = 1;
-    pthread_mutex_unlock(&qemu_mutex);
+
+    if (kvm_eventfd(fds) == -1) {
+	fprintf(stderr, "failed to create eventfd\n");
+	return -errno;
+    }
+
+    qemu_set_fd_handler2(fds[0], NULL, io_thread_wakeup, NULL,
+			 (void *)(unsigned long)fds[0]);
+
+    io_thread_fd = fds[1];
+
+    sigemptyset(&mask);
+    sigaddset(&mask, SIGIO);
+    sigaddset(&mask, SIGALRM);
+    sigaddset(&mask, SIGUSR2);
+    sigprocmask(SIG_BLOCK, &mask, NULL);
+
+    sigfd = kvm_signalfd(&mask);
+    if (sigfd == -1) {
+	fprintf(stderr, "failed to create signalfd\n");
+	return -errno;
+    }
+
+    fcntl(sigfd, F_SETFL, O_NONBLOCK);
+
+    qemu_set_fd_handler2(sigfd, NULL, sigfd_handler, NULL,
+			 (void *)(unsigned long)sigfd);
 
     pthread_cond_broadcast(&qemu_system_cond);
 
+    cpu_single_env = NULL;
+
     while (1) {
-        kvm_eat_signal(&io_signal_table, NULL, 1000);
-        pthread_mutex_lock(&qemu_mutex);
-        cpu_single_env = NULL;
-        main_loop_wait(0);
+        main_loop_wait(1000);
         if (qemu_shutdown_requested())
             break;
         else if (qemu_powerdown_requested())
@@ -471,7 +551,6 @@ int kvm_main_loop(void)
             pthread_kill(vcpu_info[0].thread, SIG_IPI);
             qemu_kvm_reset_requested = 1;
         }
-        pthread_mutex_unlock(&qemu_mutex);
     }
 
     pause_all_threads();
@@ -834,10 +913,7 @@ void qemu_kvm_aio_wait(void)
     CPUState *cpu_single = cpu_single_env;
 
     if (!cpu_single_env) {
-        pthread_mutex_unlock(&qemu_mutex);
-        kvm_eat_signal(&io_signal_table, NULL, 1000);
-        pthread_mutex_lock(&qemu_mutex);
-        cpu_single_env = NULL;
+	main_loop_wait(1000);
     } else {
         pthread_cond_wait(&qemu_aio_cond, &qemu_mutex);
         cpu_single_env = cpu_single;
@@ -864,3 +940,14 @@ void kvm_cpu_destroy_phys_mem(target_phys_addr_t start_addr,
 {
     kvm_destroy_phys_mem(kvm_context, start_addr, size);
 }
+
+void kvm_mutex_unlock(void)
+{
+    pthread_mutex_unlock(&qemu_mutex);
+}
+
+void kvm_mutex_lock(void)
+{
+    pthread_mutex_lock(&qemu_mutex);
+    cpu_single_env = NULL;
+}
diff --git a/qemu/qemu-kvm.h b/qemu/qemu-kvm.h
index 024a653..e1e461a 100644
--- a/qemu/qemu-kvm.h
+++ b/qemu/qemu-kvm.h
@@ -10,6 +10,8 @@
 
 #include "cpu.h"
 
+#include <signal.h>
+
 int kvm_main_loop(void);
 int kvm_qemu_init(void);
 int kvm_qemu_create_context(void);
@@ -97,4 +99,40 @@ extern kvm_context_t kvm_context;
 #define qemu_kvm_pit_in_kernel() (0)
 #endif
 
+void kvm_mutex_unlock(void);
+void kvm_mutex_lock(void);
+
+static inline void kvm_sleep_begin(void)
+{
+    if (kvm_enabled())
+	kvm_mutex_unlock();
+}
+
+static inline void kvm_sleep_end(void)
+{
+    if (kvm_enabled())
+	kvm_mutex_lock();
+}
+
+int kvm_check_received_signal(void);
+
+static inline int kvm_received_signal(void)
+{
+    if (kvm_enabled())
+	return kvm_check_received_signal();
+    return 0;
+}
+
+#if !defined(SYS_signalfd)
+struct signalfd_siginfo {
+    uint32_t ssi_signo;
+    uint8_t pad[124];
+};
+#else
+#include <linux/signalfd.h>
+#endif
+
+int kvm_signalfd(const sigset_t *mask);
+int kvm_eventfd(int *fds);
+
 #endif
diff --git a/qemu/vl.c b/qemu/vl.c
index 74be059..1192759 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -7836,6 +7836,23 @@ void qemu_system_powerdown_request(void)
         cpu_interrupt(cpu_single_env, CPU_INTERRUPT_EXIT);
 }
 
+static int qemu_select(int max_fd, fd_set *rfds, fd_set *wfds, fd_set *xfds,
+		       struct timeval *tv)
+{
+    int ret;
+
+    /* KVM holds a mutex while QEMU code is running, we need hooks to
+       release the mutex whenever QEMU code sleeps. */
+
+    kvm_sleep_begin();
+
+    ret = select(max_fd, rfds, wfds, xfds, tv);
+
+    kvm_sleep_end();
+
+    return ret;
+}
+
 void main_loop_wait(int timeout)
 {
     IOHandlerRecord *ioh;
@@ -7907,11 +7924,12 @@ void main_loop_wait(int timeout)
         }
     }
 
-    tv.tv_sec = 0;
 #ifdef _WIN32
+    tv.tv_sec = 0;
     tv.tv_usec = 0;
 #else
-    tv.tv_usec = timeout * 1000;
+    tv.tv_sec = timeout / 1000;
+    tv.tv_usec = (timeout % 1000) * 1000;
 #endif
 #if defined(CONFIG_SLIRP)
     if (slirp_inited) {
@@ -7919,7 +7937,7 @@ void main_loop_wait(int timeout)
     }
 #endif
  moreio:
-    ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
+    ret = qemu_select(nfds + 1, &rfds, &wfds, &xfds, &tv);
     if (ret > 0) {
         IOHandlerRecord **pioh;
         int more = 0;
@@ -7948,7 +7966,7 @@ void main_loop_wait(int timeout)
             } else
                 pioh = &ioh->next;
         }
-        if (more)
+        if (more && !kvm_received_signal())
             goto moreio;
     }
 #if defined(CONFIG_SLIRP)

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] Interrupt io thread in qemu_set_fd_handler2
  2008-05-04 20:20 [PATCH 1/3] Use signalfd() in io-thread Anthony Liguori
@ 2008-05-04 20:20 ` Anthony Liguori
  2008-05-04 20:20 ` [PATCH 3/3] Only select once per-main_loop iteration Anthony Liguori
  2008-05-05  7:23 ` [PATCH 1/3] Use signalfd() in io-thread Avi Kivity
  2 siblings, 0 replies; 5+ messages in thread
From: Anthony Liguori @ 2008-05-04 20:20 UTC (permalink / raw)
  To: kvm-devel; +Cc: Anthony Liguori, Marcelo Tosatti, Avi Kivity

The select() in the IO thread may wait a long time before rebuilding the
fd set.  Whenever we do something that changes the fd set, we should interrupt
the IO thread.

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

diff --git a/qemu/vl.c b/qemu/vl.c
index 1192759..e9f0ca4 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -260,6 +260,16 @@ static int event_pending = 1;
 
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
 
+/* KVM runs the main loop in a separate thread.  If we update one of the lists
+ * that are polled before or after select(), we need to make sure to break out
+ * of the select() to ensure the new item is serviced.
+ */
+static void main_loop_break(void)
+{
+    if (kvm_enabled())
+	qemu_kvm_notify_work();
+}
+
 void decorate_application_name(char *appname, int max_len)
 {
     if (kvm_enabled())
@@ -5680,6 +5690,7 @@ int qemu_set_fd_handler2(int fd,
         ioh->opaque = opaque;
         ioh->deleted = 0;
     }
+    main_loop_break();
     return 0;
 }
 
@@ -7606,8 +7617,7 @@ void qemu_bh_schedule(QEMUBH *bh)
     if (env) {
         cpu_interrupt(env, CPU_INTERRUPT_EXIT);
     }
-    if (kvm_enabled())
-        qemu_kvm_notify_work();
+    main_loop_break();
 }
 
 void qemu_bh_cancel(QEMUBH *bh)

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] Only select once per-main_loop iteration
  2008-05-04 20:20 [PATCH 1/3] Use signalfd() in io-thread Anthony Liguori
  2008-05-04 20:20 ` [PATCH 2/3] Interrupt io thread in qemu_set_fd_handler2 Anthony Liguori
@ 2008-05-04 20:20 ` Anthony Liguori
  2008-05-05  7:23 ` [PATCH 1/3] Use signalfd() in io-thread Avi Kivity
  2 siblings, 0 replies; 5+ messages in thread
From: Anthony Liguori @ 2008-05-04 20:20 UTC (permalink / raw)
  To: kvm-devel; +Cc: Anthony Liguori, Marcelo Tosatti, Avi Kivity

QEMU is rather aggressive about exhausting the wait period when selecting.
This is fine when the wait period is low and when there is significant delays
in-between selects as it improves IO throughput.

With the IO thread, there is a very small delay between selects and our wait
period for select is very large.  This patch changes main_loop_wait to only
select once before doing the various other things in the main loop.  This
generally improves responsiveness of things like SDL but also improves
individual file descriptor throughput quite dramatically.

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

diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index e16b261..6a90e68 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -423,24 +423,6 @@ void qemu_kvm_notify_work(void)
 	fprintf(stderr, "failed to notify io thread\n");
 }
 
-static int received_signal;
-
-/* QEMU relies on periodically breaking out of select via EINTR to poll for IO
-   and timer signals.  Since we're now using a file descriptor to handle
-   signals, select() won't be interrupted by a signal.  We need to forcefully
-   break the select() loop when a signal is received hence
-   kvm_check_received_signal(). */
-
-int kvm_check_received_signal(void)
-{
-    if (received_signal) {
-	received_signal = 0;
-	return 1;
-    }
-
-    return 0;
-}
-
 /* If we have signalfd, we mask out the signals we want to handle and then
  * use signalfd to listen for them.  We rely on whatever the current signal
  * handler is to dispatch the signals when we receive them.
@@ -474,8 +456,6 @@ static void sigfd_handler(void *opaque)
 	    pthread_cond_signal(&qemu_aio_cond); 
 	}
     }
-
-    received_signal = 1;
 }
 
 /* Used to break IO thread out of select */
@@ -497,8 +477,6 @@ static void io_thread_wakeup(void *opaque)
 
 	offset += len;
     }
-
-    received_signal = 1;
 }
 
 int kvm_main_loop(void)
diff --git a/qemu/qemu-kvm.h b/qemu/qemu-kvm.h
index e1e461a..34aabd2 100644
--- a/qemu/qemu-kvm.h
+++ b/qemu/qemu-kvm.h
@@ -114,15 +114,6 @@ static inline void kvm_sleep_end(void)
 	kvm_mutex_lock();
 }
 
-int kvm_check_received_signal(void);
-
-static inline int kvm_received_signal(void)
-{
-    if (kvm_enabled())
-	return kvm_check_received_signal();
-    return 0;
-}
-
 #if !defined(SYS_signalfd)
 struct signalfd_siginfo {
     uint32_t ssi_signo;
diff --git a/qemu/vl.c b/qemu/vl.c
index e9f0ca4..6935a82 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -7946,23 +7946,18 @@ void main_loop_wait(int timeout)
         slirp_select_fill(&nfds, &rfds, &wfds, &xfds);
     }
 #endif
- moreio:
     ret = qemu_select(nfds + 1, &rfds, &wfds, &xfds, &tv);
     if (ret > 0) {
         IOHandlerRecord **pioh;
-        int more = 0;
 
         for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) {
             if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
                 ioh->fd_read(ioh->opaque);
-                if (!ioh->fd_read_poll || ioh->fd_read_poll(ioh->opaque))
-                    more = 1;
-                else
+                if (!(ioh->fd_read_poll && ioh->fd_read_poll(ioh->opaque)))
                     FD_CLR(ioh->fd, &rfds);
             }
             if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
                 ioh->fd_write(ioh->opaque);
-                more = 1;
             }
         }
 
@@ -7976,8 +7971,6 @@ void main_loop_wait(int timeout)
             } else
                 pioh = &ioh->next;
         }
-        if (more && !kvm_received_signal())
-            goto moreio;
     }
 #if defined(CONFIG_SLIRP)
     if (slirp_inited) {

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] Use signalfd() in io-thread
  2008-05-04 20:20 [PATCH 1/3] Use signalfd() in io-thread Anthony Liguori
  2008-05-04 20:20 ` [PATCH 2/3] Interrupt io thread in qemu_set_fd_handler2 Anthony Liguori
  2008-05-04 20:20 ` [PATCH 3/3] Only select once per-main_loop iteration Anthony Liguori
@ 2008-05-05  7:23 ` Avi Kivity
       [not found]   ` <481F121A.9020207@us.ibm.com>
  2 siblings, 1 reply; 5+ messages in thread
From: Avi Kivity @ 2008-05-05  7:23 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: kvm-devel, Marcelo Tosatti

Anthony Liguori wrote:
> This patch reworks the IO thread to use signalfd() instead of sigtimedwait().
> This will eliminate the need to use SIGIO everywhere.  In this version of the
> patch, we use signalfd() when it's available.  When it isn't available, we
> create a separate thread and use sigwaitinfo() to simulate signalfd().
>
> We cannot handle thread-specific signals with signalfd() emulation so also
> replace SIGUSR1 notifications to the io-thread with an eventfd.  Since eventfd
> isn't always available, use pipe() to emulate eventfd.
>   

Please break the SIGUSR1 changes into a separate patch.  Ditto with *fd 
syscall compat.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] Use signalfd() in io-thread
       [not found]   ` <481F121A.9020207@us.ibm.com>
@ 2008-05-05 14:27     ` Avi Kivity
  0 siblings, 0 replies; 5+ messages in thread
From: Avi Kivity @ 2008-05-05 14:27 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: kvm-devel, Marcelo Tosatti

Anthony Liguori wrote:
>>
>> Please break the SIGUSR1 changes into a separate patch.  Ditto with 
>> *fd syscall compat.
>
> Done.  I didn't make the syscall compat stuff separate patches because 
> that would break bisect on older hosts.  However, I did split it up 
> logically between the remove sigusr1 patch and the signalfd patch.
>

Not if you placed the compat patch before the use (e.g. a patch which 
adds kvm_signalfd() but no uses).

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-05-05 14:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-04 20:20 [PATCH 1/3] Use signalfd() in io-thread Anthony Liguori
2008-05-04 20:20 ` [PATCH 2/3] Interrupt io thread in qemu_set_fd_handler2 Anthony Liguori
2008-05-04 20:20 ` [PATCH 3/3] Only select once per-main_loop iteration Anthony Liguori
2008-05-05  7:23 ` [PATCH 1/3] Use signalfd() in io-thread Avi Kivity
     [not found]   ` <481F121A.9020207@us.ibm.com>
2008-05-05 14:27     ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox