From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 1 of 6 v2] xen: sched_credit: improve picking up the idlal CPU for a VCPU Date: Wed, 12 Dec 2012 11:19:13 +0100 Message-ID: <1355307553.3992.32.camel@Abyss> References: <50C864D402000078000AFDB0@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0810598598097089303==" Return-path: In-Reply-To: <50C864D402000078000AFDB0@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Ian Campbell , Keir Fraser , George Dunlap , xen-devel List-Id: xen-devel@lists.xenproject.org --===============0810598598097089303== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-69b/YboXRwmnXqCMlsxH" --=-69b/YboXRwmnXqCMlsxH Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2012-12-12 at 10:04 +0000, Jan Beulich wrote:=20 > > - weight_cpu =3D cpumask_weight(&cpu_idlers); > > - weight_nxt =3D cpumask_weight(&nxt_idlers); > > + nr_idlers_cpu =3D cpumask_weight(&cpu_idlers); > > + nr_idlers_nxt =3D cpumask_weight(&nxt_idlers); > > /* smt_power_savings: consolidate work rather than spreading i= t */ > > if ( sched_smt_power_savings ? > > - weight_cpu > weight_nxt : > > - weight_cpu * migrate_factor < weight_nxt ) > > + nr_idlers_cpu > nr_idlers_nxt : > > + nr_idlers_cpu * migrate_factor < nr_idlers_nxt ) > > { > > cpumask_and(&nxt_idlers, &cpus, &nxt_idlers); > > spc =3D CSCHED_PCPU(nxt); >=20 > Despite you mentioning this in the description, these last two hunks > are, afaict, only renaming variables (and that's even debatable, as > the current names aren't really misleading imo), and hence I don't > think belong in a patch that clearly has the potential for causing > (performance) regressions. >=20 Ok, I think I can live with the current names too... Just a matter of taste. :-) > That said - I don't think it will (and even more, I'm agreeable to the > change done). >=20 It has been benchmarked, together with the next change, and the results are in the changelog of 2/6. Numbers there show that the combination of those two changes are much more an improvement than anything else, at least for the workloads I considered (which includes sysbench and specjbb2005). Anyway, I think I see your point, and I can either move the remane somewhere else or kill it entirely. > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -396,6 +396,9 @@ extern struct vcpu *idle_vcpu[NR_CPUS]; > > #define is_idle_domain(d) ((d)->domain_id =3D=3D DOMID_IDLE) > > #define is_idle_vcpu(v) (is_idle_domain((v)->domain)) > > =20 > > +#define current_on_cpu(_c) \ > > + ( (per_cpu(schedule_data, _c).curr) ) > > + >=20 > This, imo, really belings into sched-if.h. >=20 Ok. > Plus - what's the point of double parentheses, when in fact none > at all would be needed? >=20 > And finally, why "_c" and not just "c"? >=20 Nothing particular, just "personal macro style", I guess, which I can convert to what you ask and resend. Thanks, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-69b/YboXRwmnXqCMlsxH 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 v1.4.12 (GNU/Linux) iEYEABECAAYFAlDIWiEACgkQk4XaBE3IOsTJpACgoQLHtamf1x64g6aEqFAkOyQe QOoAnR8DLTmnTa92Ti9fHduOp8Yj/UnO =pncH -----END PGP SIGNATURE----- --=-69b/YboXRwmnXqCMlsxH-- --===============0810598598097089303== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============0810598598097089303==--