Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-07-07 Thread Frederic Weisbecker
On Sun, Jun 14, 2015 at 03:00:12PM +0530, Preeti U Murthy wrote:
> On 06/12/2015 06:08 PM, Frederic Weisbecker wrote:
> > On Fri, Jun 12, 2015 at 09:32:45AM +0200, Peter Zijlstra wrote:
> >> On Thu, Jun 11, 2015 at 07:36:05PM +0200, Frederic Weisbecker wrote:
> >>> Restart the tick when necessary from the irq exit path. It makes nohz
> >>> full more flexible and allow it to piggyback the tick restart on the
> >>> scheduler IPI in the future instead of sending a dedicated IPI that
> >>> often doubles the scheduler IPI on task wakeup. This will require
> >>> careful review of resched_curr() callers.
> >>
> >> This seems to assume schedule_ipi() callers use irq_exit(), this is
> >> false.
> > 
> > Indeed there will be that too. Note the current patch doesn't yet rely on
> > schedule_ipi(), we are still using the nohz ipis. But introducing the
> > tick restart on irq exit prepares for later piggybacking on scheduler_ipi().
> > 
> > I think this will involve changes on got_nohz_idle_kick(), renamed to
> > got_nohz_kick() and include nohz full related checks to trigger the
> > irq_enter()/exit() pair.
> 
> I maybe saying something obvious here, nevertheless; I am not sure about
> other archs, but atleast on powerpc after handling an interrupt, we will
> call irq_exit() and reevaluate starting of ticks. So in our case even if
> scheduler_ipi() callers do not call irq_exit(), it will be called after
> handling the reschedule interrupt.

scheduler_ipi() takes care of the call to irq_enter() and irq_exit() when
necessary. Which means that the arch low level handler for scheduler_ipi()
shouldn't call these functions. If it does then it's buggy.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-07-07 Thread Frederic Weisbecker
On Sun, Jun 14, 2015 at 02:49:16PM +0530, Preeti U Murthy wrote:
> On 06/11/2015 11:06 PM, Frederic Weisbecker wrote:
> > Restart the tick when necessary from the irq exit path. It makes nohz
> > full more flexible and allow it to piggyback the tick restart on the
> > scheduler IPI in the future instead of sending a dedicated IPI that
> > often doubles the scheduler IPI on task wakeup. This will require
> 
> You can piggy back on the scheduler ipi when you add a timer/hrtimer and
> add a new task to the runqueue of the nohz_full cpus, since we call
> resched_curr() in these code paths. But what about the calls to kick
> nohz_full cpus by perf events and posix cpu timers ? These call sites
> seem to be concerned about specifically waking up nohz_full cpus as far
> as I can see. IOW there is no scheduling ipi that we can fall back on in
> these paths.

Sure, those will need to keep the current IPI. They are less of a worry
because they should be rare events compared to the scheduler.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-14 Thread Preeti U Murthy
On 06/12/2015 06:08 PM, Frederic Weisbecker wrote:
> On Fri, Jun 12, 2015 at 09:32:45AM +0200, Peter Zijlstra wrote:
>> On Thu, Jun 11, 2015 at 07:36:05PM +0200, Frederic Weisbecker wrote:
>>> Restart the tick when necessary from the irq exit path. It makes nohz
>>> full more flexible and allow it to piggyback the tick restart on the
>>> scheduler IPI in the future instead of sending a dedicated IPI that
>>> often doubles the scheduler IPI on task wakeup. This will require
>>> careful review of resched_curr() callers.
>>
>> This seems to assume schedule_ipi() callers use irq_exit(), this is
>> false.
> 
> Indeed there will be that too. Note the current patch doesn't yet rely on
> schedule_ipi(), we are still using the nohz ipis. But introducing the
> tick restart on irq exit prepares for later piggybacking on scheduler_ipi().
> 
> I think this will involve changes on got_nohz_idle_kick(), renamed to
> got_nohz_kick() and include nohz full related checks to trigger the
> irq_enter()/exit() pair.

I maybe saying something obvious here, nevertheless; I am not sure about
other archs, but atleast on powerpc after handling an interrupt, we will
call irq_exit() and reevaluate starting of ticks. So in our case even if
scheduler_ipi() callers do not call irq_exit(), it will be called after
handling the reschedule interrupt.

Regards
Preeti U Murthy
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-14 Thread Preeti U Murthy
On 06/11/2015 11:06 PM, Frederic Weisbecker wrote:
> Restart the tick when necessary from the irq exit path. It makes nohz
> full more flexible and allow it to piggyback the tick restart on the
> scheduler IPI in the future instead of sending a dedicated IPI that
> often doubles the scheduler IPI on task wakeup. This will require

You can piggy back on the scheduler ipi when you add a timer/hrtimer and
add a new task to the runqueue of the nohz_full cpus, since we call
resched_curr() in these code paths. But what about the calls to kick
nohz_full cpus by perf events and posix cpu timers ? These call sites
seem to be concerned about specifically waking up nohz_full cpus as far
as I can see. IOW there is no scheduling ipi that we can fall back on in
these paths.

