From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752767AbbC3Fwt (ORCPT ); Mon, 30 Mar 2015 01:52:49 -0400 Received: from e36.co.us.ibm.com ([32.97.110.154]:39918 "EHLO e36.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751288AbbC3Fwo (ORCPT ); Mon, 30 Mar 2015 01:52:44 -0400 Message-ID: <5518E49C.3090507@linux.vnet.ibm.com> Date: Mon, 30 Mar 2015 11:22:28 +0530 From: Preeti U Murthy User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Viresh Kumar , Ingo Molnar , Peter Zijlstra , Thomas Gleixner CC: linaro-kernel@lists.linaro.org, linux-kernel@vger.kernel.org, Kevin Hilman , Daniel Lezcano , Frederic Weisbecker Subject: Re: [PATCH 2/3] clockevents: Restart clockevent device before using it again References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-6 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15033005-0021-0000-0000-00000980FAD5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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 > --- > 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