Re: [RFC][PATCH] Dynamic Tick: Allow 32-bit machines to sleep formorethan2.15 seconds

[Jon Hunter <jon-hunter@ti.com>]

john stultz wrote:
> Well, the mult adjustments should be quite small, especially compared to
> the NSEC_PER_SEC/HZ adjustment.
> 
> Hmm... Although, I guess we could get bitten if the max_deferment was
> like an hour, and the adjustment was enough that it scaled out to and we
> ended up being a second late or so. So you have a point.
> 
> But since the clockevent driver is not scaled, we probably can get away
> with using the orig_mult value instead of mult, and be ok.
> 
> Alternatively instead of NSEC_PER_SEC/HZ, we could always drop the
> larger of NSEC_PER_SEC/HZ or max_deferment/10? That way we should scale
> up without a problem. 

Yes, may be this would be a safer option. Thinking about this I was 
wondering if we should always use max_deferement/10, because I did not 
think that there would ever be a case where NSEC_PER_SEC/HZ would be 
greater. If NSEC_PER_SEC/HZ was greater than max_deferement/10 this 
would imply that the clocksource would wrap after only 10 jiffies, if I 
have the math right...

> I suspect it would be tough to hit this issue though.

Agree.

> Two patches should be fine.

Ok, I will re-post as two once we have the final version.

> Looks good overall. We may want to add the -10% (or -5%) to be totally
> safe, but that's likely just me being paranoid.

I am paranoid too! Do you care if we use 6.25% or 12.5% margin instead 
of 10% or 5%? This way we can avoid a 64-bit division by using a simple 
shift. See below. I have implemented a 6.25% margin for now. Let me know 
your thoughts.

One final question, I noticed in clocksource.h that the definition of 
function cyc2ns returns a type of s64, however, in the function itself a 
variable of type u64 is used and returned. Should this function be 
modified as follows?

  static inline s64 cyc2ns(struct clocksource *cs, cycle_t cycles)
  {
-       u64 ret = (u64)cycles;
+       s64 ret = (s64)cycles;
         ret = (ret * cs->mult) >> cs->shift;
         return ret;
  }

Cheers
Jon


Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
  include/linux/time.h      |    1 +
  kernel/time/tick-sched.c  |   36 +++++++++++++++++++++++++-----------
  kernel/time/timekeeping.c |   28 ++++++++++++++++++++++++++++
  3 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index 242f624..090be07 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -130,6 +130,7 @@ extern void monotonic_to_bootbased(struct timespec *ts);

  extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
  extern int timekeeping_valid_for_hres(void);
+extern s64 timekeeping_max_deferment(void);
  extern void update_wall_time(void);
  extern void update_xtime_cache(u64 nsec);

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index d3f1ef4..f0155ae 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -217,6 +217,7 @@ void tick_nohz_stop_sched_tick(int inidle)
  	ktime_t last_update, expires, now;
  	struct clock_event_device *dev = __get_cpu_var(tick_cpu_device).evtdev;
  	int cpu;
+	s64 time_delta, max_time_delta;

  	local_irq_save(flags);

@@ -264,6 +265,7 @@ void tick_nohz_stop_sched_tick(int inidle)
  		seq = read_seqbegin(&xtime_lock);
  		last_update = last_jiffies_update;
  		last_jiffies = jiffies;
+		max_time_delta = timekeeping_max_deferment();
  	} while (read_seqretry(&xtime_lock, seq));

  	/* Get the next timer wheel timer */
@@ -283,11 +285,22 @@ void tick_nohz_stop_sched_tick(int inidle)
  	if ((long)delta_jiffies >= 1) {

  		/*
-		* calculate the expiry time for the next timer wheel
-		* timer
-		*/
-		expires = ktime_add_ns(last_update, tick_period.tv64 *
-				   delta_jiffies);
+		 * Calculate the time delta for the next timer event.
+		 * If the time delta exceeds the maximum time delta
+		 * permitted by the current clocksource then adjust
+		 * the time delta accordingly to ensure the
+		 * clocksource does not wrap.
+		 */
+		time_delta = tick_period.tv64 * delta_jiffies;
+
+		if (time_delta > max_time_delta)
+			time_delta = max_time_delta;
+
+		/*
+		 * calculate the expiry time for the next timer wheel
+		 * timer
+		 */
+		expires = ktime_add_ns(last_update, time_delta);

  		/*
  		 * If this cpu is the one which updates jiffies, then
@@ -300,7 +313,7 @@ void tick_nohz_stop_sched_tick(int inidle)
  		if (cpu == tick_do_timer_cpu)
  			tick_do_timer_cpu = TICK_DO_TIMER_NONE;

-		if (delta_jiffies > 1)
+		if (time_delta > tick_period.tv64)
  			cpumask_set_cpu(cpu, nohz_cpu_mask);

  		/* Skip reprogram of event if its not changed */
@@ -332,12 +345,13 @@ void tick_nohz_stop_sched_tick(int inidle)
  		ts->idle_sleeps++;

  		/*
-		 * delta_jiffies >= NEXT_TIMER_MAX_DELTA signals that
-		 * there is no timer pending or at least extremly far
-		 * into the future (12 days for HZ=1000). In this case
-		 * we simply stop the tick timer:
+		 * time_delta >= (tick_period.tv64 * NEXT_TIMER_MAX_DELTA)
+		 * signals that there is no timer pending or at least
+		 * extremely far into the future (12 days for HZ=1000).
+		 * In this case we simply stop the tick timer:
  		 */
-		if (unlikely(delta_jiffies >= NEXT_TIMER_MAX_DELTA)) {
+		if (unlikely(time_delta >=
+				(tick_period.tv64 * NEXT_TIMER_MAX_DELTA))) {
  			ts->idle_expires.tv64 = KTIME_MAX;
  			if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
  				hrtimer_cancel(&ts->sched_timer);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 687dff4..e764ac8 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -271,6 +271,34 @@ int timekeeping_valid_for_hres(void)
  }

  /**
+ * timekeeping_max_deferment - Returns max time the clocksource can be 
deferred
+ *
+ * IMPORTANT: Must be called with xtime_lock held!
+ */
+s64 timekeeping_max_deferment(void)
+{
+	s64 max_nsecs, margin;
+
+	max_nsecs = cyc2ns(clock, clock->mask);
+
+	/*
+	 * To ensure that the clocksource does not wrap whilst we are idle,
+	 * let's limit the time the clocksource can be deferred by 6.25% of
+	 * the total time the clocksource can count. Please note a margin
+	 * of 6.25% is used because this can be computed with a shift,
+	 * versus say 5% which would require division.
+	 */
+	margin = max_nsecs >> 4;
+
+	max_nsecs = max_nsecs - margin;
+
+	if (max_nsecs < 0)
+		max_nsecs = 0;
+
+	return max_nsecs;
+}
+
+/**
   * read_persistent_clock -  Return time in seconds from the persistent 
clock.
   *
   * Weak dummy function for arches that do not yet support it.
-- 
1.6.1