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...
>
next prev parent 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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome