hrtimer: increase clock min delta threshold while interrupt hanging

Impact: avoid hanging on slow systems

While using the function graph tracer on a virtualized system, the hrtimer_interrupt can hang the system on an infinite loop. This can be caused on several situation where something intrusive is slowing the system (ie: tracing) and the next clock events to program are always before the current time.
This patch implements a reasonable compromise. If such a situation is detected, we share the CPUs time in 1/4 to process the hrtimer interrupts. This is enough to let the system running without serious starvation.

It has been successfully tested under VirtualBox with 1000 HZ and 100 HZ with function graph tracer launched. On both cases, the clock events were increased until about 25 ms periodic ticks, which means 40 HZ.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Thomas Gleixner <[email protected]>

---
 kernel/hrtimer.c |   30 +++++++++++++++++++++++++++++-

 1 files changed, 29 insertions(+), 1 deletions(-)

kernel/hrtimer.c +0 -0

index bda9cb9..02f2477 100644
@@ -1171,6 +1171,29 @@ static void __run_hrtimer(struct hrtimer *timer)

 #ifdef CONFIG_HIGH_RES_TIMERS

+static int force_clock_reprogram;

kernel/hrtimer.c +22 -0

+
+/*
+ * After 5 iteration's attempts, we consider that hrtimer_interrupt()
+ * is hanging, which could happen with something that slows the interrupt
+ * such as the tracing. Then we force the clock reprogramming for each future
+ * hrtimer interrupts to avoid infinite loops and use the min_delta_ns 
+ * threshold that we will overwrite. 
+ * The next tick event will be scheduled to 3 times we currently spend on
+ * hrtimer_interrupt(). This gives a good compromise, the cpus will spend
+ * 1/4 of their time to process the hrtimer interrupts. This is enough to
+ * let it running without serious starvation. 
+ */
+
+static inline void
+hrtimer_interrupt_hanging(struct clock_event_device *dev,
+                       ktime_t try_time)
+{
+       force_clock_reprogram = 1;
+       dev->min_delta_ns = (unsigned long)try_time.tv64 * 3;
+       printk(KERN_WARNING "hrtimer: interrupt too slow, "
+               "forcing clock min delta to %lu ns\n", dev->min_delta_ns);
+} 

 /*
  * High resolution timer interrupt
  * Called with interrupts disabled

kernel/hrtimer.c +1 -0

@@ -1180,6 +1203,7 @@ void hrtimer_interrupt(struct clock_event_device *dev)
        struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
        struct hrtimer_clock_base *base; 
         ktime_t expires_next, now;
+       int nr_retries = 0;
         int i;

        BUG_ON(!cpu_base->hres_active);

kernel/hrtimer.c +0 -0

@@ -1187,6 +1211,10 @@ void hrtimer_interrupt(struct clock_event_device *dev)
        dev->next_event.tv64 = KTIME_MAX; 

  retry:
+       /* 5 retries is enough to notice a hang */
+       if (!(++nr_retries % 5))
+               hrtimer_interrupt_hanging(dev, ktime_sub(ktime_get(), now)); + now = ktime_get();

        expires_next.tv64 = KTIME_MAX;

Quoted 10 lines

Impact: avoid hanging on slow systems

While using the function graph tracer on a virtualized system, the hrtimer_interrupt can hang the system on an infinite loop. This can be caused on several situation where something intrusive is slowing the system (ie: tracing) and the next clock events to program are always before the current time. This patch implements a reasonable compromise. If such a situation is detected, we share the CPUs time in 1/4 to process the hrtimer interrupts. This is enough to let the system running without serious starvation.

Should there maybe also be a mechanism to allow the system to automatically "recover" to higher (the original?) clockfrequencies, for example if the danger of loops has passed after tracing has been disabled?

Quoted 1 line

+static int force_clock_reprogram;

Shouldn't this be initialized to 0?

kernel/hrtimer.c void hrtimer_interrupt(struct clock_event_device *dev) · line 1272

Quoted 8 lines

kernel/hrtimer.c +1 -1

@@ -1239,7 +1267,7 @@ void hrtimer_interrupt(struct clock_event_device *dev) 
         /* Reprogramming necessary ? */
         if (expires_next.tv64 != KTIME_MAX) {
-               if (tick_program_event(expires_next, 0))
+               if (tick_program_event(expires_next, force_clock_reprogram))
 			goto retry; 
         }
  }

Shouldn't force_clock_reprogram be reset to 0 after it has fired and been handled?

