Re: kvm guest loops_per_jiffy miscalibration under host load

Glauber Costa
2008-07-07T18:27:17Z

[-- Attachment #1: Type: text/plain, Size: 3864 bytes --]

On Sun, Jul 6, 2008 at 10:56 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Glauber Costa wrote:
>>
>> Marcelo Tosatti wrote:
>>>
>>> Hello,
>>>
>>> I have been discussing with Glauber and Gerd the problem where KVM
>>> guests miscalibrate loops_per_jiffy if there's sufficient load on the
>>> host.
>>>
>>> calibrate_delay_direct() failed to get a good estimate for
>>> loops_per_jiffy.
>>> Probably due to long platform interrupts. Consider using "lpj=" boot
>>> option.
>>> Calibrating delay loop... <3>107.00 BogoMIPS (lpj=214016)
>>>
>>> While this particular host calculates lpj=1597041.
>>>
>>> This means that udelay() can delay for less than what asked for, with
>>> fatal results such as:
>>>
>>> ..MP-BIOS bug: 8254 timer not connected to IO-APIC
>>> Kernel panic - not syncing: IO-APIC + timer doesn't work! Try using the
>>> 'noapic' kernel parameter
>>>
>>> This bug is easily triggered with a CPU hungry task on nice -20
>>> running only during guest calibration (so that the timer check code on
>>> io_apic_{32,64}.c fails to wait long enough for PIT interrupts to fire).
>>>
>>> The problem is that the calibration routines assume a stable relation
>>> between timer interrupt frequency (PIT at this boot stage) and
>>> TSC/execution frequency.
>>>
>>> The emulated timer frequency is based on the host system time and
>>> therefore virtually resistant against heavy load, while the execution
>>> of these routines on the guest is suspectible to scheduling of the QEMU
>>> process.
>>>
>>> To fix this in a transparent way (without direct "lpj=" boot parameter
>>> assignment or a paravirt equivalent), it would be necessary to base the
>>> emulated timer frequency on guest execution time instead of host system
>>> time. But this can introduce timekeeping issues (recent Linux guests
>>> seem to handle lost/late interrupts fine as long as the clocksource is
>>> reliable) and just sounds scary.
>>>
>>> Possible solutions:
>>>
>>> - Require the admin to preset "lpj=". Nasty, not user friendly.
>>> - Pass the proper lpj value via a paravirt interface. Won't cover
>>> fullvirt guests.
>>> - Have the management app guarantee a minimum amount of CPU required
>>> for proper calibration during guest initialization.
>>
>> I don't like any of these solutions, and won't defend any of "the one". So
>> no hard feelings. But I think the "less worse" among them IMHO is the
>> paravirt one. At least it goes in the general direction of "paravirt if
>> you need to scale over xyz".
>
> I agree. A paravirt solution solves the problem.

Please, look at the patch I've attached.

It does __delay with host help. This may have the nice effect of not
busy waiting for long-enough delays, and may well.

It is _completely_ PoC, just to show the idea. It's ugly, broken,
obviously have to go through pv-ops, etc.

Also, I intend to add a lpj field in the kvm clock memory area. We
could do just this later, do both, etc.

If we agree this is a viable solution, I'll start working on a patch

>> I think passing lpj is out of question, and giving the cpu resources for
>> that time is kind of a kludge.
>
> It's all heuristics unfortunately.
>
>> Or maybe we could put the timer expiration alone in a separate thread,
>> with maximum priority (maybe rt priority)? dunno...
>
> But then if you have high-load because of a lot of guests running, you
> defeat yourself. Any attempt to guarantee time to a guest will be defeated
> by lots of guests all attempting calibration at the same time.
>
> Regards,
>
> Anthony Liguori
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

--
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

[-- Attachment #2: proposal.patch --]
[-- Type: application/octet-stream, Size: 1753 bytes --]

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 63a77ca..4a137d9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2539,6 +2539,12 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
case KVM_HC_MMU_OP:
r = kvm_pv_mmu_op(vcpu, a0, hc_gpa(vcpu, a1, a2), &ret);
break;
+ case KVM_DELAY: {
+ u32 wait; /* make it 64 */
+ wait = a0 / cpu_data(raw_smp_processor_id()).loops_per_jiffy;
+ ret = schedule_timeout_uninterruptible(wait);
+ break;
+ }
default:
ret = -KVM_ENOSYS;
break;
diff --git a/arch/x86/lib/delay.c b/arch/x86/lib/delay.c
index f456860..28fd9a0 100644
--- a/arch/x86/lib/delay.c
+++ b/arch/x86/lib/delay.c
@@ -17,6 +17,7 @@
#include <linux/preempt.h>
#include <linux/delay.h>
#include <linux/init.h>
+#include <linux/kvm_para.h>

#include <asm/processor.h>
#include <asm/delay.h>
@@ -84,6 +85,10 @@ static void delay_tsc(unsigned long loops)
preempt_enable();
}

+static void delay_pv(unsigned long loops)
+{
+ kvm_hypercall1(KVM_DELAY, loops);
+}
/*
* Since we calibrate only once at boot, this
* function should be set once at boot and not changed
@@ -106,7 +111,10 @@ int __devinit read_current_timer(unsigned long *timer_val)

void __delay(unsigned long loops)
{
- delay_fn(loops);
+ if (paravirt_enabled())
+ delay_pv(loops);
+ else
+ delay_fn(loops);
}
EXPORT_SYMBOL(__delay);

diff --git a/include/asm-x86/kvm_para.h b/include/asm-x86/kvm_para.h
index bfd9900..00fc997 100644
--- a/include/asm-x86/kvm_para.h
+++ b/include/asm-x86/kvm_para.h
@@ -23,6 +23,7 @@
#define KVM_MMU_OP_WRITE_PTE 1
#define KVM_MMU_OP_FLUSH_TLB 2
#define KVM_MMU_OP_RELEASE_PT 3
+#define KVM_DELAY 4

/* Payload for KVM_HC_MMU_OP */
struct kvm_mmu_op_header {