We already got rid of the hrtimer reprogramming loops and hoops as
hrtimer now enforces an interrupt if the enqueued time is in the past.
Do the same for the nohz non highres mode. That gets rid of the need
to raise the softirq which only serves the purpose of getting the
machine out of the inner idle loop.
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Preeti U Murthy <[email protected]>
Cc: Marcelo Tosatti <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
---
kernel/time/tick-sched.c | 83 ++++++++++++++++-------------------------------
1 file changed, 29 insertions(+), 54 deletions(-)
Index: tip/kernel/time/tick-sched.c
===================================================================
--- tip.orig/kernel/time/tick-sched.c
+++ tip/kernel/time/tick-sched.c
@@ -565,6 +565,20 @@ u64 get_cpu_iowait_time_us(int cpu, u64
}
EXPORT_SYMBOL_GPL(get_cpu_iowait_time_us);
+static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
+{
+ hrtimer_cancel(&ts->sched_timer);
+ hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
+
+ /* Forward the time to expire in the future */
+ hrtimer_forward(&ts->sched_timer, now, tick_period);
+
+ if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
+ hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED);
+ else
+ tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
+}
+
static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
ktime_t now, int cpu)
{
@@ -691,22 +705,18 @@ static ktime_t tick_nohz_stop_sched_tick
if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
hrtimer_cancel(&ts->sched_timer);
goto out;
- }
+ }
- if (ts->nohz_mode == NOHZ_MODE_HIGHRES) {
- hrtimer_start(&ts->sched_timer, expires,
- HRTIMER_MODE_ABS_PINNED);
- goto out;
- } else 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
- * softirq.
- */
- tick_do_update_jiffies64(ktime_get());
+ if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
+ hrtimer_start(&ts->sched_timer, expires,
+ HRTIMER_MODE_ABS_PINNED);
+ else
+ tick_program_event(expires, 1);
+ } else {
+ /* Tick is stopped, but required now. Enforce it */
+ tick_nohz_restart(ts, now);
}
- raise_softirq_irqoff(TIMER_SOFTIRQ);
+
out:
ts->next_jiffies = next_jiffies;
ts->last_jiffies = last_jiffies;
@@ -874,30 +884,6 @@ ktime_t tick_nohz_get_sleep_length(void)
return ts->sleep_length;
}
-static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
-{
- hrtimer_cancel(&ts->sched_timer);
- hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
-
- while (1) {
- /* Forward the time to expire in the future */
- hrtimer_forward(&ts->sched_timer, now, tick_period);
-
- if (ts->nohz_mode == NOHZ_MODE_HIGHRES) {
- hrtimer_start_expires(&ts->sched_timer,
- HRTIMER_MODE_ABS_PINNED);
- break;
- } else {
- if (!tick_program_event(
- hrtimer_get_expires(&ts->sched_timer), 0))
- break;
- }
- /* Reread time and update jiffies */
- now = ktime_get();
- tick_do_update_jiffies64(now);
- }
-}
-
static void tick_nohz_restart_sched_tick(struct tick_sched *ts, ktime_t now)
{
/* Update jiffies first */
@@ -968,12 +954,6 @@ void tick_nohz_idle_exit(void)
local_irq_enable();
}
-static int tick_nohz_reprogram(struct tick_sched *ts, ktime_t now)
-{
- hrtimer_forward(&ts->sched_timer, now, tick_period);
- return tick_program_event(hrtimer_get_expires(&ts->sched_timer), 0);
-}
-
/*
* The nohz low res interrupt handler
*/
@@ -992,10 +972,8 @@ static void tick_nohz_handler(struct clo
if (unlikely(ts->tick_stopped))
return;
- while (tick_nohz_reprogram(ts, now)) {
- now = ktime_get();
- tick_do_update_jiffies64(now);
- }
+ hrtimer_forward(&ts->sched_timer, now, tick_period);
+ tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
}
/**
@@ -1025,12 +1003,9 @@ static void tick_nohz_switch_to_nohz(voi
/* Get the next period */
next = tick_init_jiffy_update();
- for (;;) {
- hrtimer_set_expires(&ts->sched_timer, next);
- if (!tick_program_event(next, 0))
- break;
- next = ktime_add(next, tick_period);
- }
+ hrtimer_forward_now(&ts->sched_timer, tick_period);
+ hrtimer_set_expires(&ts->sched_timer, next);
+ tick_program_event(next, 1);
local_irq_enable();
}
On Tue, Apr 14, 2015 at 09:08:54PM -0000, Thomas Gleixner wrote:
> We already got rid of the hrtimer reprogramming loops and hoops as
> hrtimer now enforces an interrupt if the enqueued time is in the past.
>
> Do the same for the nohz non highres mode. That gets rid of the need
> to raise the softirq which only serves the purpose of getting the
> machine out of the inner idle loop.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: John Stultz <[email protected]>
> Cc: Preeti U Murthy <[email protected]>
> Cc: Marcelo Tosatti <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
>
> ---
> kernel/time/tick-sched.c | 83 ++++++++++++++++-------------------------------
> 1 file changed, 29 insertions(+), 54 deletions(-)
>
> Index: tip/kernel/time/tick-sched.c
> ===================================================================
> --- tip.orig/kernel/time/tick-sched.c
> +++ tip/kernel/time/tick-sched.c
> @@ -565,6 +565,20 @@ u64 get_cpu_iowait_time_us(int cpu, u64
> }
> EXPORT_SYMBOL_GPL(get_cpu_iowait_time_us);
>
> +static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
> +{
> + hrtimer_cancel(&ts->sched_timer);
> + hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
> +
> + /* Forward the time to expire in the future */
> + hrtimer_forward(&ts->sched_timer, now, tick_period);
> +
> + if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> + hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED);
> + else
> + tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
> +}
> +
> static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> ktime_t now, int cpu)
> {
> @@ -691,22 +705,18 @@ static ktime_t tick_nohz_stop_sched_tick
> if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> hrtimer_cancel(&ts->sched_timer);
> goto out;
> - }
> + }
>
> - if (ts->nohz_mode == NOHZ_MODE_HIGHRES) {
> - hrtimer_start(&ts->sched_timer, expires,
> - HRTIMER_MODE_ABS_PINNED);
> - goto out;
> - } else 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
> - * softirq.
> - */
> - tick_do_update_jiffies64(ktime_get());
> + if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> + hrtimer_start(&ts->sched_timer, expires,
> + HRTIMER_MODE_ABS_PINNED);
> + else
> + tick_program_event(expires, 1);
> + } else {
> + /* Tick is stopped, but required now. Enforce it */
> + tick_nohz_restart(ts, now);
> }
> - raise_softirq_irqoff(TIMER_SOFTIRQ);
> +
But the reprogramming happens only under "if ((long)delta_jiffies >= 1)".
Probably this condition should go away as well.
In the end, the possible side effect, at least on low-res, is that timers
which are already expired will be handled on the next tick instead of now.
But probably it doesn't matter much to have a one-tick delay.
On Wed, 22 Apr 2015, Frederic Weisbecker wrote:
> On Tue, Apr 14, 2015 at 09:08:54PM -0000, Thomas Gleixner wrote:
> > We already got rid of the hrtimer reprogramming loops and hoops as
> > hrtimer now enforces an interrupt if the enqueued time is in the past.
> >
> > Do the same for the nohz non highres mode. That gets rid of the need
> > to raise the softirq which only serves the purpose of getting the
> > machine out of the inner idle loop.
> >
> > Signed-off-by: Thomas Gleixner <[email protected]>
> > Cc: John Stultz <[email protected]>
> > Cc: Preeti U Murthy <[email protected]>
> > Cc: Marcelo Tosatti <[email protected]>
> > Cc: Frederic Weisbecker <[email protected]>
> >
> > ---
> > kernel/time/tick-sched.c | 83 ++++++++++++++++-------------------------------
> > 1 file changed, 29 insertions(+), 54 deletions(-)
> >
> > Index: tip/kernel/time/tick-sched.c
> > ===================================================================
> > --- tip.orig/kernel/time/tick-sched.c
> > +++ tip/kernel/time/tick-sched.c
> > @@ -565,6 +565,20 @@ u64 get_cpu_iowait_time_us(int cpu, u64
> > }
> > EXPORT_SYMBOL_GPL(get_cpu_iowait_time_us);
> >
> > +static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
> > +{
> > + hrtimer_cancel(&ts->sched_timer);
> > + hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
> > +
> > + /* Forward the time to expire in the future */
> > + hrtimer_forward(&ts->sched_timer, now, tick_period);
> > +
> > + if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> > + hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED);
> > + else
> > + tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
> > +}
> > +
> > static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
> > ktime_t now, int cpu)
> > {
> > @@ -691,22 +705,18 @@ static ktime_t tick_nohz_stop_sched_tick
> > if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> > hrtimer_cancel(&ts->sched_timer);
> > goto out;
> > - }
> > + }
> >
> > - if (ts->nohz_mode == NOHZ_MODE_HIGHRES) {
> > - hrtimer_start(&ts->sched_timer, expires,
> > - HRTIMER_MODE_ABS_PINNED);
> > - goto out;
> > - } else 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
> > - * softirq.
> > - */
> > - tick_do_update_jiffies64(ktime_get());
> > + if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> > + hrtimer_start(&ts->sched_timer, expires,
> > + HRTIMER_MODE_ABS_PINNED);
> > + else
> > + tick_program_event(expires, 1);
> > + } else {
> > + /* Tick is stopped, but required now. Enforce it */
> > + tick_nohz_restart(ts, now);
> > }
> > - raise_softirq_irqoff(TIMER_SOFTIRQ);
> > +
>
> But the reprogramming happens only under "if ((long)delta_jiffies >= 1)".
> Probably this condition should go away as well.
Errm.
if (!ts->tick_stopped && delta_jiffies <= 1)
goto out;
So if the tick is NOT stopped and delta_jiffies <= 1 we let it tick
and do nothing.
if (delta_jiffies >= 1)
Do the magic nohz stuff
else
tick_nohz_restart()
We want the distinction here because if the tick IS stopped and the
next event is due we need to kick it into gear again. So the condition
needs to stay. It probably should be if (delta > 1), but that's a
different story.
> In the end, the possible side effect, at least on low-res, is that timers
> which are already expired will be handled on the next tick instead of now.
> But probably it doesn't matter much to have a one-tick delay.
We really don't care. That stuff has no guarantees aside of the
guarantee that it does not expire early :)
Thanks,
tglx
Commit-ID: 0ff53d09642204c648424def0caa9117e7a3caaf
Gitweb: http://git.kernel.org/tip/0ff53d09642204c648424def0caa9117e7a3caaf
Author: Thomas Gleixner <[email protected]>
AuthorDate: Tue, 14 Apr 2015 21:08:54 +0000
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 22 Apr 2015 17:06:50 +0200
tick: sched: Force tick interrupt and get rid of softirq magic
We already got rid of the hrtimer reprogramming loops and hoops as
hrtimer now enforces an interrupt if the enqueued time is in the past.
Do the same for the nohz non highres mode. That gets rid of the need
to raise the softirq which only serves the purpose of getting the
machine out of the inner idle loop.
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Cc: Preeti U Murthy <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Marcelo Tosatti <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Marcelo Tosatti <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
kernel/time/tick-sched.c | 83 +++++++++++++++++-------------------------------
1 file changed, 29 insertions(+), 54 deletions(-)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index dc586c3..0f07ff2 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -565,6 +565,20 @@ u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time)
}
EXPORT_SYMBOL_GPL(get_cpu_iowait_time_us);
+static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
+{
+ hrtimer_cancel(&ts->sched_timer);
+ hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
+
+ /* Forward the time to expire in the future */
+ hrtimer_forward(&ts->sched_timer, now, tick_period);
+
+ if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
+ hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED);
+ else
+ tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
+}
+
static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
ktime_t now, int cpu)
{
@@ -691,22 +705,18 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
hrtimer_cancel(&ts->sched_timer);
goto out;
- }
+ }
- if (ts->nohz_mode == NOHZ_MODE_HIGHRES) {
- hrtimer_start(&ts->sched_timer, expires,
- HRTIMER_MODE_ABS_PINNED);
- goto out;
- } else 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
- * softirq.
- */
- tick_do_update_jiffies64(ktime_get());
+ if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
+ hrtimer_start(&ts->sched_timer, expires,
+ HRTIMER_MODE_ABS_PINNED);
+ else
+ tick_program_event(expires, 1);
+ } else {
+ /* Tick is stopped, but required now. Enforce it */
+ tick_nohz_restart(ts, now);
}
- raise_softirq_irqoff(TIMER_SOFTIRQ);
+
out:
ts->next_jiffies = next_jiffies;
ts->last_jiffies = last_jiffies;
@@ -874,30 +884,6 @@ ktime_t tick_nohz_get_sleep_length(void)
return ts->sleep_length;
}
-static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
-{
- hrtimer_cancel(&ts->sched_timer);
- hrtimer_set_expires(&ts->sched_timer, ts->last_tick);
-
- while (1) {
- /* Forward the time to expire in the future */
- hrtimer_forward(&ts->sched_timer, now, tick_period);
-
- if (ts->nohz_mode == NOHZ_MODE_HIGHRES) {
- hrtimer_start_expires(&ts->sched_timer,
- HRTIMER_MODE_ABS_PINNED);
- break;
- } else {
- if (!tick_program_event(
- hrtimer_get_expires(&ts->sched_timer), 0))
- break;
- }
- /* Reread time and update jiffies */
- now = ktime_get();
- tick_do_update_jiffies64(now);
- }
-}
-
static void tick_nohz_restart_sched_tick(struct tick_sched *ts, ktime_t now)
{
/* Update jiffies first */
@@ -968,12 +954,6 @@ void tick_nohz_idle_exit(void)
local_irq_enable();
}
-static int tick_nohz_reprogram(struct tick_sched *ts, ktime_t now)
-{
- hrtimer_forward(&ts->sched_timer, now, tick_period);
- return tick_program_event(hrtimer_get_expires(&ts->sched_timer), 0);
-}
-
/*
* The nohz low res interrupt handler
*/
@@ -992,10 +972,8 @@ static void tick_nohz_handler(struct clock_event_device *dev)
if (unlikely(ts->tick_stopped))
return;
- while (tick_nohz_reprogram(ts, now)) {
- now = ktime_get();
- tick_do_update_jiffies64(now);
- }
+ hrtimer_forward(&ts->sched_timer, now, tick_period);
+ tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
}
/**
@@ -1025,12 +1003,9 @@ static void tick_nohz_switch_to_nohz(void)
/* Get the next period */
next = tick_init_jiffy_update();
- for (;;) {
- hrtimer_set_expires(&ts->sched_timer, next);
- if (!tick_program_event(next, 0))
- break;
- next = ktime_add(next, tick_period);
- }
+ hrtimer_forward_now(&ts->sched_timer, tick_period);
+ hrtimer_set_expires(&ts->sched_timer, next);
+ tick_program_event(next, 1);
local_irq_enable();
}
On Wed, Apr 22, 2015 at 04:32:11PM +0200, Thomas Gleixner wrote:
> On Wed, 22 Apr 2015, Frederic Weisbecker wrote:
> > But the reprogramming happens only under "if ((long)delta_jiffies >= 1)".
> > Probably this condition should go away as well.
>
> Errm.
>
> if (!ts->tick_stopped && delta_jiffies <= 1)
> goto out;
>
> So if the tick is NOT stopped and delta_jiffies <= 1 we let it tick
> and do nothing.
>
> if (delta_jiffies >= 1)
> Do the magic nohz stuff
> else
> tick_nohz_restart()
>
> We want the distinction here because if the tick IS stopped and the
> next event is due we need to kick it into gear again. So the condition
> needs to stay. It probably should be if (delta > 1), but that's a
> different story.
Yes but what if the tick is stopped already and delta_jiffies < 1? Say the
tick was last programmed to fire in 5 seconds. An irq fires and enqueues
a timer to fire now. If it's soon enough that delta_jiffies < 1, it seems
we are missing the clock reprogramming and even the softirq from that irq exit.
Because we have:
if (!ts->tick_stopped && delta_jiffies <= 1)
goto out;
if ((long)delta_jiffies >= 1) {
//do clock reprogramming or restart
...
}
- raise_softirq(TIMER_SOFTIRQ);
out:
...
>
> > In the end, the possible side effect, at least on low-res, is that timers
> > which are already expired will be handled on the next tick instead of now.
> > But probably it doesn't matter much to have a one-tick delay.
>
> We really don't care. That stuff has no guarantees aside of the
> guarantee that it does not expire early :)
Good :)
>
> Thanks,
>
> tglx
>
On Thu, 23 Apr 2015, Frederic Weisbecker wrote:
> On Wed, Apr 22, 2015 at 04:32:11PM +0200, Thomas Gleixner wrote:
> > On Wed, 22 Apr 2015, Frederic Weisbecker wrote:
> > > But the reprogramming happens only under "if ((long)delta_jiffies >= 1)".
> > > Probably this condition should go away as well.
> >
> > Errm.
> >
> > if (!ts->tick_stopped && delta_jiffies <= 1)
> > goto out;
> >
> > So if the tick is NOT stopped and delta_jiffies <= 1 we let it tick
> > and do nothing.
> >
> > if (delta_jiffies >= 1)
> > Do the magic nohz stuff
> > else
> > tick_nohz_restart()
> >
> > We want the distinction here because if the tick IS stopped and the
> > next event is due we need to kick it into gear again. So the condition
> > needs to stay. It probably should be if (delta > 1), but that's a
> > different story.
>
> Yes but what if the tick is stopped already and delta_jiffies < 1? Say the
> tick was last programmed to fire in 5 seconds. An irq fires and enqueues
> a timer to fire now. If it's soon enough that delta_jiffies < 1, it seems
> we are missing the clock reprogramming and even the softirq from that irq exit.
>
> Because we have:
>
> if (!ts->tick_stopped && delta_jiffies <= 1)
> goto out;
>
> if ((long)delta_jiffies >= 1) {
> //do clock reprogramming or restart
> ...
> }
>
> - raise_softirq(TIMER_SOFTIRQ);
Did you actually read what I wrote?
> > if (delta_jiffies >= 1)
> > Do the magic nohz stuff
> > else
> > tick_nohz_restart()
So if ticked is stopped AND delta < 1 we call
tick_nohz_restart(). Which will programm the tick for the next
possible period to expire. Which will kick the timer softirq and
expire the timer.
Thanks,
tglx
On Thu, Apr 23, 2015 at 03:07:59PM +0200, Thomas Gleixner wrote:
> On Thu, 23 Apr 2015, Frederic Weisbecker wrote:
> > On Wed, Apr 22, 2015 at 04:32:11PM +0200, Thomas Gleixner wrote:
> > > On Wed, 22 Apr 2015, Frederic Weisbecker wrote:
> > > > But the reprogramming happens only under "if ((long)delta_jiffies >= 1)".
> > > > Probably this condition should go away as well.
> > >
> > > Errm.
> > >
> > > if (!ts->tick_stopped && delta_jiffies <= 1)
> > > goto out;
> > >
> > > So if the tick is NOT stopped and delta_jiffies <= 1 we let it tick
> > > and do nothing.
> > >
> > > if (delta_jiffies >= 1)
> > > Do the magic nohz stuff
> > > else
> > > tick_nohz_restart()
> > >
> > > We want the distinction here because if the tick IS stopped and the
> > > next event is due we need to kick it into gear again. So the condition
> > > needs to stay. It probably should be if (delta > 1), but that's a
> > > different story.
> >
> > Yes but what if the tick is stopped already and delta_jiffies < 1? Say the
> > tick was last programmed to fire in 5 seconds. An irq fires and enqueues
> > a timer to fire now. If it's soon enough that delta_jiffies < 1, it seems
> > we are missing the clock reprogramming and even the softirq from that irq exit.
> >
> > Because we have:
> >
> > if (!ts->tick_stopped && delta_jiffies <= 1)
> > goto out;
> >
> > if ((long)delta_jiffies >= 1) {
> > //do clock reprogramming or restart
> > ...
> > }
> >
> > - raise_softirq(TIMER_SOFTIRQ);
>
> Did you actually read what I wrote?
>
> > > if (delta_jiffies >= 1)
> > > Do the magic nohz stuff
> > > else
> > > tick_nohz_restart()
>
> So if ticked is stopped AND delta < 1 we call
> tick_nohz_restart(). Which will programm the tick for the next
> possible period to expire. Which will kick the timer softirq and
> expire the timer.
Right, I missed the "else" that moved.