Viresh Kumar viresh.kumar@linaro.org writes:
Hi Guys,
Here is another try to migrate clockevents to use set_dev_mode(). I have tried to fix all issues raised by Kevin/Preeti/Daniel. Also some updates are done to drivers to make sure they don't disable clockevents for unsupported modes, i.e. 'default' case.
Please do another round of review, it would be very helpful in getting these through tglx.
Overall a much better changelog, but some more comments below...
@Kevin: I have updated my branch again, please see if you do another round of automated tests for it.
Cover-letter for mainline:
A clockevent device can be stopped, or its events can be masked, if next event is expected at KTIME_MAX, i.e. no events are required for very long time.
The wording here is the wrong way around IMO. The way it's worded, it sounds like it's already the case. You're trying to make the case that this *should* happen, so make sure it's worded.
This normally happens with NO_HZ_FULL (when tick-sched timer is removed and we don't have any more timers scheduled for long), not sure if it can happen otherwise.
This isn't specific to NO_HZ_FULL, but will happen in regular tickless idle (NO_HZ_IDLE) also.
If we don't STOP clockevent device, we are guaranteed to receive atleast one fake interrupt, as hrtimer_force_reprogram() isn't reprogramming the event device.
I don't think "fake" is the right word. There's nothing fake about them. You might say unneeded or spurious instead.
Depending on particular implementation of clockevent device, this can be a fake interrupt at tick-rate.. (This was observed atleast on one implementation: arm_arch_timer.c).
I don't follow this part (or see the relevance to the series.) Was the continued spurious interrupt because that clockevent didn't implement one-shot mode, so was left in periodic mode?
A simple solution to get this fixed could be: update hrtimer_force_reprogram()
s/simple/simple (yet hacky)/ :)
to always reprogram clockevent device and update clockevent drivers to STOP generating events (or delay it to max time), when 'expires' is set to KTIME_MAX. But the drawback here is that every clockevent driver has to be hacked for this particular case and its very easy for new ones to miss this.
Thomas suggested to add an optional mode: CLOCK_EVT_MODE_ONESHOT_STOPPED
However, Thomas suggested...
(lkml.org/lkml/2014/5/9/508) to get tihs fixed.
s/get this fixed/solve this problem/
This requires ->set_mode() callback to have capability to return 'success' or 'failure' for this new mode.
Why? (yes, I know the answer, but it should be described here.)
Currently its return type is 'void'.
To change return type of set_mode() Thomas suggested to implement another callback: ->set_dev_mode(), with return type 'int'. We can then convert clockevent drivers to use this interface instead of existing ->set_mode() and then finally remove ->set_mode()'s support.
This patchset first adds another callback with return capability, set_dev_mode(), then it migrates all drivers one by one and finally removes earlier callback set_mode() when nobody is using it.
This is looking much better,
Kevin