Re: kvm guest: hrtimer: interrupt too slow

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 8 Oct 2009, Marcelo Tosatti wrote:
> On Thu, Oct 08, 2009 at 10:05:01AM +0200, Thomas Gleixner wrote:
> > On Wed, 7 Oct 2009, Marcelo Tosatti wrote:
> > > On Thu, Oct 08, 2009 at 01:17:35AM +0200, Frederic Weisbecker wrote:
> > > What about getting rid of the retry loop, instead? So something
> > > like:
> > > 
> > > - run hrtimer callbacks (once)
> > > - while (tick_program_event(expires))
> > > 	expires = ktime_add_ns(expires, dev->min_delta_ns)
> > > 
> > > This way there's no static tuning involved.
> > 
> > And what does that buy us ? We get an timer interrupt right away, so
> > it's not that much different from the retry loop. See below.
> > 
> > > Its not clear to me why the loop is there in the first place.
> > 
> > We get a timer interrupt and handle the expired timers and find out
> > the timer which is going to expire next to reprogram the hardware. Now
> > when we program that expiry time we find out that the timer is already
> > expired. So instead of programming the hardware to fire an interrupt
> > in the very near future which you would do with your loop above we
> > stay in the interrupt handler and expire the timer and any other by
> > now expired timers right away.
> > 
> > The hang check is just there to avoid starving (slow) machines. We do
> > this by spreading the timer interrupts out so that the system can do
> > something else than expiring timers.
> 
> OK, makes sense.
> 
> So why not program only the next tick using the heuristic, without 
> touching min_delta_ns?

That makes a certain amount of sense albeit I really hate that
heuristics crap especially when we know that we run as a guest. 

We better add a function pointer to the clock event device struct
which defaults to "force it to be slow" for real hardware and can be
overridden by paravirt guests.

Also it's not clear to me why the problem does only happen with
kvm_clock and not with acpi_pm timer emulation (according to the
reporter) and is restricted to SMP guests.

>   retry:
>  	/* 5 retries is enough to notice a hang */
> -	if (!(++nr_retries % 5))
> -		hrtimer_interrupt_hanging(dev, ktime_sub(ktime_get(), now));
> +	if (!(++nr_retries % 5)) {
> +		ktime_t try_time = ktime_sub(ktime_get(), now);
> +
> +		do {
> +			for (i = 0; i < 3; i++)
> +				expires_next = ktime_add(expires_next,try_time);
> +		} while (tick_program_event(expires_next, 0));

  This needs at least a WARN_ON_ONCE() or some other way (sysfs, proc,
  ...) where we can find out how often this happens.

Thanks,

	tglx