Date: Wed, 21 Jun 2017 15:23:25 +0200
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Mike Galbraith <efault@gmx.de>
Cc: Rik van Riel <riel@redhat.com>, Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH 3/3] sched: Spare idle load balancing on nohz_full CPUs
Message-ID: <20170621132323.GA18957@lerouge>
References: <1497838322-10913-1-git-send-email-fweisbec@gmail.com>
 <1497838322-10913-4-git-send-email-fweisbec@gmail.com>
 <1497980547.20270.106.camel@redhat.com>
 <1497985608.18887.62.camel@gmx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1497985608.18887.62.camel@gmx.de>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3098
Lines: 77

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.