From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 2/7] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS Date: Thu, 8 Oct 2015 17:49:34 +0200 Message-ID: <1444319374.22254.42.camel@citrix.com> References: <20151008124027.12522.42552.stgit@Solace.station> <20151008125243.12522.8999.stgit@Solace.station> <561688D9.30509@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1796063054305852156==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZkDRw-0002Qn-Lh for xen-devel@lists.xenproject.org; Thu, 08 Oct 2015 15:49:44 +0000 In-Reply-To: <561688D9.30509@citrix.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: George Dunlap , xen-devel@lists.xenproject.org Cc: George Dunlap , Andrew Cooper , Meng Xu , Jan Beulich List-Id: xen-devel@lists.xenproject.org --===============1796063054305852156== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-CL5V3o83OjtV3PJINCSq" --=-CL5V3o83OjtV3PJINCSq Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2015-10-08 at 16:16 +0100, George Dunlap wrote: > On 08/10/15 13:52, Dario Faggioli wrote: > > diff --git a/xen/common/schedule.c b/xen/common/schedule.c > > index c5f640f..9aa209d 100644 > > --- a/xen/common/schedule.c > > +++ b/xen/common/schedule.c > > @@ -1488,9 +1488,7 @@ void __init scheduler_init(void) > > =20 > > int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) > > { > > - unsigned long flags; > > struct vcpu *idle; > > - spinlock_t *lock; > > void *ppriv, *ppriv_old, *vpriv, *vpriv_old; > > struct scheduler *old_ops =3D per_cpu(scheduler, cpu); > > struct scheduler *new_ops =3D (c =3D=3D NULL) ? &ops : c->sched; > > @@ -1509,8 +1507,6 @@ int schedule_cpu_switch(unsigned int cpu, > > struct cpupool *c) > > return -ENOMEM; > > } > > =20 > > - lock =3D pcpu_schedule_lock_irqsave(cpu, &flags); > > - > > SCHED_OP(old_ops, tick_suspend, cpu); > > vpriv_old =3D idle->sched_priv; > > idle->sched_priv =3D vpriv; > > @@ -1520,8 +1516,6 @@ int schedule_cpu_switch(unsigned int cpu, > > struct cpupool *c) > > SCHED_OP(new_ops, tick_resume, cpu); > > SCHED_OP(new_ops, insert_vcpu, idle); > > =20 > > - pcpu_schedule_unlock_irqrestore(lock, flags, cpu); >=20 > It seems to me that the locking here wasn't to protect insert_vcpu, > but > to prevent any scheduling events from happening on cpu until all the > expected infrastructure (ticks, idle vcpu, &c) were ready. I can't > immediately convince myself that removing these is safe in that > regard. > Can you address this? >=20 Scheduling can't happen on the cpu, until later than the end of this function, when, in cpupool_assign_cpu_locked(), we set to 1 its corresponding bit in the target cpupool's cpu_valid mask. In fact, scheduling events happening before that, would basically mean that a cpu outside of any cpupool is somehow being considered for scheduling, which, as said, would be a bug. In fact, I sent patches back in July to cure occurrences of that behavior. We've been discussing, basically about the same issue, with Jan in here: https://www.choon.net/forum/read.php?22,3817262,3817489 And I'll add the promised ASSERT() and comment, when sending v2 of that patch. :-) Make sense? 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) --=-CL5V3o83OjtV3PJINCSq 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 iEYEABECAAYFAlYWkI4ACgkQk4XaBE3IOsSr8QCcCyX/S0qQYVdWYfQM5uNltKus 5+kAoKNx6Aq+kvNhHnQp3o5lzMcaoL9r =CQhq -----END PGP SIGNATURE----- --=-CL5V3o83OjtV3PJINCSq-- --===============1796063054305852156== 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 --===============1796063054305852156==--