lkml.org 
[lkml]   [2014]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V2 2/2] tick/cpuidle: Initialize hrtimer mode of broadcast
> +++ b/kernel/time/tick-broadcast-hrtimer.c
> +/*
> + * This is called from the guts of the broadcast code when the cpu
> + * which is about to enter idle has the earliest broadcast timer event.
> + */
> +static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
> +{
> + ktime_t now, interval;
> + /*
> + * We try to cancel the timer first. If the callback is on
> + * flight on some other cpu then we let it handle it. If we
> + * were able to cancel the timer nothing can rearm it as we
> + * own broadcast_lock.
> + *
> + * However if we are called from the hrtimer interrupt handler
> + * itself, reprogram it.
> + */
> + if (hrtimer_try_to_cancel(&bctimer) >= 0) {
> + hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED);
> + /* Bind the "device" to the cpu */
> + bc->bound_on = smp_processor_id();
> + } else if (bc->bound_on == smp_processor_id()) {

This part really wants a proper comment. It took me a while to figure
out why this is correct and what the call chain is.


> + now = ktime_get();
> + interval = ktime_sub(expires, now);
> + hrtimer_forward_now(&bctimer, interval);

We are in the event handler called from bc_handler() and expires is
absolute time. So what's wrong with calling
hrtimer_set_expires(&bctimer, expires)?

> +static enum hrtimer_restart bc_handler(struct hrtimer *t)
> +{
> + ce_broadcast_hrtimer.event_handler(&ce_broadcast_hrtimer);
> + return HRTIMER_RESTART;

We probably want to check whether the timer needs to be restarted at
all.

if (ce_broadcast_timer.next_event.tv64 == KTIME_MAX)
return HRTIMER_NORESTART;

return HRTIMER_RESTART;

Hmm?

Thanks,

tglx


\
 
 \ /
  Last update: 2014-02-04 12:01    [W:0.092 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site