On Tue, Mar 25, 2008 at 04:58:06PM +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
> >Avi was concerned that this would cause problems with migration. I
> >haven't specifically tested it yet, but it seems there will be no
> >problems introduced by this change: the IO thread will stop all vcpu's
> >in the same way the vcpu0 thread did before.
> >
>
> I believe this is broken for smp_cpus > 1, and will with this change
> will be broken even for non smp. The pause/resume logic is rotten.
Ok, I'll look into it in more detail.
> >QEMU/KVM: separate thread for IO handling
> >
> >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.
> >
>
> I think we call main_loop_wait() is called unconditionally after every
> signal.
We exit the kvm_run() loop if CPU_INTERRUPT_EXIT is detected by pre_kvm_run().
But the SIGALRM handler won't set it:
static void host_alarm_handler(int host_signum)
{
...
CPUState *env = next_cpu;
alarm_timer->flags |= ALARM_FLAG_EXPIRED;
if (env) {
/* stop the currently executing cpu because a timer occured */
cpu_interrupt(env, CPU_INTERRUPT_EXIT);
#ifdef USE_KQEMU
if (env->kqemu_enabled) {
kqemu_cpu_interrupt(env);
}
#endif
Because the KVM path does not initialize "next_cpu":
static int main_loop(void)
{
int ret, timeout;
#ifdef CONFIG_PROFILER
int64_t ti;
#endif
CPUState *env;
if (kvm_enabled()) {
kvm_main_loop();
cpu_disable_ticks();
return 0;
}
cur_cpu = first_cpu;
next_cpu = cur_cpu->next_cpu ?: first_cpu;
>
> >- Processing of IO events will not be unnecessarily interrupted.
> >
> >
> >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;
> > }
> >
>
> Please put this and the corresponding qemu change in a separate patch.
>
> [...lots more...]
>
> Looks good.
Will do.
-------------------------------------------------------------------------
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
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
