I agree with Yamahata partially,
If I'm right, credit scheduler only schedules runnable vcpus,
So when context_switch is called, the scheduled in vcpu's hlt_timer must be
Stopped, it is unnecessary to call migrate_timer in context_switch.
My suggestion is like following,
@@ -233,7 +233,10 @@ fw_hypercall (struct pt_regs *regs)
}
else {
perfc_incrc(pal_halt_light);
- do_sched_op_compat(SCHEDOP_yield, 0);
<<<<<<< v->arch.hlt_timer.cpu=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;
>-----Original Message-----
>From: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
>[mailto:xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Atsushi
>SAKAI
>Sent: 2006?8?23? 20:31
>To: Isaku Yamahata
>Cc: xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>Subject: Re: [Xen-ia64-devel] [PATCH] pal_halt_light emulatefor domU TAKE3
>
>Hi, Isaku
>
> Thank you for your suggestion.
>I decided the location of migrate_timer function
>by seeing the other migrate_timer calling codes in Xen.
>I think your modification changes negiligible small performance effect.
>
>
>This migrate_timer purposes to solve
>the problem of timer is up during set_timer/do_block (vcpu hangs).
>
>So, the candidate location of migrate_timer is
>from context_switch (vcpu in) to pal_halt_light (vcpu out),
>(see below figure)
>because these positions are before set_timer/do_block.
>I select context_switch position because I think it is conservative (see line
>2).
>
> vcpu in ==================> pal_halt_light => set_timer/do_block(vcpu
>out)
>(context_switch)^^^^^^^^^^^^^^^^^|
> candidate position|
> of migrate timer |
> |========>by timer (vcpu out)
>
>Anyway, for considering performance issue, I consider 2 cases.
>1)CPU intensive process is rarely occur the context switch.
> (Only timer interrupt occurs)
> In this case, So performance is negligible for this function in
> context_switch
>10^{-6}.
>
>2)For Heavy I/O issue, pal_halt_light frequently signals.
> So position of migrate_timer in context_switch and pal_halt_light
> is very small.
> It means the number of context_switch and pal_halt_light event closes.
>
>In these two cases, migrate_timer location is not important for performance.
>
>
>Do you still think on moving this function?
>
>Anyway, thank you for your comments.
>It is clearify my idea by writing document.
>
>Thanks,
>Atsushi SAKAI
>
>
>
>>
>>Calling migrate_timer from context_switch() seems to introduce
>>unnecessary overhead.
>>Why did you choose to insert migrate_timer() to context_switch()
>>instead of inserting it ot the following position?
>>
>>diff -r 8c6bb45901e7 xen/arch/ia64/xen/hypercall.c
>>--- a/xen/arch/ia64/xen/hypercall.c Wed Aug 16 14:28:57 2006 -0600
>>+++ b/xen/arch/ia64/xen/hypercall.c Mon Aug 21 13:46:05 2006 +0900
>>@@ -233,7 +233,10 @@ fw_hypercall (struct pt_regs *regs)
>> }
>> else {
>> perfc_incrc(pal_halt_light);
>>- do_sched_op_compat(SCHEDOP_yield, 0);
>> <<<<<<<<<<<<< migrate_timer() >>>>>>>>>>>>>>>>>>>>>>>>>>>
>>+ 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;
>>
>>
>>On Wed, Aug 23, 2006 at 07:29:11PM +0900, Atsushi SAKAI wrote:
>>> Hi, Isaku
>>>
>>> Sorry for confusing.
>>> It should replace from "for context_switch" to "to context_switch"
>>> migrate_timer is in context_switch.
>>>
>>> Thanks
>>> Atsushi
>>>
>>> >Hi Atsushi.
>>> >
>>> >On Wed, Aug 23, 2006 at 05:48:15PM +0900, Atsushi SAKAI wrote:
>>> >
>>> >> 1)migrate_timer for hlt_timer_fn is added for context_switch
>>> >> This makes correct pCPU work for timer.
>>> >
>>> >Is it necessary to call migrate_timer() every context switch
>>> >instead of calling it right before set_timer(&hlt_timer)?
>>> >
>>> >--
>>> >yamahata
>>> >
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Xen-ia64-devel mailing list
>>> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>>> http://lists.xensource.com/xen-ia64-devel
>>
>>--
>>yamahata
>>
>
>
>
>
>
>
>
>_______________________________________________
>Xen-ia64-devel mailing list
>Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>http://lists.xensource.com/xen-ia64-devel
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|