On Tue, Jun 17, 2014 at 02:18:13PM +0530, Viresh Kumar wrote:
In lowres mode, hrtimers are serviced by the tick. When a hrtimer is enqueued on a target, that CPU must re-evaluate the next tick to service that hrtimer.
Reusing the changelogs of periodic timers kind of works here. But it's a bit awkward to put things that way.
The changelog seem to assume that it's obvious that we must kick the target of a lowres hrtimer whereas we actually never did it.
I mean here the issue is not some missing pieces to be fixed as in periodic timers, we need to zoom out a bit on the whole subsystem support for dynticks.
Maybe that could be reworded to something like:
"In lowres mode, hrtimers are serviced by tick instead of a clock event. Now being serviced by the tick imply that we must deal with dynticks mode, pretty much like timer list timers do.
There are two requirements to correctly handle dynticks:
1) On target's tick stop time, we must not delay the next tick further the next hrtimer 2) On hrtimer queue time. If the tick of the target is stopped, we must wake up that CPU such that it sees the new hrtimer and recalculate the next tick.
The point 1 is well handled currently through get_nohz_timer_interrupt() and cmp_next_hrtimer_event().
But the point 2 isn't handled at all.
Fixing this is easy though as we have the necessary API ready for that. All we need is to call wake_up_nohz_cpu() on a target when a newly enqueued timer requires tick rescheduling, like timer list timer do."
Thanks.
get_nohz_timer_target() doesn't return a remote idle target but there is a small race window here. The elected target has all the time to turn dynticks idle between the call to get_nohz_timer_target() and the locking of its base. Hence a risk that we enqueue a hrtimer on a dynticks idle destination without kicking it. As a result, the hrtimer might be serviced too late in the future.
Also a target elected by get_nohz_timer_target() can be in full dynticks mode and thus require to be kicked as well. And unlike idle dynticks, this concern both local and remote targets.
To fix this, lets centralize dynticks kick to __hrtimer_start_range_ns(), so that it is well handled for all sort of hrtimer enqueue APIs. It will be performed by an IPI kick on the target, exactly the way it is performed for periodic timers currently.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
kernel/hrtimer.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index dd6fb1d..8f0c79c 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -1013,14 +1013,25 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, leftmost = enqueue_hrtimer(timer, new_base);
- /*
* Only allow reprogramming if the new base is on this CPU.* (it might still be on another CPU if the timer was pending)** XXX send_remote_softirq() ?*/- if (leftmost && new_base->cpu_base == &__get_cpu_var(hrtimer_bases)
&& hrtimer_enqueue_reprogram(timer, new_base)) {
- if (!leftmost) {
unlock_hrtimer_base(timer, &flags);return 0;- }
- if (!hrtimer_is_hres_active(timer)) {
/** Kicked to reevaluate the timer wheel for both idle and full* dynticks target.*/wake_up_nohz_cpu(base->cpu_base->cpu);- } else if (new_base->cpu_base == &__get_cpu_var(hrtimer_bases) &&
hrtimer_enqueue_reprogram(timer, new_base)) {/** Only allow reprogramming if the new base is on this CPU.* (it might still be on another CPU if the timer was pending)** XXX send_remote_softirq() ?*/-- 2.0.0.rc2