All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jon-hunter@ti.com>
To: John Stultz <johnstul@us.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>, Thomas Gleixner <tglx@linutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC][PATCH] Dynamic Tick: Allow 32-bit machines to sleep formorethan2.15 seconds
Date: Fri, 22 May 2009 13:21:40 -0500	[thread overview]
Message-ID: <4A16ED34.9060808@ti.com> (raw)
In-Reply-To: <1242436726.29511.198.camel@jstultz-laptop>


John Stultz wrote:
> So cyc2ns is a very very hot path, so I don't think we want to muck with
> that much.
> 
> Instead of catching the overflows, and then trying to handle them, we
> really want to prevent overflows from happening. That is the main point
> of the timekeeping_max_deferrment() interface after all ;)
> 
> So thinking about it a bit more, what we really want from
> timekeeping_max_deferrment() is roughly:
> 
> 	/* find the max cycle value that would overflow the mult */
> 	max_cycles = -1UUL/clocksource->mult;
> 
> 	/* pick the smaller of max_cycles or the mask value */
> 	max_cycles = min(max_cycles, clocksource->mask);
> 
> 	/* convert max_cycles into ns */
> 	max_time = cyc2ns(clocksource, max_cycles);
> 
> 	/* take off 5% of the max to make sure we don't show up late */
> 	max_time = max_time - max_time/20;
> 
> 
> We should be able to make this reasonably fast via:
> 	max_cycles = 1<<(64 - ilog2(clocksource->mult) - 1);
> 	max_cycles = min(max_cycles, clocksource->mask);
> 	max_time = cyc2ns(clocksource, max_cycles);
> 	max_time = max_time - max_time >> 4;
> 
> Does that seem reasonable?

Yes, good idea. Sorry for the delay in getting back to this. I have 
implemented the above and incorporated into the patch below.

Please note that the only modification I made to the above was to add 1 
to result of the log2. I found that the result of the log2 was rounded 
down and so this was still making max_cycles too big. Therefore to be on 
the safe side I add 1 to the result of log2 which will ensure max_cycles 
is not too big.

Thanks for the inputs. Let me know your thoughts.

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 |   47 
+++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 73 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..608fc6f 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -271,6 +271,53 @@ 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;
+	u64 max_cycles;
+
+	/*
+	 * Calculate the maximum number of cycles that we can pass to the
+	 * cyc2ns function without overflowing a 64-bit signed result. The
+	 * maximum number of cycles is equal to ULLONG_MAX/clock->mult which
+	 * is equivalent to the below.
+	 * max_cycles < (2^63)/clock->mult
+	 * max_cycles < 2^(log2((2^63)/clock->mult))
+	 * max_cycles < 2^(log2(2^63) - log2(clock->mult))
+	 * max_cycles < 2^(63 - log2(clock->mult))
+	 * max_cycles < 1 << (63 - log2(clock->mult))
+	 * Please note that we add 1 to the result of the log2 to account for
+	 * any rounding errors, ensure the above inequality is satisfied and
+	 * no overflow will occur.
+	 */
+	max_cycles = 1ULL << (63 - (ilog2(clock->mult) + 1));
+
+	/*
+	 * The actual maximum number of cycles we can defer the clocksource is
+	 * determined by the minimum of max_cycles and clock->mask.
+	 */
+	max_cycles = min(max_cycles, clock->mask);
+	max_nsecs = cyc2ns(clock, max_cycles);
+
+	/*
+	 * To ensure that the clocksource does not wrap whilst we are idle,
+	 * limit the time the clocksource can be deferred by 6.25%. Please
+	 * note a margin of 6.25% is used because this can be computed with
+	 * a shift, versus say 5% which would require division.
+	 */
+	max_nsecs = max_nsecs - (max_nsecs >> 4);
+
+	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







  reply	other threads:[~2009-05-22 18:22 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-20 21:16 [RFC][PATCH] Dynamic Tick: Allow 32-bit machines to sleep for more than 2.15 seconds Jon Hunter
2009-04-21  6:35 ` Ingo Molnar
2009-04-21 20:32   ` john stultz
2009-04-21 23:20     ` Jon Hunter
2009-04-22  0:02       ` john stultz
2009-05-07 14:52         ` Jon Hunter
2009-05-08  0:54           ` [RFC][PATCH] Dynamic Tick: Allow 32-bit machines to sleep formore " john stultz
2009-05-08 16:05             ` Jon Hunter
2009-05-09  0:51               ` [RFC][PATCH] Dynamic Tick: Allow 32-bit machines to sleep formorethan " john stultz
2009-05-12 23:35                 ` Jon Hunter
2009-05-12 23:58                   ` [RFC][PATCH] Dynamic Tick: Allow 32-bit machines to sleep formorethan2.15 seconds john stultz
2009-05-13 15:14                     ` Jon Hunter
2009-05-13 16:41                       ` John Stultz
2009-05-13 17:54                         ` Jon Hunter
2009-05-13 19:21                           ` John Stultz
2009-05-15 16:35                             ` Jon Hunter
2009-05-15 18:55                               ` Jon Hunter
2009-05-16  1:29                                 ` John Stultz
2009-05-16  1:18                               ` John Stultz
2009-05-22 18:21                                 ` Jon Hunter [this message]
2009-05-22 19:23                                   ` john stultz
2009-05-22 19:54                                     ` Thomas Gleixner
2009-05-26 15:12                                       ` Jon Hunter
2009-05-26 20:26                                         ` john stultz
2009-05-22 19:59                                   ` Thomas Gleixner
2009-04-22  0:05       ` [RFC][PATCH] Dynamic Tick: Allow 32-bit machines to sleep for more than 2.15 seconds john stultz
2009-04-22  3:07         ` Jon Hunter
2009-04-22 15:30           ` Chris Friesen
2009-04-22 17:04             ` Jon Hunter
2009-04-22 18:53               ` Geert Uytterhoeven

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=4A16ED34.9060808@ti.com \
    --to=jon-hunter@ti.com \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.