From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 1/3] xen: sched: introduce the 'null' semi-static scheduler Date: Thu, 6 Apr 2017 17:07:11 +0200 Message-ID: <1491491231.18721.20.camel@citrix.com> References: <148977585611.29510.906390949919041674.stgit@Palanthas.fritz.box> <148977617833.29510.4160128186395621610.stgit@Palanthas.fritz.box> <070ec8e5-ee87-ef5f-e83c-099c9743e926@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7884291855645290948==" 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 1cw90u-0006SY-6K for xen-devel@lists.xenproject.org; Thu, 06 Apr 2017 15:07:56 +0000 In-Reply-To: <070ec8e5-ee87-ef5f-e83c-099c9743e926@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: Jonathan Davies , Julien Grall , Stefano Stabellini , Marcus Granado List-Id: xen-devel@lists.xenproject.org --===============7884291855645290948== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-i+F7/VbEKJgCs4SPyJD+" --=-i+F7/VbEKJgCs4SPyJD+ Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2017-03-27 at 11:31 +0100, George Dunlap wrote: > On 17/03/17 18:42, Dario Faggioli wrote: > > +static void null_vcpu_insert(const struct scheduler *ops, struct > > vcpu *v) > > +{ > > +=C2=A0=C2=A0=C2=A0=C2=A0struct null_private *prv =3D null_priv(ops); > > +=C2=A0=C2=A0=C2=A0=C2=A0struct null_vcpu *nvc =3D null_vcpu(v); > > +=C2=A0=C2=A0=C2=A0=C2=A0unsigned int cpu; > > +=C2=A0=C2=A0=C2=A0=C2=A0spinlock_t *lock; > > + > > +=C2=A0=C2=A0=C2=A0=C2=A0ASSERT(!is_idle_vcpu(v)); > > + > > + retry: > > +=C2=A0=C2=A0=C2=A0=C2=A0lock =3D vcpu_schedule_lock_irq(v); > > + > > +=C2=A0=C2=A0=C2=A0=C2=A0cpu =3D pick_cpu(prv, v); > > + > > +=C2=A0=C2=A0=C2=A0=C2=A0/* We hold v->processor's runq lock, but we ne= ed cpu's one */ > > +=C2=A0=C2=A0=C2=A0=C2=A0if ( cpu !=3D v->processor ) > > +=C2=A0=C2=A0=C2=A0=C2=A0{ > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0spin_unlock(lock); > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0lock =3D pcpu_schedule= _lock(cpu); >=20 > Don't we need to hold the lock for v->processor until we change > v->processor?=C2=A0=C2=A0Otherwise someone might call vcpu_schedule_lock(= v) at > this point and reasonably believe that is has the right to modify v. >=20 Yes, this is actually the case. > Or does this not matter because we're just now calling insert (and so > nobody else is going to call vcpu_schedule_lock() on v? >=20 Indeed no one will. But I still prefer to turn this into something much more similar to what other schedulers do, and what is the most correct and safe to do, i.e., as you suggest, change v->processor while holding the original lock. > > diff --git a/xen/common/schedule.c b/xen/common/schedule.c > > index 223a120..b482037 100644 > > --- a/xen/common/schedule.c > > +++ b/xen/common/schedule.c > > @@ -1785,6 +1785,8 @@ int schedule_cpu_switch(unsigned int cpu, > > struct cpupool *c) > > =C2=A0 > > =C2=A0 out: > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0per_cpu(cpupool, cpu) =3D c; > > +=C2=A0=C2=A0=C2=A0=C2=A0/* Trigger a reschedule so the CPU can pick up= some work ASAP. > > */ > > +=C2=A0=C2=A0=C2=A0=C2=A0cpu_raise_softirq(cpu, SCHEDULE_SOFTIRQ); >=20 > Is this a more generic fix / improvement? >=20 Yep, I'm moving it in its own patch. 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) --=-i+F7/VbEKJgCs4SPyJD+ 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 iQIcBAABCAAGBQJY5lmfAAoJEBZCeImluHPuUeMQAKNaeEh0YiN5y1/5z4G0DN9+ ra3ku/Z9NMv/La9uBQUY/ZQJbY/TiHeFok+qI8Wgened6Z+YK6VsxtoBCLP6aWUm k3IWDpEYH1hBWtbjsqUaIA4rA2Y0EZ8Bu+vuArfFlpvAflbO5aCZBodBC+et/gir WxMbBIR9AwqeFRgAmaAkQJNkjULXUJIovnQIYWFX0ZNFkMEtBLD+cbaFK9qyaUTd GTMdpxlXw71NBuUj7E5PwnIl1rlWJzPoJOx3OeZdLoVCdH6u0eFTDw+5eKyTPh3G 9iolbznoiZ7vIM5a76TTjt+b1bwhTPVb4dL+N1e4tAuo2X5fvkmS6W0RRqfOWCd0 MglidvMWVJg/TiZxF4lGl6t3bsNz8Trpi8QZGKwb4z4vC3g4eoJgbVK9xbDrFdP4 dhPBnFdijqtjDhV28CCCG8Ikw2labbtI4DVhIOvBHu3cgLLwqVLeZW8heOYLc/sy tUT/+6b4daQaot5UzdqHfDZDUEB8wBVxRq3yf45Iv/3sQKYO6IUJ0fOTGe5i3dtR YvtWrYcC0z+QKQhbrxs764t+F4T1sjuPwxt+Z2ogSQlaqhbJY1jpX8tdx7Wo/Iem qSeFf+jbuI0o5qEnxc/1nbD20pS8hTTtaMHPNKpq8UbSg1a5nxE/WQUqHaHc6LEU T5i5e6R0GuSYrBB3Cih1 =I/34 -----END PGP SIGNATURE----- --=-i+F7/VbEKJgCs4SPyJD+-- --===============7884291855645290948== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============7884291855645290948==--