Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1670999 > unrolled thread

Re: [PATCH 3/3] sched: Spare idle load balancing on nohz_full CPUs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3/3] sched: Spare idle load balancing on nohz_full CPUs Rik van Riel <riel@redhat.com> - 2017-06-20 19:50 +0200
    Re: [PATCH 3/3] sched: Spare idle load balancing on nohz_full CPUs Mike Galbraith <efault@gmx.de> - 2017-06-20 21:10 +0200
      Re: [PATCH 3/3] sched: Spare idle load balancing on nohz_full CPUs Frederic Weisbecker <fweisbec@gmail.com> - 2017-06-21 15:30 +0200
    Re: [PATCH 3/3] sched: Spare idle load balancing on nohz_full CPUs Frederic Weisbecker <fweisbec@gmail.com> - 2017-06-20 22:30 +0200

#1670999 — Re: [PATCH 3/3] sched: Spare idle load balancing on nohz_full CPUs

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2017-06-19 at 04:12 +0200, Frederic Weisbecker wrote:
> Although idle load balancing obviously only concern idle CPUs, it can
> be a disturbance on a busy nohz_full CPU. Indeed a CPU can only get
> rid
> of an idle load balancing duty once a tick fires while it runs a task
> and this can take a while in a nohz_full CPU.
> 
> We could fix that and escape the idle load balancing duty from the
> very
> idle exit path but that would bring unecessary overhead. Lets just
> not
> bother and leave that job to housekeeping CPUs (those outside
> nohz_full
> range). The nohz_full CPUs simply don't want any disturbance.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
>  kernel/sched/fair.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d711093..cfca960 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8659,6 +8659,10 @@ void nohz_balance_enter_idle(int cpu)
>  	if (!cpu_active(cpu))
>  		return;
>  
> +	/* Spare idle load balancing on CPUs that don't want to be
> disturbed */
> +	if (!is_housekeeping_cpu(cpu))
> +		return;
> +
>  	if (test_bit(NOHZ_TICK_STOPPED, nohz_flags(cpu)))
>  		return;

I am not entirely convinced on this one.

Doesn't the if (on_null_domain(cpu_rq(cpu)) test
a few lines down take care of this already?

Do we want nohz_full to always automatically
imply that no idle balancing will happen, like
on isolated CPUs?

-- 
All rights reversed

[toc] | [next] | [standalone]

#1671027

On Tue, 2017-06-20 at 13:42 -0400, Rik van Riel wrote:
> On Mon, 2017-06-19 at 04:12 +0200, Frederic Weisbecker wrote:
> > Although idle load balancing obviously only concern idle CPUs, it can
> > be a disturbance on a busy nohz_full CPU. Indeed a CPU can only get
> > rid
> > of an idle load balancing duty once a tick fires while it runs a task
> > and this can take a while in a nohz_full CPU.
> > 
> > We could fix that and escape the idle load balancing duty from the
> > very
> > idle exit path but that would bring unecessary overhead. Lets just
> > not
> > bother and leave that job to housekeeping CPUs (those outside
> > nohz_full
> > range). The nohz_full CPUs simply don't want any disturbance.
> > 
> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Rik van Riel <riel@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > ---
> >  kernel/sched/fair.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index d711093..cfca960 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8659,6 +8659,10 @@ void nohz_balance_enter_idle(int cpu)
> >  	if (!cpu_active(cpu))
> >  		return;
> >  
> > +	/* Spare idle load balancing on CPUs that don't want to be
> > disturbed */
> > +	if (!is_housekeeping_cpu(cpu))
> > +		return;
> > +
> >  	if (test_bit(NOHZ_TICK_STOPPED, nohz_flags(cpu)))
> >  		return;
> 
> I am not entirely convinced on this one.
> 
> Doesn't the if (on_null_domain(cpu_rq(cpu)) test
> a few lines down take care of this already?
> 
> Do we want nohz_full to always automatically
> imply that no idle balancing will happen, like
> on isolated CPUs?

IMO, nohz_full capable CPUs that are not isolated should automatically
become housekeepers, and nohz_full _active_ upon becoming isolated.
 When a used as a housekeeper, you still pay a price for having the
nohz_full capability available, but it doesn't have to be as high. 

In my kernels, I use cpusets to turn nohz on/off set wise, so CPUs can
be ticking, dyntick, nohz_full or housekeeper, RT load balancing and
cpupri on/off as well if you want to assume full responsibility.  It's
a tad (from box of xxl tads) ugly, but more flexible.

	-Mike

[toc] | [prev] | [next] | [standalone]

#1671664

