Re: [PATCH] hrtimer: increase clock min delta threshold while interrupt hanging

[Frans Pop <elendil@planet.nl>]

> 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?

> 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 <fweisbec@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
>  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;

Shouldn't this be initialized to 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();
>  
>         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?