All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dario Faggioli <dario.faggioli@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: andrew.cooper3@citrix.com, julien.grall@arm.com,
	sstabellini@kernel.org, xen-devel@lists.xenproject.org
Subject: Re: [PATCH 5/5] xen: RCU: avoid busy waiting until the end of grace period.
Date: Thu, 10 Aug 2017 15:55:33 +0200	[thread overview]
Message-ID: <1502373333.5719.29.camel@citrix.com> (raw)
In-Reply-To: <1502300045.5719.23.camel@citrix.com>


[-- Attachment #1.1: Type: text/plain, Size: 2384 bytes --]

On Wed, 2017-08-09 at 19:34 +0200, Dario Faggioli wrote:
> On Mon, 2017-08-07 at 02:54 -0600, Jan Beulich wrote:
> > > > > Dario Faggioli <dario.faggioli@citrix.com> 07/27/17 10:01 AM
> > > +/*
> > > + * Timer for making sure the CPU where a callback is queued does
> > > + * periodically poke rcu_pedning(), so that it will invoke the
> > > callback
> > > + * not too late after the end of the grace period.
> > > + */
> > > +void rcu_idle_timer_start()
> > > +{
> > > +    struct rcu_data *rdp = &this_cpu(rcu_data);
> > > +
> > > +    if (likely(!rdp->curlist))
> > > +        return;
> > 
> > I would have expected this to be the inverse of the original
> > condition in
> > rcu_needs_cpu() - why is there no rcu_pending() invocation here?
> > 
> 
> [...]
>
> Actually, it's entirely possible that it is having rcu_pending(cpu)
> in
> rcu_needs_cpu() is, for us, redundant. In fact, although it does make
> sense in Linux, both code inspection and some investigation I've just
> done, makes me believe that there won't be cases where a CPU is
> denied
> going offline because it sees rcu_pending() returning 1.
> 
> In fact, when we call rcu_pending(), within cpu_is_haltable(), we
> have
> already gone through it before. And if there were pending work, we've
> raised the softirq and dealt with it. If there weren't, neither there
> is now.
> 
> I'm therefore leaning toward removing rcu_pending() from the
> rcu_needs_cpu() check as well. At that point, we'll indeed have the
> check inside rcu_start_idle_timer(), be the opposite of the original
> check in rcu_needs_cpu(). :-)
> 
FTR, I'm not so sure of this last thing any longer. I mean, the
analysis I provided is still correct, but I'm investigating the other
possible race existing in the code that Tim has hinted at in his mail,
and I think it could be useful to have rcu_pending() checked in here,
to solve/avoid that one.

It's also possible that I'll actually remove it from rcu_needs_cpu(),
but to move it somewhere else... As I said, I'm still looking into the
problem.

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

      reply	other threads:[~2017-08-10 13:55 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
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 [this message]

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=1502373333.5719.29.camel@citrix.com \
    --to=dario.faggioli@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@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 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.