From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 3/7] xen: credit2: soft-affinity awareness in fallback_cpu() Date: Tue, 25 Jul 2017 18:47:23 +0200 Message-ID: <1501001243.26429.8.camel@citrix.com> References: <149762114626.11899.6393770850121347748.stgit@Solace.fritz.box> <149762243723.11899.13163340131516329714.stgit@Solace.fritz.box> <45a42cae-9683-9fd7-690f-6f5424e24ea9@citrix.com> <1500998459.26429.4.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5549177808229653645==" 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 1da2ze-0005CN-76 for xen-devel@lists.xenproject.org; Tue, 25 Jul 2017 16:47:34 +0000 In-Reply-To: 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: George Dunlap , Anshul Makkar , "Justin T. Weaver" List-Id: xen-devel@lists.xenproject.org --===============5549177808229653645== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-gRnVCaf+BMuhms7XvWUr" --=-gRnVCaf+BMuhms7XvWUr Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2017-07-25 at 17:17 +0100, George Dunlap wrote: > On 07/25/2017 05:00 PM, Dario Faggioli wrote: > > On Tue, 2017-07-25 at 11:19 +0100, George Dunlap wrote: > > >=20 > > Mmm.. I think you're right. In fact, in a properly configured > > system, > > we'll never go past step 3 (from the comment at the top). > >=20 > > Which is not ideal, or at least not what I had in mind. In fact, I > > think it's better to check step 4 (svc->vcpu->processor in hard- > > affinity) and step 5 (a CPU from svc's runqueue in hard affinity), > > as > > that would mean avoiding a runqueue migration. > >=20 > > What about I basically kill step 3, i.e., if we reach this point > > during > > the soft-affinity step, I just continue to the hard-affinity one? >=20 > Hmm, well *normally* we would rather have a vcpu running within its > soft > affinity, even if that means moving it to another runqueue.=C2=A0=C2=A0 > Yes, but both *ideally* and *normally*, we just should not be here. :-) If we did end up here, we're in guessing territory, and, although what you say about a guest wanting to run on within its soft-affinity is always true, from the guest own point of view, our job as the scheduler is to do what would be best for the system as a whole. But we are in a situation where we could not gather the information to make such a decision. > Is your > idea that, the only reason we're in this particular code is because > we > couldn't grab the lock we need to make a more informed decision; so > defer if possible to previous decisions, which (we might presume) was > able to make a more informed decision? >=20 Kind of, yes. Basically I think we should "escape" from this situation as quickly as possible, and causing as few troubles as possible to both ourself and to others, in the hope that it will go better next time. Trying to stay in the same runqueue seems to me to fit this requirement, as: - as you say, we're here because a previous (presumably well informed) decision brought us here so, hopefully, staying here is not too bad,=C2= =A0 neither for us nor overall; - staying here is quicker and means less overhead for svc; - staying here means less overhead overall. In fact, if we decide to=C2=A0 change runqueue, we will have to take the remote runqueue lock at=C2=A0 some point... And I'd prefer that to be for good reasons. All that being said, it probably would be good to add a performance counter, and try to get a sense of how frequently we actually end up in this function as a fallback. But in the meantime, yes, I'd try to make svc stay in the runqueue where it is, in this case, if possible. > > ASSERT_UNREACHABLE() is indeed much better. What do you mean with > > "something random"? The value to be assigned to cpu? >=20 > Er, yes, I meant the return value.=C2=A0=C2=A0Returning 0, or v->processo= r > would > be simple options.=C2=A0=C2=A0*Really* defensive programming would attemp= t to > chose something somewhat sensible with the minimal risk of triggering > some other hidden assumptions (say, any cpu on our current runqueue). > But part of me says even thinking too long about it is a waste of > time > for something we're 99.99% sure can never happen. :-) >=20 Agreed. IAC, I'll go for ASSERT_UNREACHABLE() and then see about using either v->processor (with a comment), or a cpumask_any(something). Of course the latter is expensive, but it should not be a big problem, considering we'll never get there (I'll have a look at generated the assembly, to confirm that). Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-gRnVCaf+BMuhms7XvWUr 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 iQIcBAABCAAGBQJZd3YcAAoJEBZCeImluHPu6E8P/2HoqhTH6ZbDvto7QoB0gYq4 WBhe16n6zlyDMLJxAWCYIic8cOrCOfTUVeK6Eg2jn71jCHiAIYHQoBfeOUc8GVNR q3cb7Xh2+20fXpaBxiRNDSky8lE5VZbgMsN3/X56Sq9SBvPd3swXutuCeBbzDCXg cPbGvsk/WMQi7gaxVYU1OEOlqtYJd8wkmAlir6/iy54nOJy5qb/N37bpbNwbZvkR 6JbUYCQQEXb96QQliWLiQod1AItq4e/GgErzrCA7+kyjyz200W16rM8zdRz+p5SR 4K7kBToKLPAW8JJnd7TnBijU7BDxpNvZIWhYfU7r4hEsoMWptdO5q7nrYb0U3rI7 kOkp67zXoozy7XB75MRV/ZH4bZtU3dbUcIF9NSuBZrReXpTAENWQNUH/tQr81Aj2 Pi0Zp8b1k0Q0bPwPi6Dw/DKH9uPcRZqperLCANtDZQb9DK/ve6AfFhIla+3k2lUy fOVby3hkwHMA8NC3SgPJQMkScUN+sdL3tlhiajYaEtg52v/sJ+XUqCSP5513pKNb C9EBZ1lsStVTeSq0ysA0g95485wNohWoQ8u1fWr9AH4f1hZ14HDxy271iX5+Q37x UUQFsTgfIZKy3wWJBnN+Gm7PBJUqqIDKQfdD2H/SeEhDIOwvT9w8vY1p4NdYnkxo 7rBUv3X8ww5dMn5awz84 =pcT/ -----END PGP SIGNATURE----- --=-gRnVCaf+BMuhms7XvWUr-- --===============5549177808229653645== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============5549177808229653645==--