From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 2/3] xen: RCU: make the period of the idle timer configurable. Date: Thu, 28 Sep 2017 16:58:06 +0200 Message-ID: <1506610686.5001.11.camel@citrix.com> References: <150659250903.4057.6425247157210641083.stgit@Solace.fritz.box> <150659376652.4057.12723423135216244659.stgit@Solace.fritz.box> <59CD100D0200007800180825@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4420116096230245471==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dxaGb-0008CP-Tb for xen-devel@lists.xenproject.org; Thu, 28 Sep 2017 14:58:22 +0000 In-Reply-To: <59CD100D0200007800180825@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 Cc: Stefano Stabellini , Andrew Cooper , Tim Deegan , George Dunlap , Julien Grall , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org --===============4420116096230245471== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-1EILxO6RDHSe+fUox2la" --=-1EILxO6RDHSe+fUox2la Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2017-09-28 at 07:06 -0600, Jan Beulich wrote: > > > > On 28.09.17 at 12:16, wrote: > >=20 > > @@ -569,6 +579,16 @@ void __init rcu_init(void) > > =C2=A0{ > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0void *cpu =3D (void *)(long)smp_processor= _id(); > > =C2=A0 > > +=C2=A0=C2=A0=C2=A0=C2=A0/* We don't allow 0, or anything higher than > > IDLE_TIMER_PERIOD_MAX */ > > +=C2=A0=C2=A0=C2=A0=C2=A0if ( idle_timer_period_ms < 1 || >=20 > The literal 1 here looks suspicious. How about simply refusing 0 > (as well as too high values)? The also simply document the value > must be non-zero in the command line doc. >=20 Ok, sure. > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0idle_timer_perio= d_ms > IDLE_TIMER_PERIOD_MAX / > > MILLISECS(1) ) > > +=C2=A0=C2=A0=C2=A0=C2=A0{ > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0printk("WARNING: rcu-i= dle-timer-period-ms outside of > > [%d,%ld]ms!\n", > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A01, IDLE_TIMER_PERIOD_MAX / MILLISECS(1)); >=20 > Clearly the %d can be literal 1 if the above literal 1 was to stay. > Yes it can. It's actually rather ugly to look at it, the way I managed to write it... Ewww... sorry! :-P > If you follow my suggestion, use "(0," instead.=20 > Yeah, I like this too. I was afraid it was a bit too formal, that not everyone would understand it, but I guess it's actually fine. > As to the %ld - > wouldn't that rather need to be PRI_stime (due to MILLISECS() > returning s_time_t)? >=20 Yes. > And then, as a cosmetic thing, idle_timer_period_ms now isn't > really needed outside of this function. I'd prefer if you moved it > and the integer_param() into this function, to limit their scopes > as much as possible. >=20 Ok. idle_timer_period_ms still wants to go into __initdata, right? Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-1EILxO6RDHSe+fUox2la 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 iQIcBAABCAAGBQJZzQ3/AAoJEBZCeImluHPuroIP/2hvTCWJCGNpVXwDbNxJrdF5 w9xJIPp5jOOkjWI8EYdVAJh8dfIihaqKG9gGJDgWCvWpWL0svHMUjtXPZ8B6yz7U ecznn9xCppOYsGdC+p8q/C9HnOF0I+IlKjJOzNFteJ9KQqemLtMJ0OWtwgDaVKjM ciZd1uakXZ/iXEo+H72/2Q6OPRoO9GEAbZknSX77mHayXkCStl+fGPZnIY6gvUOM cXD3loHwD3UelRq0YJG9GFfSPid25U+k3AgEeLmQQX7ERPvGry64g/9md8Zw5kzu uMggfgZvVBIeIDJhULDnDlNDmImqrRaWei8osoU1U3PqxvVg+fr58Vqis7MoS+xJ p9Bh+xqkAI3tdWRYTlNOlbIeyHqhYEdJjpZmf/x8EIdEJqPpNfyWQJHpOlCmCIkR +7V5KpNx7vHBtr9127Fv7S2vb2NP5dwctajVRLgV3pm4ts7d91HuW5XL6fjo2Rqs qxdRpkOVFC1cS6mAQvWv7xKF5C0stqrFO1KRvPOG1xSxrfapPy9ps0cVWouVd9xU hwVQuQriATXY/xjFtI9f0c0CsOP847o5JyVCspakcH7YZ9wQ+4QYBZuQ0TSBaFco fYUy+8Wzz1dt4/hZ82fnoW16xYp5D2UXFFh0RzksF/EVJonPoQvz+U4zPBo3jQsn RYant8IPXE8c24GUuBaZ =cUzi -----END PGP SIGNATURE----- --=-1EILxO6RDHSe+fUox2la-- --===============4420116096230245471== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============4420116096230245471==--