From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH for Xen 4.5 v2 1/2] xen: sanity check input and serialization in sched_rt.c Date: Wed, 29 Oct 2014 10:41:32 +0100 Message-ID: <1414575692.20696.42.camel@Abyss> References: <1414246599-3914-1-git-send-email-mengxu@cis.upenn.edu> <1414246599-3914-2-git-send-email-mengxu@cis.upenn.edu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8177451891698453279==" Return-path: In-Reply-To: <1414246599-3914-2-git-send-email-mengxu@cis.upenn.edu> 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@eu.citrix.com, "konrad.r.wilk" , xumengpanda@gmail.com, JBeulich@suse.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org --===============8177451891698453279== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-YjgKKti2c0u+eQwIpKYK" --=-YjgKKti2c0u+eQwIpKYK Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable [Ditto, about the Cc list] On Sat, 2014-10-25 at 10:16 -0400, Meng Xu wrote: > Sanity check input params in rt_dom_cntl(); > Serialize rt_dom_cntl() against the global lock. >=20 > Signed-off-by: Meng Xu > Reviewed-by: Dario Faggioli Just a note. See below. > diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c > index 6c8faeb..b87c95b 100644 > --- a/xen/common/sched_rt.c > +++ b/xen/common/sched_rt.c > @@ -1042,9 +1042,11 @@ rt_dom_cntl( > struct domain *d, > struct xen_domctl_scheduler_op *op) > { > + struct rt_private *prv =3D rt_priv(ops); > struct rt_dom * const sdom =3D rt_dom(d); > struct rt_vcpu *svc; > struct list_head *iter; > + unsigned long flags; > int rc =3D 0; > =20 > switch ( op->cmd ) > @@ -1055,12 +1057,19 @@ rt_dom_cntl( > op->u.rtds.budget =3D svc->budget / MICROSECS(1); > break; > case XEN_DOMCTL_SCHEDOP_putinfo: > + if ( op->u.rtds.period =3D=3D 0 || op->u.rtds.budget =3D=3D 0 ) > + { > + rc =3D -EINVAL; > + break; > + } > + spin_lock_irqsave(&prv->lock, flags); > list_for_each( iter, &sdom->vcpu ) > { > struct rt_vcpu * svc =3D list_entry(iter, struct rt_vcpu, sd= om_elem); > svc->period =3D MICROSECS(op->u.rtds.period); /* transfer to= nanosec */ > svc->budget =3D MICROSECS(op->u.rtds.budget); > } > + spin_unlock_irqrestore(&prv->lock, flags); > Thinking more about this, I actually prefer the approach where both the get and set side are properly serialized. Races are very unlikely, and probably not that harmful, but I don't see why leaving room for them, and it looks more consistent to me that way. I appreciate why you've done it this way in this patch, and I am fine with it. As said, it's not a huge deal, and this code is experimental, so we can always change it later. I just wanted to let you know that, if you end up resending the patches for some reasons, I'd personally go for the fully locked approach. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-YjgKKti2c0u+eQwIpKYK 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 iEYEABECAAYFAlRQtkwACgkQk4XaBE3IOsRm2QCfXP8eObcIzvPCRmnzkh5aUE0W 0TUAoJxg2fKVtLwVeVNVUTGLRRQcTzAJ =NbcK -----END PGP SIGNATURE----- --=-YjgKKti2c0u+eQwIpKYK-- --===============8177451891698453279== 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 --===============8177451891698453279==--