RE: [Xen-ia64-devel][PATCH] found a small bugRE:[Xen-ia64-devel][PATCH] pal_halt_light emulatefor domU TAKE3 - Alex Williamson - com.xensource.lists.xen-ia64-devel

4 messages in com.xensource.lists.xen-ia64-develRE: [Xen-ia64-devel][PATCH] found a s...

From	Sent On	Attachments
Xu, Anthony	30 Aug 2006 18:43
Atsushi SAKAI	30 Aug 2006 19:06
Atsushi SAKAI	31 Aug 2006 18:21
Alex Williamson	01 Sep 2006 09:38

Subject:	RE: [Xen-ia64-devel][PATCH] found a small bugRE:[Xen-ia64-devel][PATCH] pal_halt_light emulatefor domU TAKE3
From:	Alex Williamson (alex...@hp.com)
Date:	09/01/2006 09:38:18 AM
List:	com.xensource.lists.xen-ia64-devel

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);