> careful review of resched_curr() callers.
> 

Regards
Preeti U Murthy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-14 Thread Preeti U Murthy
On 06/11/2015 11:06 PM, Frederic Weisbecker wrote:
> Restart the tick when necessary from the irq exit path. It makes nohz
> full more flexible and allow it to piggyback the tick restart on the
> scheduler IPI in the future instead of sending a dedicated IPI that
> often doubles the scheduler IPI on task wakeup. This will require

You can piggy back on the scheduler ipi when you add a timer/hrtimer and
add a new task to the runqueue of the nohz_full cpus, since we call
resched_curr() in these code paths. But what about the calls to kick
nohz_full cpus by perf events and posix cpu timers ? These call sites
seem to be concerned about specifically waking up nohz_full cpus as far
as I can see. IOW there is no scheduling ipi that we can fall back on in
these paths.

> careful review of resched_curr() callers.
> 

Regards
Preeti U Murthy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-12 Thread Frederic Weisbecker
On Fri, Jun 12, 2015 at 02:59:41PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 12, 2015 at 02:38:36PM +0200, Frederic Weisbecker wrote:
> > On Fri, Jun 12, 2015 at 09:32:45AM +0200, Peter Zijlstra wrote:
> > > On Thu, Jun 11, 2015 at 07:36:05PM +0200, Frederic Weisbecker wrote:
> > > > Restart the tick when necessary from the irq exit path. It makes nohz
> > > > full more flexible and allow it to piggyback the tick restart on the
> > > > scheduler IPI in the future instead of sending a dedicated IPI that
> > > > often doubles the scheduler IPI on task wakeup. This will require
> > > > careful review of resched_curr() callers.
> > > 
> > > This seems to assume schedule_ipi() callers use irq_exit(), this is
> > > false.
> > 
> > Indeed there will be that too. Note the current patch doesn't yet rely on
> > schedule_ipi(), we are still using the nohz ipis.
> 
> Ah, then I just didn't understand your changelog right.

I think I should make it clearer, I must admit it's a bit confused :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-12 Thread Peter Zijlstra
On Fri, Jun 12, 2015 at 02:38:36PM +0200, Frederic Weisbecker wrote:
> On Fri, Jun 12, 2015 at 09:32:45AM +0200, Peter Zijlstra wrote:
> > On Thu, Jun 11, 2015 at 07:36:05PM +0200, Frederic Weisbecker wrote:
> > > Restart the tick when necessary from the irq exit path. It makes nohz
> > > full more flexible and allow it to piggyback the tick restart on the
> > > scheduler IPI in the future instead of sending a dedicated IPI that
> > > often doubles the scheduler IPI on task wakeup. This will require
> > > careful review of resched_curr() callers.
> > 
> > This seems to assume schedule_ipi() callers use irq_exit(), this is
> > false.
> 
> Indeed there will be that too. Note the current patch doesn't yet rely on
> schedule_ipi(), we are still using the nohz ipis.

Ah, then I just didn't understand your changelog right.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-12 Thread Frederic Weisbecker
On Fri, Jun 12, 2015 at 09:32:45AM +0200, Peter Zijlstra wrote:
> On Thu, Jun 11, 2015 at 07:36:05PM +0200, Frederic Weisbecker wrote:
> > Restart the tick when necessary from the irq exit path. It makes nohz
> > full more flexible and allow it to piggyback the tick restart on the
> > scheduler IPI in the future instead of sending a dedicated IPI that
> > often doubles the scheduler IPI on task wakeup. This will require
> > careful review of resched_curr() callers.
> 
> This seems to assume schedule_ipi() callers use irq_exit(), this is
> false.

Indeed there will be that too. Note the current patch doesn't yet rely on
schedule_ipi(), we are still using the nohz ipis. But introducing the
tick restart on irq exit prepares for later piggybacking on scheduler_ipi().

I think this will involve changes on got_nohz_idle_kick(), renamed to
got_nohz_kick() and include nohz full related checks to trigger the
irq_enter()/exit() pair.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-12 Thread Peter Zijlstra
On Thu, Jun 11, 2015 at 07:36:05PM +0200, Frederic Weisbecker wrote:
> Restart the tick when necessary from the irq exit path. It makes nohz
> full more flexible and allow it to piggyback the tick restart on the
> scheduler IPI in the future instead of sending a dedicated IPI that
> often doubles the scheduler IPI on task wakeup. This will require
> careful review of resched_curr() callers.

This seems to assume schedule_ipi() callers use irq_exit(), this is
false.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-11 Thread Rik van Riel
On 06/11/2015 01:36 PM, Frederic Weisbecker wrote:
> Restart the tick when necessary from the irq exit path. It makes nohz
> full more flexible and allow it to piggyback the tick restart on the
> scheduler IPI in the future instead of sending a dedicated IPI that
> often doubles the scheduler IPI on task wakeup. This will require
> careful review of resched_curr() callers.
> 
> Cc: Christoph Lameter 
> Cc: Ingo Molnar 
> Cc; John Stultz 
> Cc: Peter Zijlstra 
> Cc: Preeti U Murthy 
> Cc: Rik van Riel 
> Cc: Thomas Gleixner 
> Cc: Viresh Kumar 
> Signed-off-by: Frederic Weisbecker 

