QEMU/KVM: dedicated IO thread

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.

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.

- Processing of IO events will not be unnecessarily interrupted.

Index: kvm-userspace.io/libkvm/libkvm.c
===================================================================

Quoted 5 lines · Marcelo Tosatti

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.

Quoted 10 lines

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:

Quoted 4 lines

- 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.

Quoted 10 lines

- Processing of IO events will not be unnecessarily interrupted.

Index: kvm-userspace.io/libkvm/libkvm.c
===================================================================

libkvm/libkvm.c int kvm_create(kvm_context_t kvm, unsign · line 393

Quoted 9 lines

libkvm/libkvm.c +0 -3

@@ -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.

Quoted 16 lines

Quoted 16 lines

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.

Quoted 3 lines

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().

Quoted 16 lines · Marcelo Tosatti

Quoted 16 lines

Quoted 16 lines

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.

Quoted 7 lines

Quoted 3 lines

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 why do we need to exit the kvm_run() loop? As I understand it, the I/O thread wakes up when the signal is queued and calls main_loop_wait() to process any events (through qemu_run_timers()). If a timer needs to wake up a vcpu, it will raise an interrupt line which will wake the vcpu up, either in the kernel or in userspace depending on -no-kvm-irqchip.

Quoted 25 lines · Avi Kivity

Quoted 16 lines · Marcelo Tosatti

Quoted 16 lines

Quoted 16 lines

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.

Quoted 7 lines

Quoted 3 lines

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().

Quoted 5 lines

But why do we need to exit the kvm_run() loop? As I understand it, the I/O thread wakes up when the signal is queued and calls main_loop_wait() to process any events (through qemu_run_timers()). If a timer needs to wake up a vcpu, it will raise an interrupt line which will wake the vcpu up, either in the kernel or in userspace depending on -no-kvm-irqchip.

In the current state of vcpu0 thread handling IO, kvm_run() loop must bail out for main_loop_wait->qemu_run_timers() to run.

If using an userspace timer such as RTC (brought to attention by Dor's patches), the following will happen:

- signal wakes up vcpu0 thread, goes back to userspace.
- host_alarm_handler runs but fails to set CPU_INTERRUPT_EXIT because "next_cpu" is not initialized.
- pre_kvm_run() checks for CPU_INTERRUPT_EXIT and determines its not necessary to exit kvm_run(), so vcpu0 thread goes back into kernel to enter guest mode.

No interrupt was raised even though SIGALRM handler has executed.

AFAICS next_cpu is only initialized here:

static int main_loop(void)
{
...

    if (kvm_enabled()) {
        kvm_main_loop();
        cpu_disable_ticks();
        return 0;
    }

    cur_cpu = first_cpu;
    next_cpu = cur_cpu->next_cpu ?: first_cpu;
    for(;;) {

See ? I pointed this out as it appears to be another factor in unreliable userspace timers.