From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 2/2] sched_credit2.c: runqueue_per_core code Date: Wed, 18 Mar 2015 08:53:57 +0000 Message-ID: <1426668836.2560.13.camel@citrix.com> References: <20150313181109.GA3179@gmail.com> <55032C85.6090805@citrix.com> <550336EE.60909@eu.citrix.com> <5506DF40020000780006A407@mail.emea.novell.com> <5506D1E1.8050404@eu.citrix.com> <5506E11F020000780006A426@mail.emea.novell.com> <1426616308.32500.98.camel@citrix.com> <55093DD1020000780006B1F2@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4065682996646994804==" Return-path: In-Reply-To: <55093DD1020000780006B1F2@mail.emea.novell.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "JBeulich@suse.com" Cc: Andrew Cooper , "xen-devel@lists.xen.org" , George Dunlap , "uma.sharma523@gmail.com" List-Id: xen-devel@lists.xenproject.org --===============4065682996646994804== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-zaRzWUqzQtM+B/u4pxWw" --=-zaRzWUqzQtM+B/u4pxWw Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2015-03-18 at 07:56 +0000, Jan Beulich wrote: > >>> On 17.03.15 at 19:18, wrote: > > --- a/xen/common/sched_credit2.c > > +++ b/xen/common/sched_credit2.c > > @@ -1936,12 +1936,8 @@ static void init_pcpu(const struct scheduler *op= s, int cpu) > > } > > =20 > > /* Figure out which runqueue to put it in */ > > - rqi =3D 0; > > - > > - /* Figure out which runqueue to put it in */ > > - /* NB: cpu 0 doesn't get a STARTING callback, so we hard-code it t= o runqueue 0. */ > > - if ( cpu =3D=3D 0 ) > > - rqi =3D 0; > > + if ( system_state =3D=3D SYS_STATE_boot ) >=20 > ... I'd suggest using <=3D SYS_STATE_boot or < SYS_STATE_smp_boot. >=20 Ok. > > @@ -1986,9 +1982,13 @@ static void init_pcpu(const struct scheduler *op= s, int cpu) > > static void * > > csched2_alloc_pdata(const struct scheduler *ops, int cpu) > > { > > - /* Check to see if the cpu is online yet */ > > - /* Note: cpu 0 doesn't get a STARTING callback */ > > - if ( cpu =3D=3D 0 || cpu_to_socket(cpu) >=3D 0 ) > > + /* > > + * Actual initialization is deferred to when the pCPU will be > > + * online, via a STARTING callback. The only exception is > > + * the boot cpu, which does not get such a notification, and > > + * hence needs to be taken care of here. > > + */ > > + if ( system_state =3D=3D SYS_STATE_boot ) > > init_pcpu(ops, cpu); >=20 > Same here, plus the new condition at the first glance isn't matching > the old one, but perhaps that's intentional. >=20 It is intentional, and that is why we're changing it! :-) Let me try to explain: The idea, in both old and new code, is to call init_pcpu() either: a) on the boot cpu, during boot b) on non-boot cpu, *only* if they are online. The issue is that, for assessing b), it checks whether cpu_to_socket(cpu) returns >=3D0 and, if it does, it assumes the cpu is online. That is not true, as cpu_data.phys_proc_id (which is what cpu_to_socket() go reading) is 0 even before any onlining and topolog identification has happened (it's a global). Therefore, all the pcpus are initialized right away, and all are assigned to runqueue 0, as returned by cpu_to_socket() at this stage, which is clearly wrong. In fact, this is the reason why, previous versions of this took the approach of initializing things such as cpu_to_socket() returned -1 before topology identification. In the new version, as you suggested, I'm using system_state to figure out whether we are dealing with the boot cpu, and that's it. :-) Hope this clarifies things... If yes, I'll make sure to put a similar explanation in the changelog, when sending the patch officially. Regards, Dario --=-zaRzWUqzQtM+B/u4pxWw 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 iEYEABECAAYFAlUJPSQACgkQk4XaBE3IOsTihQCeMMjNZ9UdWUodIfsyFdHLGZfI jnQAnRJoVcqhoebkP3Lk3vs+M44l/+SB =/YJy -----END PGP SIGNATURE----- --=-zaRzWUqzQtM+B/u4pxWw-- --===============4065682996646994804== 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 --===============4065682996646994804==--