Re: [PATCH 2/3] clockevents: Restart clockevent device before using it again

[Preeti U Murthy]

On 03/27/2015 10:44 PM, Viresh Kumar wrote:
> Next commit would update clockevents core to switch clockevent devices to
> ONESHOT_STOPPED state to avoid getting spurious interrupts on a tickless CPU.
> 
> Before programming the clockevent device for next event, we must switch its
> state to ONESHOT.
> 
> Switch state back to ONESHOT from ONESHOT_STOPPED at following places:
> 
> 1.) NOHZ_MODE_LOWRES Mode
> 
> Timers & hrtimers are dependent on tick for their working in this mode and the
> only place from where clockevent device is programmed is the tick-code.
> 
> Two routines can restart ticks in LOWRES mode: tick_nohz_restart() and
> tick_nohz_stop_sched_tick().
> 
> 2.) NOHZ_MODE_HIGHRES Mode
> 
> Tick & timers are dependent on hrtimers for their working in this mode and the
> only place from where clockevent device is programmed is the hrtimer-code.
> 
> Only hrtimer_reprogram() is responsible for programming the clockevent device
> for next event, if the clockevent device is stopped earlier. And updating that
> alone is sufficient here.
> 
> To make sure we haven't missed any corner case, add a WARN() for the case where
> we try to reprogram clockevent device while we aren't configured in
> ONESHOT_STOPPED state.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> ---
>  kernel/time/clockevents.c |  4 ++++
>  kernel/time/hrtimer.c     |  5 +++++
>  kernel/time/tick-sched.c  | 14 +++++++++++++-
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
> index 04f6c3433f8e..e95896fd21fd 100644
> --- a/kernel/time/clockevents.c
> +++ b/kernel/time/clockevents.c
> @@ -335,6 +335,10 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
>  	if (dev->state == CLOCK_EVT_STATE_SHUTDOWN)
>  		return 0;
> 
> +	/* We must be in ONESHOT state here */
> +	WARN_ONCE(dev->state != CLOCK_EVT_STATE_ONESHOT, "Current state: %d\n",
> +		  dev->state);
> +
>  	/* Shortcut for clockevent devices that can deal with ktime. */
>  	if (dev->features & CLOCK_EVT_FEAT_KTIME)
>  		return dev->set_next_ktime(expires, dev);
> diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
> index bee0c1f78091..045ba7e2be6c 100644
> --- a/kernel/time/hrtimer.c
> +++ b/kernel/time/hrtimer.c
> @@ -566,6 +566,7 @@ static int hrtimer_reprogram(struct hrtimer *timer,
>  {
>  	struct hrtimer_cpu_base *cpu_base = this_cpu_ptr(&hrtimer_bases);
>  	ktime_t expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
> +	struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev);
>  	int res;
> 
>  	WARN_ON_ONCE(hrtimer_get_expires_tv64(timer) < 0);
> @@ -610,6 +611,10 @@ static int hrtimer_reprogram(struct hrtimer *timer,
>  	if (cpu_base->hang_detected)
>  		return 0;
> 
> +	/* Switchback to ONESHOT state */
> +	if (unlikely(dev->state == CLOCK_EVT_STATE_ONESHOT_STOPPED))
> +		clockevents_set_state(dev, CLOCK_EVT_STATE_ONESHOT);
> +
>  	/*
>  	 * Clockevents returns -ETIME, when the event was in the past.
>  	 */
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index a4c4edac4528..47c04edd07df 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -694,8 +694,14 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
>  			/* Check, if the timer was already in the past */
>  			if (hrtimer_active(&ts->sched_timer))
>  				goto out;
> -		} else if (!tick_program_event(expires, 0))
> +		} else {
> +			/* Switchback to ONESHOT state */
> +			if (unlikely(dev->state == CLOCK_EVT_STATE_ONESHOT_STOPPED))
> +				clockevents_set_state(dev, CLOCK_EVT_STATE_ONESHOT);
> +
> +			if (!tick_program_event(expires, 0))
>  				goto out;
> +		}
>  		/*
>  		 * We are past the event already. So we crossed a
>  		 * jiffie boundary. Update jiffies and raise the
> @@ -873,6 +879,8 @@ ktime_t tick_nohz_get_sleep_length(void)
> 
>  static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
>  {
> +	struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev);
> +
>  	hrtimer_cancel(&ts->sched_timer);
>  	hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
> 
> @@ -887,6 +895,10 @@ static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
>  			if (hrtimer_active(&ts->sched_timer))
>  				break;
>  		} else {
> +			/* Switchback to ONESHOT state */
> +			if (likely(dev->state == CLOCK_EVT_STATE_ONESHOT_STOPPED))
> +				clockevents_set_state(dev, CLOCK_EVT_STATE_ONESHOT);
> +
>  			if (!tick_program_event(
>  				hrtimer_get_expires(&ts->sched_timer), 0))
>  				break;
> 
Reviewed-by: Preeti U. Murthy <preeti@xxxxxxxxxxxxxxxxxx>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/