linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>
Cc: linaro-kernel@lists.linaro.org, linux-kernel@vger.kernel.org,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Kevin Hilman <khilman@linaro.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Preeti U Murthy <preeti@linux.vnet.ibm.com>,
	Viresh Kumar <viresh.kumar@linaro.org>
Subject: [PATCH V2 1/2] clockevents: Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED state
Date: Fri,  3 Apr 2015 09:04:04 +0530	[thread overview]
Message-ID: <b8b383a03ac07b13312c16850b5106b82e4245b5.1428031396.git.viresh.kumar@linaro.org> (raw)
In-Reply-To: <cover.1428031396.git.viresh.kumar@linaro.org>
In-Reply-To: <cover.1428031396.git.viresh.kumar@linaro.org>

When no timers/hrtimers are pending, the expiry time is set to a special value:
'KTIME_MAX'. This normally happens with NO_HZ_{IDLE|FULL} in both LOWRES/HIGHRES
modes.

When 'expiry == KTIME_MAX', we either cancel the 'tick-sched' hrtimer
(NOHZ_MODE_HIGHRES) or skip reprogramming clockevent device (NOHZ_MODE_LOWRES).
But, the clockevent device is already reprogrammed from the tick-handler for
next tick.

As the clock event device is programmed in ONESHOT mode it will at least fire
one more time (unnecessarily). Timers on few implementations (like
arm_arch_timer, etc.) only support PERIODIC mode and their drivers emulate
ONESHOT over that. Which means that on these platforms we will get spurious
interrupts periodically (at last programmed interval rate, normally tick rate).

In order to avoid spurious interrupts, the clockevent device should be stopped
or its interrupts should be masked.

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.

However, Thomas suggested to add an optional state ONESHOT_STOPPED to solve this
problem: lkml.org/lkml/2014/5/9/508.

This patch adds support for ONESHOT_STOPPED state in clockevents core. It will
only be available to drivers that implement the state-specific callbacks instead
of the legacy ->set_mode() callback.

Reviewed-by: Preeti U. Murthy <preeti@linux.vnet.ibm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/linux/clockchips.h |  7 ++++++-
 kernel/time/clockevents.c  | 14 +++++++++++++-
 kernel/time/timer_list.c   |  6 ++++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index caeca76d7b32..ee63865d3cd7 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -49,12 +49,15 @@ enum clock_event_mode {
  *		reached from DETACHED or SHUTDOWN.
  * ONESHOT:	Device is programmed to generate event only once. Can be reached
  *		from DETACHED or SHUTDOWN.
+ * ONESHOT_STOPPED: Device was programmed in ONESHOT mode and is temporarily
+ *		    stopped.
  */
 enum clock_event_state {
 	CLOCK_EVT_STATE_DETACHED,
 	CLOCK_EVT_STATE_SHUTDOWN,
 	CLOCK_EVT_STATE_PERIODIC,
 	CLOCK_EVT_STATE_ONESHOT,
+	CLOCK_EVT_STATE_ONESHOT_STOPPED,
 };
 
 /*
@@ -102,6 +105,7 @@ enum clock_event_state {
  * @set_mode:		legacy set mode function, only for modes <= CLOCK_EVT_MODE_RESUME.
  * @set_state_periodic:	switch state to periodic, if !set_mode
  * @set_state_oneshot:	switch state to oneshot, if !set_mode
+ * @set_state_oneshot_stopped: switch state to oneshot_stopped, if !set_mode
  * @set_state_shutdown:	switch state to shutdown, if !set_mode
  * @tick_resume:	resume clkevt device, if !set_mode
  * @broadcast:		function to broadcast events
@@ -133,11 +137,12 @@ struct clock_event_device {
 	 * State transition callback(s): Only one of the two groups should be
 	 * defined:
 	 * - set_mode(), only for modes <= CLOCK_EVT_MODE_RESUME.
-	 * - set_state_{shutdown|periodic|oneshot}(), tick_resume().
+	 * - set_state_{shutdown|periodic|oneshot|oneshot_stopped}(), tick_resume().
 	 */
 	void			(*set_mode)(enum clock_event_mode mode, struct clock_event_device *);
 	int			(*set_state_periodic)(struct clock_event_device *);
 	int			(*set_state_oneshot)(struct clock_event_device *);
+	int			(*set_state_oneshot_stopped)(struct clock_event_device *);
 	int			(*set_state_shutdown)(struct clock_event_device *);
 	int			(*tick_resume)(struct clock_event_device *);
 
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 7af614829da1..49ed2ea4294c 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -138,6 +138,17 @@ static int __clockevents_set_state(struct clock_event_device *dev,
 			return -ENOSYS;
 		return dev->set_state_oneshot(dev);
 
+	case CLOCK_EVT_STATE_ONESHOT_STOPPED:
+		/* Core internal bug */
+		if (WARN_ONCE(dev->state != CLOCK_EVT_STATE_ONESHOT,
+			      "Current state: %d\n", dev->state))
+			return -EINVAL;
+
+		if (dev->set_state_oneshot_stopped)
+			return dev->set_state_oneshot_stopped(dev);
+		else
+			return -ENOSYS;
+
 	default:
 		return -ENOSYS;
 	}
@@ -449,7 +460,8 @@ static int clockevents_sanity_check(struct clock_event_device *dev)
 	if (dev->set_mode) {
 		/* We shouldn't be supporting new modes now */
 		WARN_ON(dev->set_state_periodic || dev->set_state_oneshot ||
-			dev->set_state_shutdown || dev->tick_resume);
+			dev->set_state_shutdown || dev->tick_resume ||
+			dev->set_state_oneshot_stopped);
 
 		BUG_ON(dev->mode != CLOCK_EVT_MODE_UNUSED);
 		return 0;
diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index e878c2e0ba45..fbc902ccd72a 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -251,6 +251,12 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu)
 			SEQ_printf(m, "\n");
 		}
 
+		if (dev->set_state_oneshot_stopped) {
+			SEQ_printf(m, " oneshot stopped: ");
+			print_name_offset(m, dev->set_state_oneshot_stopped);
+			SEQ_printf(m, "\n");
+		}
+
 		if (dev->tick_resume) {
 			SEQ_printf(m, " resume:   ");
 			print_name_offset(m, dev->tick_resume);
-- 
2.3.0.rc0.44.ga94655d


  reply	other threads:[~2015-04-03  3:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-03  3:34 [PATCH V2 0/2] clockevents: Stop unused clockevent devices Viresh Kumar
2015-04-03  3:34 ` Viresh Kumar [this message]
2015-05-19 14:39   ` [tip:timers/core] clockevents: Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED state tip-bot for Viresh Kumar
2015-04-03  3:34 ` [PATCH V2 2/2] Clockevents: Stop unused clockevent devices Viresh Kumar
2015-05-19 14:39   ` [tip:timers/core] clockevents: " tip-bot for Viresh Kumar
2015-05-07  7:50 ` [PATCH V2 0/2] " 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=b8b383a03ac07b13312c16850b5106b82e4245b5.1428031396.git.viresh.kumar@linaro.org \
    --to=viresh.kumar@linaro.org \
    --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=preeti@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).