From: Dario Faggioli <dario.faggioli@citrix.com>
To: Julien Grall <julien.grall@arm.com>, xen-devel@lists.xenproject.org
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
nd@arm.com, Stefano Stabellini <sstabellini@kernel.org>,
Jan Beulich <jbeulich@suse.com>
Subject: Re: [PATCH 5/5] xen: RCU: avoid busy waiting until the end of grace period.
Date: Tue, 1 Aug 2017 11:17:39 +0200 [thread overview]
Message-ID: <1501579059.30551.7.camel@citrix.com> (raw)
In-Reply-To: <ac3e66e4-3962-4fae-3fc2-56410f845ad4@arm.com>
[-- Attachment #1.1: Type: text/plain, Size: 3555 bytes --]
On Tue, 2017-08-01 at 09:54 +0100, Julien Grall wrote:
> Hi Dario,
>
> On 27/07/2017 09:01, Dario Faggioli wrote:
> > Instead of having the CPU where a callback is queued, busy
> > looping on rcu_pending(), use a timer.
> >
> > In fact, we let the CPU go idla,e but we program a timer
> > that will periodically wake it up, for checking whether the
> > grace period has actually ended.
> >
> > It is kind of similar to introducing a periodic tick, but
> > with a much more limited scope, and a lot less overhead. In
> > fact, this timer is:
> > - only active for the CPU(s) that have callbacks queued,
> > waiting for the end of a grace period;
> > - only active when those CPU(s) are idle (and stopped as
> > soon as they resume execution).
>
> If I read this correctly, it means on ARM the idling will now get
> interrupted periodically. This is a bit unfortunate, given that if
> you
> have a CPU doing nothing, you would still interrupt it
> intermittently.
>
Not really periodically, not always, at least. What this really means
is that a CPU that is idle, *but* have pending RCU callbacks, will be
interrupted periodically to see if the grace period ended, so it can
invoke the callbacks.
As soon as this (callbacks being invoked) will have happened, we won't
interrupt it any longer.
And idle CPUs _without_ queued RCU callbacks, won't be interrupted at
all.
> I was expected that we could remove the CPU from the RCU whilst it
> is
> idle. Is there any reason for not doing that?
>
I'm not sure I understand what you mean here. I tried to explain as
good as I could how this works, and why I think it can't work in other
ways, in this reply to Stefano: <1501548445.30551.5.camel@citrix.com>
Every CPU that participates in the grace period, and has already
quiesced, is "removed from RCU", and hence, when it becomes idle, it is
never interrupted (by this timer). With the only exception of the
CPU(s) that has queued callbacks.
We simply can't forget about these CPUs, even if they go idle. If we
do, the callbacks won't be invoked never (or will only be invoked when
the CPU becomes active again, which may happen really really late,
which is part of the reason why we're seeing the bug we're seeing).
Linux does this, in the *exact* same way (well, actually, in a totally
different way, from an implementation point of view, but the effect is
indeed exactly the same):
See here:
http://elixir.free-electrons.com/linux/v2.6.21/source/kernel/time/tick-sched.c#L198
We're in tick_nohz_stop_sched_tick(), i.e., the CPU is going idle, and
the periodic timer tick is being stopped (no interruptions). But
if (rcu_needs_cpu(cpu))
delta_jiffies = 1;
Where, rcu_needs_cpu() means:
/*
* Check to see if any future RCU-related work will need to be done
* by the current CPU, even if none need be done immediately, returning
* 1 if so.
*/
int rcu_needs_cpu(int cpu)
Then, again in tick_nohz_stop_sched_tick()
/*
* Do not stop the tick, if we are only one off
* or if the cpu is required for rcu
*/
if (!ts->tick_stopped && delta_jiffies == 1)
goto out;
And in fact, in my testing, without patches 3 and 5 applied, the bug is
still there.
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2017-08-01 9:17 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-27 8:01 [PATCH 0/5] xen: RCU: x86/ARM: Add support of rcu_idle_{enter, exit} Dario Faggioli
2017-07-27 8:01 ` [PATCH 1/5] xen: in do_softirq() sample smp_processor_id() once and for all Dario Faggioli
2017-07-27 8:01 ` [PATCH 2/5] xen: ARM: suspend the tick (if in use) when going idle Dario Faggioli
2017-07-31 20:59 ` Stefano Stabellini
2017-08-01 8:53 ` Julien Grall
2017-08-01 9:26 ` Dario Faggioli
2017-07-27 8:01 ` [PATCH 3/5] xen: RCU/x86/ARM: discount CPUs that were idle when grace period started Dario Faggioli
2017-07-31 21:17 ` Stefano Stabellini
2017-08-07 8:35 ` Jan Beulich
2017-08-09 8:48 ` Dario Faggioli
2017-08-09 8:57 ` Jan Beulich
2017-08-09 9:20 ` Dario Faggioli
2017-08-09 9:26 ` Jan Beulich
2017-08-09 11:38 ` Tim Deegan
2017-08-11 17:25 ` Dario Faggioli
2017-08-14 10:39 ` Tim Deegan
2017-08-14 13:24 ` Dario Faggioli
2017-08-14 13:54 ` Tim Deegan
2017-08-14 16:21 ` Dario Faggioli
2017-08-14 16:47 ` Tim Deegan
2017-08-14 9:19 ` Dario Faggioli
2017-07-27 8:01 ` [PATCH 4/5] xen: RCU: don't let a CPU with a callback go idle Dario Faggioli
2017-08-07 8:38 ` Jan Beulich
2017-07-27 8:01 ` [PATCH 5/5] xen: RCU: avoid busy waiting until the end of grace period Dario Faggioli
2017-07-31 21:20 ` Stefano Stabellini
2017-07-31 22:03 ` Dario Faggioli
2017-07-31 23:58 ` Stefano Stabellini
2017-08-01 0:47 ` Dario Faggioli
2017-08-01 19:13 ` Stefano Stabellini
2017-08-02 10:14 ` Dario Faggioli
2017-08-01 8:45 ` Julien Grall
2017-08-01 8:54 ` Julien Grall
2017-08-01 9:17 ` Dario Faggioli [this message]
2017-08-01 10:22 ` Julien Grall
2017-08-01 10:33 ` Dario Faggioli
2017-08-07 8:54 ` Jan Beulich
2017-08-09 17:34 ` Dario Faggioli
2017-08-10 13:55 ` Dario Faggioli
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=1501579059.30551.7.camel@citrix.com \
--to=dario.faggioli@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=jbeulich@suse.com \
--cc=julien.grall@arm.com \
--cc=nd@arm.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/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;
as well as URLs for NNTP newsgroup(s).