What''s the meaning of "division by zero" in the change log of
changeset
15943 : c0d1825f5189 (Don''t count "missed ticks" on one-shot
timers.)?
I found the c/s breaks Linux 2.6.23-rc4 when ACPI=1 in HVM config file.
I don''t think the 2 lines below are correct for one_shot vpt:
pt->enabled = 0;
list_del(&pt->list);
because i.e., it may drop one-shot local timer interrupt wrongly (this
breaks ACPI Linux 2.6.23-rc4...):
1) an one-shot timer interrupt is triggered in pt_timer_fn(), then
c/s 15943
sets pt->enabled to 0, and removes the vpt from the tm_list;
2) in vmx_intr_assit() - > pt_update_irq(), we can''t find the pt in
the tm_list, so the timer
interrupt is dropped...
Actually we don''t need to remove an one_shot vpt from tm_list, since
pt_update_irq() ignores a vpt if pt->pending_intr_nr == 0.
Am I missing something?
-- Dexuan
> # Node ID c0d1825f51899b329495efb2078dd15e0fb3b479
> # Parent d17532dc1725e58b787329f64ce2e4e0e79516f0
> [HVM] Don''t count "missed ticks" on one-shot timers.
> It''s not clear what it would mean, and it leads to division by
zero.
> Signed-off-by: Tim Deegan <Tim.Deegan@xensource.com
> <mailto:Tim.Deegan@xensource.com> > ---
> xen/arch/x86/hvm/vpt.c | 19 ++++++++++++++-----
> 1 files changed, 14 insertions(+), 5 deletions(-)
>
> diff -r d17532dc1725 -r c0d1825f5189 xen/arch/x86/hvm/vpt.c
> --- a/xen/arch/x86/hvm/vpt.c Sun Sep 23 12:56:11 2007 +0100
> +++ b/xen/arch/x86/hvm/vpt.c Mon Sep 24 13:44:29 2007 +0100
> @@ -46,6 +46,9 @@ static void missed_ticks(struct periodic
> {
> s_time_t missed_ticks;
>
> + if ( unlikely(pt->one_shot) )
> + return;
> +
> missed_ticks = NOW() - pt->scheduled;
> if ( missed_ticks <= 0 )
> return;
> @@ -111,12 +114,18 @@ static void pt_timer_fn(void *data)
> pt_lock(pt);
>
> pt->pending_intr_nr++;
> - pt->scheduled += pt->period;
> -
> - missed_ticks(pt);
> -
> - if ( !pt->one_shot )
> +
> + if ( unlikely(pt->one_shot) )
> + {
> + pt->enabled = 0;
> + list_del(&pt->list);
> + }
> + else
> + {
> + pt->scheduled += pt->period;
> + missed_ticks(pt);
> set_timer(&pt->timer, pt->scheduled);
> + }
>
> vcpu_kick(pt->vcpu);
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Hi, At 18:53 +0800 on 17 Oct (1192647209), Cui, Dexuan wrote:> What''s the meaning of "division by zero" in the change log of changeset > 15943 : c0d1825f5189 (Don''t count "missed ticks" on one-shot timers.)?An OS that set up a one-shot ACPI timer could cause the timer to fire with pt->period set to zero, which crashes Xen in the missed_ticks calculation. (vpt.c:56 missed_ticks = missed_ticks / (s_time_t) pt->period + 1;) Also, it''s surely wrong to calculate "missed" ticks on a non-repeating timer.> I found the c/s breaks Linux 2.6.23-rc4 when ACPI=1 in HVM config file. > > I don''t think the 2 lines below are correct for one_shot vpt: > pt->enabled = 0; > list_del(&pt->list); > because i.e., it may drop one-shot local timer interrupt wrongly (this > breaks ACPI Linux 2.6.23-rc4...): > 1) an one-shot timer interrupt is triggered in pt_timer_fn(), then > c/s 15943 > sets pt->enabled to 0, and removes the vpt from the tm_list; > 2) in vmx_intr_assit() - > pt_update_irq(), we can''t find the pt in > the tm_list, so the timer > interrupt is dropped...Ah, I see. Yes, those lines need to move to after the interrupt is delivered. :)> Actually we don''t need to remove an one_shot vpt from tm_list, since > pt_update_irq() ignores a vpt if pt->pending_intr_nr == 0.We should do it, though, because otherwise we''re just making pt_update_irq''s list walk more expensive for no benefit. Cheers, Tim. -- Tim Deegan <Tim.Deegan@xensource.com>, XenSource UK Limited Registered office c/o EC2Y 5EB, UK; company number 05334508 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Yes, the newly-checked-in patch fixes the 2.6.23 issue. Thanks! -- Dexuan Tim Deegan wrote:> Hi, > > At 18:53 +0800 on 17 Oct (1192647209), Cui, Dexuan wrote: >> What''s the meaning of "division by zero" in the change log of >> changeset 15943 : c0d1825f5189 (Don''t count "missed ticks" on >> one-shot timers.)? > > An OS that set up a one-shot ACPI timer could cause the timer to fire > with pt->period set to zero, which crashes Xen in the missed_ticks > calculation. (vpt.c:56 missed_ticks = missed_ticks / (s_time_t) > pt->period + 1;) Also, it''s surely wrong to calculate "missed" ticks > on a non-repeating timer. > >> I found the c/s breaks Linux 2.6.23-rc4 when ACPI=1 in HVM config >> file. >> >> I don''t think the 2 lines below are correct for one_shot vpt: >> pt->enabled = 0; list_del(&pt->list); >> because i.e., it may drop one-shot local timer interrupt wrongly >> (this breaks ACPI Linux 2.6.23-rc4...): >> 1) an one-shot timer interrupt is triggered in pt_timer_fn(), then >> c/s 15943 >> sets pt->enabled to 0, and removes the vpt from the tm_list; >> 2) in vmx_intr_assit() - > pt_update_irq(), we can''t find the pt in >> the tm_list, so the timer interrupt is dropped... > > Ah, I see. Yes, those lines need to move to after the interrupt is > delivered. :) > >> Actually we don''t need to remove an one_shot vpt from tm_list, since >> pt_update_irq() ignores a vpt if pt->pending_intr_nr == 0. > > We should do it, though, because otherwise we''re just making > pt_update_irq''s list walk more expensive for no benefit. > > Cheers, > > Tim._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel