From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v3 2/6] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS Date: Mon, 2 Nov 2015 09:03:45 +0100 Message-ID: <1446451425.2750.3.camel@citrix.com> References: <20151029225158.25219.4625.stgit@Solace.station> <20151029230420.25219.74544.stgit@Solace.station> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5902234148071612539==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZtA5y-0005bX-By for xen-devel@lists.xenproject.org; Mon, 02 Nov 2015 08:04:02 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Meng Xu Cc: George Dunlap , "xen-devel@lists.xenproject.org" , Meng Xu , Jan Beulich , Andrew Cooper List-Id: xen-devel@lists.xenproject.org --===============5902234148071612539== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-ag5Eowx5xqeAhrO/UXh0" --=-ag5Eowx5xqeAhrO/UXh0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2015-10-30 at 19:00 -0400, Meng Xu wrote: > Hi Dario, > Hi, > > This is fixed as follows: > > - take the lock in the hook implementations, in specific > > schedulers' code; > > - avoid calling insert_vcpu(), for the idle vCPU, in > > schedule_cpu_switch(). In fact, idle vCPUs are set to run > > immediately, and the various schedulers won't insert them > > in their runqueues anyway, even when explicitly asked to. > >=20 > > While there, still in schedule_cpu_switch(), locking with > > _irq() is enough (there's no need to do *_irqsave()). > >=20 > > Signed-off-by: Dario Faggioli > > --- > > diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c > > index 6f71e0d..e16bd3a 100644 > > --- a/xen/common/sched_credit.c > > +++ b/xen/common/sched_credit.c > > @@ -903,10 +903,16 @@ static void > > csched_vcpu_insert(const struct scheduler *ops, struct vcpu *vc) > > { > > struct csched_vcpu *svc =3D vc->sched_priv; > > + spinlock_t *lock; > > + unsigned long flags; > > + > > + lock =3D vcpu_schedule_lock_irqsave(vc, &flags); >=20 >=20 > This is inconsistent with the commit log. > Mmm... no, the changelog speaks about schedule_cpu_switch(), not this functions. For this function, the conversion from _irqsave() to _irq() is done later in the series, when the call to insert_vcpu() is removed from the boot path, and hence does not require IRQs to be disabled when called (and the changelog of that later patch explains this). > I also checked the >=20 > branch rel/sched/fix-vcpu-ins-rem-v2 in your repo., it is the > following code: >=20 > lock =3D vcpu_schedule_lock_irq(vc, &flags); >=20 > I guess maybe you forgot to change it in this commit but change it > the > following commit? >=20 No, this is one of the few thing that changed between v2 and v3. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-ag5Eowx5xqeAhrO/UXh0 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 iEYEABECAAYFAlY3GOEACgkQk4XaBE3IOsSZvgCghCrEY1+FJzersuEYZwB2bl1Y Q4kAnRohpxyvIz1f5q2ZJRq6WgMf8Lch =Slj9 -----END PGP SIGNATURE----- --=-ag5Eowx5xqeAhrO/UXh0-- --===============5902234148071612539== 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 --===============5902234148071612539==--