On 3 June 2014 01:51, Kevin Hilman khilman@linaro.org wrote:
In general, your cover letters and change logs still mix up what things are like currently and what things will be like (or should be like) after this series. Now your adding another layer of adding things that will likely need fix *in addition to* this series. IMO, all three categories are still mixed up in the descriptions.
Hmm.. I tried to rewrite it a bit again after I found some more corner cases where we might have problems.. Tried to make it as clear as possible. Please have a look if it makes it more clear now:
A clockevent device is used to service timers/hrtimers requests and the next event (when it should fire) is decided by the timer/hrtimer expiring next. When no timers/hrtimers are pending to be serviced, the expiry time is set to a special value: KTIME_MAX. This means that no events are required for indefinite amount of time.
This would normally happen with NO_HZ_{IDLE|FULL} in both LOWRES/HIGHRES modes.
When expiry == KTIME_MAX, either we cancel the tick-sched hrtimer (NOHZ_MODE_HIGHRES) or skip reprogramming clockevent device (NOHZ_MODE_LOWRES). But, the clockevent device is already reprogrammed from tick-handler for next tick.
So, the clockevent device will fire one more time. In NOHZ_MODE_HIGHRES, we will consider it as a spurious interrupt and just return from hrtimer_interrupt(). In NOHZ_MODE_LOWRES, we schedule the next tick again from tick_nohz_handler()? (This is what I could read from the code, not very sure though. Otherwise, it means that in NOHZ_MODE_LOWRES we are never tickless).
Ideally, as the clock event device is programmed in ONESHOT mode it should just fire one more time and that's it. But many implementations (like arm_arch_timer, etc) only have PERIODIC mode available and their drivers emulate ONESHOT over that. Which means that on these platforms we will get spurious interrupts at tick rate and that will hurt our tickless-ness badly.
At this time the clockevent device should be stopped, or its interrupts may be masked in order to get these issues fixed.
A simple (yet hacky) solution to get this fixed could be: update hrtimer_force_reprogram() 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. Also, NOHZ_MODE_LOWRES problem mentioned above wouldn't be fixed by this.
However, Thomas suggested to add an optional mode: ONESHOT_STOPPED (lkml.org/lkml/2014/5/9/508) to solve this problem.
With this proposal, introducing a new ONESHOT_STOPPED mode would require the core to know whether the platform implements this mode so it could be reprogrammed later.
In order for the core to tell if the mode is implemented, ->set_mode() callback needs to be able to return success or failure.
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.