From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v3 5/6] xen: RCU: avoid busy waiting until the end of grace period. Date: Tue, 5 Sep 2017 19:13:09 +0200 Message-ID: <1504631589.30217.1.camel@citrix.com> References: <150307710991.29525.3681195976643263117.stgit@Solace.fritz.box> <150307947767.29525.16150424729950084786.stgit@Solace.fritz.box> <599C480402000078001720A3@prv-mh.provo.novell.com> <51e7db09-c337-479a-b9c8-15f898bf5839@citrix.com> <59A682CC02000078001755E4@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6355037265729172823==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dpHdm-0007Om-4l for xen-devel@lists.xenproject.org; Tue, 05 Sep 2017 17:27:58 +0000 In-Reply-To: <59A682CC02000078001755E4@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Jan Beulich , George Dunlap Cc: Stefano Stabellini , Wei Liu , George Dunlap , Andrew Cooper , Ian Jackson , Tim Deegan , Julien Grall , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org --===============6355037265729172823== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-OOUPsrZQJmh3apl6FdLb" --=-OOUPsrZQJmh3apl6FdLb Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2017-08-30 at 01:18 -0600, Jan Beulich wrote: > > > > On 29.08.17 at 18:06, wrote: > >=20 > > On 08/22/2017 02:04 PM, Jan Beulich wrote: > > > > > > On 18.08.17 at 20:04, wrote: > > > >=20 > > > > --- a/xen/arch/x86/cpu/mwait-idle.c > > > > +++ b/xen/arch/x86/cpu/mwait-idle.c > > > > @@ -741,9 +741,8 @@ static void mwait_idle(void) > > > > =C2=A0 } > > > > =C2=A0 > > > > =C2=A0 cpufreq_dbs_timer_suspend(); > > > > - > > > > =C2=A0 sched_tick_suspend(); > > > > - /* sched_tick_suspend() can raise TIMER_SOFTIRQ. > > > > Process it now. */ > > > > + /* Timer related operations can raise TIMER_SOFTIRQ. > > > > Process it now. */ > > > > =C2=A0 process_pending_softirqs(); > > >=20 > > > Is this a leftover from v1? Otherwise, why do you do the > > > adjustment > > > here but not in acpi_processor_idle()? > > >=20 > > > > --- a/xen/common/rcupdate.c > > > > +++ b/xen/common/rcupdate.c > > > > @@ -84,8 +84,37 @@ struct rcu_data { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int cpu; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct rcu_head barrier; > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0long=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0last_rs_qlen;=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0/* qlen during the last > > > > resched */ > > > > + > > > > +=C2=A0=C2=A0=C2=A0=C2=A0/* 3) idle CPUs handling */ > > > > +=C2=A0=C2=A0=C2=A0=C2=A0struct timer idle_timer; > > > > +=C2=A0=C2=A0=C2=A0=C2=A0bool idle_timer_active; > > > > =C2=A0}; > > > > =C2=A0 > > > > +/* > > > > + * If a CPU with RCU callbacks queued goes idle, when the > > > > grace period is > > > > + * not finished yet, how can we make sure that the callbacks > > > > will=C2=A0 > >=20 > > eventually > > > > + * be executed? In Linux (2.6.21, the first "tickless idle" > > > > Linux kernel), > > > > + * the periodic timer tick would not be stopped for such CPU. > > > > Here in Xen, > > > > + * we (may) don't even have a periodic timer tick, so we need > > > > to use a > > > > + * special purpose timer. > > > > + * > > > > + * Such timer: > > > > + * 1) is armed only when a CPU with an RCU callback(s) queued > > > > goes idle > > > > + *=C2=A0=C2=A0=C2=A0=C2=A0before the end of the current grace peri= od (_not_ for > > > > any CPUs that > > > > + *=C2=A0=C2=A0=C2=A0=C2=A0go idle!); > > > > + * 2) when it fires, it is only re-armed if the grace period > > > > is still > > > > + *=C2=A0=C2=A0=C2=A0=C2=A0running; > > > > + * 3) it is stopped immediately, if the CPU wakes up from idle > > > > and > > > > + *=C2=A0=C2=A0=C2=A0=C2=A0resumes 'normal' execution. > > > > + * > > > > + * About how far in the future the timer should be programmed > > > > each time, > > > > + * it's hard to tell (guess!!). Since this mimics Linux's > > > > periodic timer > > > > + * tick, take values used there as an indication. In Linux > > > > 2.6.21, tick > > > > + * period can be 10ms, 4ms, 3.33ms or 1ms. Let's use 10ms, to > > > > enable > > > > + * at least some power saving on the CPU that is going idle. > > > > + */ > > > > +#define RCU_IDLE_TIMER_PERIOD MILLISECS(10) > > >=20 > > > With you even mentioning that the original Linux code has ways > > > to use different values, wouldn't it be worth allowing this to be > > > command line controllable? > >=20 > > Dario is on holiday, and I think it would be good to get this > > functionality in sooner rather than later to shake out as many bugs > > as > > possible.=C2=A0=C2=A0Would you be willing to let the idle timer period = be set > > with > > a follow-up patch? >=20 So, I'm back, and can do such a patch. Do we want to enforce a maximum value, to try to at least avoid severe injuries, even for users that shot themselves in the foot? Or we just accept anything which is below STIME_MAX? I personally would only accept values smaller than 100ms (In fact, I was keeping it below that level in patch 6, where the heuristics was implemnted) or, if we really want, 1s. Going above these values is basically equivalent to saying that the bug this series has fixed was not really a bug! :-/ Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-OOUPsrZQJmh3apl6FdLb Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIbBAABCAAGBQJZrtslAAoJEBZCeImluHPuNWAP+KZbFy/3pHzuQjgkuqQAAw9A tEy1Ns5h460fJQxVGv1vdLi40kLa65YETHKSP+BK4nbjZturhh5uszqqojWiuJWp UfLXbW48fSLLmszCJUupzER98AL6W5XScj+URl+lSdR+w//bhGAwAitYxEMplLk4 emF84Nm7Jm5aZlg2UF8tvM/vzhVNK7YPpjlXqNU0+eTJIO8A/o4A9R3Bck0AbSyy LKLcBuUwTe7lE4IDyPC/K0syS8wZFWykvIr5qmjUmE0+Jt5XdtXCvclR/oiQ8NF8 oMOGHfnikQpQiRPL9w/USb8LGUkblIR3vuZMmvW3W9HSdL3YeTpx0L730OqY120n 1a7t+2f8IPfozML26SUCpNLmnP/bPnpwHhaORayTcISqU2n99a1G0k8uAwhxHLX3 TY10z9FNzJ9fAy5HqQs0i/GaeSg0esGTQeRjKsyyJoiovkbtT79pUkNxHbL842bl ah9w1UGTLYfh/p9zQqY0vyxgfEZMb3e1fmO5JG/oG+MjfXBUHsgd6qvl6BIptRJ6 xs6rFzxDdWJxEzn8Qk7NtId7DkCPfcJOQBFiHJxWhmpKJ9scik/qQYs5KZO9Gtwn H2nuzP7KikNXZnJ+WFB6AvTWX3VVzLP1g1D0mZJo0DN/FfNJfsrGTxQB/YIay/8a dpVx1yOEfYz5ovpDCVM= =KCou -----END PGP SIGNATURE----- --=-OOUPsrZQJmh3apl6FdLb-- --===============6355037265729172823== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============6355037265729172823==--