From b778fb62efcd19cda580ad98365c3f89afd48a15 Mon Sep 17 00:00:00 2001 From: aliguori Date: Wed, 10 Sep 2008 15:45:19 +0000 Subject: [PATCH] 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 git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@5187 c046a42c-6fe2-441c-8c8c-71466251a162 --- Makefile | 4 +- Makefile.target | 2 +- block-raw-posix.c | 129 ++++++++++++++++++++---------------------------------- block-raw-win32.c | 12 ----- block.c | 14 ++---- block.h | 3 -- vl.c | 1 - 7 files changed, 55 insertions(+), 110 deletions(-) diff --git a/Makefile b/Makefile index 9c219c3..e676900 100644 --- a/Makefile +++ b/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 diff --git a/Makefile.target b/Makefile.target index 2e8e0a0..dd511ef 100644 --- a/Makefile.target +++ b/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 diff --git a/block-raw-posix.c b/block-raw-posix.c index 96edab4..f6dc72a 100644 --- a/block-raw-posix.c +++ b/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 #ifdef CONFIG_AIO #include @@ -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) -{ -#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) +static void qemu_aio_poll(void *opaque) { 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(); + 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_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) diff --git a/block-raw-win32.c b/block-raw-win32.c index 43d3f6c..7b88dcf 100644 --- a/block-raw-win32.c +++ b/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), diff --git a/block.c b/block.c index db8244c..a6fd0b1 100644 --- a/block.c +++ b/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) { - 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 @@ 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) { - qemu_aio_wait_end(); + if (acb == NULL) return -1; - } while (async_ret == NOT_DONE) { qemu_aio_wait(); } - qemu_aio_wait_end(); return async_ret; } diff --git a/block.h b/block.h index fa741b5..0443585 100644 --- a/block.h +++ b/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); diff --git a/vl.c b/vl.c index c20091a..18bcc1f 100644 --- a/vl.c +++ b/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))) -- 2.7.4