LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
To: Viresh Kumar <viresh.kumar@linaro.org>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: linaro-kernel@lists.linaro.org, linux-kernel@vger.kernel.org,
	Kevin Hilman <khilman@linaro.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Frederic Weisbecker <fweisbec@gmail.com>
Subject: Re: [PATCH 2/3] clockevents: Restart clockevent device before using it again
Date: Mon, 30 Mar 2015 11:22:28 +0530	[thread overview]
Message-ID: <5518E49C.3090507@linux.vnet.ibm.com> (raw)
In-Reply-To: <a8bce41503c36f6de6be94b0f8854ab646f6745d.1427475606.git.viresh.kumar@linaro.org>

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@linaro.org>
> ---
>  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@linux.vnet.ibm.com>


  reply	other threads:[~2015-03-30  5:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-27 17:14 [PATCH 0/3] clockevents: Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED Viresh Kumar
2015-03-27 17:14 ` [PATCH 1/3] clockevents: Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED state Viresh Kumar
2015-03-30  5:49   ` Preeti U Murthy
2015-04-06 21:26   ` Kevin Hilman
2015-03-27 17:14 ` [PATCH 2/3] clockevents: Restart clockevent device before using it again Viresh Kumar
2015-03-30  5:52   ` Preeti U Murthy [this message]
2015-04-02 13:34   ` Peter Zijlstra
2015-04-02 13:50     ` Viresh Kumar
2015-04-02 15:06       ` Peter Zijlstra
2015-04-02 16:04         ` Ingo Molnar
2015-03-27 17:14 ` [PATCH 3/3] clockevents: Switch state to ONESHOT_STOPPED for unused clockevent devices Viresh Kumar
2015-03-30  5:50   ` Preeti U Murthy
2015-04-02 13:37   ` Peter Zijlstra
2015-04-02 13:51     ` Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5518E49C.3090507@linux.vnet.ibm.com \
    --to=preeti@linux.vnet.ibm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=fweisbec@gmail.com \
    --cc=khilman@linaro.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=viresh.kumar@linaro.org \
    --subject='Re: [PATCH 2/3] clockevents: Restart clockevent device before using it again' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).