On Thu, May 22, 2014 at 06:38:29PM +0530, Viresh Kumar wrote:
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.
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 should also happen with NO_HZ_IDLE. It's rather a wide NO_HZ concern.
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. 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).
A simple 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.
Thomas suggested to add an optional mode: CLOCK_EVT_MODE_ONESHOT_STOPPED (lkml.org/lkml/2014/5/9/508) to get tihs fixed.
This requires ->set_mode() callback to have capability to return 'success' or 'failure' for this new mode. 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 patch adds support for ->set_dev_mode() in clockevents core.
For now, just add a WARN_ON() to callers of ->set_dev_mode() as none of currently implemented modes are expected/allowed to fail.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
include/linux/clockchips.h | 5 ++++- kernel/time/clockevents.c | 17 ++++++++++++++--- kernel/time/timer_list.c | 5 ++++- 3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 2e4cb67..8ab1a86 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -81,7 +81,8 @@ enum clock_event_mode {
- @mode: operating mode assigned by the management code
- @features: features
- @retries: number of forced programming retries
- @set_mode: set mode function
- @set_dev_mode: set dev mode function
- @set_mode: set mode function (deprecated, use set_dev_mode instead)
- @broadcast: function to broadcast events
- @min_delta_ticks: minimum delta value in ticks stored for reconfiguration
- @max_delta_ticks: maximum delta value in ticks stored for reconfiguration
@@ -109,6 +110,8 @@ struct clock_event_device { unsigned long retries; void (*broadcast)(const struct cpumask *mask);
- int (*set_dev_mode)(enum clock_event_mode mode,
void (*set_mode)(enum clock_event_mode mode, struct clock_event_device *); void (*suspend)(struct clock_event_device *);struct clock_event_device *);
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c index ad362c2..3cf7455 100644 --- a/kernel/time/clockevents.c +++ b/kernel/time/clockevents.c @@ -105,7 +105,14 @@ void clockevents_set_mode(struct clock_event_device *dev, enum clock_event_mode mode) { if (dev->mode != mode) {
dev->set_mode(mode, dev);
if (dev->set_dev_mode) {
/* WARN: Currently available modes shouldn't fail */
if (WARN_ON(dev->set_dev_mode(mode, dev)))
Rather use WARN_ON_ONCE(), in general it's much better. Also probably the error handling should be done in this patchset?
Thanks.
return;
} else {
dev->set_mode(mode, dev);
}
- dev->mode = mode;
/* @@ -446,8 +453,12 @@ int __clockevents_update_freq(struct clock_event_device *dev, u32 freq) if (dev->mode == CLOCK_EVT_MODE_ONESHOT) return clockevents_program_event(dev, dev->next_event, false);
- if (dev->mode == CLOCK_EVT_MODE_PERIODIC)
dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);
- if (dev->mode == CLOCK_EVT_MODE_PERIODIC) {
if (dev->set_dev_mode)
WARN_ON(dev->set_dev_mode(CLOCK_EVT_MODE_PERIODIC, dev));
else
dev->set_mode(CLOCK_EVT_MODE_PERIODIC, dev);
- }
return 0; } diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c index 61ed862..957a049 100644 --- a/kernel/time/timer_list.c +++ b/kernel/time/timer_list.c @@ -229,7 +229,10 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu) SEQ_printf(m, "\n"); SEQ_printf(m, " set_mode: ");
- print_name_offset(m, dev->set_mode);
- if (dev->set_dev_mode)
print_name_offset(m, dev->set_dev_mode);
- else
SEQ_printf(m, "\n");print_name_offset(m, dev->set_mode);
SEQ_printf(m, " event_handler: "); -- 2.0.0.rc2