From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v3 for Xen 4.6 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler Date: Wed, 8 Jul 2015 10:33:54 +0200 Message-ID: <1436344434.22672.186.camel@citrix.com> References: <1435545899-22751-1-git-send-email-chong.li@wustl.edu> <1435545899-22751-2-git-send-email-chong.li@wustl.edu> <559BB101020000780008D371@mail.emea.novell.com> <1436279944.22672.104.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7902093637673252746==" 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: Meng Xu Cc: Chong Li , Sisu Xi , George Dunlap , "xen-devel@lists.xen.org" , Meng Xu , Jan Beulich , Chong Li , Dagaen Golomb List-Id: xen-devel@lists.xenproject.org --===============7902093637673252746== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-i5Z8lwzwLk8IqON2bQ5g" --=-i5Z8lwzwLk8IqON2bQ5g Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2015-07-07 at 23:06 -0700, Meng Xu wrote: > 2015-07-07 7:39 GMT-07:00 Dario Faggioli : > > On Tue, 2015-07-07 at 09:59 +0100, Jan Beulich wrote: > >> >>> On 29.06.15 at 04:44, wrote: > >> > --- a/xen/common/Makefile > >> > +++ b/xen/common/Makefile > >> > @@ -31,7 +31,6 @@ obj-y +=3D rbtree.o > >> > obj-y +=3D rcupdate.o > >> > obj-y +=3D sched_credit.o > >> > obj-y +=3D sched_credit2.o > >> > -obj-y +=3D sched_sedf.o > >> > obj-y +=3D sched_arinc653.o > >> > obj-y +=3D sched_rt.o > >> > obj-y +=3D schedule.o > >> > >> Stray change. Or perhaps the file doesn't build anymore, in which case > >> you should instead have stated that the patch is dependent upon the > >> series removing SEDF. > >> > > This indeed does not belong in here. And of course, things should > > build... So, Chong, either deal with SEDF as well, if basing your > > patches on a tree where it is still there, or base on top of my patches= , > > ignore it, but state the dependency, as Jan is asking. > > > >> > @@ -1157,8 +1158,75 @@ rt_dom_cntl( > > > >> > + case XEN_DOMCTL_SCHEDOP_putvcpuinfo: > >> > + spin_lock_irqsave(&prv->lock, flags); > >> > + for( index =3D 0; index < op->u.v.nr_vcpus; index++ ) > >> > + { > >> > + if ( copy_from_guest_offset(&local_sched, > >> > + op->u.v.vcpus, index, 1) ) > >> > + { > >> > + rc =3D -EFAULT; > >> > + break; > >> > + } > >> > + if ( local_sched.vcpuid >=3D d->max_vcpus > >> > + || d->vcpu[local_sched.vcpuid] =3D=3D NULL ) > >> > + { > >> > + rc =3D -EINVAL; > >> > + break; > >> > + } > >> > + svc =3D rt_vcpu(d->vcpu[local_sched.vcpuid]); > >> > + svc->period =3D MICROSECS(local_sched.s.rtds.period); > >> > + svc->budget =3D MICROSECS(local_sched.s.rtds.budget); > >> > >> Are all input values valid here? > >> > > That's a good point, actually. Right now, SEDF does some range > > enforcement, by means of these values: > > > > #define PERIOD_MAX MILLISECS(10000) /* 10s */ > > #define PERIOD_MIN (MICROSECS(10)) /* 10us */ > > #define SLICE_MIN (MICROSECS(5)) /* 5us */ > > > > Chong, it probably makes sense to (in a separate patch), introduce > > something like this in RTDS too (with SLICE_MIN-->BUDGET_MIN), and then > > use them, in this patch, for sanity checking the input. > > > > It also makes sense to check and enforce budget<=3Dperiod, IMO. > > > > About the specific values, I'm open to proposals. I think something lik= e > > the SEDF's one is fine. Meng? >=20 > We are trying to make some range enforcement for RTDS scheduler. Is my > understanding correct? (It should be, but just in case. :-) ) >=20 We are wondering whether that could be necessary/useful, and IMO, it would. > As to the range of period, I think the max value can be as large as > the type of period (ie. s_time_t) can represent. When we want a > dedicated CPU for a guest, we will set budget=3Dperiod and can set the > period to a very very large value to avoid the unnecessarily > invocation of the scheduler. > Makes sense. We do have STIME_MAX and, given that period is something that is added to current time during scheduling, STIME_DELTA_MAX. Maybe, put something together basing on those?=20 > As to the min value of period, I think it should be >=3D100us. The > scheduler overhead of running a large box could be 1us if the runq is > long and competetion of the runq lock is heavy. If the scheduler is > potentially invoked every 10us, the scheduler overhead will be 10% of > total computation time, which seems a lot to me. >=20 Ok. > As to the range of budget, the min value can be 5us, the same with > SEDF;=20 > Well, wouldn't the above reasoning about overhead apply here too? Budgets of 5us mean the scheduler can be invoked every 5us for budget enforcement. If 10us was unreasonable, 5 is even more so. Therefore, 100us here too? Or maybe let's allow for lower values (like 50us or 10us), but print a warning? > the max value is the value of period of the same VCPU. >=20 Yep. And, whatever the values, it would be useful to have comments somewhere (either when the values are defined or enforced), stating what you said above. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-i5Z8lwzwLk8IqON2bQ5g 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 iEYEABECAAYFAlWc4HkACgkQk4XaBE3IOsQY8wCglYorJwZI70FIcSzeXAk44Xgm kbAAn3Iov0JrKhuPkHaAgIMz0by7tw7z =Nl9b -----END PGP SIGNATURE----- --=-i5Z8lwzwLk8IqON2bQ5g-- --===============7902093637673252746== 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 --===============7902093637673252746==--