* [patch 0/3] QEMU dedicated IO thread
@ 2008-03-27 15:09 Marcelo Tosatti
2008-03-27 15:09 ` [patch 1/3] QEMU/KVM: separate thread for IO handling Marcelo Tosatti
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Marcelo Tosatti @ 2008-03-27 15:09 UTC (permalink / raw)
To: Avi Kivity, Anthony Liguori; +Cc: kvm-devel
--
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
^ permalink raw reply [flat|nested] 16+ messages in thread* [patch 1/3] QEMU/KVM: separate thread for IO handling 2008-03-27 15:09 [patch 0/3] QEMU dedicated IO thread Marcelo Tosatti @ 2008-03-27 15:09 ` Marcelo Tosatti 2008-03-27 15:23 ` Avi Kivity 2008-03-27 15:09 ` [patch 2/3] QEMU/KVM: add function to handle signals Marcelo Tosatti ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2008-03-27 15:09 UTC (permalink / raw) To: Avi Kivity, Anthony Liguori; +Cc: kvm-devel [-- Attachment #1: io-thread --] [-- Type: text/plain, Size: 9144 bytes --] Move IO processing from vcpu0 to a dedicated thread. This removes load on vcpu0 by allowing better cache locality and also improves latency. We can now block signal handling for IO events, so sigtimedwait won't race with handlers: - Currently the SIGALRM handler fails to set CPU_INTERRUPT_EXIT because the "next_cpu" variable is not initialized in the KVM path, meaning that processing of timer expiration might be delayed until the next vcpu0 exit. - Processing of IO events will not be unnecessarily interrupted. Index: kvm-userspace.io/qemu/qemu-kvm.c =================================================================== --- kvm-userspace.io.orig/qemu/qemu-kvm.c +++ kvm-userspace.io/qemu/qemu-kvm.c @@ -38,6 +38,7 @@ struct qemu_kvm_signal_table { }; static struct qemu_kvm_signal_table io_signal_table; +static struct qemu_kvm_signal_table vcpu_signal_table; #define SIG_IPI (SIGRTMIN+4) @@ -51,6 +52,8 @@ struct vcpu_info { int stopped; } vcpu_info[256]; +pthread_t io_thread; + static inline unsigned long kvm_get_thread_id(void) { return syscall(SYS_gettid); @@ -67,12 +70,19 @@ static void sig_ipi_handler(int n) void kvm_update_interrupt_request(CPUState *env) { - if (env && vcpu && env != vcpu->env) { - if (vcpu_info[env->cpu_index].signalled) - return; - vcpu_info[env->cpu_index].signalled = 1; - if (vcpu_info[env->cpu_index].thread) - pthread_kill(vcpu_info[env->cpu_index].thread, SIG_IPI); + int signal = 0; + + if (env) { + if (!vcpu) + signal = 1; + if (vcpu && env != vcpu->env && !vcpu_info[env->cpu_index].signalled) + signal = 1; + + if (signal) { + vcpu_info[env->cpu_index].signalled = 1; + if (vcpu_info[env->cpu_index].thread) + pthread_kill(vcpu_info[env->cpu_index].thread, SIG_IPI); + } } } @@ -105,7 +115,7 @@ static void post_kvm_run(void *opaque, i static int pre_kvm_run(void *opaque, int vcpu) { - CPUState *env = cpu_single_env; + CPUState *env = qemu_kvm_cpu_env(vcpu); kvm_arch_pre_kvm_run(opaque, vcpu); @@ -151,7 +161,8 @@ static int has_work(CPUState *env) return kvm_arch_has_work(env); } -static int kvm_eat_signal(CPUState *env, int timeout) +static int kvm_eat_signal(struct qemu_kvm_signal_table *waitset, CPUState *env, + int timeout) { struct timespec ts; int r, e, ret = 0; @@ -160,12 +171,12 @@ static int kvm_eat_signal(CPUState *env, ts.tv_sec = timeout / 1000; ts.tv_nsec = (timeout % 1000) * 1000000; - r = sigtimedwait(&io_signal_table.sigset, &siginfo, &ts); + r = sigtimedwait(&waitset->sigset, &siginfo, &ts); if (r == -1 && (errno == EAGAIN || errno == EINTR) && !timeout) return 0; e = errno; pthread_mutex_lock(&qemu_mutex); - if (vcpu) + if (env && vcpu) cpu_single_env = vcpu->env; if (r == -1 && !(errno == EAGAIN || errno == EINTR)) { printf("sigtimedwait: %s\n", strerror(e)); @@ -181,7 +192,7 @@ static int kvm_eat_signal(CPUState *env, if (env && vcpu_info[env->cpu_index].stop) { vcpu_info[env->cpu_index].stop = 0; vcpu_info[env->cpu_index].stopped = 1; - pthread_kill(vcpu_info[0].thread, SIG_IPI); + pthread_kill(io_thread, SIGUSR1); } pthread_mutex_unlock(&qemu_mutex); @@ -192,24 +203,16 @@ static int kvm_eat_signal(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(env, 0)) + while (kvm_eat_signal(waitset, env, 0)) r = 1; if (!r && timeout) { - r = kvm_eat_signal(env, timeout); + r = kvm_eat_signal(waitset, env, timeout); if (r) - while (kvm_eat_signal(env, 0)) + while (kvm_eat_signal(waitset, env, 0)) ; } - /* - * we call select() even if no signal was received, to account for - * for which there is no signal handler installed. - */ - pthread_mutex_lock(&qemu_mutex); - cpu_single_env = vcpu->env; - if (env->cpu_index == 0) - main_loop_wait(0); - pthread_mutex_unlock(&qemu_mutex); } static void kvm_main_loop_wait(CPUState *env, int timeout) @@ -225,29 +228,29 @@ static int all_threads_paused(void) { int i; - for (i = 1; i < smp_cpus; ++i) + for (i = 0; i < smp_cpus; ++i) if (vcpu_info[i].stopped) return 0; return 1; } -static void pause_other_threads(void) +static void pause_all_threads(void) { int i; - for (i = 1; i < smp_cpus; ++i) { + for (i = 0; i < smp_cpus; ++i) { vcpu_info[i].stop = 1; pthread_kill(vcpu_info[i].thread, SIG_IPI); } while (!all_threads_paused()) - kvm_eat_signals(vcpu->env, 0); + kvm_eat_signal(&io_signal_table, NULL, 1000); } -static void resume_other_threads(void) +static void resume_all_threads(void) { int i; - for (i = 1; i < smp_cpus; ++i) { + for (i = 0; i < smp_cpus; ++i) { vcpu_info[i].stop = 0; vcpu_info[i].stopped = 0; pthread_kill(vcpu_info[i].thread, SIG_IPI); @@ -257,9 +260,9 @@ static void resume_other_threads(void) static void kvm_vm_state_change_handler(void *context, int running) { if (running) - resume_other_threads(); + resume_all_threads(); else - pause_other_threads(); + pause_all_threads(); } static void update_regs_for_sipi(CPUState *env) @@ -281,8 +284,6 @@ static void setup_kernel_sigmask(CPUStat sigprocmask(SIG_BLOCK, NULL, &set); sigdelset(&set, SIG_IPI); - if (env->cpu_index == 0) - sigandset(&set, &set, &io_signal_table.negsigset); kvm_set_signal_mask(kvm_context, env->cpu_index, &set); } @@ -337,7 +338,7 @@ static void *ap_main_loop(void *_env) vcpu->env = env; vcpu->env->thread_id = kvm_get_thread_id(); sigfillset(&signals); - //sigdelset(&signals, SIG_IPI); + sigdelset(&signals, SIG_IPI); sigprocmask(SIG_BLOCK, &signals, NULL); kvm_create_vcpu(kvm_context, env->cpu_index); kvm_qemu_init_env(env); @@ -364,38 +365,58 @@ void kvm_init_new_ap(int cpu, CPUState * pthread_create(&vcpu_info[cpu].thread, NULL, ap_main_loop, env); } +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) { - CPUState *env = first_cpu->next_cpu; + CPUState *env = first_cpu; int i; #ifdef TARGET_I386 kvm_tpr_opt_setup(); #endif qemu_add_vm_change_state_handler(kvm_vm_state_change_handler, NULL); - qemu_kvm_init_signal_table(&io_signal_table); - kvm_add_signal(&io_signal_table, SIGIO); - kvm_add_signal(&io_signal_table, SIGALRM); - kvm_add_signal(&io_signal_table, SIGUSR2); - kvm_add_signal(&io_signal_table, SIG_IPI); - sigprocmask(SIG_BLOCK, &io_signal_table.sigset, NULL); + qemu_kvm_init_signal_tables(); - vcpu = &vcpu_info[0]; - vcpu->env = first_cpu; - vcpu->env->thread_id = kvm_get_thread_id(); signal(SIG_IPI, sig_ipi_handler); - for (i = 1; i < smp_cpus; ++i) { + for (i = 0; i < smp_cpus; ++i) { kvm_init_new_ap(i, env); env = env->next_cpu; } 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(). + */ + int kvm_main_loop(void) { - vcpu_info[0].thread = pthread_self(); + io_thread = pthread_self(); pthread_mutex_unlock(&qemu_mutex); - return kvm_main_loop_cpu(first_cpu); + while (1) { + kvm_eat_signal(&io_signal_table, NULL, 1000); + pthread_mutex_lock(&qemu_mutex); + cpu_single_env = NULL; + main_loop_wait(0); + pthread_mutex_unlock(&qemu_mutex); + } + return 0; } static int kvm_debug(void *opaque, int vcpu) @@ -749,12 +770,16 @@ void qemu_kvm_aio_wait_start(void) void qemu_kvm_aio_wait(void) { - if (!cpu_single_env || cpu_single_env->cpu_index == 0) { - pthread_mutex_unlock(&qemu_mutex); - kvm_eat_signal(cpu_single_env, 1000); - pthread_mutex_lock(&qemu_mutex); + CPUState *cpu_single = cpu_single_env; + + if (!cpu_single_env) { + pthread_mutex_unlock(&qemu_mutex); + kvm_eat_signal(&io_signal_table, cpu_single_env, 1000); + pthread_mutex_lock(&qemu_mutex); + cpu_single_env = NULL; } else { - pthread_cond_wait(&qemu_aio_cond, &qemu_mutex); + pthread_cond_wait(&qemu_aio_cond, &qemu_mutex); + cpu_single_env = cpu_single; } } -- ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 1/3] QEMU/KVM: separate thread for IO handling 2008-03-27 15:09 ` [patch 1/3] QEMU/KVM: separate thread for IO handling Marcelo Tosatti @ 2008-03-27 15:23 ` Avi Kivity 2008-03-27 15:48 ` Marcelo Tosatti 0 siblings, 1 reply; 16+ messages in thread From: Avi Kivity @ 2008-03-27 15:23 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm-devel Marcelo Tosatti wrote: > Move IO processing from vcpu0 to a dedicated thread. > > This removes load on vcpu0 by allowing better cache locality and also > improves latency. > > Does live migration (and stop/cont) still work with this? > We can now block signal handling for IO events, so sigtimedwait won't > race with handlers: > > - Currently the SIGALRM handler fails to set CPU_INTERRUPT_EXIT because > the "next_cpu" variable is not initialized in the KVM path, meaning that > processing of timer expiration might be delayed until the next vcpu0 exit. > I still don't understand this. Timer expiration ought to be processed in the iothread, when SIGALRM is dequeued, and be completely unrelated to whatever vcpu 0 is doing. > - Processing of IO events will not be unnecessarily interrupted. > What do you mean by this? -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 1/3] QEMU/KVM: separate thread for IO handling 2008-03-27 15:23 ` Avi Kivity @ 2008-03-27 15:48 ` Marcelo Tosatti 2008-03-27 15:49 ` Avi Kivity 0 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2008-03-27 15:48 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm-devel On Thu, Mar 27, 2008 at 05:23:38PM +0200, Avi Kivity wrote: > Marcelo Tosatti wrote: > >Move IO processing from vcpu0 to a dedicated thread. > > > >This removes load on vcpu0 by allowing better cache locality and also > >improves latency. > > > > > > Does live migration (and stop/cont) still work with this? Yes, live migration works with UP. SMP migration is broken, but its not an issue related to pausing/resuming of threads (its broken even without the change). What happens is that all threads run successfully on the target (you can see the guest reading the PM timer), but its locked wasting lots of CPU cycles (KVM is emulating some instruction(s)). Will look into it later. > > >We can now block signal handling for IO events, so sigtimedwait won't > >race with handlers: > > > >- Currently the SIGALRM handler fails to set CPU_INTERRUPT_EXIT because > >the "next_cpu" variable is not initialized in the KVM path, meaning that > >processing of timer expiration might be delayed until the next vcpu0 exit. > > > > I still don't understand this. Timer expiration ought to be processed > in the iothread, when SIGALRM is dequeued, and be completely unrelated > to whatever vcpu 0 is doing. Yes, now it is unrelated. But before the patch it wasnt. So "currently the SIGALRM handler..." means the current code in kvm-userspace.git. > >- Processing of IO events will not be unnecessarily interrupted. > > > > What do you mean by this? That SIGIO, SIGALRM and SIGUSR2 signals will not interrupt main_loop_wait(), since they are now blocked. Better latency, less time holding the mutex lock. ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 1/3] QEMU/KVM: separate thread for IO handling 2008-03-27 15:48 ` Marcelo Tosatti @ 2008-03-27 15:49 ` Avi Kivity 0 siblings, 0 replies; 16+ messages in thread From: Avi Kivity @ 2008-03-27 15:49 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm-devel Marcelo Tosatti wrote: >>> >>> >> I still don't understand this. Timer expiration ought to be processed >> in the iothread, when SIGALRM is dequeued, and be completely unrelated >> to whatever vcpu 0 is doing. >> > > Yes, now it is unrelated. But before the patch it wasnt. So "currently > the SIGALRM handler..." means the current code in kvm-userspace.git. > > I see now. Okay, will merge. >>> - Processing of IO events will not be unnecessarily interrupted. >>> >>> >> What do you mean by this? >> > > That SIGIO, SIGALRM and SIGUSR2 signals will not interrupt > main_loop_wait(), since they are now blocked. Better latency, less time > holding the mutex lock Signals are blocked in current git, they can only interrupt the guest, not the host. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch 2/3] QEMU/KVM: add function to handle signals 2008-03-27 15:09 [patch 0/3] QEMU dedicated IO thread Marcelo Tosatti 2008-03-27 15:09 ` [patch 1/3] QEMU/KVM: separate thread for IO handling Marcelo Tosatti @ 2008-03-27 15:09 ` Marcelo Tosatti 2008-03-27 15:25 ` Avi Kivity 2008-03-27 16:55 ` Avi Kivity 2008-03-27 15:09 ` [patch 3/3] QEMU/libkvm: dont create vcpu0 thread Marcelo Tosatti 2008-03-27 15:54 ` [patch 0/3] QEMU dedicated IO thread Avi Kivity 3 siblings, 2 replies; 16+ messages in thread From: Marcelo Tosatti @ 2008-03-27 15:09 UTC (permalink / raw) To: Avi Kivity, Anthony Liguori; +Cc: kvm-devel [-- Attachment #1: io-thread-sig --] [-- Type: text/plain, Size: 1892 bytes --] SIGUSR1 has no handler, and the SIGUSR2 one does nothing useful anymore (thats also true for SIGIO on tap fd, which runs host_alarm_handler unnecessarily). Index: kvm-userspace.io/qemu/qemu-kvm.c =================================================================== --- kvm-userspace.io.orig/qemu/qemu-kvm.c +++ kvm-userspace.io/qemu/qemu-kvm.c @@ -161,13 +161,30 @@ 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) { struct timespec ts; int r, e, ret = 0; siginfo_t siginfo; - struct sigaction sa; ts.tv_sec = timeout / 1000; ts.tv_nsec = (timeout % 1000) * 1000000; @@ -182,13 +199,9 @@ static int kvm_eat_signal(struct qemu_kv printf("sigtimedwait: %s\n", strerror(e)); exit(1); } - if (r != -1) { - sigaction(siginfo.si_signo, NULL, &sa); - sa.sa_handler(siginfo.si_signo); - if (siginfo.si_signo == SIGUSR2) - pthread_cond_signal(&qemu_aio_cond); - ret = 1; - } + if (r != -1) + ret = kvm_process_signal(siginfo.si_signo); + if (env && vcpu_info[env->cpu_index].stop) { vcpu_info[env->cpu_index].stop = 0; vcpu_info[env->cpu_index].stopped = 1; -- ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 2/3] QEMU/KVM: add function to handle signals 2008-03-27 15:09 ` [patch 2/3] QEMU/KVM: add function to handle signals Marcelo Tosatti @ 2008-03-27 15:25 ` Avi Kivity 2008-03-27 15:57 ` Marcelo Tosatti 2008-03-27 16:55 ` Avi Kivity 1 sibling, 1 reply; 16+ messages in thread From: Avi Kivity @ 2008-03-27 15:25 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm-devel Marcelo Tosatti wrote: > SIGUSR1 has no handler, and the SIGUSR2 one does nothing useful anymore > (thats also true for SIGIO on tap fd, which runs host_alarm_handler > unnecessarily). > The sigaction(); sa.sa_handler() stuff was added as a stopgap; do we actually need to run _any_ signal handlers? I think that main_loop_wait() does all that's necessary. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 2/3] QEMU/KVM: add function to handle signals 2008-03-27 15:25 ` Avi Kivity @ 2008-03-27 15:57 ` Marcelo Tosatti 2008-03-27 15:57 ` Avi Kivity 0 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2008-03-27 15:57 UTC (permalink / raw) To: Avi Kivity, Anders Melchiorsen; +Cc: kvm-devel On Thu, Mar 27, 2008 at 05:25:20PM +0200, Avi Kivity wrote: > Marcelo Tosatti wrote: > >SIGUSR1 has no handler, and the SIGUSR2 one does nothing useful anymore > >(thats also true for SIGIO on tap fd, which runs host_alarm_handler > >unnecessarily). > > > > The sigaction(); sa.sa_handler() stuff was added as a stopgap; do we > actually need to run _any_ signal handlers? Yes, we need to run host_alarm_handler() to set ALARM_EXPIRED, so the alarm timer gets rearmed by main_loop_wait. Thats the only one. > I think that main_loop_wait() does all that's necessary. We need this patch to separate SIGALRM from network SIGIO: http://www.mail-archive.com/qemu-devel@nongnu.org/msg16020.html Anders, can you resubmit this patch please? Any idea why it has not been merged yet? With this in place we can get rid of the sigaction syscall for SIGIO. ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 2/3] QEMU/KVM: add function to handle signals 2008-03-27 15:57 ` Marcelo Tosatti @ 2008-03-27 15:57 ` Avi Kivity 0 siblings, 0 replies; 16+ messages in thread From: Avi Kivity @ 2008-03-27 15:57 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm-devel Marcelo Tosatti wrote: > On Thu, Mar 27, 2008 at 05:25:20PM +0200, Avi Kivity wrote: > >> Marcelo Tosatti wrote: >> >>> SIGUSR1 has no handler, and the SIGUSR2 one does nothing useful anymore >>> (thats also true for SIGIO on tap fd, which runs host_alarm_handler >>> unnecessarily). >>> >>> >> The sigaction(); sa.sa_handler() stuff was added as a stopgap; do we >> actually need to run _any_ signal handlers? >> > > Yes, we need to run host_alarm_handler() to set ALARM_EXPIRED, so the > alarm timer gets rearmed by main_loop_wait. Thats the only one. > > We can call it explicitly. I'm worried about the slirp code. I think I checked it once but it is sufficiently obfuscated to be worrying. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 2/3] QEMU/KVM: add function to handle signals 2008-03-27 15:09 ` [patch 2/3] QEMU/KVM: add function to handle signals Marcelo Tosatti 2008-03-27 15:25 ` Avi Kivity @ 2008-03-27 16:55 ` Avi Kivity 1 sibling, 0 replies; 16+ messages in thread From: Avi Kivity @ 2008-03-27 16:55 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm-devel Marcelo Tosatti wrote: > SIGUSR1 has no handler, and the SIGUSR2 one does nothing useful anymore > (thats also true for SIGIO on tap fd, which runs host_alarm_handler > unnecessarily). > > This one prevents alt+F4 from closing the sdl window (X over ssh), so I'm backing it out. I'm getting some regression test failures, I'll see if they're caused by patch 1 or 2. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch 3/3] QEMU/libkvm: dont create vcpu0 thread 2008-03-27 15:09 [patch 0/3] QEMU dedicated IO thread Marcelo Tosatti 2008-03-27 15:09 ` [patch 1/3] QEMU/KVM: separate thread for IO handling Marcelo Tosatti 2008-03-27 15:09 ` [patch 2/3] QEMU/KVM: add function to handle signals Marcelo Tosatti @ 2008-03-27 15:09 ` Marcelo Tosatti 2008-03-27 15:55 ` Avi Kivity 2008-03-27 15:54 ` [patch 0/3] QEMU dedicated IO thread Avi Kivity 3 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2008-03-27 15:09 UTC (permalink / raw) To: Avi Kivity, Anthony Liguori; +Cc: kvm-devel [-- Attachment #1: io-thread-2 --] [-- Type: text/plain, Size: 742 bytes --] Since the vcpu0 thread is not special anymore and created by qemu-kvm.c. Index: kvm-userspace.io/libkvm/libkvm.c =================================================================== --- kvm-userspace.io.orig/libkvm/libkvm.c +++ kvm-userspace.io/libkvm/libkvm.c @@ -388,9 +388,6 @@ int kvm_create(kvm_context_t kvm, unsign if (r < 0) return r; kvm_create_irqchip(kvm); - r = kvm_create_vcpu(kvm, 0); - if (r < 0) - return r; return 0; } -- ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 3/3] QEMU/libkvm: dont create vcpu0 thread 2008-03-27 15:09 ` [patch 3/3] QEMU/libkvm: dont create vcpu0 thread Marcelo Tosatti @ 2008-03-27 15:55 ` Avi Kivity 0 siblings, 0 replies; 16+ messages in thread From: Avi Kivity @ 2008-03-27 15:55 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm-devel Marcelo Tosatti wrote: > Since the vcpu0 thread is not special anymore and created by qemu-kvm.c. > > Index: kvm-userspace.io/libkvm/libkvm.c > =================================================================== > --- kvm-userspace.io.orig/libkvm/libkvm.c > +++ kvm-userspace.io/libkvm/libkvm.c > @@ -388,9 +388,6 @@ int kvm_create(kvm_context_t kvm, unsign > if (r < 0) > return r; > kvm_create_irqchip(kvm); > - r = kvm_create_vcpu(kvm, 0); > - if (r < 0) > - return r; > > return 0; > } > > This isn't a standalone patch, so I folded it into the first patch (I expected this patch would have the corresponding addition of a kvm_create_vcpu(kvm, 0) to qemu). -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 0/3] QEMU dedicated IO thread 2008-03-27 15:09 [patch 0/3] QEMU dedicated IO thread Marcelo Tosatti ` (2 preceding siblings ...) 2008-03-27 15:09 ` [patch 3/3] QEMU/libkvm: dont create vcpu0 thread Marcelo Tosatti @ 2008-03-27 15:54 ` Avi Kivity 2008-03-27 19:06 ` Avi Kivity 3 siblings, 1 reply; 16+ messages in thread From: Avi Kivity @ 2008-03-27 15:54 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm-devel Marcelo Tosatti wrote: Applied all, thanks. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 0/3] QEMU dedicated IO thread 2008-03-27 15:54 ` [patch 0/3] QEMU dedicated IO thread Avi Kivity @ 2008-03-27 19:06 ` Avi Kivity 2008-03-27 20:21 ` Marcelo Tosatti 0 siblings, 1 reply; 16+ messages in thread From: Avi Kivity @ 2008-03-27 19:06 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm-devel Avi Kivity wrote: > Marcelo Tosatti wrote: > > > Applied all, thanks. > > And backed out, as the regression tests indicate a bazillion failures. I haven't investigated , but I think things are just slow so the tests time out. -- Any sufficiently difficult bug is indistinguishable from a feature. ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch 0/3] QEMU dedicated IO thread 2008-03-27 19:06 ` Avi Kivity @ 2008-03-27 20:21 ` Marcelo Tosatti 0 siblings, 0 replies; 16+ messages in thread From: Marcelo Tosatti @ 2008-03-27 20:21 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm-devel On Thu, Mar 27, 2008 at 09:06:58PM +0200, Avi Kivity wrote: > Avi Kivity wrote: > >Marcelo Tosatti wrote: > > > > > >Applied all, thanks. > > > > > And backed out, as the regression tests indicate a bazillion failures. > I haven't investigated , but I think things are just slow so the tests > time out. What tests did you run? ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch 0/3] separate thread for IO handling V3 @ 2008-04-02 23:20 Marcelo Tosatti 2008-04-02 23:20 ` [patch 2/3] QEMU/KVM: add function to handle signals Marcelo Tosatti 0 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2008-04-02 23:20 UTC (permalink / raw) To: Avi Kivity, Dor Laor; +Cc: kvm-devel This version fixes the vmdk problems found by the regression testing. Dor, regarding the option to disable the IO thread, it would require duplicating most of the changed code. For now I believe its better to get the patch into a state where its considered stable enough for inclusion. Please rerun the regression tests. Thanks. -- ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
* [patch 2/3] QEMU/KVM: add function to handle signals 2008-04-02 23:20 [patch 0/3] separate thread for IO handling V3 Marcelo Tosatti @ 2008-04-02 23:20 ` Marcelo Tosatti 0 siblings, 0 replies; 16+ messages in thread From: Marcelo Tosatti @ 2008-04-02 23:20 UTC (permalink / raw) To: Avi Kivity, Dor Laor; +Cc: kvm-devel, Marcelo Tosatti [-- Attachment #1: io-thread-sig --] [-- Type: text/plain, Size: 1946 bytes --] SIGUSR1 has no handler, and the SIGUSR2 one does nothing useful anymore (thats also true for SIGIO on tap fd, which runs host_alarm_handler unnecessarily). Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> Index: kvm-userspace.io/qemu/qemu-kvm.c =================================================================== --- kvm-userspace.io.orig/qemu/qemu-kvm.c +++ kvm-userspace.io/qemu/qemu-kvm.c @@ -163,13 +163,30 @@ 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) { struct timespec ts; int r, e, ret = 0; siginfo_t siginfo; - struct sigaction sa; ts.tv_sec = timeout / 1000; ts.tv_nsec = (timeout % 1000) * 1000000; @@ -184,13 +201,9 @@ static int kvm_eat_signal(struct qemu_kv printf("sigtimedwait: %s\n", strerror(e)); exit(1); } - if (r != -1) { - sigaction(siginfo.si_signo, NULL, &sa); - sa.sa_handler(siginfo.si_signo); - if (siginfo.si_signo == SIGUSR2) - pthread_cond_signal(&qemu_aio_cond); - ret = 1; - } + if (r != -1) + ret = kvm_process_signal(siginfo.si_signo); + if (env && vcpu_info[env->cpu_index].stop) { vcpu_info[env->cpu_index].stop = 0; vcpu_info[env->cpu_index].stopped = 1; -- ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2008-04-02 23:20 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-03-27 15:09 [patch 0/3] QEMU dedicated IO thread Marcelo Tosatti 2008-03-27 15:09 ` [patch 1/3] QEMU/KVM: separate thread for IO handling Marcelo Tosatti 2008-03-27 15:23 ` Avi Kivity 2008-03-27 15:48 ` Marcelo Tosatti 2008-03-27 15:49 ` Avi Kivity 2008-03-27 15:09 ` [patch 2/3] QEMU/KVM: add function to handle signals Marcelo Tosatti 2008-03-27 15:25 ` Avi Kivity 2008-03-27 15:57 ` Marcelo Tosatti 2008-03-27 15:57 ` Avi Kivity 2008-03-27 16:55 ` Avi Kivity 2008-03-27 15:09 ` [patch 3/3] QEMU/libkvm: dont create vcpu0 thread Marcelo Tosatti 2008-03-27 15:55 ` Avi Kivity 2008-03-27 15:54 ` [patch 0/3] QEMU dedicated IO thread Avi Kivity 2008-03-27 19:06 ` Avi Kivity 2008-03-27 20:21 ` Marcelo Tosatti -- strict thread matches above, loose matches on Subject: below -- 2008-04-02 23:20 [patch 0/3] separate thread for IO handling V3 Marcelo Tosatti 2008-04-02 23:20 ` [patch 2/3] QEMU/KVM: add function to handle signals Marcelo Tosatti
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox