From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 for Xen 4.6 1/4] xen: enabling XL to set per-VCPU parameters of a domain for RTDS scheduler Date: Thu, 4 Jun 2015 17:14:12 +0200 Message-ID: <1433430852.6291.16.camel@citrix.com> References: <1432598752-20826-1-git-send-email-chong.li@wustl.edu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4808621849937175479==" Return-path: 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: George Dunlap Cc: Chong Li , Sisu Xi , "xen-devel@lists.xen.org" , Meng Xu , Jan Beulich , Chong Li , Dagaen Golomb List-Id: xen-devel@lists.xenproject.org --===============4808621849937175479== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-4LOPob8GG//5RDa+fph7" --=-4LOPob8GG//5RDa+fph7 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2015-06-02 at 17:28 +0100, George Dunlap wrote: > On Tue, May 26, 2015 at 1:05 AM, Chong Li wrote: > > Add two hypercalls(XEN_DOMCTL_SCHEDOP_getvcpuinfo/putvcpuinfo) to get/s= et a domain's > > per-VCPU parameters. Hypercalls are handled by newly added hook (.adjus= t_vcpu) in the > > scheduler interface. > > > > Add a new data structure (struct xen_domctl_scheduler_vcpu_op) for tran= sferring data > > between tool and hypervisor. > > > > Signed-off-by: Chong Li > > Signed-off-by: Meng Xu > > Signed-off-by: Sisu Xi >=20 > One comment that I didn't see addressed already... >=20 > > + switch ( op->cmd ) > > + { > > + case XEN_DOMCTL_SCHEDOP_getvcpuinfo: > > + spin_lock_irqsave(&prv->lock, flags); > > + list_for_each( iter, &sdom->vcpu ) > > + { > > + svc =3D list_entry(iter, struct rt_vcpu, sdom_elem); > > + vcpuid =3D svc->vcpu->vcpu_id; > > + > > + local_sched.budget =3D svc->budget / MICROSECS(1); > > + local_sched.period =3D svc->period / MICROSECS(1); > > + if ( copy_to_guest_offset(op->u.rtds.vcpus, vcpuid, > > + &local_sched, 1) ) > > + { > > + spin_unlock_irqrestore(&prv->lock, flags); > > + return -EFAULT; > > + } > > + hypercall_preempt_check(); > > + } > > + spin_unlock_irqrestore(&prv->lock, flags); > > + break; > > + case XEN_DOMCTL_SCHEDOP_putvcpuinfo: > > + spin_lock_irqsave(&prv->lock, flags); > > + for( i =3D 0; i < op->u.rtds.nr_vcpus; i++ ) > > + { > > + if ( copy_from_guest_offset(&local_sched, > > + op->u.rtds.vcpus, i, 1) ) > > + { > > + spin_unlock_irqrestore(&prv->lock, flags); > > + return -EFAULT; > > + } > > + if ( local_sched.period <=3D 0 || local_sched.budget <=3D = 0 ) > > + { > > + spin_unlock_irqrestore(&prv->lock, flags); > > + return -EINVAL; > > + } > > + svc =3D rt_vcpu(d->vcpu[local_sched.vcpuid]); > > + svc->period =3D MICROSECS(local_sched.period); > > + svc->budget =3D MICROSECS(local_sched.budget); > > + hypercall_preempt_check(); > > + } >=20 > It looks like the interface here is assymetric. That is, on > "putvcpuinfo", you assume there is an array of rtds.nr_vcpus length, > and you read vcpu_id from each one. In "getvcpuinfo", you assume that > the array is the number of vcpus in the guest, and you just copy all > the vcpu data into it. >=20 > I think it would make more sense for it to work the same both ways -- > so either always pass an array of all vcpus of the guest in and out; > or, have an array potentially specifying a subset of cpus in and out. >=20 > Thoughts? >=20 It would indeed. And in fact, just FTR, I did say pretty much the same in <1432904488.5077.30.camel@citrix.com> :-D Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-4LOPob8GG//5RDa+fph7 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 iEYEABECAAYFAlVwa0QACgkQk4XaBE3IOsSQ9wCePGRTNZ+cLpreCPba3KDYEkXY t3sAoJACv5o2wVLdZsinsbZVecFqZP4q =XuLw -----END PGP SIGNATURE----- --=-4LOPob8GG//5RDa+fph7-- --===============4808621849937175479== 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 --===============4808621849937175479==--