Message-Id: <20160916201204.9424-4-nicstange@gmail.com>
Date:   Fri, 16 Sep 2016 22:11:56 +0200
From:   Nicolai Stange <nicstange@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     John Stultz <john.stultz@...aro.org>, linux-kernel@...r.kernel.org,
        Nicolai Stange <nicstange@...il.com>
Subject: [RFC v7 15/23] clockevents: do comparison of delta against minimum in terms of cycles

Before converting the given delta from ns to cycles by means of the
mult/shift pair, clockevents_program_event() enforces it to be greater or
equal than ->max_delta_ns. Simplified, this reads as

  delta = max(delta, dev->min_delta_ns);
  clc = (delta * dev->mult) >> dev->shift;

Note that ->min_delta_ns is more or less ->min_delta_ticks converted to
nanoseconds and thus, it depends on the device's mult/shift pair as well.
Thus, any update to ->mult would require a corresponding change of
->min_delta_ns as well and these two had to get consumed in the clockevent
programming path atomically. This would have a negative performance impact
as soon as we start adjusting ->mult from a different CPU with upcoming
changes making the clockevents core NTP correction aware: some kind of
locking would have been needed in the event programming path.

These costly atomic reads in the event programming path can be avoided by
doing the comparison against the lower bound in terms of cycles instead of
nanoseconds.

Now, as it stands, ->min_delta_ns is not always completely equivalent
to ->min_delta_ticks: first, it is forced to be >= 1us and second,
clockevents_program_min_delta() can increase it if
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y.

In order to account for this, introduce ->min_delta_ticks_adjusted which
is supposed to be equivalent to ->min_delta_ns. Keep the original
->min_delta_ticks for reconfiguration.

The invariant ->min_delta_ticks <= ->min_delta_ticks_adjusted will be
guaranteed to hold at all times. This will allow for non-atomic updates of
 ->mult and ->min_delta_ticks_adjusted -- as long as we stay within a
device's allowed bounds, we don't care for small deviations.

Make clockevents_program_event() use ->min_delta_ticks_adjusted for the
enforcement of the lower bound on the delta value.

Finally, slightly reorder the members of struct clock_event_device in order
to keep the now often used ones close together.

Signed-off-by: Nicolai Stange <nicstange@...il.com>
---
 include/linux/clockchips.h |  6 ++++--
 kernel/time/clockevents.c  | 11 ++++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index be5f222..7c3a193 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -80,8 +80,8 @@ enum clock_event_state {
  * @set_next_ktime:	set next event function using a direct ktime value
  * @next_event:		local storage for the next event in oneshot mode
  * @max_delta_ns:	maximum delta value in ns
- * @min_delta_ns:	minimum delta value in ns
  * @max_delta_ticks:	maximum delta value in ticks
+ * @min_delta_ticks_adjusted:	minimum delta value, increased as needed
  * @mult:		nanosecond to cycles multiplier
  * @shift:		nanoseconds to cycles divisor (power of two)
  * @state_use_accessors:current state of the device, assigned by the core code
@@ -94,6 +94,7 @@ enum clock_event_state {
  * @tick_resume:	resume clkevt device
  * @broadcast:		function to broadcast events
  * @min_delta_ticks:	minimum delta value in ticks stored for reconfiguration
+ * @min_delta_ns:	minimum delta value in ns
  * @name:		ptr to clock event name
  * @rating:		variable to rate clock event devices
  * @irq:		IRQ number (only for non CPU local devices)
@@ -108,8 +109,8 @@ struct clock_event_device {
 	int			(*set_next_ktime)(ktime_t expires, struct clock_event_device *);
 	ktime_t			next_event;
 	u64			max_delta_ns;
-	u64			min_delta_ns;
 	unsigned long		max_delta_ticks;
+	unsigned long		min_delta_ticks_adjusted;
 	u32			mult;
 	u32			shift;
 	enum clock_event_state	state_use_accessors;
@@ -126,6 +127,7 @@ struct clock_event_device {
 	void			(*suspend)(struct clock_event_device *);
 	void			(*resume)(struct clock_event_device *);
 	unsigned long		min_delta_ticks;
+	u64			min_delta_ns;
 
 	const char		*name;
 	int			rating;
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 472fcc2..f41f584 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -226,6 +226,11 @@ static int clockevents_increase_min_delta(struct clock_event_device *dev)
 	if (dev->min_delta_ns > MIN_DELTA_LIMIT)
 		dev->min_delta_ns = MIN_DELTA_LIMIT;
 
+	dev->min_delta_ticks_adjusted = (unsigned long)((dev->min_delta_ns *
+						dev->mult) >> dev->shift);
+	dev->min_delta_ticks_adjusted = max(dev->min_delta_ticks_adjusted,
+						dev->min_delta_ticks);
+
 	printk_deferred(KERN_WARNING
 			"CE: %s increased min_delta_ns to %llu nsec\n",
 			dev->name ? dev->name : "?",
@@ -334,11 +339,11 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
 		return force ? clockevents_program_min_delta(dev) : -ETIME;
 
 	delta = min(delta, (int64_t) dev->max_delta_ns);
-	delta = max(delta, (int64_t) dev->min_delta_ns);
 
 	clc = ((unsigned long long) delta * dev->mult) >> dev->shift;
 
 	clc = min_t(unsigned long, clc, dev->max_delta_ticks);
+	clc = max_t(unsigned long, clc, dev->min_delta_ticks_adjusted);
 
 	rc = dev->set_next_event((unsigned long) clc, dev);
 
@@ -501,6 +506,10 @@ static void __clockevents_update_bounds(struct clock_event_device *dev)
 	 * we'll never program any ced with anything less.
 	 */
 	dev->min_delta_ns = cev_delta2ns(dev->min_delta_ticks, dev, false);
+	dev->min_delta_ticks_adjusted = (unsigned long)((dev->min_delta_ns *
+						dev->mult) >> dev->shift);
+	dev->min_delta_ticks_adjusted = max(dev->min_delta_ticks_adjusted,
+						dev->min_delta_ticks);
 }
 
 /**
-- 
2.10.0

Powered by blists - more mailing lists