From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 3/9] xen: sched: make locking for {insert, remove}_vcpu consistent Date: Thu, 1 Oct 2015 13:59:16 +0200 Message-ID: <1443700756.14525.22.camel@citrix.com> References: <20150929164726.17589.96920.stgit@Solace.station> <20150929165549.17589.76223.stgit@Solace.station> <560D04F102000078000A755C@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5839035158432802311==" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZhcWL-0003IU-So for xen-devel@lists.xenproject.org; Thu, 01 Oct 2015 11:59:33 +0000 In-Reply-To: <560D04F102000078000A755C@prv-mh.provo.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: George Dunlap , xen-devel@lists.xenproject.org, Meng Xu List-Id: xen-devel@lists.xenproject.org --===============5839035158432802311== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-2mE0nK+oRmcXGRhnxVNK" --=-2mE0nK+oRmcXGRhnxVNK Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2015-10-01 at 02:03 -0600, Jan Beulich wrote: > > > > On 29.09.15 at 18:55, wrote: > > In case of credit1, remove_vcpu() handling needs some > > fixing remove_vcpu() too, i.e.: > > - it manipulates runqueues, so the runqueue lock must > > be acquired; >=20 > Is this really a fix needed only as a result of the insert side > changes? If not, since the insert side looks more like an > improvement than a bug fix, perhaps splitting the two (and > requesting backport of the remove side change) would be > better? >=20 Yes, I like the idea, I'll do this. > > --- a/xen/common/sched_credit.c > > +++ b/xen/common/sched_credit.c > > @@ -905,8 +905,19 @@ csched_vcpu_insert(const struct scheduler > > *ops, struct vcpu *vc) > > { > > struct csched_vcpu *svc =3D vc->sched_priv; > > =20 > > - if ( !__vcpu_on_runq(svc) && vcpu_runnable(vc) && !vc > > ->is_running ) > > - __runq_insert(vc->processor, svc); > > + /* > > + * For the idle domain, this is called, during boot, before > > + * than alloc_pdata() has been called for the pCPU. > > + */ > > + if ( !is_idle_vcpu(vc) ) >=20 > Looking through the patch I can't spot why this would all of the > sudden need special casing. Can you explain that please? In > particular (in case it's just the acquiring of the lock that now > becomes an issue) - where would the runqueue insertion be > happening now?=20 > So, this is an issue in Credit2 (from where the comment was taken) and RTDS. I've checked further and, for Credit, it is an issue only without patch 4 of this series, and even in that case, it can be cured in another (better) way, I think. So, I'll not only split, but also rework this patch a bit, and resubmit. > Or if one of the latter two conditions makes it > so that insertion turns out not to be needed, perhaps they could > be moved up to replace the is-idle check? >=20 Yes, it was the !vc->is_running part which prevents the insertion of idle vcpus in runqueues already. I wouldn't like having it guarding the whole thing, for various reasons, but as I said, I think I can rework the patch to make things more clear. > Also, not the least since the comment gets repeated below, if > it is to stay then I think the "than" should be dropped (or > re-worded more extensively). >=20 Yep, as a part of the rework, I'll reword the comment(s). 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) --=-2mE0nK+oRmcXGRhnxVNK 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 iEYEABECAAYFAlYNIBQACgkQk4XaBE3IOsTl7gCfZjoLEiS6mA/26I/HTyTtADnA jmgAn0CGdhfNTpZkaYhIeHEsRoaCA89E =a3SX -----END PGP SIGNATURE----- --=-2mE0nK+oRmcXGRhnxVNK-- --===============5839035158432802311== 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 --===============5839035158432802311==--