| Message ID | 1314019498-8550-1-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 2011-08-22 15:24, Anthony Liguori wrote: > Enabling the I/O thread by default seems like an important part of declaring > 1.0. Besides allowing true SMP support with KVM, the I/O thread means that the > TCG VCPU doesn't have to multiplex itself with the I/O dispatch routines which > currently requires a (racey) signal based alarm system. > > I know there have been concerns about performance. I think so far the ones that > have come up (virtio-net) are most likely due to secondary reasons like > decreased batching. iothread enabled without [1] gives unacceptable performance for ARM emulation (Musicpal board) here. With that series applied, the host CPU load is measurably higher, but no longer excessively. As that series demonstrates, a major issue for TCG is that the iothread and the VCPU thread run in lock-step, almost nothing is parallelized. At least so far. Paolo and Peter (and to some degree /me) discussed some better TCG treading last Thursday. Jan [1] http://thread.gmane.org/gmane.comp.emulators.qemu/99658 > > I think we ought to force enabling I/O thread early in 1.0 development and > commit to resolving any lingering issues. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > configure | 13 ----- > cpus.c | 143 ---------------------------------------------------------- > hw/qxl.c | 4 -- > kvm-all.c | 2 +- > qemu-timer.c | 53 --------------------- > vl.c | 17 ------- > 6 files changed, 1 insertions(+), 231 deletions(-) > > diff --git a/configure b/configure > index 234a4c5..ef8d7ae 100755 > --- a/configure > +++ b/configure > @@ -165,7 +165,6 @@ darwin_user="no" > bsd_user="no" > guest_base="" > uname_release="" > -io_thread="no" > mixemu="no" > aix="no" > blobs="yes" > @@ -722,8 +721,6 @@ for opt do > ;; > --enable-attr) attr="yes" > ;; > - --enable-io-thread) io_thread="yes" > - ;; > --disable-blobs) blobs="no" > ;; > --with-pkgversion=*) pkgversion=" ($optarg)" > @@ -2159,12 +2156,6 @@ EOF > > if compile_prog "" "" ; then > signalfd=yes > -elif test "$kvm" = "yes" -a "$io_thread" != "yes"; then > - echo > - echo "ERROR: Host kernel lacks signalfd() support," > - echo "but KVM depends on it when the IO thread is disabled." > - echo > - exit 1 > fi > > # check if eventfd is supported > @@ -2711,7 +2702,6 @@ echo "NPTL support $nptl" > echo "GUEST_BASE $guest_base" > echo "PIE user targets $user_pie" > echo "vde support $vde" > -echo "IO thread $io_thread" > echo "Linux AIO support $linux_aio" > echo "ATTR/XATTR support $attr" > echo "Install blobs $blobs" > @@ -2969,9 +2959,6 @@ if test "$xen" = "yes" ; then > echo "CONFIG_XEN_BACKEND=y" >> $config_host_mak > echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version" >> $config_host_mak > fi > -if test "$io_thread" = "yes" ; then > - echo "CONFIG_IOTHREAD=y" >> $config_host_mak > -fi > if test "$linux_aio" = "yes" ; then > echo "CONFIG_LINUX_AIO=y" >> $config_host_mak > fi > diff --git a/cpus.c b/cpus.c > index c996ac5..fe18a60 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -173,12 +173,9 @@ static void cpu_handle_guest_debug(CPUState *env) > { > gdb_set_stop_cpu(env); > qemu_system_debug_request(); > -#ifdef CONFIG_IOTHREAD > env->stopped = 1; > -#endif > } > > -#ifdef CONFIG_IOTHREAD > static void cpu_signal(int sig) > { > if (cpu_single_env) { > @@ -186,7 +183,6 @@ static void cpu_signal(int sig) > } > exit_request = 1; > } > -#endif > > #ifdef CONFIG_LINUX > static void sigbus_reraise(void) > @@ -262,12 +258,6 @@ static void qemu_kvm_eat_signals(CPUState *env) > exit(1); > } > } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS)); > - > -#ifndef CONFIG_IOTHREAD > - if (sigismember(&chkset, SIGIO) || sigismember(&chkset, SIGALRM)) { > - qemu_notify_event(); > - } > -#endif > } > > #else /* !CONFIG_LINUX */ > @@ -390,7 +380,6 @@ static int qemu_signal_init(void) > int sigfd; > sigset_t set; > > -#ifdef CONFIG_IOTHREAD > /* SIGUSR2 used by posix-aio-compat.c */ > sigemptyset(&set); > sigaddset(&set, SIGUSR2); > @@ -409,18 +398,6 @@ static int qemu_signal_init(void) > sigaddset(&set, SIGIO); > sigaddset(&set, SIGALRM); > sigaddset(&set, SIGBUS); > -#else > - sigemptyset(&set); > - sigaddset(&set, SIGBUS); > - if (kvm_enabled()) { > - /* > - * We need to process timer signals synchronously to avoid a race > - * between exit_request check and KVM vcpu entry. > - */ > - sigaddset(&set, SIGIO); > - sigaddset(&set, SIGALRM); > - } > -#endif > pthread_sigmask(SIG_BLOCK, &set, NULL); > > sigfd = qemu_signalfd(&set); > @@ -447,7 +424,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) > sigact.sa_handler = dummy_signal; > sigaction(SIG_IPI, &sigact, NULL); > > -#ifdef CONFIG_IOTHREAD > pthread_sigmask(SIG_BLOCK, NULL, &set); > sigdelset(&set, SIG_IPI); > sigdelset(&set, SIGBUS); > @@ -456,17 +432,7 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) > fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); > exit(1); > } > -#else > - sigemptyset(&set); > - sigaddset(&set, SIG_IPI); > - sigaddset(&set, SIGIO); > - sigaddset(&set, SIGALRM); > - pthread_sigmask(SIG_BLOCK, &set, NULL); > > - pthread_sigmask(SIG_BLOCK, NULL, &set); > - sigdelset(&set, SIGIO); > - sigdelset(&set, SIGALRM); > -#endif > sigdelset(&set, SIG_IPI); > sigdelset(&set, SIGBUS); > r = kvm_set_signal_mask(env, &set); > @@ -478,7 +444,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) > > static void qemu_tcg_init_cpu_signals(void) > { > -#ifdef CONFIG_IOTHREAD > sigset_t set; > struct sigaction sigact; > > @@ -489,7 +454,6 @@ static void qemu_tcg_init_cpu_signals(void) > sigemptyset(&set); > sigaddset(&set, SIG_IPI); > pthread_sigmask(SIG_UNBLOCK, &set, NULL); > -#endif > } > > #else /* _WIN32 */ > @@ -535,106 +499,6 @@ static void qemu_tcg_init_cpu_signals(void) > } > #endif /* _WIN32 */ > > -#ifndef CONFIG_IOTHREAD > -int qemu_init_main_loop(void) > -{ > - int ret; > - > - ret = qemu_signal_init(); > - if (ret) { > - return ret; > - } > - > - qemu_init_sigbus(); > - > - return qemu_event_init(); > -} > - > -void qemu_main_loop_start(void) > -{ > -} > - > -void qemu_init_vcpu(void *_env) > -{ > - CPUState *env = _env; > - int r; > - > - env->nr_cores = smp_cores; > - env->nr_threads = smp_threads; > - > - if (kvm_enabled()) { > - r = kvm_init_vcpu(env); > - if (r < 0) { > - fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r)); > - exit(1); > - } > - qemu_kvm_init_cpu_signals(env); > - } else { > - qemu_tcg_init_cpu_signals(); > - } > -} > - > -int qemu_cpu_is_self(void *env) > -{ > - return 1; > -} > - > -void run_on_cpu(CPUState *env, void (*func)(void *data), void *data) > -{ > - func(data); > -} > - > -void resume_all_vcpus(void) > -{ > -} > - > -void pause_all_vcpus(void) > -{ > -} > - > -void qemu_cpu_kick(void *env) > -{ > -} > - > -void qemu_cpu_kick_self(void) > -{ > -#ifndef _WIN32 > - assert(cpu_single_env); > - > - raise(SIG_IPI); > -#else > - abort(); > -#endif > -} > - > -void qemu_notify_event(void) > -{ > - CPUState *env = cpu_single_env; > - > - qemu_event_increment (); > - if (env) { > - cpu_exit(env); > - } > - if (next_cpu && env != next_cpu) { > - cpu_exit(next_cpu); > - } > - exit_request = 1; > -} > - > -void qemu_mutex_lock_iothread(void) {} > -void qemu_mutex_unlock_iothread(void) {} > - > -void cpu_stop_current(void) > -{ > -} > - > -void vm_stop(int reason) > -{ > - do_vm_stop(reason); > -} > - > -#else /* CONFIG_IOTHREAD */ > - > QemuMutex qemu_global_mutex; > static QemuCond qemu_io_proceeded_cond; > static bool iothread_requesting_mutex; > @@ -1036,8 +900,6 @@ void vm_stop(int reason) > do_vm_stop(reason); > } > > -#endif > - > static int tcg_cpu_exec(CPUState *env) > { > int ret; > @@ -1092,11 +954,6 @@ bool cpu_exec_all(void) > qemu_clock_enable(vm_clock, > (env->singlestep_enabled & SSTEP_NOTIMER) == 0); > > -#ifndef CONFIG_IOTHREAD > - if (qemu_alarm_pending()) { > - break; > - } > -#endif > if (cpu_can_run(env)) { > if (kvm_enabled()) { > r = kvm_cpu_exec(env); > diff --git a/hw/qxl.c b/hw/qxl.c > index bab60a5..c3a3e97 100644 > --- a/hw/qxl.c > +++ b/hw/qxl.c > @@ -1388,11 +1388,7 @@ static void init_pipe_signaling(PCIQXLDevice *d) > dprint(d, 1, "%s: pipe creation failed\n", __FUNCTION__); > return; > } > -#ifdef CONFIG_IOTHREAD > fcntl(d->pipe[0], F_SETFL, O_NONBLOCK); > -#else > - fcntl(d->pipe[0], F_SETFL, O_NONBLOCK /* | O_ASYNC */); > -#endif > fcntl(d->pipe[1], F_SETFL, O_NONBLOCK); > fcntl(d->pipe[0], F_SETOWN, getpid()); > > diff --git a/kvm-all.c b/kvm-all.c > index 0ae2e26..fbb9ff3 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -479,7 +479,7 @@ static int kvm_check_many_ioeventfds(void) > * Older kernels have a 6 device limit on the KVM io bus. Find out so we > * can avoid creating too many ioeventfds. > */ > -#if defined(CONFIG_EVENTFD) && defined(CONFIG_IOTHREAD) > +#if defined(CONFIG_EVENTFD) > int ioeventfds[7]; > int i, ret = 0; > for (i = 0; i < ARRAY_SIZE(ioeventfds); i++) { > diff --git a/qemu-timer.c b/qemu-timer.c > index 19313d3..46dd483 100644 > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -101,22 +101,6 @@ static int64_t cpu_get_clock(void) > } > } > > -#ifndef CONFIG_IOTHREAD > -static int64_t qemu_icount_delta(void) > -{ > - if (!use_icount) { > - return 5000 * (int64_t) 1000000; > - } else if (use_icount == 1) { > - /* When not using an adaptive execution frequency > - we tend to get badly out of sync with real time, > - so just delay for a reasonable amount of time. */ > - return 0; > - } else { > - return cpu_get_icount() - cpu_get_clock(); > - } > -} > -#endif > - > /* enable cpu_get_ticks() */ > void cpu_enable_ticks(void) > { > @@ -688,9 +672,7 @@ void configure_icount(const char *option) > if (!option) > return; > > -#ifdef CONFIG_IOTHREAD > vm_clock->warp_timer = qemu_new_timer_ns(rt_clock, icount_warp_rt, NULL); > -#endif > > if (strcmp(option, "auto") != 0) { > icount_time_shift = strtol(option, NULL, 0); > @@ -1178,41 +1160,6 @@ void quit_timers(void) > > int qemu_calculate_timeout(void) > { > -#ifndef CONFIG_IOTHREAD > - int timeout; > - > - if (!vm_running) > - timeout = 5000; > - else { > - /* XXX: use timeout computed from timers */ > - int64_t add; > - int64_t delta; > - /* Advance virtual time to the next event. */ > - delta = qemu_icount_delta(); > - if (delta > 0) { > - /* If virtual time is ahead of real time then just > - wait for IO. */ > - timeout = (delta + 999999) / 1000000; > - } else { > - /* Wait for either IO to occur or the next > - timer event. */ > - add = qemu_next_icount_deadline(); > - /* We advance the timer before checking for IO. > - Limit the amount we advance so that early IO > - activity won't get the guest too far ahead. */ > - if (add > 10000000) > - add = 10000000; > - delta += add; > - qemu_icount += qemu_icount_round (add); > - timeout = delta / 1000000; > - if (timeout < 0) > - timeout = 0; > - } > - } > - > - return timeout; > -#else /* CONFIG_IOTHREAD */ > return 1000; > -#endif > } > > diff --git a/vl.c b/vl.c > index d661e8e..11f5669 100644 > --- a/vl.c > +++ b/vl.c > @@ -1438,17 +1438,6 @@ void main_loop_wait(int nonblocking) > > } > > -#ifndef CONFIG_IOTHREAD > -static int vm_request_pending(void) > -{ > - return powerdown_requested || > - reset_requested || > - shutdown_requested || > - debug_requested || > - vmstop_requested; > -} > -#endif > - > qemu_irq qemu_system_powerdown; > > static void main_loop(void) > @@ -1462,12 +1451,6 @@ static void main_loop(void) > qemu_main_loop_start(); > > for (;;) { > -#ifndef CONFIG_IOTHREAD > - nonblocking = cpu_exec_all(); > - if (vm_request_pending()) { > - nonblocking = true; > - } > -#endif > #ifdef CONFIG_PROFILER > ti = profile_getclock(); > #endif
On 08/22/2011 08:35 AM, Jan Kiszka wrote: > On 2011-08-22 15:24, Anthony Liguori wrote: >> Enabling the I/O thread by default seems like an important part of declaring >> 1.0. Besides allowing true SMP support with KVM, the I/O thread means that the >> TCG VCPU doesn't have to multiplex itself with the I/O dispatch routines which >> currently requires a (racey) signal based alarm system. >> >> I know there have been concerns about performance. I think so far the ones that >> have come up (virtio-net) are most likely due to secondary reasons like >> decreased batching. > > iothread enabled without [1] gives unacceptable performance for ARM > emulation (Musicpal board) here. With that series applied, the host CPU > load is measurably higher, but no longer excessively. Can you rebase and resubmit? I think there was confusion about which tree they were supposed to go through. I'll apply them directly. > > As that series demonstrates, a major issue for TCG is that the iothread > and the VCPU thread run in lock-step, almost nothing is parallelized. At > least so far. Paolo and Peter (and to some degree /me) discussed some > better TCG treading last Thursday. I still think we should enable unconditionally. We have a few months to try and improve the situation further. Fundamentally, using a dedicated VCPU thread is the Right Architecture for TCG so I think we are just delaying the inevitable by leaving it disabled. Regards, Anthony Liguori > > Jan > > [1] http://thread.gmane.org/gmane.comp.emulators.qemu/99658 > >> >> I think we ought to force enabling I/O thread early in 1.0 development and >> commit to resolving any lingering issues. >> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >> --- >> configure | 13 ----- >> cpus.c | 143 ---------------------------------------------------------- >> hw/qxl.c | 4 -- >> kvm-all.c | 2 +- >> qemu-timer.c | 53 --------------------- >> vl.c | 17 ------- >> 6 files changed, 1 insertions(+), 231 deletions(-) >> >> diff --git a/configure b/configure >> index 234a4c5..ef8d7ae 100755 >> --- a/configure >> +++ b/configure >> @@ -165,7 +165,6 @@ darwin_user="no" >> bsd_user="no" >> guest_base="" >> uname_release="" >> -io_thread="no" >> mixemu="no" >> aix="no" >> blobs="yes" >> @@ -722,8 +721,6 @@ for opt do >> ;; >> --enable-attr) attr="yes" >> ;; >> - --enable-io-thread) io_thread="yes" >> - ;; >> --disable-blobs) blobs="no" >> ;; >> --with-pkgversion=*) pkgversion=" ($optarg)" >> @@ -2159,12 +2156,6 @@ EOF >> >> if compile_prog "" "" ; then >> signalfd=yes >> -elif test "$kvm" = "yes" -a "$io_thread" != "yes"; then >> - echo >> - echo "ERROR: Host kernel lacks signalfd() support," >> - echo "but KVM depends on it when the IO thread is disabled." >> - echo >> - exit 1 >> fi >> >> # check if eventfd is supported >> @@ -2711,7 +2702,6 @@ echo "NPTL support $nptl" >> echo "GUEST_BASE $guest_base" >> echo "PIE user targets $user_pie" >> echo "vde support $vde" >> -echo "IO thread $io_thread" >> echo "Linux AIO support $linux_aio" >> echo "ATTR/XATTR support $attr" >> echo "Install blobs $blobs" >> @@ -2969,9 +2959,6 @@ if test "$xen" = "yes" ; then >> echo "CONFIG_XEN_BACKEND=y">> $config_host_mak >> echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version">> $config_host_mak >> fi >> -if test "$io_thread" = "yes" ; then >> - echo "CONFIG_IOTHREAD=y">> $config_host_mak >> -fi >> if test "$linux_aio" = "yes" ; then >> echo "CONFIG_LINUX_AIO=y">> $config_host_mak >> fi >> diff --git a/cpus.c b/cpus.c >> index c996ac5..fe18a60 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -173,12 +173,9 @@ static void cpu_handle_guest_debug(CPUState *env) >> { >> gdb_set_stop_cpu(env); >> qemu_system_debug_request(); >> -#ifdef CONFIG_IOTHREAD >> env->stopped = 1; >> -#endif >> } >> >> -#ifdef CONFIG_IOTHREAD >> static void cpu_signal(int sig) >> { >> if (cpu_single_env) { >> @@ -186,7 +183,6 @@ static void cpu_signal(int sig) >> } >> exit_request = 1; >> } >> -#endif >> >> #ifdef CONFIG_LINUX >> static void sigbus_reraise(void) >> @@ -262,12 +258,6 @@ static void qemu_kvm_eat_signals(CPUState *env) >> exit(1); >> } >> } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS)); >> - >> -#ifndef CONFIG_IOTHREAD >> - if (sigismember(&chkset, SIGIO) || sigismember(&chkset, SIGALRM)) { >> - qemu_notify_event(); >> - } >> -#endif >> } >> >> #else /* !CONFIG_LINUX */ >> @@ -390,7 +380,6 @@ static int qemu_signal_init(void) >> int sigfd; >> sigset_t set; >> >> -#ifdef CONFIG_IOTHREAD >> /* SIGUSR2 used by posix-aio-compat.c */ >> sigemptyset(&set); >> sigaddset(&set, SIGUSR2); >> @@ -409,18 +398,6 @@ static int qemu_signal_init(void) >> sigaddset(&set, SIGIO); >> sigaddset(&set, SIGALRM); >> sigaddset(&set, SIGBUS); >> -#else >> - sigemptyset(&set); >> - sigaddset(&set, SIGBUS); >> - if (kvm_enabled()) { >> - /* >> - * We need to process timer signals synchronously to avoid a race >> - * between exit_request check and KVM vcpu entry. >> - */ >> - sigaddset(&set, SIGIO); >> - sigaddset(&set, SIGALRM); >> - } >> -#endif >> pthread_sigmask(SIG_BLOCK,&set, NULL); >> >> sigfd = qemu_signalfd(&set); >> @@ -447,7 +424,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) >> sigact.sa_handler = dummy_signal; >> sigaction(SIG_IPI,&sigact, NULL); >> >> -#ifdef CONFIG_IOTHREAD >> pthread_sigmask(SIG_BLOCK, NULL,&set); >> sigdelset(&set, SIG_IPI); >> sigdelset(&set, SIGBUS); >> @@ -456,17 +432,7 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) >> fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); >> exit(1); >> } >> -#else >> - sigemptyset(&set); >> - sigaddset(&set, SIG_IPI); >> - sigaddset(&set, SIGIO); >> - sigaddset(&set, SIGALRM); >> - pthread_sigmask(SIG_BLOCK,&set, NULL); >> >> - pthread_sigmask(SIG_BLOCK, NULL,&set); >> - sigdelset(&set, SIGIO); >> - sigdelset(&set, SIGALRM); >> -#endif >> sigdelset(&set, SIG_IPI); >> sigdelset(&set, SIGBUS); >> r = kvm_set_signal_mask(env,&set); >> @@ -478,7 +444,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) >> >> static void qemu_tcg_init_cpu_signals(void) >> { >> -#ifdef CONFIG_IOTHREAD >> sigset_t set; >> struct sigaction sigact; >> >> @@ -489,7 +454,6 @@ static void qemu_tcg_init_cpu_signals(void) >> sigemptyset(&set); >> sigaddset(&set, SIG_IPI); >> pthread_sigmask(SIG_UNBLOCK,&set, NULL); >> -#endif >> } >> >> #else /* _WIN32 */ >> @@ -535,106 +499,6 @@ static void qemu_tcg_init_cpu_signals(void) >> } >> #endif /* _WIN32 */ >> >> -#ifndef CONFIG_IOTHREAD >> -int qemu_init_main_loop(void) >> -{ >> - int ret; >> - >> - ret = qemu_signal_init(); >> - if (ret) { >> - return ret; >> - } >> - >> - qemu_init_sigbus(); >> - >> - return qemu_event_init(); >> -} >> - >> -void qemu_main_loop_start(void) >> -{ >> -} >> - >> -void qemu_init_vcpu(void *_env) >> -{ >> - CPUState *env = _env; >> - int r; >> - >> - env->nr_cores = smp_cores; >> - env->nr_threads = smp_threads; >> - >> - if (kvm_enabled()) { >> - r = kvm_init_vcpu(env); >> - if (r< 0) { >> - fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r)); >> - exit(1); >> - } >> - qemu_kvm_init_cpu_signals(env); >> - } else { >> - qemu_tcg_init_cpu_signals(); >> - } >> -} >> - >> -int qemu_cpu_is_self(void *env) >> -{ >> - return 1; >> -} >> - >> -void run_on_cpu(CPUState *env, void (*func)(void *data), void *data) >> -{ >> - func(data); >> -} >> - >> -void resume_all_vcpus(void) >> -{ >> -} >> - >> -void pause_all_vcpus(void) >> -{ >> -} >> - >> -void qemu_cpu_kick(void *env) >> -{ >> -} >> - >> -void qemu_cpu_kick_self(void) >> -{ >> -#ifndef _WIN32 >> - assert(cpu_single_env); >> - >> - raise(SIG_IPI); >> -#else >> - abort(); >> -#endif >> -} >> - >> -void qemu_notify_event(void) >> -{ >> - CPUState *env = cpu_single_env; >> - >> - qemu_event_increment (); >> - if (env) { >> - cpu_exit(env); >> - } >> - if (next_cpu&& env != next_cpu) { >> - cpu_exit(next_cpu); >> - } >> - exit_request = 1; >> -} >> - >> -void qemu_mutex_lock_iothread(void) {} >> -void qemu_mutex_unlock_iothread(void) {} >> - >> -void cpu_stop_current(void) >> -{ >> -} >> - >> -void vm_stop(int reason) >> -{ >> - do_vm_stop(reason); >> -} >> - >> -#else /* CONFIG_IOTHREAD */ >> - >> QemuMutex qemu_global_mutex; >> static QemuCond qemu_io_proceeded_cond; >> static bool iothread_requesting_mutex; >> @@ -1036,8 +900,6 @@ void vm_stop(int reason) >> do_vm_stop(reason); >> } >> >> -#endif >> - >> static int tcg_cpu_exec(CPUState *env) >> { >> int ret; >> @@ -1092,11 +954,6 @@ bool cpu_exec_all(void) >> qemu_clock_enable(vm_clock, >> (env->singlestep_enabled& SSTEP_NOTIMER) == 0); >> >> -#ifndef CONFIG_IOTHREAD >> - if (qemu_alarm_pending()) { >> - break; >> - } >> -#endif >> if (cpu_can_run(env)) { >> if (kvm_enabled()) { >> r = kvm_cpu_exec(env); >> diff --git a/hw/qxl.c b/hw/qxl.c >> index bab60a5..c3a3e97 100644 >> --- a/hw/qxl.c >> +++ b/hw/qxl.c >> @@ -1388,11 +1388,7 @@ static void init_pipe_signaling(PCIQXLDevice *d) >> dprint(d, 1, "%s: pipe creation failed\n", __FUNCTION__); >> return; >> } >> -#ifdef CONFIG_IOTHREAD >> fcntl(d->pipe[0], F_SETFL, O_NONBLOCK); >> -#else >> - fcntl(d->pipe[0], F_SETFL, O_NONBLOCK /* | O_ASYNC */); >> -#endif >> fcntl(d->pipe[1], F_SETFL, O_NONBLOCK); >> fcntl(d->pipe[0], F_SETOWN, getpid()); >> >> diff --git a/kvm-all.c b/kvm-all.c >> index 0ae2e26..fbb9ff3 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -479,7 +479,7 @@ static int kvm_check_many_ioeventfds(void) >> * Older kernels have a 6 device limit on the KVM io bus. Find out so we >> * can avoid creating too many ioeventfds. >> */ >> -#if defined(CONFIG_EVENTFD)&& defined(CONFIG_IOTHREAD) >> +#if defined(CONFIG_EVENTFD) >> int ioeventfds[7]; >> int i, ret = 0; >> for (i = 0; i< ARRAY_SIZE(ioeventfds); i++) { >> diff --git a/qemu-timer.c b/qemu-timer.c >> index 19313d3..46dd483 100644 >> --- a/qemu-timer.c >> +++ b/qemu-timer.c >> @@ -101,22 +101,6 @@ static int64_t cpu_get_clock(void) >> } >> } >> >> -#ifndef CONFIG_IOTHREAD >> -static int64_t qemu_icount_delta(void) >> -{ >> - if (!use_icount) { >> - return 5000 * (int64_t) 1000000; >> - } else if (use_icount == 1) { >> - /* When not using an adaptive execution frequency >> - we tend to get badly out of sync with real time, >> - so just delay for a reasonable amount of time. */ >> - return 0; >> - } else { >> - return cpu_get_icount() - cpu_get_clock(); >> - } >> -} >> -#endif >> - >> /* enable cpu_get_ticks() */ >> void cpu_enable_ticks(void) >> { >> @@ -688,9 +672,7 @@ void configure_icount(const char *option) >> if (!option) >> return; >> >> -#ifdef CONFIG_IOTHREAD >> vm_clock->warp_timer = qemu_new_timer_ns(rt_clock, icount_warp_rt, NULL); >> -#endif >> >> if (strcmp(option, "auto") != 0) { >> icount_time_shift = strtol(option, NULL, 0); >> @@ -1178,41 +1160,6 @@ void quit_timers(void) >> >> int qemu_calculate_timeout(void) >> { >> -#ifndef CONFIG_IOTHREAD >> - int timeout; >> - >> - if (!vm_running) >> - timeout = 5000; >> - else { >> - /* XXX: use timeout computed from timers */ >> - int64_t add; >> - int64_t delta; >> - /* Advance virtual time to the next event. */ >> - delta = qemu_icount_delta(); >> - if (delta> 0) { >> - /* If virtual time is ahead of real time then just >> - wait for IO. */ >> - timeout = (delta + 999999) / 1000000; >> - } else { >> - /* Wait for either IO to occur or the next >> - timer event. */ >> - add = qemu_next_icount_deadline(); >> - /* We advance the timer before checking for IO. >> - Limit the amount we advance so that early IO >> - activity won't get the guest too far ahead. */ >> - if (add> 10000000) >> - add = 10000000; >> - delta += add; >> - qemu_icount += qemu_icount_round (add); >> - timeout = delta / 1000000; >> - if (timeout< 0) >> - timeout = 0; >> - } >> - } >> - >> - return timeout; >> -#else /* CONFIG_IOTHREAD */ >> return 1000; >> -#endif >> } >> >> diff --git a/vl.c b/vl.c >> index d661e8e..11f5669 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -1438,17 +1438,6 @@ void main_loop_wait(int nonblocking) >> >> } >> >> -#ifndef CONFIG_IOTHREAD >> -static int vm_request_pending(void) >> -{ >> - return powerdown_requested || >> - reset_requested || >> - shutdown_requested || >> - debug_requested || >> - vmstop_requested; >> -} >> -#endif >> - >> qemu_irq qemu_system_powerdown; >> >> static void main_loop(void) >> @@ -1462,12 +1451,6 @@ static void main_loop(void) >> qemu_main_loop_start(); >> >> for (;;) { >> -#ifndef CONFIG_IOTHREAD >> - nonblocking = cpu_exec_all(); >> - if (vm_request_pending()) { >> - nonblocking = true; >> - } >> -#endif >> #ifdef CONFIG_PROFILER >> ti = profile_getclock(); >> #endif >
On 22 August 2011 14:24, Anthony Liguori <aliguori@us.ibm.com> wrote: > Enabling the I/O thread by default seems like an important part of declaring > 1.0. Besides allowing true SMP support with KVM, the I/O thread means that the > TCG VCPU doesn't have to multiplex itself with the I/O dispatch routines which > currently requires a (racey) signal based alarm system. Even with iothread it's still signal based (and still racy) -- the only way to get a thread currently executing TCG code to stop doing so is to send it a signal. -- PMM
On 08/22/2011 03:50 PM, Peter Maydell wrote: > > Enabling the I/O thread by default seems like an important part of declaring > > 1.0. Besides allowing true SMP support with KVM, the I/O thread means that the > > TCG VCPU doesn't have to multiplex itself with the I/O dispatch routines which > > currently requires a (racey) signal based alarm system. > > Even with iothread it's still signal based (and still racy) -- the only way > to get a thread currently executing TCG code to stop doing so is to send it > a signal. It's signal-based, but I'm not sure it's racy when single-threaded. This: /* ... tb_add_jump ... */ barrier(); if (likely(!env->exit_request)) { in cpu_exec, vs. this in the signal handler: void cpu_exit(CPUState *env) { env->exit_request = 1; cpu_unlink_tb(env); } together will make sure that only a single basic block is executed after an exit request. The problems with user-level emulation arise from multiple threads concurrently execute the tb_add_jump or cpu_unlink_tb operations. My knowledge of user-level emulation is approximately zero, but I think it should be possible to make the race outcome predictable. That's because (1) cpu_unlink_tb is idempotent; (2) you don't really need to do anything in cpu_unlink_tb if the other thread is running tb_add_jump, because setting env->exit_request will avoid entering the CPU. Paolo
On 2011-08-22 16:00, Paolo Bonzini wrote: > On 08/22/2011 03:50 PM, Peter Maydell wrote: >>> Enabling the I/O thread by default seems like an important part of declaring >>> 1.0. Besides allowing true SMP support with KVM, the I/O thread means that the >>> TCG VCPU doesn't have to multiplex itself with the I/O dispatch routines which >>> currently requires a (racey) signal based alarm system. >> >> Even with iothread it's still signal based (and still racy) -- the only way >> to get a thread currently executing TCG code to stop doing so is to send it >> a signal. > > It's signal-based, but I'm not sure it's racy when single-threaded. This: > > /* ... tb_add_jump ... */ > barrier(); > if (likely(!env->exit_request)) { > > in cpu_exec, vs. this in the signal handler: > > void cpu_exit(CPUState *env) > { > env->exit_request = 1; > cpu_unlink_tb(env); > } > > together will make sure that only a single basic block is executed after > an exit request. > > The problems with user-level emulation arise from multiple threads > concurrently execute the tb_add_jump or cpu_unlink_tb operations. My > knowledge of user-level emulation is approximately zero, but I think it > should be possible to make the race outcome predictable. That's because > (1) cpu_unlink_tb is idempotent; (2) you don't really need to do > anything in cpu_unlink_tb if the other thread is running tb_add_jump, > because setting env->exit_request will avoid entering the CPU. Current multi-threaded user mode emulation is just (too) optimistically designed. But once VCPUs start to use their own TBs and/or TB chains (maybe it can be beneficial to decouple the translation buffer from the linking), this problem should go away. Jan
On 08/22/2011 08:50 AM, Peter Maydell wrote: > On 22 August 2011 14:24, Anthony Liguori<aliguori@us.ibm.com> wrote: >> Enabling the I/O thread by default seems like an important part of declaring >> 1.0. Besides allowing true SMP support with KVM, the I/O thread means that the >> TCG VCPU doesn't have to multiplex itself with the I/O dispatch routines which >> currently requires a (racey) signal based alarm system. > > Even with iothread it's still signal based (and still racy) -- the only way > to get a thread currently executing TCG code to stop doing so is to send it > a signal. What I meant is that we use SIGALRM to effectively do preemptive multiple tasking between the VCPU thread and the I/O thread. We still need to use signals with the I/O thread because we run the two in lock step. The race I was referring to with SIGALRM has to do with a guest disabling timer interrupts and any other event source. It'll cause a live lock in TCG. The only way to fix this is by moving to the I/O thread or setting SIGIO on every fd. Since every fd doesn't support SIGIO, I/O thread is really the only correct solution. Regards, Anthony Liguori > > -- PMM >
On 22 August 2011 15:00, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 08/22/2011 03:50 PM, Peter Maydell wrote: >> Even with iothread it's still signal based (and still racy) -- the only >> way to get a thread currently executing TCG code to stop doing so is to >> send it a signal. > > It's signal-based, but I'm not sure it's racy when single-threaded. This: > > /* ... tb_add_jump ... */ > barrier(); > if (likely(!env->exit_request)) { > > in cpu_exec, vs. this in the signal handler: > > void cpu_exit(CPUState *env) > { > env->exit_request = 1; > cpu_unlink_tb(env); > } > > together will make sure that only a single basic block is executed after an > exit request. If the cpu thread is in the middle of tb_add_jump() (updating the jmp_next[] circular list) when the signal fires, cpu_unlink_tb() will also try to modify the same list and corrupt things. (Also an arbitrary number of basic blocks may be executed because it's (in theory) possible for the cpu thread to stay one jump ahead of the thread running cpu_unlink_tb(). This is why cpu_unlink_tb() has to traverse the entire tb graph from the current tb unlinking things rather than only operating on the current tb. In practice of course only one or two tbs will be executed.) > The problems with user-level emulation arise from multiple threads > concurrently execute the tb_add_jump or cpu_unlink_tb operations. My > knowledge of user-level emulation is approximately zero, but I think it > should be possible to make the race outcome predictable. That's because (1) > cpu_unlink_tb is idempotent; (2) you don't really need to do anything in > cpu_unlink_tb if the other thread is running tb_add_jump, because setting > env->exit_request will avoid entering the CPU. Yeah, but we don't actually do enough locking or other cleverness at this point, which is why it's racy. (I think that cpu_unlink_tb() is just fundamentally too much work to be doing in a signal handler...) Anyway, dropping non-iothread will reduce the number of configurations we have to reason about in fixing this kind of thing, which is a good thing. -- PMM
On 08/22/2011 08:35 AM, Jan Kiszka wrote: > On 2011-08-22 15:24, Anthony Liguori wrote: >> Enabling the I/O thread by default seems like an important part of declaring >> 1.0. Besides allowing true SMP support with KVM, the I/O thread means that the >> TCG VCPU doesn't have to multiplex itself with the I/O dispatch routines which >> currently requires a (racey) signal based alarm system. >> >> I know there have been concerns about performance. I think so far the ones that >> have come up (virtio-net) are most likely due to secondary reasons like >> decreased batching. > > iothread enabled without [1] gives unacceptable performance for ARM > emulation (Musicpal board) here. With that series applied, the host CPU > load is measurably higher, but no longer excessively. Given that [1] has been applied now, is there any objections to this patch? Blue, Aurelien? Regards, Anthony Liguori > > As that series demonstrates, a major issue for TCG is that the iothread > and the VCPU thread run in lock-step, almost nothing is parallelized. At > least so far. Paolo and Peter (and to some degree /me) discussed some > better TCG treading last Thursday. > > Jan > > [1] http://thread.gmane.org/gmane.comp.emulators.qemu/99658 > >> >> I think we ought to force enabling I/O thread early in 1.0 development and >> commit to resolving any lingering issues. >> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >> --- >> configure | 13 ----- >> cpus.c | 143 ---------------------------------------------------------- >> hw/qxl.c | 4 -- >> kvm-all.c | 2 +- >> qemu-timer.c | 53 --------------------- >> vl.c | 17 ------- >> 6 files changed, 1 insertions(+), 231 deletions(-) >> >> diff --git a/configure b/configure >> index 234a4c5..ef8d7ae 100755 >> --- a/configure >> +++ b/configure >> @@ -165,7 +165,6 @@ darwin_user="no" >> bsd_user="no" >> guest_base="" >> uname_release="" >> -io_thread="no" >> mixemu="no" >> aix="no" >> blobs="yes" >> @@ -722,8 +721,6 @@ for opt do >> ;; >> --enable-attr) attr="yes" >> ;; >> - --enable-io-thread) io_thread="yes" >> - ;; >> --disable-blobs) blobs="no" >> ;; >> --with-pkgversion=*) pkgversion=" ($optarg)" >> @@ -2159,12 +2156,6 @@ EOF >> >> if compile_prog "" "" ; then >> signalfd=yes >> -elif test "$kvm" = "yes" -a "$io_thread" != "yes"; then >> - echo >> - echo "ERROR: Host kernel lacks signalfd() support," >> - echo "but KVM depends on it when the IO thread is disabled." >> - echo >> - exit 1 >> fi >> >> # check if eventfd is supported >> @@ -2711,7 +2702,6 @@ echo "NPTL support $nptl" >> echo "GUEST_BASE $guest_base" >> echo "PIE user targets $user_pie" >> echo "vde support $vde" >> -echo "IO thread $io_thread" >> echo "Linux AIO support $linux_aio" >> echo "ATTR/XATTR support $attr" >> echo "Install blobs $blobs" >> @@ -2969,9 +2959,6 @@ if test "$xen" = "yes" ; then >> echo "CONFIG_XEN_BACKEND=y">> $config_host_mak >> echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version">> $config_host_mak >> fi >> -if test "$io_thread" = "yes" ; then >> - echo "CONFIG_IOTHREAD=y">> $config_host_mak >> -fi >> if test "$linux_aio" = "yes" ; then >> echo "CONFIG_LINUX_AIO=y">> $config_host_mak >> fi >> diff --git a/cpus.c b/cpus.c >> index c996ac5..fe18a60 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -173,12 +173,9 @@ static void cpu_handle_guest_debug(CPUState *env) >> { >> gdb_set_stop_cpu(env); >> qemu_system_debug_request(); >> -#ifdef CONFIG_IOTHREAD >> env->stopped = 1; >> -#endif >> } >> >> -#ifdef CONFIG_IOTHREAD >> static void cpu_signal(int sig) >> { >> if (cpu_single_env) { >> @@ -186,7 +183,6 @@ static void cpu_signal(int sig) >> } >> exit_request = 1; >> } >> -#endif >> >> #ifdef CONFIG_LINUX >> static void sigbus_reraise(void) >> @@ -262,12 +258,6 @@ static void qemu_kvm_eat_signals(CPUState *env) >> exit(1); >> } >> } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS)); >> - >> -#ifndef CONFIG_IOTHREAD >> - if (sigismember(&chkset, SIGIO) || sigismember(&chkset, SIGALRM)) { >> - qemu_notify_event(); >> - } >> -#endif >> } >> >> #else /* !CONFIG_LINUX */ >> @@ -390,7 +380,6 @@ static int qemu_signal_init(void) >> int sigfd; >> sigset_t set; >> >> -#ifdef CONFIG_IOTHREAD >> /* SIGUSR2 used by posix-aio-compat.c */ >> sigemptyset(&set); >> sigaddset(&set, SIGUSR2); >> @@ -409,18 +398,6 @@ static int qemu_signal_init(void) >> sigaddset(&set, SIGIO); >> sigaddset(&set, SIGALRM); >> sigaddset(&set, SIGBUS); >> -#else >> - sigemptyset(&set); >> - sigaddset(&set, SIGBUS); >> - if (kvm_enabled()) { >> - /* >> - * We need to process timer signals synchronously to avoid a race >> - * between exit_request check and KVM vcpu entry. >> - */ >> - sigaddset(&set, SIGIO); >> - sigaddset(&set, SIGALRM); >> - } >> -#endif >> pthread_sigmask(SIG_BLOCK,&set, NULL); >> >> sigfd = qemu_signalfd(&set); >> @@ -447,7 +424,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) >> sigact.sa_handler = dummy_signal; >> sigaction(SIG_IPI,&sigact, NULL); >> >> -#ifdef CONFIG_IOTHREAD >> pthread_sigmask(SIG_BLOCK, NULL,&set); >> sigdelset(&set, SIG_IPI); >> sigdelset(&set, SIGBUS); >> @@ -456,17 +432,7 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) >> fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); >> exit(1); >> } >> -#else >> - sigemptyset(&set); >> - sigaddset(&set, SIG_IPI); >> - sigaddset(&set, SIGIO); >> - sigaddset(&set, SIGALRM); >> - pthread_sigmask(SIG_BLOCK,&set, NULL); >> >> - pthread_sigmask(SIG_BLOCK, NULL,&set); >> - sigdelset(&set, SIGIO); >> - sigdelset(&set, SIGALRM); >> -#endif >> sigdelset(&set, SIG_IPI); >> sigdelset(&set, SIGBUS); >> r = kvm_set_signal_mask(env,&set); >> @@ -478,7 +444,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) >> >> static void qemu_tcg_init_cpu_signals(void) >> { >> -#ifdef CONFIG_IOTHREAD >> sigset_t set; >> struct sigaction sigact; >> >> @@ -489,7 +454,6 @@ static void qemu_tcg_init_cpu_signals(void) >> sigemptyset(&set); >> sigaddset(&set, SIG_IPI); >> pthread_sigmask(SIG_UNBLOCK,&set, NULL); >> -#endif >> } >> >> #else /* _WIN32 */ >> @@ -535,106 +499,6 @@ static void qemu_tcg_init_cpu_signals(void) >> } >> #endif /* _WIN32 */ >> >> -#ifndef CONFIG_IOTHREAD >> -int qemu_init_main_loop(void) >> -{ >> - int ret; >> - >> - ret = qemu_signal_init(); >> - if (ret) { >> - return ret; >> - } >> - >> - qemu_init_sigbus(); >> - >> - return qemu_event_init(); >> -} >> - >> -void qemu_main_loop_start(void) >> -{ >> -} >> - >> -void qemu_init_vcpu(void *_env) >> -{ >> - CPUState *env = _env; >> - int r; >> - >> - env->nr_cores = smp_cores; >> - env->nr_threads = smp_threads; >> - >> - if (kvm_enabled()) { >> - r = kvm_init_vcpu(env); >> - if (r< 0) { >> - fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r)); >> - exit(1); >> - } >> - qemu_kvm_init_cpu_signals(env); >> - } else { >> - qemu_tcg_init_cpu_signals(); >> - } >> -} >> - >> -int qemu_cpu_is_self(void *env) >> -{ >> - return 1; >> -} >> - >> -void run_on_cpu(CPUState *env, void (*func)(void *data), void *data) >> -{ >> - func(data); >> -} >> - >> -void resume_all_vcpus(void) >> -{ >> -} >> - >> -void pause_all_vcpus(void) >> -{ >> -} >> - >> -void qemu_cpu_kick(void *env) >> -{ >> -} >> - >> -void qemu_cpu_kick_self(void) >> -{ >> -#ifndef _WIN32 >> - assert(cpu_single_env); >> - >> - raise(SIG_IPI); >> -#else >> - abort(); >> -#endif >> -} >> - >> -void qemu_notify_event(void) >> -{ >> - CPUState *env = cpu_single_env; >> - >> - qemu_event_increment (); >> - if (env) { >> - cpu_exit(env); >> - } >> - if (next_cpu&& env != next_cpu) { >> - cpu_exit(next_cpu); >> - } >> - exit_request = 1; >> -} >> - >> -void qemu_mutex_lock_iothread(void) {} >> -void qemu_mutex_unlock_iothread(void) {} >> - >> -void cpu_stop_current(void) >> -{ >> -} >> - >> -void vm_stop(int reason) >> -{ >> - do_vm_stop(reason); >> -} >> - >> -#else /* CONFIG_IOTHREAD */ >> - >> QemuMutex qemu_global_mutex; >> static QemuCond qemu_io_proceeded_cond; >> static bool iothread_requesting_mutex; >> @@ -1036,8 +900,6 @@ void vm_stop(int reason) >> do_vm_stop(reason); >> } >> >> -#endif >> - >> static int tcg_cpu_exec(CPUState *env) >> { >> int ret; >> @@ -1092,11 +954,6 @@ bool cpu_exec_all(void) >> qemu_clock_enable(vm_clock, >> (env->singlestep_enabled& SSTEP_NOTIMER) == 0); >> >> -#ifndef CONFIG_IOTHREAD >> - if (qemu_alarm_pending()) { >> - break; >> - } >> -#endif >> if (cpu_can_run(env)) { >> if (kvm_enabled()) { >> r = kvm_cpu_exec(env); >> diff --git a/hw/qxl.c b/hw/qxl.c >> index bab60a5..c3a3e97 100644 >> --- a/hw/qxl.c >> +++ b/hw/qxl.c >> @@ -1388,11 +1388,7 @@ static void init_pipe_signaling(PCIQXLDevice *d) >> dprint(d, 1, "%s: pipe creation failed\n", __FUNCTION__); >> return; >> } >> -#ifdef CONFIG_IOTHREAD >> fcntl(d->pipe[0], F_SETFL, O_NONBLOCK); >> -#else >> - fcntl(d->pipe[0], F_SETFL, O_NONBLOCK /* | O_ASYNC */); >> -#endif >> fcntl(d->pipe[1], F_SETFL, O_NONBLOCK); >> fcntl(d->pipe[0], F_SETOWN, getpid()); >> >> diff --git a/kvm-all.c b/kvm-all.c >> index 0ae2e26..fbb9ff3 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -479,7 +479,7 @@ static int kvm_check_many_ioeventfds(void) >> * Older kernels have a 6 device limit on the KVM io bus. Find out so we >> * can avoid creating too many ioeventfds. >> */ >> -#if defined(CONFIG_EVENTFD)&& defined(CONFIG_IOTHREAD) >> +#if defined(CONFIG_EVENTFD) >> int ioeventfds[7]; >> int i, ret = 0; >> for (i = 0; i< ARRAY_SIZE(ioeventfds); i++) { >> diff --git a/qemu-timer.c b/qemu-timer.c >> index 19313d3..46dd483 100644 >> --- a/qemu-timer.c >> +++ b/qemu-timer.c >> @@ -101,22 +101,6 @@ static int64_t cpu_get_clock(void) >> } >> } >> >> -#ifndef CONFIG_IOTHREAD >> -static int64_t qemu_icount_delta(void) >> -{ >> - if (!use_icount) { >> - return 5000 * (int64_t) 1000000; >> - } else if (use_icount == 1) { >> - /* When not using an adaptive execution frequency >> - we tend to get badly out of sync with real time, >> - so just delay for a reasonable amount of time. */ >> - return 0; >> - } else { >> - return cpu_get_icount() - cpu_get_clock(); >> - } >> -} >> -#endif >> - >> /* enable cpu_get_ticks() */ >> void cpu_enable_ticks(void) >> { >> @@ -688,9 +672,7 @@ void configure_icount(const char *option) >> if (!option) >> return; >> >> -#ifdef CONFIG_IOTHREAD >> vm_clock->warp_timer = qemu_new_timer_ns(rt_clock, icount_warp_rt, NULL); >> -#endif >> >> if (strcmp(option, "auto") != 0) { >> icount_time_shift = strtol(option, NULL, 0); >> @@ -1178,41 +1160,6 @@ void quit_timers(void) >> >> int qemu_calculate_timeout(void) >> { >> -#ifndef CONFIG_IOTHREAD >> - int timeout; >> - >> - if (!vm_running) >> - timeout = 5000; >> - else { >> - /* XXX: use timeout computed from timers */ >> - int64_t add; >> - int64_t delta; >> - /* Advance virtual time to the next event. */ >> - delta = qemu_icount_delta(); >> - if (delta> 0) { >> - /* If virtual time is ahead of real time then just >> - wait for IO. */ >> - timeout = (delta + 999999) / 1000000; >> - } else { >> - /* Wait for either IO to occur or the next >> - timer event. */ >> - add = qemu_next_icount_deadline(); >> - /* We advance the timer before checking for IO. >> - Limit the amount we advance so that early IO >> - activity won't get the guest too far ahead. */ >> - if (add> 10000000) >> - add = 10000000; >> - delta += add; >> - qemu_icount += qemu_icount_round (add); >> - timeout = delta / 1000000; >> - if (timeout< 0) >> - timeout = 0; >> - } >> - } >> - >> - return timeout; >> -#else /* CONFIG_IOTHREAD */ >> return 1000; >> -#endif >> } >> >> diff --git a/vl.c b/vl.c >> index d661e8e..11f5669 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -1438,17 +1438,6 @@ void main_loop_wait(int nonblocking) >> >> } >> >> -#ifndef CONFIG_IOTHREAD >> -static int vm_request_pending(void) >> -{ >> - return powerdown_requested || >> - reset_requested || >> - shutdown_requested || >> - debug_requested || >> - vmstop_requested; >> -} >> -#endif >> - >> qemu_irq qemu_system_powerdown; >> >> static void main_loop(void) >> @@ -1462,12 +1451,6 @@ static void main_loop(void) >> qemu_main_loop_start(); >> >> for (;;) { >> -#ifndef CONFIG_IOTHREAD >> - nonblocking = cpu_exec_all(); >> - if (vm_request_pending()) { >> - nonblocking = true; >> - } >> -#endif >> #ifdef CONFIG_PROFILER >> ti = profile_getclock(); >> #endif >
Am 29.08.2011 um 20:03 schrieb Anthony Liguori: > On 08/22/2011 08:35 AM, Jan Kiszka wrote: >> On 2011-08-22 15:24, Anthony Liguori wrote: >>> Enabling the I/O thread by default seems like an important part of >>> declaring >>> 1.0. Besides allowing true SMP support with KVM, the I/O thread >>> means that the >>> TCG VCPU doesn't have to multiplex itself with the I/O dispatch >>> routines which >>> currently requires a (racey) signal based alarm system. >>> >>> I know there have been concerns about performance. I think so far >>> the ones that >>> have come up (virtio-net) are most likely due to secondary reasons >>> like >>> decreased batching. >> >> iothread enabled without [1] gives unacceptable performance for ARM >> emulation (Musicpal board) here. With that series applied, the host >> CPU >> load is measurably higher, but no longer excessively. > > Given that [1] has been applied now, is there any objections to this > patch? There had been additional patches to fix I/O thread issues on Darwin, have any such fixes been applied the last few weeks? The patches that I tested some time ago only fixed the non-iothread case reliably. Andreas
On 08/29/2011 03:21 PM, Andreas Färber wrote: > Am 29.08.2011 um 20:03 schrieb Anthony Liguori: > >> On 08/22/2011 08:35 AM, Jan Kiszka wrote: >>> On 2011-08-22 15:24, Anthony Liguori wrote: >>>> Enabling the I/O thread by default seems like an important part of >>>> declaring >>>> 1.0. Besides allowing true SMP support with KVM, the I/O thread >>>> means that the >>>> TCG VCPU doesn't have to multiplex itself with the I/O dispatch >>>> routines which >>>> currently requires a (racey) signal based alarm system. >>>> >>>> I know there have been concerns about performance. I think so far >>>> the ones that >>>> have come up (virtio-net) are most likely due to secondary reasons like >>>> decreased batching. >>> >>> iothread enabled without [1] gives unacceptable performance for ARM >>> emulation (Musicpal board) here. With that series applied, the host CPU >>> load is measurably higher, but no longer excessively. >> >> Given that [1] has been applied now, is there any objections to this >> patch? > > There had been additional patches to fix I/O thread issues on Darwin, > have any such fixes been applied the last few weeks? I don't see any outstanding Darwin patches in my queue so either they were applied or there were issues with them. Regards, Anthony Liguori > > The patches that I tested some time ago only fixed the non-iothread case > reliably. > > Andreas >
Am 29.08.2011 um 22:24 schrieb Anthony Liguori: > On 08/29/2011 03:21 PM, Andreas Färber wrote: >> Am 29.08.2011 um 20:03 schrieb Anthony Liguori: >> >>> On 08/22/2011 08:35 AM, Jan Kiszka wrote: >>>> On 2011-08-22 15:24, Anthony Liguori wrote: >>>>> Enabling the I/O thread by default seems like an important part of >>>>> declaring >>>>> 1.0. >>> >>> Given that [1] has been applied now, is there any objections to this >>> patch? >> >> There had been additional patches to fix I/O thread issues on Darwin, >> have any such fixes been applied the last few weeks? > > I don't see any outstanding Darwin patches in my queue so either > they were applied or there were issues with them. Alexandre recalled his series on July 25, I spot neither explanation of what was wrong with it nor a v2. Hope he'll explain. Andreas
On 08/29/2011 04:23 PM, Andreas Färber wrote: > Am 29.08.2011 um 22:24 schrieb Anthony Liguori: > >> On 08/29/2011 03:21 PM, Andreas Färber wrote: >>> Am 29.08.2011 um 20:03 schrieb Anthony Liguori: >>> >>>> On 08/22/2011 08:35 AM, Jan Kiszka wrote: >>>>> On 2011-08-22 15:24, Anthony Liguori wrote: >>>>>> Enabling the I/O thread by default seems like an important part of >>>>>> declaring >>>>>> 1.0. >>>> >>>> Given that [1] has been applied now, is there any objections to this >>>> patch? >>> >>> There had been additional patches to fix I/O thread issues on Darwin, >>> have any such fixes been applied the last few weeks? >> >> I don't see any outstanding Darwin patches in my queue so either they >> were applied or there were issues with them. > > Alexandre recalled his series on July 25, I spot neither explanation of > what was wrong with it nor a v2. Hope he'll explain. If there's uncertainty, I'd prefer to enable it sooner than later. This is a key 1.0 feature IMHO and I'd prefer to have more time to work through any issues. Regards, Anthony Liguroi > Andreas
On 2011-08-29 23:25, Anthony Liguori wrote: > On 08/29/2011 04:23 PM, Andreas Färber wrote: >> Am 29.08.2011 um 22:24 schrieb Anthony Liguori: >> >>> On 08/29/2011 03:21 PM, Andreas Färber wrote: >>>> Am 29.08.2011 um 20:03 schrieb Anthony Liguori: >>>> >>>>> On 08/22/2011 08:35 AM, Jan Kiszka wrote: >>>>>> On 2011-08-22 15:24, Anthony Liguori wrote: >>>>>>> Enabling the I/O thread by default seems like an important part of >>>>>>> declaring >>>>>>> 1.0. >>>>> >>>>> Given that [1] has been applied now, is there any objections to this >>>>> patch? >>>> >>>> There had been additional patches to fix I/O thread issues on Darwin, >>>> have any such fixes been applied the last few weeks? >>> >>> I don't see any outstanding Darwin patches in my queue so either they >>> were applied or there were issues with them. >> >> Alexandre recalled his series on July 25, I spot neither explanation of >> what was wrong with it nor a v2. Hope he'll explain. > > If there's uncertainty, I'd prefer to enable it sooner than later. This > is a key 1.0 feature IMHO and I'd prefer to have more time to work > through any issues. What about making --enable-io-thread default as an intermediate step? That would leave --disable-io-thread as temporary workaround until all issues are fixed. The latter could generate a big fat warning that this mode will be removed before 1.0. Jan
Am 30.08.2011 um 00:42 schrieb Jan Kiszka: > On 2011-08-29 23:25, Anthony Liguori wrote: >> On 08/29/2011 04:23 PM, Andreas Färber wrote: >>> Am 29.08.2011 um 22:24 schrieb Anthony Liguori: >>> >>>> On 08/29/2011 03:21 PM, Andreas Färber wrote: >>>>> Am 29.08.2011 um 20:03 schrieb Anthony Liguori: >>>>> >>>>>> On 08/22/2011 08:35 AM, Jan Kiszka wrote: >>>>>>> On 2011-08-22 15:24, Anthony Liguori wrote: >>>>>>>> Enabling the I/O thread by default seems like an important >>>>>>>> part of >>>>>>>> declaring >>>>>>>> 1.0. >>>>>> >>>>>> Given that [1] has been applied now, is there any objections to >>>>>> this >>>>>> patch? >>>>> >>>>> There had been additional patches to fix I/O thread issues on >>>>> Darwin, >>>>> have any such fixes been applied the last few weeks? >>>> >>>> I don't see any outstanding Darwin patches in my queue so either >>>> they >>>> were applied or there were issues with them. >>> >>> Alexandre recalled his series on July 25, I spot neither >>> explanation of >>> what was wrong with it nor a v2. Hope he'll explain. >> >> If there's uncertainty, I'd prefer to enable it sooner than later. >> This >> is a key 1.0 feature IMHO and I'd prefer to have more time to work >> through any issues. > > What about making --enable-io-thread default as an intermediate step? > That would leave --disable-io-thread as temporary workaround until all > issues are fixed. The latter could generate a big fat warning that > this > mode will be removed before 1.0. Yes please, that proposal sounds much better. If http://wiki.qemu.org/Planning/1.0 is still up-to-date, we have about six weeks to make I/O thread work everywhere. Andreas
On 08/30/2011 01:45 PM, Andreas Färber wrote: > Am 30.08.2011 um 00:42 schrieb Jan Kiszka: > >> What about making --enable-io-thread default as an intermediate step? >> That would leave --disable-io-thread as temporary workaround until all >> issues are fixed. The latter could generate a big fat warning that this >> mode will be removed before 1.0. > > Yes please, that proposal sounds much better. > > If http://wiki.qemu.org/Planning/1.0 is still up-to-date, we have about > six weeks to make I/O thread work everywhere. I'm not a big fan of just flipping the configure flag. There is other work being held up by disable-io-thread like the timer conversion. If there aren't known issues, then I want to remove the non-I/O thread code. git history is still there for anyone that wants to test w/o it. Regards, Anthony Liguori > > Andreas
Am 30.08.2011 um 21:28 schrieb Anthony Liguori: > On 08/30/2011 01:45 PM, Andreas Färber wrote: >> Am 30.08.2011 um 00:42 schrieb Jan Kiszka: >> >>> What about making --enable-io-thread default as an intermediate >>> step? >>> That would leave --disable-io-thread as temporary workaround until >>> all >>> issues are fixed. The latter could generate a big fat warning that >>> this >>> mode will be removed before 1.0. >> >> Yes please, that proposal sounds much better. >> >> If http://wiki.qemu.org/Planning/1.0 is still up-to-date, we have >> about >> six weeks to make I/O thread work everywhere. > > I'm not a big fan of just flipping the configure flag. There is > other work being held up by disable-io-thread like the timer > conversion. > > If there aren't known issues, then I want to remove the non-I/O > thread code. git history is still there for anyone that wants to > test w/o it. My problem is that at HEAD *none* of the i386,ppc,sparc guests that used to work about a month ago boot *at all* on my Darwin/ppc64 host. Might be TCG, might be the new MemoryRegion API, might be (non-)I/O thread. Means a lot of git history testing despite little time. An ultimatum to make things even worse doesn't really help there... ;) Andreas
On 01/09/11 19:31, Andreas Färber wrote: >> If there aren't known issues, then I want to remove the non-I/O thread >> code. git history is still there for anyone that wants to test w/o it. > > My problem is that at HEAD *none* of the i386,ppc,sparc guests that used > to work about a month ago boot *at all* on my Darwin/ppc64 host. At the moment, SPARC/PPC in HEAD invokes a crashing bug in OpenBIOS because of this commit: commit c7b488721d6aafe32994ac63f8d690ae6d4729fa Author: Paolo Bonzini <pbonzini@redhat.com> Date: Wed Aug 3 10:49:18 2011 +0200 scsi: report unit attention on reset Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> Blue (CC) committed an appropriate fix to OpenBIOS SVN as r1047 but hasn't yet updated the binaries in git which is why the crash still exists. Not sure about the i386 part, although a brief test on a PPC64 Mac running Linux boots the linux-0.2.img.bz2 test image fine here. ATB, Mark.
On 09/01/2011 01:31 PM, Andreas Färber wrote: > Am 30.08.2011 um 21:28 schrieb Anthony Liguori: > >> On 08/30/2011 01:45 PM, Andreas Färber wrote: >>> Am 30.08.2011 um 00:42 schrieb Jan Kiszka: >>> >>>> What about making --enable-io-thread default as an intermediate step? >>>> That would leave --disable-io-thread as temporary workaround until all >>>> issues are fixed. The latter could generate a big fat warning that this >>>> mode will be removed before 1.0. >>> >>> Yes please, that proposal sounds much better. >>> >>> If http://wiki.qemu.org/Planning/1.0 is still up-to-date, we have about >>> six weeks to make I/O thread work everywhere. >> >> I'm not a big fan of just flipping the configure flag. There is other >> work being held up by disable-io-thread like the timer conversion. >> >> If there aren't known issues, then I want to remove the non-I/O thread >> code. git history is still there for anyone that wants to test w/o it. > > My problem is that at HEAD *none* of the i386,ppc,sparc guests that used > to work about a month ago boot *at all* on my Darwin/ppc64 host. > > Might be TCG, might be the new MemoryRegion API, might be (non-)I/O > thread. Means a lot of git history testing despite little time. An > ultimatum to make things even worse doesn't really help there... ;) For a platform to be supported, it needs to be actively maintained and fixed. If there aren't enough folks testing/fixing Darwin/ppc64, then it's not a platform we can reasonable support :-/ Regards, Anthony Liguori > > Andreas
On 09/02/2011 04:31 PM, Anthony Liguori wrote: > For a platform to be supported, it needs to be actively maintained and > fixed. If there aren't enough folks testing/fixing Darwin/ppc64, then > it's not a platform we can reasonable support :-/ I agree unfortunately. I think personally that Darwin is important (at least Darwin/VNC, I care zero about Cocoa and SDL) because it keeps us honest and avoids introducing unwanted Linux-isms. But if even Windows turns out to work better than Darwin, that is not a good sign. Still, Andreas, please do try to get a report of how iothread works on Darwin with VNC graphics. Paolo
On 09/02/2011 09:42 AM, Paolo Bonzini wrote: > On 09/02/2011 04:31 PM, Anthony Liguori wrote: >> For a platform to be supported, it needs to be actively maintained and >> fixed. If there aren't enough folks testing/fixing Darwin/ppc64, then >> it's not a platform we can reasonable support :-/ > > I agree unfortunately. I think personally that Darwin is important (at > least Darwin/VNC, I care zero about Cocoa and SDL) because it keeps us > honest and avoids introducing unwanted Linux-isms. But if even Windows > turns out to work better than Darwin, that is not a good sign. > > Still, Andreas, please do try to get a report of how iothread works on > Darwin with VNC graphics. I've pushed this patch. Please do report how things behave and we can try to fix whatever we can. Regards, Anthony Liguori > > Paolo > >
diff --git a/configure b/configure index 234a4c5..ef8d7ae 100755 --- a/configure +++ b/configure @@ -165,7 +165,6 @@ darwin_user="no" bsd_user="no" guest_base="" uname_release="" -io_thread="no" mixemu="no" aix="no" blobs="yes" @@ -722,8 +721,6 @@ for opt do ;; --enable-attr) attr="yes" ;; - --enable-io-thread) io_thread="yes" - ;; --disable-blobs) blobs="no" ;; --with-pkgversion=*) pkgversion=" ($optarg)" @@ -2159,12 +2156,6 @@ EOF if compile_prog "" "" ; then signalfd=yes -elif test "$kvm" = "yes" -a "$io_thread" != "yes"; then - echo - echo "ERROR: Host kernel lacks signalfd() support," - echo "but KVM depends on it when the IO thread is disabled." - echo - exit 1 fi # check if eventfd is supported @@ -2711,7 +2702,6 @@ echo "NPTL support $nptl" echo "GUEST_BASE $guest_base" echo "PIE user targets $user_pie" echo "vde support $vde" -echo "IO thread $io_thread" echo "Linux AIO support $linux_aio" echo "ATTR/XATTR support $attr" echo "Install blobs $blobs" @@ -2969,9 +2959,6 @@ if test "$xen" = "yes" ; then echo "CONFIG_XEN_BACKEND=y" >> $config_host_mak echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version" >> $config_host_mak fi -if test "$io_thread" = "yes" ; then - echo "CONFIG_IOTHREAD=y" >> $config_host_mak -fi if test "$linux_aio" = "yes" ; then echo "CONFIG_LINUX_AIO=y" >> $config_host_mak fi diff --git a/cpus.c b/cpus.c index c996ac5..fe18a60 100644 --- a/cpus.c +++ b/cpus.c @@ -173,12 +173,9 @@ static void cpu_handle_guest_debug(CPUState *env) { gdb_set_stop_cpu(env); qemu_system_debug_request(); -#ifdef CONFIG_IOTHREAD env->stopped = 1; -#endif } -#ifdef CONFIG_IOTHREAD static void cpu_signal(int sig) { if (cpu_single_env) { @@ -186,7 +183,6 @@ static void cpu_signal(int sig) } exit_request = 1; } -#endif #ifdef CONFIG_LINUX static void sigbus_reraise(void) @@ -262,12 +258,6 @@ static void qemu_kvm_eat_signals(CPUState *env) exit(1); } } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS)); - -#ifndef CONFIG_IOTHREAD - if (sigismember(&chkset, SIGIO) || sigismember(&chkset, SIGALRM)) { - qemu_notify_event(); - } -#endif } #else /* !CONFIG_LINUX */ @@ -390,7 +380,6 @@ static int qemu_signal_init(void) int sigfd; sigset_t set; -#ifdef CONFIG_IOTHREAD /* SIGUSR2 used by posix-aio-compat.c */ sigemptyset(&set); sigaddset(&set, SIGUSR2); @@ -409,18 +398,6 @@ static int qemu_signal_init(void) sigaddset(&set, SIGIO); sigaddset(&set, SIGALRM); sigaddset(&set, SIGBUS); -#else - sigemptyset(&set); - sigaddset(&set, SIGBUS); - if (kvm_enabled()) { - /* - * We need to process timer signals synchronously to avoid a race - * between exit_request check and KVM vcpu entry. - */ - sigaddset(&set, SIGIO); - sigaddset(&set, SIGALRM); - } -#endif pthread_sigmask(SIG_BLOCK, &set, NULL); sigfd = qemu_signalfd(&set); @@ -447,7 +424,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) sigact.sa_handler = dummy_signal; sigaction(SIG_IPI, &sigact, NULL); -#ifdef CONFIG_IOTHREAD pthread_sigmask(SIG_BLOCK, NULL, &set); sigdelset(&set, SIG_IPI); sigdelset(&set, SIGBUS); @@ -456,17 +432,7 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); exit(1); } -#else - sigemptyset(&set); - sigaddset(&set, SIG_IPI); - sigaddset(&set, SIGIO); - sigaddset(&set, SIGALRM); - pthread_sigmask(SIG_BLOCK, &set, NULL); - pthread_sigmask(SIG_BLOCK, NULL, &set); - sigdelset(&set, SIGIO); - sigdelset(&set, SIGALRM); -#endif sigdelset(&set, SIG_IPI); sigdelset(&set, SIGBUS); r = kvm_set_signal_mask(env, &set); @@ -478,7 +444,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) static void qemu_tcg_init_cpu_signals(void) { -#ifdef CONFIG_IOTHREAD sigset_t set; struct sigaction sigact; @@ -489,7 +454,6 @@ static void qemu_tcg_init_cpu_signals(void) sigemptyset(&set); sigaddset(&set, SIG_IPI); pthread_sigmask(SIG_UNBLOCK, &set, NULL); -#endif } #else /* _WIN32 */ @@ -535,106 +499,6 @@ static void qemu_tcg_init_cpu_signals(void) } #endif /* _WIN32 */ -#ifndef CONFIG_IOTHREAD -int qemu_init_main_loop(void) -{ - int ret; - - ret = qemu_signal_init(); - if (ret) { - return ret; - } - - qemu_init_sigbus(); - - return qemu_event_init(); -} - -void qemu_main_loop_start(void) -{ -} - -void qemu_init_vcpu(void *_env) -{ - CPUState *env = _env; - int r; - - env->nr_cores = smp_cores; - env->nr_threads = smp_threads; - - if (kvm_enabled()) { - r = kvm_init_vcpu(env); - if (r < 0) { - fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r)); - exit(1); - } - qemu_kvm_init_cpu_signals(env); - } else { - qemu_tcg_init_cpu_signals(); - } -} - -int qemu_cpu_is_self(void *env) -{ - return 1; -} - -void run_on_cpu(CPUState *env, void (*func)(void *data), void *data) -{ - func(data); -} - -void resume_all_vcpus(void) -{ -} - -void pause_all_vcpus(void) -{ -} - -void qemu_cpu_kick(void *env) -{ -} - -void qemu_cpu_kick_self(void) -{ -#ifndef _WIN32 - assert(cpu_single_env); - - raise(SIG_IPI); -#else - abort(); -#endif -} - -void qemu_notify_event(void) -{ - CPUState *env = cpu_single_env; - - qemu_event_increment (); - if (env) { - cpu_exit(env); - } - if (next_cpu && env != next_cpu) { - cpu_exit(next_cpu); - } - exit_request = 1; -} - -void qemu_mutex_lock_iothread(void) {} -void qemu_mutex_unlock_iothread(void) {} - -void cpu_stop_current(void) -{ -} - -void vm_stop(int reason) -{ - do_vm_stop(reason); -} - -#else /* CONFIG_IOTHREAD */ - QemuMutex qemu_global_mutex; static QemuCond qemu_io_proceeded_cond; static bool iothread_requesting_mutex; @@ -1036,8 +900,6 @@ void vm_stop(int reason) do_vm_stop(reason); } -#endif - static int tcg_cpu_exec(CPUState *env) { int ret; @@ -1092,11 +954,6 @@ bool cpu_exec_all(void) qemu_clock_enable(vm_clock, (env->singlestep_enabled & SSTEP_NOTIMER) == 0); -#ifndef CONFIG_IOTHREAD - if (qemu_alarm_pending()) { - break; - } -#endif if (cpu_can_run(env)) { if (kvm_enabled()) { r = kvm_cpu_exec(env); diff --git a/hw/qxl.c b/hw/qxl.c index bab60a5..c3a3e97 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1388,11 +1388,7 @@ static void init_pipe_signaling(PCIQXLDevice *d) dprint(d, 1, "%s: pipe creation failed\n", __FUNCTION__); return; } -#ifdef CONFIG_IOTHREAD fcntl(d->pipe[0], F_SETFL, O_NONBLOCK); -#else - fcntl(d->pipe[0], F_SETFL, O_NONBLOCK /* | O_ASYNC */); -#endif fcntl(d->pipe[1], F_SETFL, O_NONBLOCK); fcntl(d->pipe[0], F_SETOWN, getpid()); diff --git a/kvm-all.c b/kvm-all.c index 0ae2e26..fbb9ff3 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -479,7 +479,7 @@ static int kvm_check_many_ioeventfds(void) * Older kernels have a 6 device limit on the KVM io bus. Find out so we * can avoid creating too many ioeventfds. */ -#if defined(CONFIG_EVENTFD) && defined(CONFIG_IOTHREAD) +#if defined(CONFIG_EVENTFD) int ioeventfds[7]; int i, ret = 0; for (i = 0; i < ARRAY_SIZE(ioeventfds); i++) { diff --git a/qemu-timer.c b/qemu-timer.c index 19313d3..46dd483 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -101,22 +101,6 @@ static int64_t cpu_get_clock(void) } } -#ifndef CONFIG_IOTHREAD -static int64_t qemu_icount_delta(void) -{ - if (!use_icount) { - return 5000 * (int64_t) 1000000; - } else if (use_icount == 1) { - /* When not using an adaptive execution frequency - we tend to get badly out of sync with real time, - so just delay for a reasonable amount of time. */ - return 0; - } else { - return cpu_get_icount() - cpu_get_clock(); - } -} -#endif - /* enable cpu_get_ticks() */ void cpu_enable_ticks(void) { @@ -688,9 +672,7 @@ void configure_icount(const char *option) if (!option) return; -#ifdef CONFIG_IOTHREAD vm_clock->warp_timer = qemu_new_timer_ns(rt_clock, icount_warp_rt, NULL); -#endif if (strcmp(option, "auto") != 0) { icount_time_shift = strtol(option, NULL, 0); @@ -1178,41 +1160,6 @@ void quit_timers(void) int qemu_calculate_timeout(void) { -#ifndef CONFIG_IOTHREAD - int timeout; - - if (!vm_running) - timeout = 5000; - else { - /* XXX: use timeout computed from timers */ - int64_t add; - int64_t delta; - /* Advance virtual time to the next event. */ - delta = qemu_icount_delta(); - if (delta > 0) { - /* If virtual time is ahead of real time then just - wait for IO. */ - timeout = (delta + 999999) / 1000000; - } else { - /* Wait for either IO to occur or the next - timer event. */ - add = qemu_next_icount_deadline(); - /* We advance the timer before checking for IO. - Limit the amount we advance so that early IO - activity won't get the guest too far ahead. */ - if (add > 10000000) - add = 10000000; - delta += add; - qemu_icount += qemu_icount_round (add); - timeout = delta / 1000000; - if (timeout < 0) - timeout = 0; - } - } - - return timeout; -#else /* CONFIG_IOTHREAD */ return 1000; -#endif } diff --git a/vl.c b/vl.c index d661e8e..11f5669 100644 --- a/vl.c +++ b/vl.c @@ -1438,17 +1438,6 @@ void main_loop_wait(int nonblocking) } -#ifndef CONFIG_IOTHREAD -static int vm_request_pending(void) -{ - return powerdown_requested || - reset_requested || - shutdown_requested || - debug_requested || - vmstop_requested; -} -#endif - qemu_irq qemu_system_powerdown; static void main_loop(void) @@ -1462,12 +1451,6 @@ static void main_loop(void) qemu_main_loop_start(); for (;;) { -#ifndef CONFIG_IOTHREAD - nonblocking = cpu_exec_all(); - if (vm_request_pending()) { - nonblocking = true; - } -#endif #ifdef CONFIG_PROFILER ti = profile_getclock(); #endif
Enabling the I/O thread by default seems like an important part of declaring 1.0. Besides allowing true SMP support with KVM, the I/O thread means that the TCG VCPU doesn't have to multiplex itself with the I/O dispatch routines which currently requires a (racey) signal based alarm system. I know there have been concerns about performance. I think so far the ones that have come up (virtio-net) are most likely due to secondary reasons like decreased batching. I think we ought to force enabling I/O thread early in 1.0 development and commit to resolving any lingering issues. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- configure | 13 ----- cpus.c | 143 ---------------------------------------------------------- hw/qxl.c | 4 -- kvm-all.c | 2 +- qemu-timer.c | 53 --------------------- vl.c | 17 ------- 6 files changed, 1 insertions(+), 231 deletions(-)