Quoted 12 lines

Quoted 11 lines

Impact: avoid hanging on slow systems

While using the function graph tracer on a virtualized system, the hrtimer_interrupt can hang the system on an infinite loop. This can be caused on several situation where something intrusive is slowing the system (ie: tracing) and the next clock events to program are always before the current time. This patch implements a reasonable compromise. If such a situation is detected, we share the CPUs time in 1/4 to process the hrtimer interrupts. This is enough to let the system running without serious starvation.

Quoted 4 lines

Should there maybe also be a mechanism to allow the system to automatically "recover" to higher (the original?) clockfrequencies, for example if the danger of loops has passed after tracing has been disabled?

I dont think that's necessary - tick_dev_program_event() already includes a gradual approach that increases the 'min delta' interval step by step - so we should find the 'system is limping along' compromise pretty accurately.

A system can get "magically faster" later on (if we turn off tracing or other kernel features that impact the cost of the timer tick), and we might not need those safety measures anymore - but here the real solution will be to make the virtualizer faster. Taking milliseconds to process a timer tick (be that under tracing or not) is rather slow. So the kernel has applied the brakes and has printed a warning about it - we should do no more.

Quoted 3 lines

Quoted 1 line

+static int force_clock_reprogram;

Shouldn't this be initialized to 0?

no - global or static scope variables are initialized to zero in C.

Quoted 11 lines

kernel/hrtimer.c void hrtimer_interrupt(struct clock_event_device *dev) · line 1272

Quoted 8 lines

kernel/hrtimer.c +1 -1

@@ -1239,7 +1267,7 @@ void hrtimer_interrupt(struct clock_event_device *dev)
        /* Reprogramming necessary ? */
        if (expires_next.tv64 != KTIME_MAX) {
-               if (tick_program_event(expires_next, 0))
+               if (tick_program_event(expires_next, force_clock_reprogram))
                    goto retry;
        }
 }

Shouldn't force_clock_reprogram be reset to 0 after it has fired and been handled?

hm, that would be interesting to see - in theory the system should become stable after a few iterations of increasing min_delta. Frederic, is the system still workable if you try what Frans has suggested?

Also, there's min_delta doubling in tick_dev_program_event() itself too - that interacts with the irq-overload logic:

+ dev->min_delta_ns = (unsigned long)try_time.tv64 * 3;

Ingo

Quoted 11 lines

Impact: avoid hanging on slow systems

While using the function graph tracer on a virtualized system, the hrtimer_interrupt can hang the system on an infinite loop. This can be caused on several situation where something intrusive is slowing the system (ie: tracing) and the next clock events to program are always before the current time.
This patch implements a reasonable compromise. If such a situation is detected, we share the CPUs time in 1/4 to process the hrtimer interrupts. This is enough to let the system running without serious starvation.

Should there maybe also be a mechanism to allow the system to automatically "recover" to higher (the original?) clockfrequencies, for example if the danger of loops has passed after tracing has been disabled?

Quoted 15 lines

It has been successfully tested under VirtualBox with 1000 HZ and 100 HZ with function graph tracer launched. On both cases, the clock events were increased until about 25 ms periodic ticks, which means 40 HZ.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Thomas Gleixner <[email protected]>

---
 kernel/hrtimer.c |   30 +++++++++++++++++++++++++++++-
 1 files changed, 29 insertions(+), 1 deletions(-)

kernel/hrtimer.c +0 -0

index bda9cb9..02f2477 100644

kernel/hrtimer.c static void __run_hrtimer(struct hrtimer *timer) · line 1174

Quoted 4 lines

kernel/hrtimer.c +1 -0

@@ -1171,6 +1171,29 @@ static void __run_hrtimer(struct hrtimer *timer)
 
 #ifdef CONFIG_HIGH_RES_TIMERS
 
+static int force_clock_reprogram;

Shouldn't this be initialized to 0?

Quoted 42 lines

kernel/hrtimer.c +26 -0

