From mboxrd@z Thu Jan 1 00:00:00 1970 From: Meng Xu Subject: Re: [PATCH RFC v1 2/4] xl for rt scheduler Date: Fri, 11 Jul 2014 10:59:57 -0400 Message-ID: References: <1405054198-29106-1-git-send-email-mengxu@cis.upenn.edu> <1405054198-29106-3-git-send-email-mengxu@cis.upenn.edu> <20140711110206.GD12584@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3788699691965341871==" Return-path: In-Reply-To: <20140711110206.GD12584@zion.uk.xensource.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: Wei Liu Cc: Ian Campbell , Sisu Xi , Stefano Stabellini , George Dunlap , Dario Faggioli , Ian Jackson , "xen-devel@lists.xen.org" , Meng Xu , Chong Li , Dagaen Golomb List-Id: xen-devel@lists.xenproject.org --===============3788699691965341871== Content-Type: multipart/alternative; boundary=089e0153821cd6dab404fdec3189 --089e0153821cd6dab404fdec3189 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Wei, First, thank you very much for your comments! > > =3Dhead1 CPUPOOLS COMMANDS > > diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h > > index 10a2e66..51b634a 100644 > > --- a/tools/libxl/xl.h > > +++ b/tools/libxl/xl.h > > @@ -67,6 +67,7 @@ int main_memset(int argc, char **argv); > > int main_sched_credit(int argc, char **argv); > > int main_sched_credit2(int argc, char **argv); > > int main_sched_sedf(int argc, char **argv); > > +int main_sched_rt(int argc, char **argv); > > int main_domid(int argc, char **argv); > > int main_domname(int argc, char **argv); > > int main_rename(int argc, char **argv); > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > index 68df548..c043f88 100644 > > --- a/tools/libxl/xl_cmdimpl.c > > +++ b/tools/libxl/xl_cmdimpl.c > > @@ -4947,6 +4947,7 @@ int main_sharing(int argc, char **argv) > > return 0; > > } > > > > + > > Stray blank line. > =E2=80=8BI will delete the blank line. =E2=80=8B > > > static int sched_domain_get(libxl_scheduler sched, int domid, > > libxl_domain_sched_params *scinfo) > > { > > @@ -5098,6 +5099,52 @@ static int sched_sedf_domain_output( > > return 0; > > } > > > > + > > +static int sched_rt_domain_output( > > + int domid) > > +{ > > + char *domname; > > + libxl_domain_sched_params scinfo; > > + int rc, i; > > + > > + if (domid < 0) { > > + printf("%-33s %4s %4s %6s %6s\n", "Name", "ID", "VCPU", > "Period", "Budget"); > > + return 0; > > + } > > + > > Just print the header and nothing more? > =E2=80=8BWe just print out the header here. This function will call =E2=80=8Bsched_domain_get(LIBXL_SCHEDULER_RT, domid, &scinfo) and print out= the VCPUs parameters of each domain in the calling function. =E2=80=8B > > > + libxl_domain_sched_params_init(&scinfo); > > + rc =3D sched_domain_get(LIBXL_SCHEDULER_RT, domid, &scinfo); > > + if (rc) > > + return rc; > > + > > This is violating libxl type paradigm. See libxl.h. > 263 * libxl types > 264 * > 265 * Most libxl types are defined by the libxl IDL (see > 266 * libxl_types.idl). The library provides a common set of methods fo= r > 267 * initialising and freeing these types. > 268 * > 269 * IDL-generated libxl types should be used as follows: the user mus= t > 270 * always call the "init" function before using a type, even if the > 271 * variable is simply being passed by reference as an out parameter > 272 * to a libxl function. The user must always calls "dispose" exactl= y > 273 * once afterwards, to clean up, regardless of whether operations on > 274 * this object succeeded or failed. See the xl code for examples. > > And you should check other places that use libxl types as well. > =E2=80=8BThank you very much for pasting the rules here! I really appreciat= e it. However, I didn't quite get why it violate the libxl type paradigm and how I should correct it. (Sorry. :-()=E2=80=8B We actually followed the way credit scheduler does in main_sched_credit(int argc, char **argv) } else { /* set credit scheduler paramaters */ libxl_domain_sched_params scinfo; libxl_domain_sched_params_init(&scinfo); scinfo.sched =3D LIBXL_SCHEDULER_CREDIT; if (opt_w) scinfo.weight =3D weight; if (opt_c) scinfo.cap =3D cap; rc =3D sched_domain_set(domid, &scinfo); libxl_domain_sched_params_dispose(&scinfo); Could you help let me know how I should modify it? I will check the rest to modify when I know how to do it. Thank you very much! Best, Meng --=20 ----------- Meng Xu PhD Student in Computer and Information Science University of Pennsylvania --089e0153821cd6dab404fdec3189 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi = Wei,

