On 9 June 2014 18:29, Stanislav Fomichev <[email protected]> wrote: > In hrtimer_get_next_event we unconditionally lock/unlock spinlock, even if > it's > not required (hrtimer_hres_active() != 0). This patch adds fast path > when highres is active so we don't execute unnecessary operations. > > We can safely do lockless check because: > - hrtimer_get_next_event is always called with interrupts disabled; > - we may switch to hres only from softirq handler with disabled interrupts. > > Because we only care about hres_active which may be changed only from > local CPU, we can use interrupt context for synchronization. > > run_timer_softirq > hrtimer_run_pending > hrtimer_switch_to_hres > local_irq_save <- > base->hres_active = 0 > > tick_nohz_idle_enter > local_irq_disable <- > __tick_nohz_idle_enter > tick_nohz_stop_sched_tick > get_next_timer_interrupt > cmp_next_hrtimer_event > hrtimer_get_next_event > check base->hres_active > > irq_exit <- irq context > tick_irq_exit > tick_nohz_irq_exit > tick_nohz_full_stop_tick > tick_nohz_stop_sched_tick > ... <see above> > __tick_nohz_idle_enter > ... <see above> > > Signed-off-by: Stanislav Fomichev <[email protected]> > --- > kernel/hrtimer.c | 27 ++++++++++++++------------- > 1 file changed, 14 insertions(+), 13 deletions(-) > > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c > index ba340739c701..731be91e927f 100644 > --- a/kernel/hrtimer.c > +++ b/kernel/hrtimer.c > @@ -1167,23 +1167,24 @@ ktime_t hrtimer_get_next_event(void) > unsigned long flags; > int i; > > + if (hrtimer_hres_active()) > + return mindelta; > + > raw_spin_lock_irqsave(&cpu_base->lock, flags); > > - if (!hrtimer_hres_active()) { > - for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) { > - struct hrtimer *timer; > - struct timerqueue_node *next; > + for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) { > + struct hrtimer *timer; > + struct timerqueue_node *next; > > - next = timerqueue_getnext(&base->active); > - if (!next) > - continue; > + next = timerqueue_getnext(&base->active); > + if (!next) > + continue; > > - timer = container_of(next, struct hrtimer, node); > - delta.tv64 = hrtimer_get_expires_tv64(timer); > - delta = ktime_sub(delta, base->get_time()); > - if (delta.tv64 < mindelta.tv64) > - mindelta.tv64 = delta.tv64; > - } > + timer = container_of(next, struct hrtimer, node); > + delta.tv64 = hrtimer_get_expires_tv64(timer); > + delta = ktime_sub(delta, base->get_time()); > + if (delta.tv64 < mindelta.tv64) > + mindelta.tv64 = delta.tv64; > } > > raw_spin_unlock_irqrestore(&cpu_base->lock, flags);
Even I was concerned about a bigger diff earlier and so didn't comment to rewrite it this way :) Codewise, looks much better.. Reviewed-by: Viresh Kumar <[email protected]> -- 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/
