From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 1/6] xen: credit1: simplify csched_runq_steal() a little bit. Date: Fri, 3 Mar 2017 14:39:38 +0100 Message-ID: <1488548378.5548.208.camel@citrix.com> References: <148844531279.23452.17528540110704914171.stgit@Solace.fritz.box> <148845108437.23452.18282287504552403796.stgit@Solace.fritz.box> <2ce79bdd-7218-02e3-a015-3e4eb6d469b0@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8958631242040808891==" Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cjnQx-0000lY-G4 for xen-devel@lists.xenproject.org; Fri, 03 Mar 2017 13:39:47 +0000 In-Reply-To: <2ce79bdd-7218-02e3-a015-3e4eb6d469b0@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: anshul makkar , xen-devel@lists.xenproject.org Cc: George Dunlap List-Id: xen-devel@lists.xenproject.org --===============8958631242040808891== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-thXo5IC9PDOw6Dnnfm1B" --=-thXo5IC9PDOw6Dnnfm1B Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2017-03-03 at 09:35 +0000, anshul makkar wrote: > On 02/03/17 10:38, Dario Faggioli wrote: > > --- a/xen/common/sched_credit.c > > +++ b/xen/common/sched_credit.c > > @@ -1593,64 +1593,65 @@ static struct csched_vcpu * > > =C2=A0 csched_runq_steal(int peer_cpu, int cpu, int pri, int > > balance_step) > > =C2=A0 { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0const struct csched_pcpu * const pe= er_pcpu =3D > > CSCHED_PCPU(peer_cpu); > > -=C2=A0=C2=A0=C2=A0=C2=A0const struct vcpu * const peer_vcpu =3D curr_o= n_cpu(peer_cpu); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct csched_vcpu *speer; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct list_head *iter; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct vcpu *vc; > > =C2=A0=C2=A0 > > +=C2=A0=C2=A0=C2=A0=C2=A0ASSERT(peer_pcpu !=3D NULL); > > + > > =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* Don't steal from an idle CP= U's runq because it's about to > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* pick up work from it itself= . > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*/ > > -=C2=A0=C2=A0=C2=A0=C2=A0if ( peer_pcpu !=3D NULL && !is_idle_vcpu(peer= _vcpu) ) > > +=C2=A0=C2=A0=C2=A0=C2=A0if ( unlikely(is_idle_vcpu(curr_on_cpu(peer_cp= u))) ) > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0goto out; > We can just use if (!is_idle_vcpu(peer_vcpu)). Why to replace it > with=C2=A0 > some code that introduces an unnecessary branch statement. > Mmm... I don't think I understand what this means. > > +=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=A0=C2=A0=C2=A0* If the vcpu ha= s no useful soft affinity, skip this > > vcpu. > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* In fact, what = we want is to check if we have any "soft- > > affine > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* work" to steal= , before starting to look at "hard-affine=20 > > work". > > +=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=A0=C2=A0=C2=A0=C2=A0* Notice that, i= f not even one vCPU on this runq has a > > useful > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* soft affinity,= we could have avoid considering this > > runq for > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* a soft balanci= ng step in the first place. This, for > > instance, > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* can be impleme= nted by taking note of on what runq there > > are > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* vCPUs with use= ful soft affinities in some sort of > > bitmap > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* or counter. > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*/ > > Isn't it a better approach that now as we have came across a vcpu > which=C2=A0 > doesn't have a desired soft affinity but is a potential candidate > for=C2=A0 > migration, so instead of just forgetting it,=C2=A0=C2=A0lets save the > information=C2=A0 > for such vcpus in some data structure in some order so that we don't=C2= =A0 > have to scan them again if we don't find anything useful in the > present run. > So, AFAIUI, you're suggesting something like this: =C2=A01. for each vcpu in the runqueue, we check soft-affinity. If it=C2=A0 =C2=A0 =C2=A0 matches, we're done; =C2=A02. if it does not match, we check hard-affinity. If it matches, we=C2= =A0 =C2=A0 =C2=A0 save that vcpu somewhere. We only need to save one vcpu, the = first=C2=A0 =C2=A0 =C2=A0 one that we find to have matching hard-affinity; =C2=A03. if we don't find any vcpu with matching soft affinity, we steal=C2= =A0 =C2=A0 =C2=A0 the one we've saved. Is this correct? If yes, if there's not anything I'm overlooking, I think it could work. It's a separate patch, of course. I can try putting that together, unless of course you want to give this a go yourself. :-) Thanks and Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-thXo5IC9PDOw6Dnnfm1B 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 iQIcBAABCAAGBQJYuXIbAAoJEBZCeImluHPu88sQAOZ2GEc5PqViNBXntuouajYe +7kiB1iE4MOReDRdiWX4l8UOWzlq2iwwl9rM9hol+zL+2n141cxMRFGK51VE4OkV CNZPkBTJMTloV8IhDUr6yxIXn2cEtb7jltNS3gJC45FzVJ4qBUp9Qv5qj76Uba4J slD86aUGofyEoPbfwf22bhrVJqfbiDkTwOwy+ObrIYBt+iC2xCJJ7LzwF+Ok6d3p oVS8zydk4vLNEH7nSm5QOmhIaeL2fk25jvzLAuOr26z0tmRnTlfVjpzJceXemnOb PL5KN8Oufk/brtMGH1vGWINbUJDaT/9bMMnCZtsWUsZr0w2r39EMx99tiNzrx3mT +vcZVzikAEaIAJYsCA1szQvlRdwGNPQ/NfSZw5HoRcuKQrljjXJQ4XAbiwr3ZDqP rDcTvN1fs2WqY5UIVqdic7W+dM4eDHRQafUipmuJCPMy6aBG193RwHl3TALr7BOe c7P1P92B3+94iI3mkmIBNL5aGmmI162kAzft9N0nHtSDeNbw459gQiE8NaegFL1R 7IZ1UyV6Ki+dvuixCMEHhSp+g7SG52iWSSlnfB/GyrFKXDYbyCzjCdnv5h6LPhdh DrQkqiVc4Qq5Jr7AK2JNsVxg6VaLXgAaZTE5hC7JT+gAXoTfQwsjKKYpZN92c5Fa vwvUWnSnW+29TNR0zyOF =yqso -----END PGP SIGNATURE----- --=-thXo5IC9PDOw6Dnnfm1B-- --===============8958631242040808891== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============8958631242040808891==--