+
+/*
+ * After 5 iteration's attempts, we consider that hrtimer_interrupt()
+ * is hanging, which could happen with something that slows the interrupt
+ * such as the tracing. Then we force the clock reprogramming for each future
+ * hrtimer interrupts to avoid infinite loops and use the min_delta_ns
+ * threshold that we will overwrite.
+ * The next tick event will be scheduled to 3 times we currently spend on
+ * hrtimer_interrupt(). This gives a good compromise, the cpus will spend
+ * 1/4 of their time to process the hrtimer interrupts. This is enough to
+ * let it running without serious starvation.
+ */
+
+static inline void
+hrtimer_interrupt_hanging(struct clock_event_device *dev,
+                       ktime_t try_time)
+{
+       force_clock_reprogram = 1;
+       dev->min_delta_ns = (unsigned long)try_time.tv64 * 3;
+       printk(KERN_WARNING "hrtimer: interrupt too slow, "
+               "forcing clock min delta to %lu ns\n", dev->min_delta_ns);
+}
 /*
  * High resolution timer interrupt
  * Called with interrupts disabled

@@ -1180,6 +1203,7 @@ void hrtimer_interrupt(struct clock_event_device *dev)
        struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
        struct hrtimer_clock_base *base;
        ktime_t expires_next, now;
+       int nr_retries = 0;
        int i;
 
        BUG_ON(!cpu_base->hres_active);

@@ -1187,6 +1211,10 @@ void hrtimer_interrupt(struct clock_event_device *dev)
        dev->next_event.tv64 = KTIME_MAX;
 
  retry:
+       /* 5 retries is enough to notice a hang */
+       if (!(++nr_retries % 5))
+               hrtimer_interrupt_hanging(dev, ktime_sub(ktime_get(), now)); +
        now = ktime_get();
 

kernel/hrtimer.c void hrtimer_interrupt(struct clock_event_device *dev) · line 1272

Quoted 9 lines

kernel/hrtimer.c +1 -1

        expires_next.tv64 = KTIME_MAX;

@@ -1239,7 +1267,7 @@ void hrtimer_interrupt(struct clock_event_device *dev)
        /* Reprogramming necessary ? */
        if (expires_next.tv64 != KTIME_MAX) {
-               if (tick_program_event(expires_next, 0))
+               if (tick_program_event(expires_next, force_clock_reprogram))
                      goto retry;
        }
 }

Shouldn't force_clock_reprogram be reset to 0 after it has fired and been handled?

Quoted 6 lines

Also, there's min_delta doubling in tick_dev_program_event() itself too - that interacts with the irq-overload logic:

+ dev->min_delta_ns = (unsigned long)try_time.tv64 * 3;

Ingo

That's what I explained above, the doubling of min delta in tick_dev_program_event() is never reached in my case. And moreover I'm not sure it is ever reached whatever the call sites of tick_dev_program_event() unless min delta has a very low value...

Quoted 4 lines

Quoted 4 lines

Also, there's min_delta doubling in tick_dev_program_event() itself too - that interacts with the irq-overload logic:

Quoted 8 lines

Quoted 3 lines

+ dev->min_delta_ns = (unsigned long)try_time.tv64 * 3;

Ingo

That's what I explained above, the doubling of min delta in tick_dev_program_event() is never reached in my case. And moreover I'm not sure it is ever reached whatever the call sites of tick_dev_program_event() unless min delta has a very low value...

yeah, that looks all rather messy and probably ineffective. Thomas, mind to take a look?

Frederic's patch solves a real timer-irq-overload extreme situation so i find it rather valuable. Maybe we should do Frederic's patch as-is, have the 'softer' min_delta behavior for the usual hrtimer codepaths - and the more agressive one for the timer tick only?

Ingo

[Frederic Weisbecker - Mon, Dec 22, 2008 at 02:24:48AM +0100]

