Hi Viresh,
On 06/17/2014 01:50 PM, Viresh Kumar wrote:
When expiry is set to KTIME_MAX, we cancel the 'tick-sched' hrtimer in highres mode and skip reprogramming clockevent device in lowres mode. But, the clockevent device is already reprogrammed from tick-handler.
We will get interrupted atleast one more time.
In highres mode, as there is no hrtimer to service, hrtimer_interrupt() will return without calling tick-handler.
But the problem is somewhat bigger in lowres mode. We unconditionally reschedule tick everytime tick_nohz_handler() is called and so even if tick_stopped is set, we never enter full dynticks mode.
To fix this, skip rescheduling next tick from tick-handler when we are running tickless.
OTOH, when expires isn't equal to KTIME_MAX, we avoid reprogramming the clkdev from the tick when it is stopped because it's going to be reprogrammed from irq_exit() anyway. So we could avoid one useless device write.
I am sceptical about what this patch does when expires is not equal to KTIME_MAX. This, being the primary timer handler function in the nohz low resolution mode, it should skip programming *only* when the tick is stopped, indicating that the cpu was in idle (which this patch takes care of) and there were no pending timers queued between the time this cpu went idle and now. If there is a pending timer, it should program the clock device even if the tick was stopped.
I realise that in the irq_exit path, the periodic tick is re-started if there are pending timers, but is it right to rely on the irq exit path for this? What if the code in the irq_exit path optimizes some parts and removes restart of periodic ticks? In which case this code will break. My point is the timer handler function should be self sufficient as much as possible in taking care of programming the clock device.
Correct me if I am wrong but I don't think any other timer handler function relies on the irq_exit path to reprogram the clock device.
Regards Preeti U Murthy
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
kernel/time/tick-sched.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 6558b7a..a4a45e0 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -956,6 +956,12 @@ static void tick_nohz_handler(struct clock_event_device *dev) tick_sched_do_timer(now); tick_sched_handle(ts, regs);
- /*
* Skip reprogramming next event if we are running tickless.*/- if (ts->tick_stopped)
return;- while (tick_nohz_reprogram(ts, now)) { now = ktime_get(); tick_do_update_jiffies64(now);