2006-01-09 15:44:41

by Clemens Ladisch

[permalink] [raw]
Subject: [PATCH] HPET RTC emulation: add watchdog timer

To prevent the emulated RTC timer from stopping when interrupts are
disabled for too long, implement the watchdog timer to restart it when
needed.

Index: linux-2.6.15/arch/i386/kernel/time_hpet.c
===================================================================
--- linux-2.6.15.orig/arch/i386/kernel/time_hpet.c 2006-01-06 16:11:10.000000000 +0100
+++ linux-2.6.15/arch/i386/kernel/time_hpet.c 2006-01-08 21:31:55.000000000 +0100
@@ -413,9 +413,45 @@ int hpet_set_periodic_freq(unsigned long

int hpet_rtc_dropped_irq(void)
{
+ unsigned int cnt, ticks_per_int, lost_ints;
+
if (!is_hpet_enabled())
return 0;

+ if (UIE_on | PIE_on | AIE_on) {
+ /*
+ * The interrupt handler schedules the next interrupt at a
+ * constant offset from the time the current interrupt was
+ * scheduled, without regard to the actual time. When the
+ * handler is delayed too long, it tries to schedule the next
+ * interrupt in the past and the hardware would not interrupt
+ * until the counter had wrapped around. We catch it here.
+ */
+ cnt = hpet_readl(HPET_COUNTER);
+ /* was the comparator set to a time in the past? */
+ if ((int)(cnt - hpet_t1_cmp) > 0) {
+ /* determine how many interrupts were actually lost */
+ ticks_per_int = (hpet_tick * HZ) / hpet_rtc_int_freq;
+ lost_ints = (cnt - hpet_t1_cmp) / ticks_per_int + 1;
+ /*
+ * Make sure that, even with the time needed to execute
+ * this code, the next scheduled interrupt has been
+ * moved back to the future.
+ */
+ lost_ints++;
+
+ cnt = hpet_t1_cmp + lost_ints * ticks_per_int;
+ hpet_writel(cnt, HPET_T1_CMP);
+ hpet_t1_cmp = cnt;
+
+ if (PIE_on)
+ PIE_count += lost_ints;
+
+ printk(KERN_WARNING "rtc: lost some interrupts"
+ " at %ldHz.\n", hpet_rtc_int_freq);
+ }
+ }
+
return 1;
}

Index: linux-2.6.15/arch/x86_64/kernel/time.c
===================================================================
--- linux-2.6.15.orig/arch/x86_64/kernel/time.c 2006-01-06 16:11:13.000000000 +0100
+++ linux-2.6.15/arch/x86_64/kernel/time.c 2006-01-08 21:42:16.000000000 +0100
@@ -1234,9 +1234,45 @@ int hpet_set_periodic_freq(unsigned long

int hpet_rtc_dropped_irq(void)
{
+ unsigned int cnt, ticks_per_int, lost_ints;
+
if (!is_hpet_enabled())
return 0;

+ if (UIE_on | PIE_on | AIE_on) {
+ /*
+ * The interrupt handler schedules the next interrupt at a
+ * constant offset from the time the current interrupt was
+ * scheduled, without regard to the actual time. When the
+ * handler is delayed too long, it tries to schedule the next
+ * interrupt in the past and the hardware would not interrupt
+ * until the counter had wrapped around. We catch it here.
+ */
+ cnt = hpet_readl(HPET_COUNTER);
+ /* was the comparator set to a time in the past? */
+ if ((int)(cnt - hpet_t1_cmp) > 0) {
+ /* determine how many interrupts were actually lost */
+ ticks_per_int = (hpet_tick * HZ) / hpet_rtc_int_freq;
+ lost_ints = (cnt - hpet_t1_cmp) / ticks_per_int + 1;
+ /*
+ * Make sure that, even with the time needed to execute
+ * this code, the next scheduled interrupt has been
+ * moved back to the future.
+ */
+ lost_ints++;
+
+ cnt = hpet_t1_cmp + lost_ints * ticks_per_int;
+ hpet_writel(cnt, HPET_T1_CMP);
+ hpet_t1_cmp = cnt;
+
+ if (PIE_on)
+ PIE_count += lost_ints;
+
+ printk(KERN_WARNING "rtc: lost some interrupts"
+ " at %ldHz.\n", hpet_rtc_int_freq);
+ }
+ }
+
return 1;
}


2006-01-09 16:15:46

by Clemens Ladisch

[permalink] [raw]
Subject: Re: [PATCH] HPET RTC emulation: add watchdog timer

OK, this time with Signed-off line ...


To prevent the emulated RTC timer from stopping when interrupts are
disabled for too long, implement the watchdog timer to restart it when
needed.

Signed-off-by: Clemens Ladisch <[email protected]>

Index: linux-2.6.15/arch/i386/kernel/time_hpet.c
===================================================================
--- linux-2.6.15.orig/arch/i386/kernel/time_hpet.c 2006-01-06 16:11:10.000000000 +0100
+++ linux-2.6.15/arch/i386/kernel/time_hpet.c 2006-01-08 21:31:55.000000000 +0100
@@ -413,9 +413,45 @@ int hpet_set_periodic_freq(unsigned long

int hpet_rtc_dropped_irq(void)
{
+ unsigned int cnt, ticks_per_int, lost_ints;
+
if (!is_hpet_enabled())
return 0;

+ if (UIE_on | PIE_on | AIE_on) {
+ /*
+ * The interrupt handler schedules the next interrupt at a
+ * constant offset from the time the current interrupt was
+ * scheduled, without regard to the actual time. When the
+ * handler is delayed too long, it tries to schedule the next
+ * interrupt in the past and the hardware would not interrupt
+ * until the counter had wrapped around. We catch it here.
+ */
+ cnt = hpet_readl(HPET_COUNTER);
+ /* was the comparator set to a time in the past? */
+ if ((int)(cnt - hpet_t1_cmp) > 0) {
+ /* determine how many interrupts were actually lost */
+ ticks_per_int = (hpet_tick * HZ) / hpet_rtc_int_freq;
+ lost_ints = (cnt - hpet_t1_cmp) / ticks_per_int + 1;
+ /*
+ * Make sure that, even with the time needed to execute
+ * this code, the next scheduled interrupt has been
+ * moved back to the future.
+ */
+ lost_ints++;
+
+ cnt = hpet_t1_cmp + lost_ints * ticks_per_int;
+ hpet_writel(cnt, HPET_T1_CMP);
+ hpet_t1_cmp = cnt;
+
+ if (PIE_on)
+ PIE_count += lost_ints;
+
+ printk(KERN_WARNING "rtc: lost some interrupts"
+ " at %ldHz.\n", hpet_rtc_int_freq);
+ }
+ }
+
return 1;
}

Index: linux-2.6.15/arch/x86_64/kernel/time.c
===================================================================
--- linux-2.6.15.orig/arch/x86_64/kernel/time.c 2006-01-06 16:11:13.000000000 +0100
+++ linux-2.6.15/arch/x86_64/kernel/time.c 2006-01-08 21:42:16.000000000 +0100
@@ -1234,9 +1234,45 @@ int hpet_set_periodic_freq(unsigned long

int hpet_rtc_dropped_irq(void)
{
+ unsigned int cnt, ticks_per_int, lost_ints;
+
if (!is_hpet_enabled())
return 0;

+ if (UIE_on | PIE_on | AIE_on) {
+ /*
+ * The interrupt handler schedules the next interrupt at a
+ * constant offset from the time the current interrupt was
+ * scheduled, without regard to the actual time. When the
+ * handler is delayed too long, it tries to schedule the next
+ * interrupt in the past and the hardware would not interrupt
+ * until the counter had wrapped around. We catch it here.
+ */
+ cnt = hpet_readl(HPET_COUNTER);
+ /* was the comparator set to a time in the past? */
+ if ((int)(cnt - hpet_t1_cmp) > 0) {
+ /* determine how many interrupts were actually lost */
+ ticks_per_int = (hpet_tick * HZ) / hpet_rtc_int_freq;
+ lost_ints = (cnt - hpet_t1_cmp) / ticks_per_int + 1;
+ /*
+ * Make sure that, even with the time needed to execute
+ * this code, the next scheduled interrupt has been
+ * moved back to the future.
+ */
+ lost_ints++;
+
+ cnt = hpet_t1_cmp + lost_ints * ticks_per_int;
+ hpet_writel(cnt, HPET_T1_CMP);
+ hpet_t1_cmp = cnt;
+
+ if (PIE_on)
+ PIE_count += lost_ints;
+
+ printk(KERN_WARNING "rtc: lost some interrupts"
+ " at %ldHz.\n", hpet_rtc_int_freq);
+ }
+ }
+
return 1;
}

2006-01-09 16:41:47

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] HPET RTC emulation: add watchdog timer

On Mon, Jan 09, 2006 at 04:43:50PM +0100, Clemens Ladisch wrote:
> To prevent the emulated RTC timer from stopping when interrupts are
> disabled for too long, implement the watchdog timer to restart it when
> needed.

The interrupt handler should just read the time (it likely
has to do that anyways) and check for that
directly. That is what I did in my noidletick HPET patch and it
worked ok.

-Andi

2006-01-09 18:02:01

by lkml

[permalink] [raw]
Subject: Re: [PATCH] HPET RTC emulation: add watchdog timer

Andi Kleen wrote:
> On Mon, Jan 09, 2006 at 04:43:50PM +0100, Clemens Ladisch wrote:
> > To prevent the emulated RTC timer from stopping when interrupts
> > are disabled for too long, implement the watchdog timer to
> > restart it when needed.
>
> The interrupt handler should just read the time (it likely
> has to do that anyways)

Not in the current implementation.

> and check for that directly.

I want to avoid the read altogether because the round trip to the
south bridge is rather slow (1.5 microseconds with VIA chipsets).


Clemens

2006-01-09 20:32:31

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] HPET RTC emulation: add watchdog timer

On Mon, Jan 09, 2006 at 07:00:34PM +0100, [email protected] wrote:
> Andi Kleen wrote:
> > On Mon, Jan 09, 2006 at 04:43:50PM +0100, Clemens Ladisch wrote:
> > > To prevent the emulated RTC timer from stopping when interrupts
> > > are disabled for too long, implement the watchdog timer to
> > > restart it when needed.
> >
> > The interrupt handler should just read the time (it likely
> > has to do that anyways)
>
> Not in the current implementation.

The standard HPET interrupt in x86-64 does this already at least.

> > and check for that directly.
>
> I want to avoid the read altogether because the round trip to the
> south bridge is rather slow (1.5 microseconds with VIA chipsets).

You can use the TSC to detect it too, but it might be unreliable.

-Andi

2006-01-10 08:27:47

by Clemens Ladisch

[permalink] [raw]
Subject: Re: [PATCH] HPET RTC emulation: add watchdog timer

Andi Kleen wrote:
> Clemens Ladisch <[email protected]> wrote:
> > Andi Kleen wrote:
> > > Clemens Ladisch wrote:
> > > > To prevent the emulated RTC timer from stopping when
> > > > interrupts are disabled for too long, implement the watchdog
> > > > timer to restart it when needed.
> > >
> > > The interrupt handler should just read the time (it likely
> > > has to do that anyways)
> >
> > Not in the current implementation.
>
> The standard HPET interrupt in x86-64 does this already at least.

But it's slower than just doing a mod_timer which the RTC code does
_anyway_, so we could just as well use it. Especially as the watchdog
timer isn't actually expected to run in normal circumstances.


Clemens