Re: [Xen-devel] [PATCH] Add a timer mode that disables pending missed ticks

[Dave Winchell <dwinchell@xxxxxxxxxxxxxxx>]

Keir,

Here are the results of the testing I've done recently.
There are three protocols tested. I'll call them ASYNC for the code
as it stood just before your last check-in, MIXED (for ASYNC+SYNC) for
the code you recently checked in, and SYNC for the method I proposed
in the last mail.



Date     Duration       Protocol                 sles, rhat error

11/07  23 hrs 40 min     ASYNC         -4.96 sec, +4.42 sec    -.006%,  
+.005%
11/08         28 min     MIXED          -.75 sec,  -.67 sec    -.045%,  
-.040%
11/08   2 hrs 21 min      SYNC          -.80 sec,  -.34 sec,   -.009%,  
-.004%
11/08   1 hr  25 min      SYNC          -.24 sec,  -.26 sec,   -.005%,  
-.005%
11/08  15 hrs 39 min     MIXED        -19.   sec,-17.4  sec,   -.034%,  
-.031%
11/09   3 hrs 19 min     ASYNC          -.13 sec, +1.44 sec,   -.001%,  
+.012%

Each protocol was run a couple of times to gage the repeatability.

Since I had a high error (~.03%) for the ASYNC method a couple of days ago,
I ran another ASYNC test. I think there may have been something
wrong with the code I used a couple of days ago for ASYNC. It may have been
missing the immediate delivery of interrupt after context switch in.

My results indicate that either SYNC or ASYNC give acceptable accuracy,
each running consistently around or under .01%. MIXED has a fairly high 
error of
greater than .03%. Probably too close to .05% ntp threshold for comfort.
I don't have an overnight run with SYNC. I plan to leave SYNC running
over the weekend. If you'd rather I can leave MIXED running instead.

It may be too early to pick the protocol and I can run more overnight tests
next week.

I have attached a patch based on what's checked-in now that shows what I
tested for SYNC and ASYNC. For MIXED I used the code that is checked-in now.
The patch looks more complicated than the change really is as
it backs out the MIXED code you put in.

Regards,
Dave


Keir Fraser wrote:

> On 8/11/07 14:43, "Dave Winchell" <dwinchell@xxxxxxxxxxxxxxx> wrote:
>
>  
>
> > I agree that this could be a problem. I have an idea that could give us full
> > SYNC and eliminate the long periods without clock interrupts.
> > In pt_process_missed_ticks() when missed_ticks > 0 set pt->run_timer = 1.
> > In pt_save_timer():
> >
> >    list_for_each_entry ( pt, head, list )
> >        if(!pt->run_timer)
> >             stop_timer(&pt->timer);
> >
> > And in pt_timer_fn():
> >
> >    pt->run_timer = 0;
> >
> > So, for a guest that misses a tick, we will interrupt him once from the
> > descheduled state and then leave him alone in the descheduled state.
> >    
> Well, I'd rather not complicate the code if it's avoidable. I checked in a
> SYNC/ASYNC combo and code simplification as changeset 16341, and it'd be
> interesting to know how that fares against your suggested scheme. I suppose
> as long as we're better than ntpd's tolerance it doesn't actually matter all
> that much.
>
> -- Keir
>
>
>  

diff -r c0bdfda5183d xen/arch/x86/hvm/vpt.c
--- a/xen/arch/x86/hvm/vpt.c    Thu Nov 08 14:50:01 2007 +0000
+++ b/xen/arch/x86/hvm/vpt.c    Fri Nov 09 09:08:59 2007 -0500
@@ -45,6 +45,26 @@ static void pt_unlock(struct periodic_ti
     spin_unlock(&pt->vcpu->arch.hvm_vcpu.tm_lock);
 }
 
+#include <xen/keyhandler.h>
+#define PT_MISSED_OPTION_SYNC 0
+#define PT_MISSED_OPTION_ASYNC 1
+int pt_missed_option = 0;
+static void pt_missed_option_fn(unsigned char key)
+{
+    pt_missed_option = pt_missed_option ^ 1;
+    if(pt_missed_option == PT_MISSED_OPTION_SYNC)
+       printk("pt_missed_option = 0 (SYNC)\n");
+    else
+       printk("pt_missed_option = 1 (ASYNC)\n");
+}
+static int __init pt_missed_option_init(void)
+{
+    register_keyhandler('D', pt_missed_option_fn, "pt_missed_option");
+    return 0;
+}
+
+__initcall(pt_missed_option_init);
+
 static void pt_process_missed_ticks(struct periodic_time *pt)
 {
     s_time_t missed_ticks, now = NOW();
@@ -56,9 +76,20 @@ static void pt_process_missed_ticks(stru
     if ( missed_ticks <= 0 )
         return;
 
-    missed_ticks = missed_ticks / (s_time_t) pt->period + 1;
-    pt->pending_intr_nr += missed_ticks;
-    pt->scheduled += missed_ticks * pt->period;
+    if ( mode_is(pt->vcpu->domain, no_missed_tick_accounting)  && 
(pt_missed_option == PT_MISSED_OPTION_ASYNC) )
+    {
+        pt->pending_intr_nr = 1;
+        pt->scheduled = now + pt->period;
+    }
+    else
+    {
+        missed_ticks = missed_ticks / (s_time_t) pt->period + 1;
+       if ( !mode_is(pt->vcpu->domain, no_missed_tick_accounting) )
+           pt->pending_intr_nr += missed_ticks;
+       else
+           pt->run_timer = 1;
+        pt->scheduled += missed_ticks * pt->period;
+    }
 }
 
 static void pt_freeze_time(struct vcpu *v)
@@ -91,8 +122,10 @@ void pt_save_timer(struct vcpu *v)
 
     spin_lock(&v->arch.hvm_vcpu.tm_lock);
 
-    list_for_each_entry ( pt, head, list )
-        stop_timer(&pt->timer);
+    list_for_each_entry ( pt, head, list ) {
+       if(!pt->run_timer)
+           stop_timer(&pt->timer);
+    }
 
     pt_freeze_time(v);
 
@@ -123,7 +156,12 @@ static void pt_timer_fn(void *data)
 
     pt_lock(pt);
 
-    pt->pending_intr_nr++;
+    if ( mode_is(pt->vcpu->domain, no_missed_tick_accounting) ) {
+        pt->pending_intr_nr = 1;
+       pt->run_timer = 0;
+    }
+    else
+        pt->pending_intr_nr++;
 
     if ( !pt->one_shot )
     {
@@ -224,16 +262,11 @@ void pt_intr_post(struct vcpu *v, struct
     }
     else
     {
+        pt->pending_intr_nr--;
         if ( mode_is(v->domain, no_missed_tick_accounting) )
-        {
             pt->last_plt_gtime = hvm_get_guest_time(v);
-            pt->pending_intr_nr = 0; /* 'collapse' all missed ticks */
-        }
         else
-        {
             pt->last_plt_gtime += pt->period_cycles;
-            pt->pending_intr_nr--;
-        }
     }
 
     if ( mode_is(v->domain, delay_for_missed_ticks) &&
diff -r c0bdfda5183d xen/include/asm-x86/hvm/vpt.h
--- a/xen/include/asm-x86/hvm/vpt.h     Thu Nov 08 14:50:01 2007 +0000
+++ b/xen/include/asm-x86/hvm/vpt.h     Fri Nov 09 08:53:14 2007 -0500
@@ -84,6 +84,7 @@ struct periodic_time {
     struct timer timer;         /* ac_timer */
     time_cb *cb;
     void *priv;                 /* point back to platform time source */
+    u32 run_timer;
 };
 
 

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