On 18 June 2014 18:23, Frederic Weisbecker fweisbec@gmail.com wrote:
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.
Correct !!
I have updated logs and pushed them back in my repo I shared with you earlier. I have updated 1/3 as well a bit with what you suggested..
Though I have kept some part from earlier log about how can we get idle/full dynticks CPU from get_nohz_timer_target(). Might be worth mentioning ?
New logs:
hrtimer: Kick lowres dynticks targets on hrtimer_start*() calls
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."
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
Any more improvements required?