From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 11/11] xen: credit2: implement true SMT support Date: Mon, 18 Jul 2016 19:24:17 +0200 Message-ID: <1468862657.13039.164.camel@citrix.com> References: <146859397891.10217.10155969474613302167.stgit@Solace.fritz.box> <146859422592.10217.3035174208345104624.stgit@Solace.fritz.box> <19d4fa2d-94ee-2fef-c106-06b8d7e72cfd@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6201206535489497944==" 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 1bPCHQ-0000UK-Hx for xen-devel@lists.xenproject.org; Mon, 18 Jul 2016 17:24:32 +0000 In-Reply-To: <19d4fa2d-94ee-2fef-c106-06b8d7e72cfd@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: George Dunlap , xen-devel@lists.xenproject.org Cc: Anshul Makkar , David Vrabel , Jan Beulich List-Id: xen-devel@lists.xenproject.org --===============6201206535489497944== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-ZmZlNZednRIhsSOVVU+2" --=-ZmZlNZednRIhsSOVVU+2 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2016-07-18 at 17:48 +0100, George Dunlap wrote: > On 15/07/16 15:50, Dario Faggioli wrote: > >=C2=A0 > > +/* > > + * If all the siblings of cpu (including cpu itself) are in > > idlers, > > + * set all their bits in mask. > > + * > > + * In order to properly take into account tickling, idlers needs > > to be > > + * set qeual to something like: > *equal (I can fix this on check-in) >=20 Oops! > > + * > > + *=C2=A0=C2=A0rqd->idle & (~rqd->tickled) > > + * > > + * This is because cpus that have been tickled will very likely > > pick up some > > + * work as soon as the manage to schedule, and hence we should > > really consider > > + * them as busy. > OK this is something that slightly confused me when I was reviewing > the > patch the first time: that rqd->idle is *all* pcpus which are > currently > idle (and thus we need to & (~tickled) when using it), but rqd- > >smt_idle > is meant to be maintained as *non-tickled* idle pcpus. >=20 Short answer is, "yes, this recap of yours is correct". In fact, the difference between idle and smt_idle is that the former is valid instantaneously, while the latter is tracking a state. IOW, if, at any given time, I want to know what pcpus are idle, I check rqd->idle. If I want to know what are idle and also are not (or are unlikely) just about to pick up work, I can check rqd->idle&(~rqd->tickled) Let's now consider smt_idle and assume that, at time t siblings pcpus 2 and 3 are idle (as in, their bit is 1 in rqd->idle). If I'd be basing smt_idle just on that, I could at this point set the bit of the core in smt_idle. This in turn means that work will likely be sent to either 2 or 3 (depending on all the other factors that influence this). Let's assume we select 2. But if either of them --although being idle-- was has actually been tickled already, we may have taken a suboptimal decision. In fact, if 3 was tickled, both 2 and 3 will pick up work, and if there is another core (say, made up of siblings pcpus 6 and 7) which is truly fully idle, we would better have chosen a pcpu from there. If 2 was the one that was tickled, that's even worse, because I most likely have 2 work items, and am tickling only 1 pcpu! So, again, yes, basically this means that I need smt_idle to be representative of the set of non-tickled idle pcpus. > Are you planning at some point to have a follow-up patch which > changes > rqd->idle to be non-tickled idle pcpus as well?=C2=A0=C2=A0Unless I misse= d > something it looks like at the moment the only times rqd->idle is > acted > upon is after &~-ing out rqd->tickled anyway. >=20 I am indeed, but I was planning to do that after this round of changes (this series, plus soft-affinity, plus caps, which I have in my queue). It's, after all, an optimization, and hence I think it is fine to leave it to when things will be proven to be working. :-) If you're saying that this discrepancy between rqd->idle's and rqd->smt_idle's semantic is, at minimum, unideal, I do agree... but I think, for now at least, it's worth living with it. Regards, Dario --=C2=A0 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-ZmZlNZednRIhsSOVVU+2 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 iQIcBAABCAAGBQJXjRDCAAoJEBZCeImluHPu3sUQAKHrtJZYpsITF9SaTDFFW3c3 kTGikhSug1kiuzWvSd5Rc8niQCMv15R8kGBMAaF7ukoqGGZHjzcUTR3nESDipifb r/+JdDuB9o1qMrKoiMCQHSdBlnXNzy6VOyH/fWrIbQ3xXf/IgScr7PIDo5Qim8WS 7a0NjvhSPdi08urg+SUAiEmq759mIM17uyIvO6SS7Iz1NiT40EXqm+gAUM/dRq96 TtKMGvy9hCMUUzaCvmuVIJreDElF0NaT619AN0H5OmKfCe+efHtPZT4pgrB9v7NW 6i9USxL0hUJi2xLj5uOZMILKADEQmzOMHfiTum8UvPqWaDqqkT7m+1mUeHmoe8im onQadIBEgfA741lTqtjtXR+biMroQsj1Chn+XLeR1HpQF1JSQPqIOdx2hJUSyJ3w 65tbKZFFxNn5PAziAlXPUGZHIKfPWEvzXnHWsLYQfYJMEGkHghx6oaaXjdEit6vN mxUAOE7NpxavPXGMZLlIhuYo4n840NfN5QRXtz4e2In48DBSztOyQlTori3c52xH Odmon7QZ5SKbXu9CXsCVQ4QUrsejYor+G9xjnhIauJd9WcUo+mMMxB727e65uaxs 6U7iWwDHbVG7iSLBxJ2BrktcpMHnLYXDSUe5WkRN81gTcfC0JANSZJ1Z0tQhD3WT WF6WkZo7TzonHBbhmh4P =IWOG -----END PGP SIGNATURE----- --=-ZmZlNZednRIhsSOVVU+2-- --===============6201206535489497944== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============6201206535489497944==--