RE: [Xen-ia64-devel][PATCH] found a small bugRE:[Xen-ia64-devel][PATCH] pal_halt_light emulatefor domU TAKE3

[Alex Williamson <alex.williamson@xxxxxx>]

On Thu, 2006-08-31 at 09:44 +0800, Xu, Anthony wrote:
> -                             do_sched_op_compat(SCHEDOP_yield, 0);  
> +                             migrate_timer(&v->arch.hlt_timer,  << 
> <<<v->arch.hlt_timer.cpu=v->processor;
> +                                           v->processor);
...
> 
> I also propose use above assignment state to substitute migrate_timer,
> Because at this time hlt_timer is definitely stopped, we can change 
> hlt_timer.cpu
> directly. As we know, migrate_timer may need to get two big spin_locks, 
> in huge box, I think this may cause performance degradation.

Hi Anthony,

   Is the patch below effectively what you are proposing?  I'm happy to
remove the migrate_timer() call from schedule_tail if it's unnecessary.
I'm less comfortable with the timer migration shortcut in the
pal_halt_light emulation.  While it may work, it violates the timer API,
which could come back to bite us later.  Thanks,

        Alex

diff -r af50fb41612c xen/arch/ia64/xen/domain.c
--- a/xen/arch/ia64/xen/domain.c        Fri Sep 01 08:46:02 2006 -0600
+++ b/xen/arch/ia64/xen/domain.c        Fri Sep 01 10:33:01 2006 -0600
@@ -122,7 +122,6 @@ void schedule_tail(struct vcpu *prev)
                  shared_info->vcpu_info[current->vcpu_id].evtchn_upcall_mask;
                __ia64_per_cpu_var(current_psr_ic_addr) = (int *)
                  (current->domain->arch.shared_info_va + XSI_PSR_IC_OFS);
-               migrate_timer(&current->arch.hlt_timer, current->processor);
        }
        flush_vtlb_for_context_switch(current);
 }
diff -r af50fb41612c xen/arch/ia64/xen/hypercall.c
--- a/xen/arch/ia64/xen/hypercall.c     Fri Sep 01 08:46:02 2006 -0600
+++ b/xen/arch/ia64/xen/hypercall.c     Fri Sep 01 10:33:01 2006 -0600
@@ -31,6 +31,12 @@
 #include <public/callback.h>
 #include <xen/event.h>
 #include <xen/perfc.h>
+
+/*
+ * Migrating the hlt_timer between CPUs is rather costly.  This avoids
+ * the locking overhead between the CPUs.
+ */
+#undef HLT_TIMER_SAFE_MIGRATION
 
 static long do_physdev_op_compat(XEN_GUEST_HANDLE(physdev_op_t) uop);
 static long do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg);
@@ -236,8 +242,13 @@ fw_hypercall (struct pt_regs *regs)
                        }
                        else {
                                perfc_incrc(pal_halt_light);
-                               migrate_timer(&v->arch.hlt_timer,
-                                             v->processor);
+#ifdef HLT_TIMER_SAFE_MIGRATION
+                               migrate_timer(&v->arch.hlt_timer, v->processor);
+#else
+                               if (active_timer(&v->arch.hlt_timer))
+                                       stop_timer(&v->arch.hlt_timer);
+                               v->arch.hlt_timer.cpu = v->processor;
+#endif
                                set_timer(&v->arch.hlt_timer,
                                          vcpu_get_next_timer_ns(v));
                                do_sched_op_compat(SCHEDOP_block, 0);

-- 
Alex Williamson                             HP Open Source & Linux Org.


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