[PATCH] LAPIC timer accounting fix

[PATCH] LAPIC timer accounting fix

[Ben Guthro]

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

Offset emulated local APIC timer so it doesn't tick during guest's
timer related processing. Otherwise, guests using the local APIC for
process accounting can see long sequences of process ticks incorrectly
charged to interrupt processing.

Signed-off-by: Ben Guthro <bguthro@virtualron.com>
Signed-off-by: Gary Grebus <ggrebus@virtualiron.com>

[-- Attachment #2: xen-lapic-timer.patch --]
[-- Type: text/x-patch, Size: 1642 bytes --]

diff -r 68521d709355 xen/arch/x86/hvm/vlapic.c
--- a/xen/arch/x86/hvm/vlapic.c	Tue Sep 11 15:33:54 2007 -0400
+++ b/xen/arch/x86/hvm/vlapic.c	Tue Sep 11 15:33:54 2007 -0400
@@ -962,6 +962,11 @@ void vlapic_destroy(struct vcpu *v)
     free_domheap_page(vlapic->regs_page);
 }
 
+int is_lvtt_vector(struct vcpu *v, int vector)
+{
+    return (vector == vlapic_lvt_vector(vcpu_vlapic(v), APIC_LVTT));
+}
+
 int is_lvtt(struct vcpu *v, int vector)
 {
     return vcpu_vlapic(v)->pt.enabled &&
diff -r 68521d709355 xen/arch/x86/hvm/vpt.c
--- a/xen/arch/x86/hvm/vpt.c	Tue Sep 11 15:33:54 2007 -0400
+++ b/xen/arch/x86/hvm/vpt.c	Tue Sep 11 15:33:54 2007 -0400
@@ -277,6 +277,13 @@ void create_periodic_time(
     pt->period_cycles = (u64)period * cpu_khz / 1000000L;
     pt->one_shot = one_shot;
     pt->scheduled = NOW() + period;
+    if (is_lvtt_vector(v, irq))
+    {
+        /* Try to offset local APIC ticks from timer ticks.  Otherwise, guests
+           using the local APIC for process accounting can see long sequences
+           of process ticks incorrectly charged to interrupt processing. */
+        pt->scheduled += (period >> 1);
+    }
     pt->cb = cb;
     pt->priv = data;
 
diff -r 68521d709355 xen/include/asm-x86/hvm/vlapic.h
--- a/xen/include/asm-x86/hvm/vlapic.h	Tue Sep 11 15:33:54 2007 -0400
+++ b/xen/include/asm-x86/hvm/vlapic.h	Tue Sep 11 15:33:54 2007 -0400
@@ -92,6 +92,7 @@ struct vlapic *apic_round_robin(
 
 int vlapic_match_logical_addr(struct vlapic *vlapic, uint8_t mda);
 
+int is_lvtt_vector(struct vcpu *v, int vector);
 int is_lvtt(struct vcpu *v, int vector);
 int is_lvtt_enabled(struct vcpu *v);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] LAPIC timer accounting fix

[Keir Fraser]

On 24/10/07 22:12, "Ben Guthro" <bguthro@virtualiron.com> wrote:

> Offset emulated local APIC timer so it doesn't tick during guest's
> timer related processing. Otherwise, guests using the local APIC for
> process accounting can see long sequences of process ticks incorrectly
> charged to interrupt processing.

Do they really get in sync like that? Seems a bit odd. Couldn't the same
happen on real hardware? I wonder why it would be more likely in the
virtualised case...

Perhaps just offsetting all periodic timers by some random fraction of the
period would be a simpler patch?

 -- Keir

Re: [PATCH] LAPIC timer accounting fix

[Gary Grebus]

On Thu, 2007-10-25 at 15:16 +0100, Keir Fraser wrote:
> On 24/10/07 22:12, "Ben Guthro" <bguthro@virtualiron.com> wrote:
> 
> > Offset emulated local APIC timer so it doesn't tick during guest's
> > timer related processing. Otherwise, guests using the local APIC for
> > process accounting can see long sequences of process ticks incorrectly
> > charged to interrupt processing.
> 
> Do they really get in sync like that? Seems a bit odd. Couldn't the same
> happen on real hardware? I wonder why it would be more likely in the
> virtualised case...

Seems like it could happen on real hardware as well.  My guess was that
real hardware timers would drift relative to each other, and make it
less likely.

The failure was seen on a 2.4 kernel (RHEL3) so the default method for
process accounting there might be poor anyway.  There was a program that
failed because it did a large amount of CPU-intensive work but then saw
zero ticks charged to the process.

> Perhaps just offsetting all periodic timers by some random fraction of the
> period would be a simpler patch?

My goal at the time was to move the local APIC timer as far out of sync
as possible, but some random fraction might work as well.

	/gary

-- 
Gary Grebus
Virtual Iron Software, Inc.