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

["Xu, Anthony" <anthony.xu@xxxxxxxxx>]

That's Ok for me.

>@@ -122,6 +122,7 @@ 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);
> }

I think we don't need to call migrate_timer in schedule_tail,
due to it is definitely stopped.

+++ b/xen/arch/ia64/xen/hypercall.c     Thu Aug 24 11:48:35 2006 -0600
@@ -235,7 +235,12 @@ fw_hypercall (struct pt_regs *regs)
                        }
                        else {
                                perfc_incrc(pal_halt_light);
-                               do_sched_op_compat(SCHEDOP_yield, 0);  
+                               migrate_timer(&v->arch.hlt_timer,  << 
<<<v->arch.hlt_timer.cpu=v->processor;
+                                             v->processor);
+                               set_timer(&v->arch.hlt_timer,
+                                         vcpu_get_next_timer_ns(v));
+                               do_sched_op_compat(SCHEDOP_block, 0);
+                               stop_timer(&v->arch.hlt_timer);
                        }
                        regs->r8 = 0;
                        regs->r9 = 0;

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.

Thanks,
Anthony



>-----Original Message-----
>From: Alex Williamson [mailto:alex.williamson@xxxxxx]
>Sent: 2006?8?29? 21:55
>To: Xu, Anthony
>Cc: Atsushi SAKAI; xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>Subject: RE: [Xen-ia64-devel][PATCH] found a small
>bugRE:[Xen-ia64-devel][PATCH] pal_halt_light emulatefor domU TAKE3
>
>On Tue, 2006-08-29 at 17:04 +0800, Xu, Anthony wrote:
>>
>> I agree with you,
>> But I didn't find a good place to call init_tiemr.
>>
>> Comment?
>
>   How about the patch below?  It calls init_timer() with a valid CPU,
>then migrates the timer in schedule_tail(), much like the vmx timer.
>Probably safer from a timer standpoint.  Thanks,
>
>       Alex
>
>Signed-off-by: Alex Williamson <alex.williamson@xxxxxx>
>---
>
>diff -r 684fdcfb251a xen/arch/ia64/xen/domain.c
>--- a/xen/arch/ia64/xen/domain.c       Mon Aug 28 16:26:37 2006 -0600
>+++ b/xen/arch/ia64/xen/domain.c       Tue Aug 29 07:52:49 2006 -0600
>@@ -122,6 +122,7 @@ 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);
> }
>@@ -305,7 +306,8 @@ struct vcpu *alloc_vcpu_struct(struct do
>           v->arch.last_processor = INVALID_PROCESSOR;
>       }
>       if (!VMX_DOMAIN(v)){
>-              init_timer(&v->arch.hlt_timer, hlt_timer_fn, v, v->processor);
>+              init_timer(&v->arch.hlt_timer, hlt_timer_fn, v,
>+                         first_cpu(cpu_online_map));
>       }
>
>       return v;

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