All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luiz Capitulino <lcapitulino@redhat.com>
To: Frederic Weisbecker <frederic@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Chris Metcalf <cmetcalf@mellanox.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Christoph Lameter <cl@linux.com>,
	"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
	Wanpeng Li <kernellwp@gmail.com>, Mike Galbraith <efault@gmx.de>,
	Rik van Riel <riel@redhat.com>
Subject: Re: [GIT PULL] isolation: 1Hz residual tick offloading v3
Date: Tue, 16 Jan 2018 11:52:11 -0500	[thread overview]
Message-ID: <20180116115211.7fd55c9a@redhat.com> (raw)
In-Reply-To: <20180116154055.GA27042@lerouge>

On Tue, 16 Jan 2018 16:41:00 +0100
Frederic Weisbecker <frederic@kernel.org> wrote:

> On Fri, Jan 12, 2018 at 02:18:13PM -0500, Luiz Capitulino wrote:
> > On Thu,  4 Jan 2018 05:25:32 +0100
> > Frederic Weisbecker <frederic@kernel.org> wrote:
> >   
> > > Ingo,
> > > 
> > > Please pull the sched/0hz branch that can be found at:
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
> > > 	sched/0hz
> > > 
> > > HEAD: 9e932b2cc707209febd130978a5eb9f4a943a3f4
> > > 
> > > --
> > > Now that scheduler_tick() has become resilient towards the absence of
> > > ticks, current->sched_class->task_tick() is the last piece that needs
> > > at least 1Hz tick to keep scheduler stats alive.
> > > 
> > > This patchset adds a flag to the isolcpus boot option to offload the
> > > residual 1Hz tick. This way the nohz_full CPUs don't have anymore tick
> > > (assuming nothing else requires it) as their residual 1Hz tick is
> > > offloaded to the housekeepers.
> > > 
> > > For quick testing, say on CPUs 1-7:
> > > 
> > > 	"isolcpus=nohz_offload,domain,1-7"  
> > 
> > Sorry for being very late to this series, but I've a few comments to
> > make (one right now and others in individual patches).
> > 
> > Why are extending isolcpus= given that it's a deprecated interface?
> > Some people have already moved away from isolcpus= now, but with this
> > new feature they will be forced back to using it.  
> 
> I tried to remove isolcpus or at least change the way it works so that its
> effects are reversible (ie: affine the init task instead of isolating domains)
> but that got nacked due to the behaviour's expectations for userspace.
> 
> That's when I realized that kernel parameters are like userspace ABIs,
> they can't be removed easily whether we deprecate them or not.
> 
> Also I needed to be able to control the various isolation features, and
> nohz_full is the wrong place to do that as nohz_full is really just an
> isolation feature like the others, nohz_full= should really just imply
> full dynticks and not watchdog, workqueue or tilegx NAPI isolation...

Yeah, I completely agree with that.

> So isolcpus= is now the place where we control the isolation features
> and nohz is one of them.

That's the part I'm not very sure about. We've been advising users to
move away from isolcpus= when possible, but this very wanted nohz_offload
feature will force everyone back to using isolcpus= again.

I have the impression this series is trying to solve two problems:

 1. How (and where) we control the various isolation features in the
    kernel

 2. Where we add the control for the tick offload feature

I think item 1 is too complex to solve right now. IMHO, this series
should focus on item 2. And regarding item 2, I think we have two
choices to make:

 1. Make tick offload a first class citizen by making it default to
    nohz_full=. If there are regressions, we handle them

 2. Add a new option to nohz_full=, like nohz_full=tick_offload

As an avid user of nohz_full I'm dying to see option 1 happening,
but I'm not totally sure what the consequences can be.

Another idea is to add CONFIG_NOHZ_TICK_OFFLOAD as an experimental
feature.

> The complain about isolcpus is the immutable result. I'm thinking about
> making it modifiable to cpuset but I only see two possible solutions:
> 
> - Make the root cpuset modifiable
> - Create a directory called "isolcpus" visible on the first cpuset mount
>   and move all processes there.

So, if we move the control of the tick offload to nohz_full= itself,
we can completely ditch any isolcpus= change in this series.

I think this should give you a great relief :)

> > What about just adding the new functionality to nohz_full=? That is,
> > no new options, just make the tick go away since this has always been
> > what nohz_full= was intended to do?  
> 
> We can, or have isolcpus=nohz to do it, as both do almost the same.
> 
> But I'm afraid about the overhead for people used to nohz_full= once
> they upgrade their kernels and see those workqueues once per second.
> 
> We can still affine those workqueues (in fact the whole unbound workqueue
> mask) outside the nohz_full range. Still current users may be surprised
> about that new overhead on housekeeping CPUs...
> 

  reply	other threads:[~2018-01-16 16:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-04  4:25 [GIT PULL] isolation: 1Hz residual tick offloading v3 Frederic Weisbecker
2018-01-04  4:25 ` [PATCH 1/5] sched: Rename init_rq_hrtick to hrtick_rq_init Frederic Weisbecker
2018-01-04  4:25 ` [PATCH 2/5] sched/isolation: Add scheduler tick offloading interface Frederic Weisbecker
2018-01-04  4:25 ` [PATCH 3/5] nohz: Allow to check if remote CPU tick is stopped Frederic Weisbecker
2018-01-04  4:25 ` [PATCH 4/5] sched/isolation: Residual 1Hz scheduler tick offload Frederic Weisbecker
2018-01-12 19:22   ` Luiz Capitulino
2018-01-16 15:57     ` Frederic Weisbecker
2018-01-16 16:53       ` Luiz Capitulino
2018-01-04  4:25 ` [PATCH 5/5] sched/isolation: Document "nohz_offload" flag Frederic Weisbecker
2018-01-12 19:18 ` [GIT PULL] isolation: 1Hz residual tick offloading v3 Luiz Capitulino
2018-01-16 15:41   ` Frederic Weisbecker
2018-01-16 16:52     ` Luiz Capitulino [this message]
2018-01-16 22:51       ` Frederic Weisbecker
2018-01-17 17:38         ` Luiz Capitulino
2018-01-18  3:04           ` Frederic Weisbecker
2018-01-18 14:02             ` Luiz Capitulino
2018-01-16 17:58     ` Mike Galbraith
2018-01-16 22:53       ` Frederic Weisbecker
2018-01-17 14:51       ` Christopher Lameter
2018-01-17 15:59         ` Mike Galbraith
2018-01-17 16:32           ` Christopher Lameter
2018-01-17 16:58             ` Mike Galbraith

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=20180116115211.7fd55c9a@redhat.com \
    --to=lcapitulino@redhat.com \
    --cc=cl@linux.com \
    --cc=cmetcalf@mellanox.com \
    --cc=efault@gmx.de \
    --cc=frederic@kernel.org \
    --cc=kernellwp@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --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.