| Impact: avoid hanging on slow systems
|
| While using the function graph tracer on a virtualized system, the hrtimer_interrupt
| can hang the system on an infinite loop.
| This can be caused on several situation where something intrusive is slowing the
| system (ie: tracing) and the next clock events to program are always before the current
| time.
| This patch implements a reasonable compromise. If such a situation is detected, we share
| the CPUs time in 1/4 to process the hrtimer interrupts. This is enough to let the system
| running without serious starvation.
|
| It has been successfully tested under VirtualBox with 1000 HZ and 100 HZ with function graph
| tracer launched. On both cases, the clock events were increased until about 25 ms periodic ticks,
| which means 40 HZ.
|
| Signed-off-by: Frederic Weisbecker <[email protected]>
| Cc: Thomas Gleixner <[email protected]>
| ---
|  kernel/hrtimer.c |   30 +++++++++++++++++++++++++++++-
|  1 files changed, 29 insertions(+), 1 deletions(-)
|
| diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
| index bda9cb9..02f2477 100644
| --- a/kernel/hrtimer.c
| +++ b/kernel/hrtimer.c
| @@ -1171,6 +1171,29 @@ static void __run_hrtimer(struct hrtimer *timer)
|
|  #ifdef CONFIG_HIGH_RES_TIMERS
|
| +static int force_clock_reprogram;
| +
| +/*
| + * After 5 iteration's attempts, we consider that hrtimer_interrupt()
| + * is hanging, which could happen with something that slows the interrupt
| + * such as the tracing. Then we force the clock reprogramming for each future
| + * hrtimer interrupts to avoid infinite loops and use the min_delta_ns
| + * threshold that we will overwrite.
| + * The next tick event will be scheduled to 3 times we currently spend on
| + * hrtimer_interrupt(). This gives a good compromise, the cpus will spend
| + * 1/4 of their time to process the hrtimer interrupts. This is enough to
| + * let it running without serious starvation.
| + */
| +
| +static inline void
| +hrtimer_interrupt_hanging(struct clock_event_device *dev,
| +                     ktime_t try_time)
| +{
| +     force_clock_reprogram = 1;
| +     dev->min_delta_ns = (unsigned long)try_time.tv64 * 3;
| +     printk(KERN_WARNING "hrtimer: interrupt too slow, "
| +             "forcing clock min delta to %lu ns\n", dev->min_delta_ns);
| +}
|  /*
|   * High resolution timer interrupt
|   * Called with interrupts disabled
| @@ -1180,6 +1203,7 @@ void hrtimer_interrupt(struct clock_event_device *dev)
|       struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
|       struct hrtimer_clock_base *base;
|       ktime_t expires_next, now;
| +     int nr_retries = 0;
|       int i;
|
|       BUG_ON(!cpu_base->hres_active);
| @@ -1187,6 +1211,10 @@ void hrtimer_interrupt(struct clock_event_device *dev)
|       dev->next_event.tv64 = KTIME_MAX;
|
|   retry:
| +     /* 5 retries is enough to notice a hang */
| +     if (!(++nr_retries % 5))
| +             hrtimer_interrupt_hanging(dev, ktime_sub(ktime_get(), now));
| +
|       now = ktime_get();

Hi Frederic,

is it really needed to use mod operation here?

Why cant we test for plain 5 and flush it to zero then? I mean something like

       if (++nr_retries > 5) {
               nr_retries = 0;
               ...
       }

Did I miss anything?

- Cyrill -

Quoted 2 lines

Shouldn't force_clock_reprogram be reset to 0 after it has fired and been handled?

No, we need to keep the clock reprogramming forcing for each next hrtimer interrupt if we want the min delta to be used inside tick_dev_program_event().

Impact: avoid hanging on slow systems

While using the function graph tracer on a virtualized system, the hrtimer_interrupt can hang the system on an infinite loop. This can be caused on several situation where something intrusive is slowing the system (ie: tracing) and the next clock events to program are always before the current time. This patch implements a reasonable compromise. If such a situation is detected, we share the CPUs time in 1/4 to process the hrtimer interrupts. This is enough to let the system running without serious starvation.

It has been successfully tested under VirtualBox with 1000 HZ and 100 HZ with function graph tracer launched. On both cases, the clock events were increased until about 25 ms periodic ticks, which means 40 HZ.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Thomas Gleixner <[email protected]>

---
 kernel/hrtimer.c |   30 +++++++++++++++++++++++++++++-
 1 files changed, 29 insertions(+), 1 deletions(-)

Quoted 15 lines · Frederic Weisbecker

Impact: avoid hanging on slow systems

While using the function graph tracer on a virtualized system, the hrtimer_interrupt can hang the system on an infinite loop. This can be caused on several situation where something intrusive is slowing the system (ie: tracing) and the next clock events to program are always before the current time. This patch implements a reasonable compromise. If such a situation is detected, we share the CPUs time in 1/4 to process the hrtimer interrupts. This is enough to let the system running without serious starvation.

It has been successfully tested under VirtualBox with 1000 HZ and 100 HZ with function graph tracer launched. On both cases, the clock events were increased until about 25 ms periodic ticks, which means 40 HZ.

Quoted 5 lines

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Thomas Gleixner <[email protected]>

---
 kernel/hrtimer.c |   30 +++++++++++++++++++++++++++++-
 1 files changed, 29 insertions(+), 1 deletions(-)

applied to tip/timers/hrtimers, thanks Frederic!

Thomas, any objections/observations?

Ingo