Reviewed-by: Rik van Riel 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 5/8] nohz: Restart the tick from irq exit

2015-06-11 Thread Frederic Weisbecker
Restart the tick when necessary from the irq exit path. It makes nohz
full more flexible and allow it to piggyback the tick restart on the
scheduler IPI in the future instead of sending a dedicated IPI that
often doubles the scheduler IPI on task wakeup. This will require
careful review of resched_curr() callers.

Cc: Christoph Lameter 
Cc: Ingo Molnar 
Cc; John Stultz 
Cc: Peter Zijlstra 
Cc: Preeti U Murthy 
Cc: Rik van Riel 
Cc: Thomas Gleixner 
Cc: Viresh Kumar 
Signed-off-by: Frederic Weisbecker 
---
 include/linux/tick.h |  8 
 kernel/time/tick-sched.c | 34 ++
 2 files changed, 10 insertions(+), 32 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index 4191b56..390559e 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -140,7 +140,6 @@ static inline void tick_nohz_full_add_cpus_to(struct 
cpumask *mask)
cpumask_or(mask, mask, tick_nohz_full_mask);
 }
 
-extern void __tick_nohz_full_check(void);
 extern void tick_nohz_full_kick(void);
 extern void tick_nohz_full_kick_cpu(int cpu);
 extern void tick_nohz_full_kick_all(void);
@@ -149,7 +148,6 @@ extern void __tick_nohz_task_switch(struct task_struct 
*tsk);
 static inline bool tick_nohz_full_enabled(void) { return false; }
 static inline bool tick_nohz_full_cpu(int cpu) { return false; }
 static inline void tick_nohz_full_add_cpus_to(struct cpumask *mask) { }
-static inline void __tick_nohz_full_check(void) { }
 static inline void tick_nohz_full_kick_cpu(int cpu) { }
 static inline void tick_nohz_full_kick(void) { }
 static inline void tick_nohz_full_kick_all(void) { }
@@ -174,12 +172,6 @@ static inline void housekeeping_affine(struct task_struct 
*t)
 #endif
 }
 
-static inline void tick_nohz_full_check(void)
-{
-   if (tick_nohz_full_enabled())
-   __tick_nohz_full_check();
-}
-
 static inline void tick_nohz_task_switch(struct task_struct *tsk)
 {
if (tick_nohz_full_enabled())
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 324482f..7cf37eb 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -197,25 +197,9 @@ static bool can_stop_full_tick(void)
return true;
 }
 
-static void tick_nohz_restart_sched_tick(struct tick_sched *ts, ktime_t now);
-
-/*
- * Re-evaluate the need for the tick on the current CPU
- * and restart it if necessary.
- */
-void __tick_nohz_full_check(void)
-{
-   struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
-
-   if (tick_nohz_full_cpu(smp_processor_id())) {
-   if (ts->tick_stopped && !can_stop_full_tick())
-   tick_nohz_restart_sched_tick(ts, ktime_get());
-   }
-}
-
 static void nohz_full_kick_work_func(struct irq_work *work)
 {
-   __tick_nohz_full_check();
+   /* Empty, the tick restart happens on tick_nohz_irq_exit() */
 }
 
 static DEFINE_PER_CPU(struct irq_work, nohz_full_kick_work) = {
@@ -250,7 +234,7 @@ void tick_nohz_full_kick_cpu(int cpu)
 
 static void nohz_full_kick_ipi(void *info)
 {
-   __tick_nohz_full_check();
+   /* Empty, the tick restart happens on tick_nohz_irq_exit() */
 }
 
 /*
@@ -703,7 +687,9 @@ out:
return tick;
 }
 
-static void tick_nohz_full_stop_tick(struct tick_sched *ts)
+static void tick_nohz_restart_sched_tick(struct tick_sched *ts, ktime_t now);
+
+static void tick_nohz_full_update_tick(struct tick_sched *ts)
 {
 #ifdef CONFIG_NO_HZ_FULL
int cpu = smp_processor_id();
@@ -714,10 +700,10 @@ static void tick_nohz_full_stop_tick(struct tick_sched 
*ts)
if (!ts->tick_stopped && ts->nohz_mode == NOHZ_MODE_INACTIVE)
return;
 
-   if (!can_stop_full_tick())
-   return;
-
-   tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
+   if (can_stop_full_tick())
+   tick_nohz_stop_sched_tick(ts, ktime_get(), cpu);
+   else if (ts->tick_stopped)
+   tick_nohz_restart_sched_tick(ts, ktime_get());
 #endif
 }
 
@@ -847,7 +833,7 @@ void tick_nohz_irq_exit(void)
if (ts->inidle)
__tick_nohz_idle_enter(ts);
else
-   tick_nohz_full_stop_tick(ts);
+   tick_nohz_full_update_tick(ts);
 }
 
 /**
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/