On Tue, Jun 20, 2017 at 09:06:48PM +0200, Mike Galbraith wrote:
> On Tue, 2017-06-20 at 13:42 -0400, Rik van Riel wrote:
> > On Mon, 2017-06-19 at 04:12 +0200, Frederic Weisbecker wrote:
> > > Although idle load balancing obviously only concern idle CPUs, it can
> > > be a disturbance on a busy nohz_full CPU. Indeed a CPU can only get
> > > rid
> > > of an idle load balancing duty once a tick fires while it runs a task
> > > and this can take a while in a nohz_full CPU.
> > > 
> > > We could fix that and escape the idle load balancing duty from the
> > > very
> > > idle exit path but that would bring unecessary overhead. Lets just
> > > not
> > > bother and leave that job to housekeeping CPUs (those outside
> > > nohz_full
> > > range). The nohz_full CPUs simply don't want any disturbance.
> > > 
> > > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Ingo Molnar <mingo@kernel.org>
> > > Cc: Rik van Riel <riel@redhat.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > ---
> > >  kernel/sched/fair.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > index d711093..cfca960 100644
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -8659,6 +8659,10 @@ void nohz_balance_enter_idle(int cpu)
> > >  	if (!cpu_active(cpu))
> > >  		return;
> > >  
> > > +	/* Spare idle load balancing on CPUs that don't want to be
> > > disturbed */
> > > +	if (!is_housekeeping_cpu(cpu))
> > > +		return;
> > > +
> > >  	if (test_bit(NOHZ_TICK_STOPPED, nohz_flags(cpu)))
> > >  		return;
> > 
> > I am not entirely convinced on this one.
> > 
> > Doesn't the if (on_null_domain(cpu_rq(cpu)) test
> > a few lines down take care of this already?
> > 
> > Do we want nohz_full to always automatically
> > imply that no idle balancing will happen, like
> > on isolated CPUs?
> 
> IMO, nohz_full capable CPUs that are not isolated should automatically
> become housekeepers, and nohz_full _active_ upon becoming isolated.
>  When a used as a housekeeper, you still pay a price for having the
> nohz_full capability available, but it doesn't have to be as high. 

That's right. So in the end checking for housekeeper on idle load balancing
is something we want, but not with the current definition of housekeepers
which is every CPU outside of nohz_full.

I should set this patch aside until I manage to decouple housekeeping from
nohz_full.

> In my kernels, I use cpusets to turn nohz on/off set wise, so CPUs can
> be ticking, dyntick, nohz_full or housekeeper, RT load balancing and
> cpupri on/off as well if you want to assume full responsibility.  It's
> a tad (from box of xxl tads) ugly, but more flexible.

Indeed I think that, in the end, driving the isolation "intensity" through
cpusets is a good idea. It's going to be quite a headache in the case
of nohz_full though if we want to avoid races against tick dependency,
cputime accounting.

But at least I can start to move the other various isolation features
to cpusets.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1671079

On Tue, Jun 20, 2017 at 01:42:27PM -0400, Rik van Riel wrote:
> On Mon, 2017-06-19 at 04:12 +0200, Frederic Weisbecker wrote:
> > Although idle load balancing obviously only concern idle CPUs, it can
> > be a disturbance on a busy nohz_full CPU. Indeed a CPU can only get
> > rid
> > of an idle load balancing duty once a tick fires while it runs a task
> > and this can take a while in a nohz_full CPU.
> > 
> > We could fix that and escape the idle load balancing duty from the
> > very
> > idle exit path but that would bring unecessary overhead. Lets just
> > not
> > bother and leave that job to housekeeping CPUs (those outside
> > nohz_full
> > range). The nohz_full CPUs simply don't want any disturbance.
> > 
> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Rik van Riel <riel@redhat.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > ---
> >  kernel/sched/fair.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index d711093..cfca960 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8659,6 +8659,10 @@ void nohz_balance_enter_idle(int cpu)
> >  	if (!cpu_active(cpu))
> >  		return;
> >  
> > +	/* Spare idle load balancing on CPUs that don't want to be
> > disturbed */
> > +	if (!is_housekeeping_cpu(cpu))
> > +		return;
> > +
> >  	if (test_bit(NOHZ_TICK_STOPPED, nohz_flags(cpu)))
> >  		return;
> 
> I am not entirely convinced on this one.
> 
> Doesn't the if (on_null_domain(cpu_rq(cpu)) test
> a few lines down take care of this already?

It shouldn't, since nohz_full= doesn't imply isolcpus= anymore.
Of course it does if the user manually adds them.

> 
> Do we want nohz_full to always automatically
> imply that no idle balancing will happen, like
> on isolated CPUs?

You're making a good point in that I would prefer that nohz_full be
only about the tick and let some sort of separate isolation subsystem
deal with individual isolation features: nohz, workqueues, idle load
balancing, etc...

That's why I rather used is_housekeeping_cpu() and not !tick_nohz_full_cpu()
because for now housekeepers are ~tick_nohz_full_mask but later it should be
cpu_possible_mask by default or some given set of CPUs defined by the future
isolation subsystem.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web