=
First, thank you ver= y much for your comments!=C2=A0

=

> =C2=A0=3Dhead1 CPUPOOLS COMMANDS
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index 10a2e66..51b634a 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -67,6 +67,7 @@ int main_memset(int argc, char **argv);
> =C2=A0int main_sched_credit(int argc, char **argv);
> =C2=A0int main_sched_credit2(int argc, char **argv);
> =C2=A0int main_sched_sedf(int argc, char **argv);
> +int main_sched_rt(int argc, char **argv);
> =C2=A0int main_domid(int argc, char **argv);
> =C2=A0int main_domname(int argc, char **argv);
> =C2=A0int main_rename(int argc, char **argv);
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 68df548..c043f88 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -4947,6 +4947,7 @@ int main_sharing(int argc, char **argv)
> =C2=A0 =C2=A0 =C2=A0return 0;
> =C2=A0}
>
> +

Stray blank line.

=E2=80=8BI will delete the blank line.
=E2=80=8B
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;pa= dding-left:1ex">

> =C2=A0static int sched_domain_get(libxl_scheduler sched, int domid, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0libxl_domain_sched_params *scinfo)
> =C2=A0{
> @@ -5098,6 +5099,52 @@ static int sched_sedf_domain_output(
> =C2=A0 =C2=A0 =C2=A0return 0;
> =C2=A0}
>
> +
> +static int sched_rt_domain_output(
> + =C2=A0 =C2=A0int domid)
> +{
> + =C2=A0 =C2=A0char *domname;
> + =C2=A0 =C2=A0libxl_domain_sched_params scinfo;
> + =C2=A0 =C2=A0int rc, i;
> +
> + =C2=A0 =C2=A0if (domid < 0) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0printf("%-33s %4s %4s %6s %6s\n"= ;, "Name", "ID", "VCPU", "Period", = "Budget");
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
> + =C2=A0 =C2=A0}
> +

Just print the header and nothing more?

=E2=80=8BWe j= ust print out the header here. This function will call =E2=80=8Bsched_domai= n_get(LIBXL_SCHEDULER_RT, domid, &scinfo) and print out the VCPUs param= eters of each domain in the calling function. =E2=80=8B

=C2=A0

> + =C2=A0 =C2=A0libxl_domain_sched_params_init(&scinfo);
> + =C2=A0 =C2=A0rc =3D sched_domain_get(LIBXL_SCHEDULER_RT, domid, &= ;scinfo);
> + =C2=A0 =C2=A0if (rc)
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0return rc;
> +

This is violating libxl type paradigm. See libxl.h.

=C2=A0263 =C2=A0* libxl types
=C2=A0264 =C2=A0*
=C2=A0265 =C2=A0* Most libxl types are defined by the libxl IDL (see
=C2=A0266 =C2=A0* libxl_types.idl). The library provides a common set of me= thods for
=C2=A0267 =C2=A0* initialising and freeing these types.
=C2=A0268 =C2=A0*
=C2=A0269 =C2=A0* IDL-generated libxl types should be used as follows: the = user must
=C2=A0270 =C2=A0* always call the "init" function before using a = type, even if the
=C2=A0271 =C2=A0* variable is simply being passed by reference as an out pa= rameter
=C2=A0272 =C2=A0* to a libxl function. =C2=A0The user must always calls &qu= ot;dispose" exactly
=C2=A0273 =C2=A0* once afterwards, to clean up, regardless of whether opera= tions on
=C2=A0274 =C2=A0* this object succeeded or failed. =C2=A0See the xl code fo= r examples.

And you should check other places that use libxl types as well.

=E2=80=8BThank you very much for pasting the rules here! I really appr= eciate it. However, I didn't quite get why it violate the libxl type pa= radigm and how I should correct it. (Sorry. :-()=E2=80=8B

We actually followed the way c= redit scheduler does in=C2=A0main_sched_credit(int argc, char **argv)
=

=C2=A0 =C2=A0 =C2=A0 =C2=A0 } else { /* set credit scheduler paramater= s */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 libxl_domain_sched_params scinfo;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 libxl_domain_sched_params_init(&a= mp;scinfo);
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 scinfo.sched =3D LIBXL_SCHEDULER_CREDIT;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (op= t_w)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 scinfo.weight =3D w= eight;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 if (opt_c)
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 scinfo.cap =3D cap;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 rc= =3D sched_domain_set(domid, &scinfo);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 libxl_domain_sched_params_dispose(&scinfo);

Could you hel= p let me know how I should modify it? I will check the rest to modify when = I know how to do it.=C2=A0

Thank you very much!

Best,

Meng




--=


-----------
Meng Xu
PhD Student in Comp= uter and Information Science
University of Pennsylvania
--089e0153821cd6dab404fdec3189-- --===============3788699691965341871== 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 --===============3788699691965341871==--