When resuming a Xen domU we were seeing an issue where the timer ticks never
seemed to start up again. This was with CONFIG_NO_HZ=y, the Xen clocksource
has CLOCK_EVT_FEAT_ONESHOT but not CLOCK_EVT_FEAT_PERIODIC.
The issue is that on resume tick_resume_oneshot() tries to program an event for
"now", e.g.
tick_program_event(ktime_get(), 1);
However further down the call chain tick_dev_program_event() then compares that
expiry time with a second call to ktime_get() and discards the event if the
timeout is negative -- which it always will be since some time must have passed
since tick_program_event was called.
Instead of asking for an immediate event on resume, instead ask for the next
actual event.
With this fix I can successfully resume a Xen domain.
Signed-off-by: Ian Campbell <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jeremy Fitzhardinge <[email protected]>
Cc: Alex.Zeffertt <[email protected]>
Cc: [email protected]
---
kernel/time/tick-oneshot.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/time/tick-oneshot.c b/kernel/time/tick-oneshot.c
index 2e8de67..8b2c8ed 100644
--- a/kernel/time/tick-oneshot.c
+++ b/kernel/time/tick-oneshot.c
@@ -81,7 +81,7 @@ void tick_resume_oneshot(void)
struct clock_event_device *dev = td->evtdev;
clockevents_set_mode(dev, CLOCK_EVT_MODE_ONESHOT);
- tick_program_event(ktime_get(), 1);
+ tick_program_event(dev->next_event, 1);
}
/**
--
1.5.6.5
Ian Campbell wrote:
> When resuming a Xen domU we were seeing an issue where the timer ticks never
> seemed to start up again. This was with CONFIG_NO_HZ=y, the Xen clocksource
> has CLOCK_EVT_FEAT_ONESHOT but not CLOCK_EVT_FEAT_PERIODIC.
>
> The issue is that on resume tick_resume_oneshot() tries to program an event for
> "now", e.g.
> tick_program_event(ktime_get(), 1);
>
> However further down the call chain tick_dev_program_event() then compares that
>
Where's that? Do you mean in clockevents_program_event():
delta = ktime_to_ns(ktime_sub(expires, now));
if (delta <= 0)
return -ETIME;
?
> expiry time with a second call to ktime_get() and discards the event if the
> timeout is negative -- which it always will be since some time must have passed
> since tick_program_event was called.
>
> Instead of asking for an immediate event on resume, instead ask for the next
> actual event.
>
What if the next event is now anyway? What timebase is it in anyway?
> With this fix I can successfully resume a Xen domain.
>
That's good, but I think there's more going on here.
J
On Wed, 2009-03-25 at 10:19 -0700, Jeremy Fitzhardinge wrote:
> Ian Campbell wrote:
> > When resuming a Xen domU we were seeing an issue where the timer ticks never
> > seemed to start up again. This was with CONFIG_NO_HZ=y, the Xen clocksource
> > has CLOCK_EVT_FEAT_ONESHOT but not CLOCK_EVT_FEAT_PERIODIC.
> >
> > The issue is that on resume tick_resume_oneshot() tries to program an event for
> > "now", e.g.
> > tick_program_event(ktime_get(), 1);
> >
> > However further down the call chain tick_dev_program_event() then compares that
> >
> Where's that? Do you mean in clockevents_program_event():
>
> delta = ktime_to_ns(ktime_sub(expires, now));
>
> if (delta <= 0)
> return -ETIME;
>
>
> ?
Yes that's the one.
> > With this fix I can successfully resume a Xen domain.
> >
>
> That's good, but I think there's more going on here.
Hmm, yes I think so too. I misread tick_dev_program_event(), it seems
like it Does The Right Thing and I do see the Xen set_next_event hook
get called which I thought wasn't getting called earlier.
Turns out the virtual timer IRQ isn't getting reinitialised before
tick_oneshot_resume runs so we are just missing the interrupt, doh!
---
Subject: xen: resume interrupts before system devices.
otherwise the first timer interrupt after resume is missed and we never
get another.
Signed-off-by: Ian Campbell <[email protected]>
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 0489ea2..5269bb4 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -68,15 +68,15 @@ static int xen_suspend(void *data)
gnttab_resume();
xen_mm_unpin_all();
- sysdev_resume();
- device_power_up(PMSG_RESUME);
-
if (!*cancelled) {
xen_irq_resume();
xen_console_resume();
xen_timer_resume();
}
+ sysdev_resume();
+ device_power_up(PMSG_RESUME);
+
return 0;
}
Ian.
>
> J
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Ian Campbell wrote:
> Hmm, yes I think so too. I misread tick_dev_program_event(), it seems
> like it Does The Right Thing and I do see the Xen set_next_event hook
> get called which I thought wasn't getting called earlier.
>
> Turns out the virtual timer IRQ isn't getting reinitialised before
> tick_oneshot_resume runs so we are just missing the interrupt, doh!
>
While that ordering is a bug, I'm still not sure it completely explains
what we're seeing here.
In drivers/xen/manage.c:do_suspend() we call clock_was_set(), which has
the specific effect of causing all the timer events to get retriggered
on all cpus. This is necessary because we don't unplug/replug all the
cpus, and the normal sysdev_resume() timer resume only resumes the
current cpu (which is cpu 0 in this case). It also deals with the
clocksource timebase shifting, as it will over suspend/resume (esp
suspend/reboot/resume, or suspend/migrate/resume). Your patch will only
re-trigger the next cpu0 timer event, and leave the rest hanging without
a next event.
So the question is why does your patch help?
I'm seeing much worse symptoms on my test machine: the resumed domain is
just sitting there spinning dead with 100% cpu use. I don't know if
this is related or something else.
J
> Subject: xen: resume interrupts before system devices.
>
> otherwise the first timer interrupt after resume is missed and we never
> get another.
>
> Signed-off-by: Ian Campbell <[email protected]>
>
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index 0489ea2..5269bb4 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -68,15 +68,15 @@ static int xen_suspend(void *data)
> gnttab_resume();
> xen_mm_unpin_all();
>
> - sysdev_resume();
> - device_power_up(PMSG_RESUME);
> -
> if (!*cancelled) {
> xen_irq_resume();
> xen_console_resume();
> xen_timer_resume();
> }
>
> + sysdev_resume();
> + device_power_up(PMSG_RESUME);
> +
> return 0;
> }
>
>
> Ian.
>
>
>
>
>> J
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>>
>
>
On Wed, 2009-03-25 at 19:40 -0400, Jeremy Fitzhardinge wrote:
> Ian Campbell wrote:
> > Hmm, yes I think so too. I misread tick_dev_program_event(), it seems
> > like it Does The Right Thing and I do see the Xen set_next_event hook
> > get called which I thought wasn't getting called earlier.
> >
> > Turns out the virtual timer IRQ isn't getting reinitialised before
> > tick_oneshot_resume runs so we are just missing the interrupt, doh!
> >
>
> While that ordering is a bug, I'm still not sure it completely explains
> what we're seeing here.
>
> In drivers/xen/manage.c:do_suspend() we call clock_was_set(), which has
> the specific effect of causing all the timer events to get retriggered
> on all cpus.
Not if CONFIG_HIGH_RES_TIMERS is not set, which I don't have. If I set
it then things work as expected even without the patch.
When CONFIG_HIGH_RES_TIMERS is set though the call to clock_was_set ends
up in hr_timer_force_reprogram, I'm not clear what the relationship
between hrtimers and ticks is, they both seem to call down to the
oneshot code eventually, so they must coexist somehow...
Ian.
> This is necessary because we don't unplug/replug all the
> cpus, and the normal sysdev_resume() timer resume only resumes the
> current cpu (which is cpu 0 in this case). It also deals with the
> clocksource timebase shifting, as it will over suspend/resume (esp
> suspend/reboot/resume, or suspend/migrate/resume). Your patch will only
> re-trigger the next cpu0 timer event, and leave the rest hanging without
> a next event.
>
> So the question is why does your patch help?
>
> I'm seeing much worse symptoms on my test machine: the resumed domain is
> just sitting there spinning dead with 100% cpu use. I don't know if
> this is related or something else.
>
> J
>
> > Subject: xen: resume interrupts before system devices.
> >
> > otherwise the first timer interrupt after resume is missed and we never
> > get another.
> >
> > Signed-off-by: Ian Campbell <[email protected]>
> >
> > diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> > index 0489ea2..5269bb4 100644
> > --- a/drivers/xen/manage.c
> > +++ b/drivers/xen/manage.c
> > @@ -68,15 +68,15 @@ static int xen_suspend(void *data)
> > gnttab_resume();
> > xen_mm_unpin_all();
> >
> > - sysdev_resume();
> > - device_power_up(PMSG_RESUME);
> > -
> > if (!*cancelled) {
> > xen_irq_resume();
> > xen_console_resume();
> > xen_timer_resume();
> > }
> >
> > + sysdev_resume();
> > + device_power_up(PMSG_RESUME);
> > +
> > return 0;
> > }
> >
> >
> > Ian.
> >
> >
> >
> >
> >> J
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >> the body of a message to [email protected]
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> Please read the FAQ at http://www.tux.org/lkml/
> >>
> >>
> >
> >
>