Juergen Gross
2012-May-22 09:16 UTC
[PATCH 0 of 3] support of setting scheduler parameters on domain creation
Support setting scheduling parameters on domain creation. Depending on the scheduler of the cpupool in which the domain is started, the default parameters are obtained from the hypervisor. Any scheduling parameters specified during domain creation will modify these defaults. This patch series consists of 3 patches: 1: support of getting scheduler default parameters in the hypervisor 2: support in libxc 3: support in xl/libxl 14 files changed, 214 insertions(+), 28 deletions(-) tools/libxc/xc_csched.c | 21 +++++++++++++ tools/libxc/xc_csched2.c | 21 +++++++++++++ tools/libxc/xc_sedf.c | 21 +++++++++++++ tools/libxc/xenctrl.h | 10 ++++++ tools/libxl/libxl.h | 2 + tools/libxl/libxl_dom.c | 65 +++++++++++++++++++++++++++++++++++++++++-- tools/libxl/libxl_types.idl | 1 tools/libxl/xl_cmdimpl.c | 10 ++++-- xen/common/sched_credit.c | 5 +++ xen/common/sched_credit2.c | 17 +++++++++++ xen/common/sched_sedf.c | 20 +++++++++++++ xen/common/schedule.c | 5 +-- xen/include/public/domctl.h | 38 +++++++++++++------------ xen/include/public/sysctl.h | 6 ++-
Support a new sysctl schedop sub-command to get the scheduling defaults of a specific scheduler. Additionally correct parameter checking of the sysctl handling in schedule.c (checked wrong sub-commands: domctl instead of sysctl). Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com> 6 files changed, 69 insertions(+), 22 deletions(-) xen/common/sched_credit.c | 5 +++++ xen/common/sched_credit2.c | 17 +++++++++++++++++ xen/common/sched_sedf.c | 20 ++++++++++++++++++++ xen/common/schedule.c | 5 +++-- xen/include/public/domctl.h | 38 ++++++++++++++++++++------------------ xen/include/public/sysctl.h | 6 ++++-- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Juergen Gross
2012-May-22 09:16 UTC
[PATCH 2 of 3] Support getting scheduler defaults in libxc
Add scheduler specific interfaces to get scheduling defaults from the hypervisor. Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com> 4 files changed, 73 insertions(+) tools/libxc/xc_csched.c | 21 +++++++++++++++++++++ tools/libxc/xc_csched2.c | 21 +++++++++++++++++++++ tools/libxc/xc_sedf.c | 21 +++++++++++++++++++++ tools/libxc/xenctrl.h | 10 ++++++++++ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Juergen Gross
2012-May-22 09:16 UTC
[PATCH 3 of 3] full support of setting scheduler parameters on domain creation
Obtains default scheduler parameters before modifying any settings specified via domain config. Corrected an error for setting sedf parameters (setting .period multiple times). Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com> 4 files changed, 72 insertions(+), 6 deletions(-) tools/libxl/libxl.h | 2 + tools/libxl/libxl_dom.c | 65 +++++++++++++++++++++++++++++++++++++++++-- tools/libxl/libxl_types.idl | 1 tools/libxl/xl_cmdimpl.c | 10 ++++-- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Campbell
2012-May-22 12:22 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote:> Support a new sysctl schedop sub-command to get the scheduling defaults of a > specific scheduler.Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here? Isn''t it actually more useful/meaningful to get the current actual setting rather than a default? Ian.> > Additionally correct parameter checking of the sysctl handling in schedule.c > (checked wrong sub-commands: domctl instead of sysctl). > > Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com> > > > 6 files changed, 69 insertions(+), 22 deletions(-) > xen/common/sched_credit.c | 5 +++++ > xen/common/sched_credit2.c | 17 +++++++++++++++++ > xen/common/sched_sedf.c | 20 ++++++++++++++++++++ > xen/common/schedule.c | 5 +++-- > xen/include/public/domctl.h | 38 ++++++++++++++++++++------------------ > xen/include/public/sysctl.h | 6 ++++-- > >
Juergen Gross
2012-May-22 12:29 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On 05/22/2012 02:22 PM, Ian Campbell wrote:> On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote: >> Support a new sysctl schedop sub-command to get the scheduling defaults of a >> specific scheduler. > Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here? > > Isn''t it actually more useful/meaningful to get the current actual > setting rather than a default?When setting the parameters from the config file we have no domain yet which we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn''t want to parse part of the config only after domain creation. A default should be okay, as this is what we want to modify. :-) The scheduler should initialize a new domain with this default, of course. Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
Ian Campbell
2012-May-22 12:30 UTC
Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
> # HG changeset patch > # User Juergen Gross <juergen.gross@ts.fujitsu.com> > # Date 1337677423 -7200 > # Node ID 953383741ff44d97587c2e75da79b092523d6e83 > # Parent 19aaa30d7fdce2f1b56cb13399d603d955a61fb8 > full support of setting scheduler parameters on domain creation > > Obtains default scheduler parameters before modifying any settings > specified > via domain config. > > Corrected an error for setting sedf parameters (setting .period > multiple > times). > > Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com> > > diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h > --- a/tools/libxl/libxl.h Tue May 22 10:31:30 2012 +0200 > +++ b/tools/libxl/libxl.h Tue May 22 11:03:43 2012 +0200 > @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx > /* May be called with info_r == NULL to check for domain''s existance > */ > int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r, > uint32_t domid); > +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid, > + libxl_sched_params *scparams);This interface really makes libxl_sched_params differ from all the other libxl structs (which have a public _init function and an internal setdefaults function). I''m not really sure its justified either, I was under the impression that you''d found that there were useful discriminating values? If this function was called libxl_sched_init (replacing the autogenerated one) then it might be ok. Although I''m still not really sure what the issue is with having a discriminating value meaning default is, doing that keeps the _init function cheap too.> int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, > libxl_sched_params *scparams) > { > libxl_ctx *ctx = libxl__gc_owner(gc); > - libxl_scheduler sched; > libxl_sched_sedf_domain sedf_info; > libxl_sched_credit_domain credit_info; > libxl_sched_credit2_domain credit2_info; > int ret; > > - sched = libxl_get_scheduler (ctx); > - switch (sched) { > + switch (scparams->sched) {What happens if scparams->sched is not the scheduler used for this domain? Should it either be checked or set somewhere?> > Ian. >
Ian Campbell
2012-May-22 12:32 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Tue, 2012-05-22 at 13:29 +0100, Juergen Gross wrote:> On 05/22/2012 02:22 PM, Ian Campbell wrote: > > On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote: > >> Support a new sysctl schedop sub-command to get the scheduling defaults of a > >> specific scheduler. > > Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here? > > > > Isn''t it actually more useful/meaningful to get the current actual > > setting rather than a default? > > When setting the parameters from the config file we have no domain yet which > we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn''t want to > parse part of the config only after domain creation.That seems like another problem with doing this up front instead of doing read-modify-write when we come to set the values for the domain.> A default should be okay, as this is what we want to modify. :-) > The scheduler should initialize a new domain with this default, of course.So I can''t use this interface to change the current one of the current settings for running a domain, while leaving the others at their current value? Which interface can I use for that and why isn''t it the same as this one? Ian.
Juergen Gross
2012-May-22 12:39 UTC
Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
On 05/22/2012 02:30 PM, Ian Campbell wrote:>> # HG changeset patch >> # User Juergen Gross<juergen.gross@ts.fujitsu.com> >> # Date 1337677423 -7200 >> # Node ID 953383741ff44d97587c2e75da79b092523d6e83 >> # Parent 19aaa30d7fdce2f1b56cb13399d603d955a61fb8 >> full support of setting scheduler parameters on domain creation >> >> Obtains default scheduler parameters before modifying any settings >> specified >> via domain config. >> >> Corrected an error for setting sedf parameters (setting .period >> multiple >> times). >> >> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com> >> >> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h >> --- a/tools/libxl/libxl.h Tue May 22 10:31:30 2012 +0200 >> +++ b/tools/libxl/libxl.h Tue May 22 11:03:43 2012 +0200 >> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx >> /* May be called with info_r == NULL to check for domain''s existance >> */ >> int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r, >> uint32_t domid); >> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid, >> + libxl_sched_params *scparams); > This interface really makes libxl_sched_params differ from all the other > libxl structs (which have a public _init function and an internal > setdefaults function). I''m not really sure its justified either, I was > under the impression that you''d found that there were useful > discriminating values?Dario opted for this solution, so I proposed a patch implementing it. I prefer this solution, too, as it isn''t exporting scheduler internals to the tools.> If this function was called libxl_sched_init (replacing the > autogenerated one) then it might be ok. Although I''m still not really > sure what the issue is with having a discriminating value meaning > default is, doing that keeps the _init function cheap too.Okay, I can rename it.> >> int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, >> libxl_sched_params *scparams) >> { >> libxl_ctx *ctx = libxl__gc_owner(gc); >> - libxl_scheduler sched; >> libxl_sched_sedf_domain sedf_info; >> libxl_sched_credit_domain credit_info; >> libxl_sched_credit2_domain credit2_info; >> int ret; >> >> - sched = libxl_get_scheduler (ctx); >> - switch (sched) { >> + switch (scparams->sched) { > What happens if scparams->sched is not the scheduler used for this > domain? Should it either be checked or set somewhere?The check would be the same as the original setting of scparams->sched. Setting of the scheduler parameters will be rejected by the hypervisor if the scheduler does not match. Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
Ian Campbell
2012-May-22 12:51 UTC
Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
On Tue, 2012-05-22 at 13:39 +0100, Juergen Gross wrote:> On 05/22/2012 02:30 PM, Ian Campbell wrote: > >> # HG changeset patch > >> # User Juergen Gross<juergen.gross@ts.fujitsu.com> > >> # Date 1337677423 -7200 > >> # Node ID 953383741ff44d97587c2e75da79b092523d6e83 > >> # Parent 19aaa30d7fdce2f1b56cb13399d603d955a61fb8 > >> full support of setting scheduler parameters on domain creation > >> > >> Obtains default scheduler parameters before modifying any settings > >> specified > >> via domain config. > >> > >> Corrected an error for setting sedf parameters (setting .period > >> multiple > >> times). > >> > >> Signed-off-by: Juergen Gross<juergen.gross@ts.fujitsu.com> > >> > >> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h > >> --- a/tools/libxl/libxl.h Tue May 22 10:31:30 2012 +0200 > >> +++ b/tools/libxl/libxl.h Tue May 22 11:03:43 2012 +0200 > >> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx > >> /* May be called with info_r == NULL to check for domain''s existance > >> */ > >> int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r, > >> uint32_t domid); > >> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid, > >> + libxl_sched_params *scparams); > > This interface really makes libxl_sched_params differ from all the other > > libxl structs (which have a public _init function and an internal > > setdefaults function). I''m not really sure its justified either, I was > > under the impression that you''d found that there were useful > > discriminating values? > > Dario opted for this solution, so I proposed a patch implementing it. > I prefer this solution, too, as it isn''t exporting scheduler internals to > the tools.I don''t think "-1 is not a valid weight" is really "exporting scheduler internals". Anyway, I am far more concerned that the libxl API is useful to users than I am about the API between libxl/libxc and the hypervisor (which is not set in stone and can be fixed etc). The libxl API is the one we want to expose and keep stable going forward etc.> > > If this function was called libxl_sched_init (replacing the > > autogenerated one) then it might be ok. Although I''m still not really > > sure what the issue is with having a discriminating value meaning > > default is, doing that keeps the _init function cheap too. > > Okay, I can rename it. > > > > >> int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, > >> libxl_sched_params *scparams) > >> { > >> libxl_ctx *ctx = libxl__gc_owner(gc); > >> - libxl_scheduler sched; > >> libxl_sched_sedf_domain sedf_info; > >> libxl_sched_credit_domain credit_info; > >> libxl_sched_credit2_domain credit2_info; > >> int ret; > >> > >> - sched = libxl_get_scheduler (ctx); > >> - switch (sched) { > >> + switch (scparams->sched) { > > What happens if scparams->sched is not the scheduler used for this > > domain? Should it either be checked or set somewhere? > > The check would be the same as the original setting of scparams->sched. > Setting of the scheduler parameters will be rejected by the hypervisor if the > scheduler does not match. > > > Juergen >
Juergen Gross
2012-May-22 12:58 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On 05/22/2012 02:32 PM, Ian Campbell wrote:> On Tue, 2012-05-22 at 13:29 +0100, Juergen Gross wrote: >> On 05/22/2012 02:22 PM, Ian Campbell wrote: >>> On Tue, 2012-05-22 at 10:16 +0100, Juergen Gross wrote: >>>> Support a new sysctl schedop sub-command to get the scheduling defaults of a >>>> specific scheduler. >>> Why is XEN_DOMCTL_SCHEDOP_getinfo not sufficient here? >>> >>> Isn''t it actually more useful/meaningful to get the current actual >>> setting rather than a default? >> When setting the parameters from the config file we have no domain yet which >> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn''t want to >> parse part of the config only after domain creation. > That seems like another problem with doing this up front instead of > doing read-modify-write when we come to set the values for the domain.Do you think it would be okay to parse the scheduler config data _after_ domain creation? If yes, the read-modify-write approach is simple and always correct. If no, you will have to initialize the parameters to something scheduler specific, and not domain specific.>> A default should be okay, as this is what we want to modify. :-) >> The scheduler should initialize a new domain with this default, of course. > So I can''t use this interface to change the current one of the current > settings for running a domain, while leaving the others at their current > value?I don''t understand this sentence :-) ...> Which interface can I use for that and why isn''t it the same as this > one?... making it hard to answer to this one. :-) Nevertheless trying: you can change the scheduling parameters of a running domain with xl sched-credit/sched-sedf/... Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
George Dunlap
2012-May-22 13:05 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Tue, May 22, 2012 at 1:58 PM, Juergen Gross <juergen.gross@ts.fujitsu.com> wrote:>>> When setting the parameters from the config file we have no domain yet >>> which >>> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn''t want to >>> parse part of the config only after domain creation. >> >> That seems like another problem with doing this up front instead of >> doing read-modify-write when we come to set the values for the domain. > > > Do you think it would be okay to parse the scheduler config data _after_ > domain creation? If yes, the read-modify-write approach is simple and always > correct. If no, you will have to initialize the parameters to something > scheduler specific, and not domain specific. > > >>> A default should be okay, as this is what we want to modify. :-) >>> The scheduler should initialize a new domain with this default, of >>> course. >> >> So I can''t use this interface to change the current one of the current >> settings for running a domain, while leaving the others at their current >> value? > > > I don''t understand this sentence :-) ...I think he means this: Suppose after the domain is created, you want to set the weight. With the proposed interface, you have to manually read the values and change them yourself. With an interface that allows -1 to be default, libxl can do that for you (making programming easier).> > >> Which interface can I use for that and why isn''t it the same as this >> one? > > > ... making it hard to answer to this one. :-) > Nevertheless trying: you can change the scheduling parameters of a running > domain with xl sched-credit/sched-sedf/...I think Ian has convinced me that just using -1 for default values would be the best option: * It doesn''t require a new special ''what are the defaults" hypercall * The same interface can be used to set only a subset of the options (leaving the other ones alone) *after* the domain is created. Thanks for doing the work to code it up, though Juergen. -George
Ian Campbell
2012-May-22 13:16 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote:> On Tue, May 22, 2012 at 1:58 PM, Juergen Gross > <juergen.gross@ts.fujitsu.com> wrote: > >>> When setting the parameters from the config file we have no domain yet > >>> which > >>> we would have to specify for XEN_DOMCTL_SCHEDOP_getinfo. I didn''t want to > >>> parse part of the config only after domain creation. > >> > >> That seems like another problem with doing this up front instead of > >> doing read-modify-write when we come to set the values for the domain. > > > > > > Do you think it would be okay to parse the scheduler config data _after_ > > domain creation?No, I don''t think there''s any sensible way to make this work at the libxl interface, you''d need to call back out to xl half way through domain build to ask it to parse the sched params for you.> If yes, the read-modify-write approach is simple and always > > correct. If no, you will have to initialize the parameters to something > > scheduler specific, and not domain specific. > > > > > >>> A default should be okay, as this is what we want to modify. :-) > >>> The scheduler should initialize a new domain with this default, of > >>> course. > >> > >> So I can''t use this interface to change the current one of the current > >> settings for running a domain, while leaving the others at their current > >> value? > > > > > > I don''t understand this sentence :-) ... > > I think he means this: Suppose after the domain is created, you want > to set the weight. With the proposed interface, you have to manually > read the values and change them yourself. With an interface that > allows -1 to be default, libxl can do that for you (making programming > easier).Right. And furthermore with this patch the interface for setting the initial values is different to the one for updating them, since this patch gets you the defaults which you change.> > > > > > >> Which interface can I use for that and why isn''t it the same as this > >> one? > > > > > > ... making it hard to answer to this one. :-) > > Nevertheless trying: you can change the scheduling parameters of a running > > domain with xl sched-credit/sched-sedf/...Right, my question above was -- why I can''t I use the same libxl interface as these commands at build time (perhaps internally inside libxl)> I think Ian has convinced me that just using -1 for default values > would be the best option: > * It doesn''t require a new special ''what are the defaults" hypercall > * The same interface can be used to set only a subset of the options > (leaving the other ones alone) *after* the domain is created. > > Thanks for doing the work to code it up, though Juergen. > > -George
Ian Campbell
2012-May-22 13:40 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote:> > I think Ian has convinced me that just using -1 for default values > would be the best option:Perhaps I should code up what I''m talking about for comparison. Ian.
Ian Campbell
2012-May-22 14:59 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Tue, 2012-05-22 at 14:40 +0100, Ian Campbell wrote:> On Tue, 2012-05-22 at 14:05 +0100, George Dunlap wrote: > > > > I think Ian has convinced me that just using -1 for default values > > would be the best option: > > Perhaps I should code up what I''m talking about for comparison.Like the below. Lightly tested with the credit scheduler. I think CAP is the only one for which 0 is a real valid value, but I''m not sure (especially with the SEDF ones which didn''t have existing limits checks in the set function I could crib from...). I''m pretty sure that libxl__sched_set_params needs to get the correct scheduler for the particular domain, but I''ve no idea how to get that... 8<--------------------------- # HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1337698727 -3600 # Node ID 355030f95eb313605a0e43aa7328e731b28a28b3 # Parent 426bbf58cea4559464b6e5d3ff0f65324a5f5926 libxl: make it possible to explicitly specify default sched params To do so we define a descriminating value which is never a valid real value for each parameter. While there: - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl domain config parser. These are global scheduler properties, not per-domain ones (and were never read in any case). - removed libxl_sched_*_domain in favour of libxl_sched_params. - rename libxl_sched_params to libxl_sched_domain_params for clarity. - use this new functionality for the various xl commands which set sched parameters, which saves an explicit read-modify-write in xl. - removed call of xc_domain_getinfolist from a few functions which weren''t actually using the result (looks like a cut and paste error) - fix xl which was setting period for a variety of different config keys. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Tue May 22 14:19:07 2012 +0100 +++ b/tools/libxl/libxl.c Tue May 22 15:58:47 2012 +0100 @@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx } int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid, - libxl_sched_credit_domain *scinfo) + libxl_sched_domain_params *scinfo) { struct xen_domctl_sched_credit sdom; int rc; - libxl_sched_credit_domain_init(scinfo); - rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom); if (rc != 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit"); return ERROR_FAIL; } + libxl_sched_domain_params_init(scinfo); + scinfo->sched = LIBXL_SCHEDULER_CREDIT; scinfo->weight = sdom.weight; scinfo->cap = sdom.cap; @@ -3188,7 +3188,7 @@ int libxl_sched_credit_domain_get(libxl_ } int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid, - libxl_sched_credit_domain *scinfo) + libxl_sched_domain_params *scinfo) { struct xen_domctl_sched_credit sdom; xc_domaininfo_t domaininfo; @@ -3202,22 +3202,33 @@ int libxl_sched_credit_domain_set(libxl_ if (rc != 1 || domaininfo.domain != domid) return ERROR_INVAL; - - if (scinfo->weight < 1 || scinfo->weight > 65535) { - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, - "Cpu weight out of range, valid values are within range from 1 to 65535"); - return ERROR_INVAL; + rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom); + if (rc != 0) { + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit"); + return ERROR_FAIL; } - if (scinfo->cap < 0 || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) { - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, - "Cpu cap out of range, valid range is from 0 to %d for specified number of vcpus", - ((domaininfo.max_vcpu_id + 1) * 100)); - return ERROR_INVAL; + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) { + if (scinfo->weight < 1 || scinfo->weight > 65535) { + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, + "Cpu weight out of range, " + "valid values are within range from 1 to 65535"); + return ERROR_INVAL; + } + sdom.weight = scinfo->weight; } - sdom.weight = scinfo->weight; - sdom.cap = scinfo->cap; + if (scinfo->cap != LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT) { + if (scinfo->cap < 0 + || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) { + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, + "Cpu cap out of range, " + "valid range is from 0 to %d for specified number of vcpus", + ((domaininfo.max_vcpu_id + 1) * 100)); + return ERROR_INVAL; + } + sdom.cap = scinfo->cap; + } rc = xc_sched_credit_domain_set(ctx->xch, domid, &sdom); if ( rc < 0 ) { @@ -3290,13 +3301,11 @@ int libxl_sched_credit_params_set(libxl_ } int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid, - libxl_sched_credit2_domain *scinfo) + libxl_sched_domain_params *scinfo) { struct xen_domctl_sched_credit2 sdom; int rc; - libxl_sched_credit2_domain_init(scinfo); - rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom); if (rc != 0) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, @@ -3304,36 +3313,37 @@ int libxl_sched_credit2_domain_get(libxl return ERROR_FAIL; } + libxl_sched_domain_params_init(scinfo); + scinfo->sched = LIBXL_SCHEDULER_CREDIT2; scinfo->weight = sdom.weight; return 0; } int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid, - libxl_sched_credit2_domain *scinfo) + libxl_sched_domain_params *scinfo) { struct xen_domctl_sched_credit2 sdom; - xc_domaininfo_t domaininfo; int rc; - rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo); - if (rc < 0) { - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list"); + rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom); + if (rc != 0) { + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, + "getting domain sched credit2"); return ERROR_FAIL; } - if (rc != 1 || domaininfo.domain != domid) - return ERROR_INVAL; - - - if (scinfo->weight < 1 || scinfo->weight > 65535) { - LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, - "Cpu weight out of range, valid values are within range from " - "1 to 65535"); - return ERROR_INVAL; + + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) { + if (scinfo->weight < 1 || scinfo->weight > 65535) { + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, + "Cpu weight out of range, " + "valid values are within range from " + "1 to 65535"); + return ERROR_INVAL; + } + sdom.weight = scinfo->weight; } - sdom.weight = scinfo->weight; - rc = xc_sched_credit2_domain_set(ctx->xch, domid, &sdom); if ( rc < 0 ) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, @@ -3345,7 +3355,7 @@ int libxl_sched_credit2_domain_set(libxl } int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid, - libxl_sched_sedf_domain *scinfo) + libxl_sched_domain_params *scinfo) { uint64_t period; uint64_t slice; @@ -3354,8 +3364,6 @@ int libxl_sched_sedf_domain_get(libxl_ct uint16_t weight; int rc; - libxl_sched_sedf_domain_init(scinfo); - rc = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency, &extratime, &weight); if (rc != 0) { @@ -3363,6 +3371,8 @@ int libxl_sched_sedf_domain_get(libxl_ct return ERROR_FAIL; } + libxl_sched_domain_params_init(scinfo); + scinfo->sched = LIBXL_SCHEDULER_SEDF; scinfo->period = period / 1000000; scinfo->slice = slice / 1000000; scinfo->latency = latency / 1000000; @@ -3373,24 +3383,37 @@ int libxl_sched_sedf_domain_get(libxl_ct } int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid, - libxl_sched_sedf_domain *scinfo) + libxl_sched_domain_params *scinfo) { - xc_domaininfo_t domaininfo; - int rc; - - rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo); - if (rc < 0) { - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list"); + uint64_t period; + uint64_t slice; + uint64_t latency; + uint16_t extratime; + uint16_t weight; + + int ret; + + ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency, + &extratime, &weight); + if (ret != 0) { + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf"); return ERROR_FAIL; } - if (rc != 1 || domaininfo.domain != domid) - return ERROR_INVAL; - - - rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000, - scinfo->slice * 1000000, scinfo->latency * 1000000, - scinfo->extratime, scinfo->weight); - if ( rc < 0 ) { + + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT) + period = scinfo->period * 1000000; + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT) + period = scinfo->slice * 1000000; + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT) + period = scinfo->latency * 1000000; + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT) + period = scinfo->extratime; + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) + period = scinfo->weight; + + ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency, + extratime, weight); + if ( ret < 0 ) { LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf"); return ERROR_FAIL; } diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.h --- a/tools/libxl/libxl.h Tue May 22 14:19:07 2012 +0100 +++ b/tools/libxl/libxl.h Tue May 22 15:58:47 2012 +0100 @@ -805,23 +805,33 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx); - -int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid, - libxl_sched_credit_domain *scinfo); -int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid, - libxl_sched_credit_domain *scinfo); +/* Per-scheduler parameters */ int libxl_sched_credit_params_get(libxl_ctx *ctx, uint32_t poolid, libxl_sched_credit_params *scinfo); int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid, libxl_sched_credit_params *scinfo); + +/* Scheduler Per-domain parameters */ + +#define LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT 0 +#define LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT -1 +#define LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT 0 +#define LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT 0 +#define LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT 0 +#define LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT 0 + +int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid, + libxl_sched_domain_params *scinfo); +int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid, + libxl_sched_domain_params *scinfo); int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid, - libxl_sched_credit2_domain *scinfo); + libxl_sched_domain_params *scinfo); int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid, - libxl_sched_credit2_domain *scinfo); + libxl_sched_domain_params *scinfo); int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid, - libxl_sched_sedf_domain *scinfo); + libxl_sched_domain_params *scinfo); int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid, - libxl_sched_sedf_domain *scinfo); + libxl_sched_domain_params *scinfo); int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid, libxl_trigger trigger, uint32_t vcpuid); int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq); diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_dom.c --- a/tools/libxl/libxl_dom.c Tue May 22 14:19:07 2012 +0100 +++ b/tools/libxl/libxl_dom.c Tue May 22 15:58:47 2012 +0100 @@ -42,36 +42,30 @@ libxl_domain_type libxl__domain_type(lib return LIBXL_DOMAIN_TYPE_PV; } -int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams) +int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, + libxl_sched_domain_params *scparams) { libxl_ctx *ctx = libxl__gc_owner(gc); - libxl_scheduler sched; - libxl_sched_sedf_domain sedf_info; - libxl_sched_credit_domain credit_info; - libxl_sched_credit2_domain credit2_info; + libxl_scheduler sched = scparams->sched; int ret; - sched = libxl_get_scheduler (ctx); + if (sched == LIBXL_SCHEDULER_UNKNOWN) + sched = libxl_get_scheduler(ctx); + switch (sched) { case LIBXL_SCHEDULER_SEDF: - sedf_info.period = scparams->period; - sedf_info.slice = scparams->slice; - sedf_info.latency = scparams->latency; - sedf_info.extratime = scparams->extratime; - sedf_info.weight = scparams->weight; - ret=libxl_sched_sedf_domain_set(ctx, domid, &sedf_info); - break; + ret=libxl_sched_sedf_domain_set(ctx, domid, scparams); + break; case LIBXL_SCHEDULER_CREDIT: - credit_info.weight = scparams->weight; - credit_info.cap = scparams->cap; - ret=libxl_sched_credit_domain_set(ctx, domid, &credit_info); - break; + ret=libxl_sched_credit_domain_set(ctx, domid, scparams); + break; case LIBXL_SCHEDULER_CREDIT2: - credit2_info.weight = scparams->weight; - ret=libxl_sched_credit2_domain_set(ctx, domid, &credit2_info); - break; + ret=libxl_sched_credit2_domain_set(ctx, domid, scparams); + break; default: - ret=-1; + LOG(ERROR, "Unknown scheduler"); + ret=ERROR_INVAL; + break; } return ret; } diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_internal.h --- a/tools/libxl/libxl_internal.h Tue May 22 14:19:07 2012 +0100 +++ b/tools/libxl/libxl_internal.h Tue May 22 15:58:47 2012 +0100 @@ -716,7 +716,7 @@ _hidden int libxl__atfork_init(libxl_ctx /* from xl_dom */ _hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid); _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid); -_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams); +_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_domain_params *scparams); #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \ libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type typedef struct { diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl_types.idl --- a/tools/libxl/libxl_types.idl Tue May 22 14:19:07 2012 +0100 +++ b/tools/libxl/libxl_types.idl Tue May 22 15:58:47 2012 +0100 @@ -109,7 +109,9 @@ libxl_bios_type = Enumeration("bios_type ]) # Consistent with values defined in domctl.h +# Except unknown which is special. libxl_scheduler = Enumeration("scheduler", [ + (0, "unknown"), (4, "sedf"), (5, "credit"), (6, "credit2"), @@ -224,15 +226,14 @@ libxl_domain_create_info = Struct("domai MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT") -libxl_sched_params = Struct("sched_params",[ - ("weight", integer), - ("cap", integer), - ("tslice_ms", integer), - ("ratelimit_us", integer), - ("period", integer), - ("slice", integer), - ("latency", integer), - ("extratime", integer), +libxl_sched_domain_params = Struct("sched_domain_params",[ + ("sched", libxl_scheduler), + ("weight", integer, {''init_val'': ''LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT''}), + ("cap", integer, {''init_val'': ''LIBXL_SCHED_DOMAIN_PARAM_CAP_DEFAULT''}), + ("period", integer, {''init_val'': ''LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT''}), + ("slice", integer, {''init_val'': ''LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT''}), + ("latency", integer, {''init_val'': ''LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT''}), + ("extratime", integer, {''init_val'': ''LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT''}), ], dir=DIR_IN) # Instances of libxl_file_reference contained in this struct which @@ -267,7 +268,7 @@ libxl_domain_build_info = Struct("domain # extra parameters pass directly to qemu for HVM guest, NULL terminated ("extra_hvm", libxl_string_list), # parameters for all type of scheduler - ("sched_params", libxl_sched_params), + ("sched_params", libxl_sched_domain_params), ("u", KeyedUnion(None, libxl_domain_type, "type", [("hvm", Struct(None, [("firmware", string), @@ -432,28 +433,11 @@ libxl_cputopology = Struct("cputopology" ("node", uint32), ], dir=DIR_OUT) -libxl_sched_credit_domain = Struct("sched_credit_domain", [ - ("weight", integer), - ("cap", integer), - ]) - libxl_sched_credit_params = Struct("sched_credit_params", [ ("tslice_ms", integer), ("ratelimit_us", integer), ], dispose_fn=None) -libxl_sched_credit2_domain = Struct("sched_credit2_domain", [ - ("weight", integer), - ]) - -libxl_sched_sedf_domain = Struct("sched_sedf_domain", [ - ("period", integer), - ("slice", integer), - ("latency", integer), - ("extratime", integer), - ("weight", integer), - ]) - libxl_domain_remus_info = Struct("domain_remus_info",[ ("interval", integer), ("blackhole", bool), diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100 @@ -627,23 +627,20 @@ static void parse_config_data(const char libxl_domain_build_info_init_type(b_info, c_info->type); - /* the following is the actual config parsing with overriding values in the structures */ + /* the following is the actual config parsing with overriding + * values in the structures */ if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0)) b_info->sched_params.weight = l; if (!xlu_cfg_get_long (config, "cap", &l, 0)) b_info->sched_params.cap = l; - if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0)) - b_info->sched_params.tslice_ms = l; - if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0)) - b_info->sched_params.ratelimit_us = l; if (!xlu_cfg_get_long (config, "period", &l, 0)) b_info->sched_params.period = l; if (!xlu_cfg_get_long (config, "slice", &l, 0)) - b_info->sched_params.period = l; + b_info->sched_params.slice = l; if (!xlu_cfg_get_long (config, "latency", &l, 0)) - b_info->sched_params.period = l; + b_info->sched_params.latency = l; if (!xlu_cfg_get_long (config, "extratime", &l, 0)) - b_info->sched_params.period = l; + b_info->sched_params.extratime = l; if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) { b_info->max_vcpus = l; @@ -4355,7 +4352,7 @@ int main_sharing(int argc, char **argv) return 0; } -static int sched_credit_domain_get(int domid, libxl_sched_credit_domain *scinfo) +static int sched_credit_domain_get(int domid, libxl_sched_domain_params *scinfo) { int rc; @@ -4366,7 +4363,7 @@ static int sched_credit_domain_get(int d return rc; } -static int sched_credit_domain_set(int domid, libxl_sched_credit_domain *scinfo) +static int sched_credit_domain_set(int domid, libxl_sched_domain_params *scinfo) { int rc; @@ -4402,7 +4399,7 @@ static int sched_credit_params_get(int p static int sched_credit_domain_output(int domid) { char *domname; - libxl_sched_credit_domain scinfo; + libxl_sched_domain_params scinfo; int rc; if (domid < 0) { @@ -4419,7 +4416,7 @@ static int sched_credit_domain_output(in scinfo.weight, scinfo.cap); free(domname); - libxl_sched_credit_domain_dispose(&scinfo); + libxl_sched_domain_params_dispose(&scinfo); return 0; } @@ -4445,7 +4442,7 @@ static int sched_credit_pool_output(uint } static int sched_credit2_domain_get( - int domid, libxl_sched_credit2_domain *scinfo) + int domid, libxl_sched_domain_params *scinfo) { int rc; @@ -4457,7 +4454,7 @@ static int sched_credit2_domain_get( } static int sched_credit2_domain_set( - int domid, libxl_sched_credit2_domain *scinfo) + int domid, libxl_sched_domain_params *scinfo) { int rc; @@ -4472,7 +4469,7 @@ static int sched_credit2_domain_output( int domid) { char *domname; - libxl_sched_credit2_domain scinfo; + libxl_sched_domain_params scinfo; int rc; if (domid < 0) { @@ -4488,12 +4485,12 @@ static int sched_credit2_domain_output( domid, scinfo.weight); free(domname); - libxl_sched_credit2_domain_dispose(&scinfo); + libxl_sched_domain_params_dispose(&scinfo); return 0; } static int sched_sedf_domain_get( - int domid, libxl_sched_sedf_domain *scinfo) + int domid, libxl_sched_domain_params *scinfo) { int rc; @@ -4505,7 +4502,7 @@ static int sched_sedf_domain_get( } static int sched_sedf_domain_set( - int domid, libxl_sched_sedf_domain *scinfo) + int domid, libxl_sched_domain_params *scinfo) { int rc; @@ -4519,7 +4516,7 @@ static int sched_sedf_domain_output( int domid) { char *domname; - libxl_sched_sedf_domain scinfo; + libxl_sched_domain_params scinfo; int rc; if (domid < 0) { @@ -4540,7 +4537,7 @@ static int sched_sedf_domain_output( scinfo.extratime, scinfo.weight); free(domname); - libxl_sched_sedf_domain_dispose(&scinfo); + libxl_sched_domain_params_dispose(&scinfo); return 0; } @@ -4620,7 +4617,6 @@ static int sched_domain_output(libxl_sch */ int main_sched_credit(int argc, char **argv) { - libxl_sched_credit_domain scinfo; const char *dom = NULL; const char *cpupool = NULL; int weight = 256, cap = 0, opt_w = 0, opt_c = 0; @@ -4695,7 +4691,7 @@ int main_sched_credit(int argc, char **a } if (opt_s) { - libxl_sched_credit_params scparam; + libxl_sched_credit_params scparam; uint32_t poolid = 0; if (cpupool) { @@ -4731,20 +4727,19 @@ int main_sched_credit(int argc, char **a } else { find_domain(dom); - rc = sched_credit_domain_get(domid, &scinfo); - if (rc) - return -rc; - if (!opt_w && !opt_c) { /* output credit scheduler info */ sched_credit_domain_output(-1); return -sched_credit_domain_output(domid); } else { /* set credit scheduler paramaters */ + libxl_sched_domain_params scinfo; + libxl_sched_domain_params_init(&scinfo); + scinfo.sched = LIBXL_SCHEDULER_CREDIT; if (opt_w) scinfo.weight = weight; if (opt_c) scinfo.cap = cap; rc = sched_credit_domain_set(domid, &scinfo); - libxl_sched_credit_domain_dispose(&scinfo); + libxl_sched_domain_params_dispose(&scinfo); if (rc) return -rc; } @@ -4755,7 +4750,6 @@ int main_sched_credit(int argc, char **a int main_sched_credit2(int argc, char **argv) { - libxl_sched_credit2_domain scinfo; const char *dom = NULL; const char *cpupool = NULL; int weight = 256, opt_w = 0; @@ -4810,18 +4804,17 @@ int main_sched_credit2(int argc, char ** } else { find_domain(dom); - rc = sched_credit2_domain_get(domid, &scinfo); - if (rc) - return -rc; - if (!opt_w) { /* output credit2 scheduler info */ sched_credit2_domain_output(-1); return -sched_credit2_domain_output(domid); } else { /* set credit2 scheduler paramaters */ + libxl_sched_domain_params scinfo; + libxl_sched_domain_params_init(&scinfo); + scinfo.sched = LIBXL_SCHEDULER_CREDIT2; if (opt_w) scinfo.weight = weight; rc = sched_credit2_domain_set(domid, &scinfo); - libxl_sched_credit2_domain_dispose(&scinfo); + libxl_sched_domain_params_dispose(&scinfo); if (rc) return -rc; } @@ -4832,7 +4825,6 @@ int main_sched_credit2(int argc, char ** int main_sched_sedf(int argc, char **argv) { - libxl_sched_sedf_domain scinfo; const char *dom = NULL; const char *cpupool = NULL; int period = 0, opt_p = 0; @@ -4915,15 +4907,15 @@ int main_sched_sedf(int argc, char **arg } else { find_domain(dom); - rc = sched_sedf_domain_get(domid, &scinfo); - if (rc) - return -rc; - if (!opt_p && !opt_s && !opt_l && !opt_e && !opt_w) { /* output sedf scheduler info */ sched_sedf_domain_output(-1); return -sched_sedf_domain_output(domid); } else { /* set sedf scheduler paramaters */ + libxl_sched_domain_params scinfo; + libxl_sched_domain_params_init(&scinfo); + scinfo.sched = LIBXL_SCHEDULER_SEDF; + if (opt_p) { scinfo.period = period; scinfo.weight = 0; @@ -4942,7 +4934,7 @@ int main_sched_sedf(int argc, char **arg scinfo.slice = 0; } rc = sched_sedf_domain_set(domid, &scinfo); - libxl_sched_sedf_domain_dispose(&scinfo); + libxl_sched_domain_params_dispose(&scinfo); if (rc) return -rc; }
Dario Faggioli
2012-May-22 22:15 UTC
Re: [PATCH 3 of 3] full support of setting scheduler parameters on domain creation
On Tue, 2012-05-22 at 14:39 +0200, Juergen Gross wrote:> > This interface really makes libxl_sched_params differ from all the other > > libxl structs (which have a public _init function and an internal > > setdefaults function). I''m not really sure its justified either, I was > > under the impression that you''d found that there were useful > > discriminating values? > > Dario opted for this solution, so I proposed a patch implementing it. > I prefer this solution, too, as it isn''t exporting scheduler internals to > the tools. >Yep. I agree with Ian that having two different procedures for the first as compared to the subsequent operations on the scheduling parameters is bad, but still, as a matter of my personal taste, I''d prefer no to have things belonging to the hypervisor/scheduler replicated in the toolstack, although they''re just simple default values. It''s still something you need to always remember to check for consistency, or bad things will happen! :-/ However, Ian''s approach seems clean and nice as well, and I''m not really sure which one I like most, so don''t count me when deciding, I''m fine with both.> >> int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, > >> libxl_sched_params *scparams) > >> { > >> libxl_ctx *ctx = libxl__gc_owner(gc); > >> - libxl_scheduler sched; > >> libxl_sched_sedf_domain sedf_info; > >> libxl_sched_credit_domain credit_info; > >> libxl_sched_credit2_domain credit2_info; > >> int ret; > >> > >> - sched = libxl_get_scheduler (ctx); > >> - switch (sched) { > >> + switch (scparams->sched) { > > What happens if scparams->sched is not the scheduler used for this > > domain? Should it either be checked or set somewhere? > > The check would be the same as the original setting of scparams->sched. > Setting of the scheduler parameters will be rejected by the hypervisor if the > scheduler does not match. >I was thinking this should go through finding out what domid''s cpupool is and then checking that scheduler, as it is being done somewhere else... Isn''t this the case? Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Dario Faggioli
2012-May-22 23:46 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Tue, 2012-05-22 at 15:59 +0100, Ian Campbell wrote:> Like the below. Lightly tested with the credit scheduler. > > I think CAP is the only one for which 0 is a real valid value, but I''m > not sure (especially with the SEDF ones which didn''t have existing > limits checks in the set function I could crib from...). >Yep, that''s because they''re mostly time values. xen/common/sched_sedf.c hosts some ranges for some of them, but I''m not sure we want to propagate those: #define PERIOD_MAX MILLISECS(10000) /* 10s */ #define PERIOD_MIN (MICROSECS(10)) /* 10us */ #define SLICE_MIN (MICROSECS(5)) /* 5us */ Also, extratime is a flag, so I think 0 and 1 are both meaningful values, maybe we can go for -1 as for cap (I''ll try and let you know).> I''m pretty sure that libxl__sched_set_params needs to get the correct > scheduler for the particular domain, but I''ve no idea how to get that... >Again, I was thinking something like what Juergen did here could help (go getting the scheduler of the cpupool the domain belongs to)... O am I misunderstanding the issue? + poolinfo = libxl_list_cpupool(ctx, &n_pools); + if (!poolinfo) + return ERROR_NOMEM; + + ret = ERROR_INVAL; + for (p = 0; p < n_pools; p++) { + if (poolinfo[p].poolid == poolid) { + scparams->sched = poolinfo[p].sched; + ret = 0; + } + libxl_cpupoolinfo_dispose(poolinfo + p); + } +> 8<--------------------------- > > # HG changeset patch > # User Ian Campbell <ian.campbell@citrix.com> > # Date 1337698727 -3600 > # Node ID 355030f95eb313605a0e43aa7328e731b28a28b3 > # Parent 426bbf58cea4559464b6e5d3ff0f65324a5f5926 > libxl: make it possible to explicitly specify default sched params > > To do so we define a descriminating value which is never a valid real value for > each parameter. > > While there: > > - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl > domain config parser. These are global scheduler properties, not per-domain > ones (and were never read in any case). > - removed libxl_sched_*_domain in favour of libxl_sched_params. > - rename libxl_sched_params to libxl_sched_domain_params for clarity. > - use this new functionality for the various xl commands which set sched > parameters, which saves an explicit read-modify-write in xl. > - removed call of xc_domain_getinfolist from a few functions which weren''t > actually using the result (looks like a cut and paste error) > - fix xl which was setting period for a variety of different config keys. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Tue May 22 14:19:07 2012 +0100 > +++ b/tools/libxl/libxl.c Tue May 22 15:58:47 2012 +0100 > @@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx > } ><snip>> > int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid, > - libxl_sched_sedf_domain *scinfo) > + libxl_sched_domain_params *scinfo) > { > - xc_domaininfo_t domaininfo; > - int rc; > - > - rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo); > - if (rc < 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list"); > + uint64_t period; > + uint64_t slice; > + uint64_t latency; > + uint16_t extratime; > + uint16_t weight; > + > + int ret; > + > + ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency, > + &extratime, &weight); > + if (ret != 0) { > + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf"); > return ERROR_FAIL; > } > - if (rc != 1 || domaininfo.domain != domid) > - return ERROR_INVAL; > - > - > - rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000, > - scinfo->slice * 1000000, scinfo->latency * 1000000, > - scinfo->extratime, scinfo->weight); > - if ( rc < 0 ) { > + > + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT) > + period = scinfo->period * 1000000; > + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT) > + period = scinfo->slice * 1000000; > + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT) > + period = scinfo->latency * 1000000; > + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT) > + period = scinfo->extratime; > + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) > + period = scinfo->weight; > + > + ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency, > + extratime, weight); > + if ( ret < 0 ) { > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf"); > return ERROR_FAIL; > } ># xl create vm1.cfg Parsing config from vm1.cfg libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument And I''m getting the above independently on what I put in the config file (valid params, no params at all, etc.). I also can''t change the scheduling parameters of a domain on-line anymore: # xl sched-sedf -d 3 -p 100 -s 50 libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument libxl_sched_sedf_domain_set failed. I''ll try digging a bit more into this ASAP. On more thing, are we ok with the _set command failing and he domain being created anyway? Or perhaps the whole process should just abort? Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Juergen Gross
2012-May-23 05:34 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On 05/23/2012 01:46 AM, Dario Faggioli wrote:> On Tue, 2012-05-22 at 15:59 +0100, Ian Campbell wrote: >> Like the below. Lightly tested with the credit scheduler. >> >> I think CAP is the only one for which 0 is a real valid value, but I''m >> not sure (especially with the SEDF ones which didn''t have existing >> limits checks in the set function I could crib from...). >> > Yep, that''s because they''re mostly time values. xen/common/sched_sedf.c > hosts some ranges for some of them, but I''m not sure we want to > propagate those: > > #define PERIOD_MAX MILLISECS(10000) /* 10s */ > #define PERIOD_MIN (MICROSECS(10)) /* 10us */ > #define SLICE_MIN (MICROSECS(5)) /* 5us */I think this should remain in the hypervisor only.> Also, extratime is a flag, so I think 0 and 1 are both meaningful > values, maybe we can go for -1 as for cap (I''ll try and let you know).Why not -1 for all values?>> I''m pretty sure that libxl__sched_set_params needs to get the correct >> scheduler for the particular domain, but I''ve no idea how to get that... >> > Again, I was thinking something like what Juergen did here could help > (go getting the scheduler of the cpupool the domain belongs to)... O am > I misunderstanding the issue? > > + poolinfo = libxl_list_cpupool(ctx,&n_pools); > + if (!poolinfo) > + return ERROR_NOMEM; > + > + ret = ERROR_INVAL; > + for (p = 0; p< n_pools; p++) { > + if (poolinfo[p].poolid == poolid) { > + scparams->sched = poolinfo[p].sched; > + ret = 0; > + } > + libxl_cpupoolinfo_dispose(poolinfo + p); > + } > +This was exactly the purpose of the sniplet.>> 8<--------------------------- >> >> # HG changeset patch >> # User Ian Campbell<ian.campbell@citrix.com> >> # Date 1337698727 -3600 >> # Node ID 355030f95eb313605a0e43aa7328e731b28a28b3 >> # Parent 426bbf58cea4559464b6e5d3ff0f65324a5f5926 >> libxl: make it possible to explicitly specify default sched params >> >> To do so we define a descriminating value which is never a valid real value for >> each parameter. >> >> While there: >> >> - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl >> domain config parser. These are global scheduler properties, not per-domain >> ones (and were never read in any case). >> - removed libxl_sched_*_domain in favour of libxl_sched_params. >> - rename libxl_sched_params to libxl_sched_domain_params for clarity. >> - use this new functionality for the various xl commands which set sched >> parameters, which saves an explicit read-modify-write in xl. >> - removed call of xc_domain_getinfolist from a few functions which weren''t >> actually using the result (looks like a cut and paste error) >> - fix xl which was setting period for a variety of different config keys. >> >> Signed-off-by: Ian Campbell<ian.campbell@citrix.com> >> >> diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c >> --- a/tools/libxl/libxl.c Tue May 22 14:19:07 2012 +0100 >> +++ b/tools/libxl/libxl.c Tue May 22 15:58:47 2012 +0100 >> @@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx >> } >> > <snip> >> >> int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid, >> - libxl_sched_sedf_domain *scinfo) >> + libxl_sched_domain_params *scinfo) >> { >> - xc_domaininfo_t domaininfo; >> - int rc; >> - >> - rc = xc_domain_getinfolist(ctx->xch, domid, 1,&domaininfo); >> - if (rc< 0) { >> - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list"); >> + uint64_t period; >> + uint64_t slice; >> + uint64_t latency; >> + uint16_t extratime; >> + uint16_t weight; >> + >> + int ret; >> + >> + ret = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency, >> +&extratime,&weight); >> + if (ret != 0) { >> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf"); >> return ERROR_FAIL; >> } >> - if (rc != 1 || domaininfo.domain != domid) >> - return ERROR_INVAL; >> - >> - >> - rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000, >> - scinfo->slice * 1000000, scinfo->latency * 1000000, >> - scinfo->extratime, scinfo->weight); >> - if ( rc< 0 ) { >> + >> + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT) >> + period = scinfo->period * 1000000; >> + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT) >> + period = scinfo->slice * 1000000; >> + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT) >> + period = scinfo->latency * 1000000; >> + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT) >> + period = scinfo->extratime; >> + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) >> + period = scinfo->weight; >> + >> + ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency, >> + extratime, weight); >> + if ( ret< 0 ) { >> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf"); >> return ERROR_FAIL; >> } >> > # xl create vm1.cfg > Parsing config from vm1.cfg > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > > And I''m getting the above independently on what I put in the config file > (valid params, no params at all, etc.). I also can''t change the > scheduling parameters of a domain on-line anymore: > > # xl sched-sedf -d 3 -p 100 -s 50 > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > libxl_sched_sedf_domain_set failed. > > I''ll try digging a bit more into this ASAP.It''s easy: Ian repeated an error he (and I) corrected elsewhere: He''s always setting period, regardless of the changed parameter...> On more thing, are we ok with the _set command failing and he domain > being created anyway? Or perhaps the whole process should just abort?Perhaps a warning might be okay. Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
Dario Faggioli
2012-May-23 07:22 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote:> > #define PERIOD_MAX MILLISECS(10000) /* 10s */ > > #define PERIOD_MIN (MICROSECS(10)) /* 10us */ > > #define SLICE_MIN (MICROSECS(5)) /* 5us */ > > I think this should remain in the hypervisor only. >Me too.> > Also, extratime is a flag, so I think 0 and 1 are both meaningful > > values, maybe we can go for -1 as for cap (I''ll try and let you know). > > Why not -1 for all values? >Would work, I guess, unless there''s some collision with big weights represented on short unsigned value (not sure it''s like that, and I''ve always been bad at this kind of math! :-P).> >> I''m pretty sure that libxl__sched_set_params needs to get the correct > >> scheduler for the particular domain, but I''ve no idea how to get that... > >> > > Again, I was thinking something like what Juergen did here could help > > (go getting the scheduler of the cpupool the domain belongs to)... O am > > I misunderstanding the issue? > > > > + poolinfo = libxl_list_cpupool(ctx,&n_pools); > > + if (!poolinfo) > > + return ERROR_NOMEM; > > + > > + ret = ERROR_INVAL; > > + for (p = 0; p< n_pools; p++) { > > + if (poolinfo[p].poolid == poolid) { > > + scparams->sched = poolinfo[p].sched; > > + ret = 0; > > + } > > + libxl_cpupoolinfo_dispose(poolinfo + p); > > + } > > + > > This was exactly the purpose of the sniplet. >Good to hear that. :-)> > # xl create vm1.cfg > > Parsing config from vm1.cfg > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > > > > And I''m getting the above independently on what I put in the config file > > (valid params, no params at all, etc.). I also can''t change the > > scheduling parameters of a domain on-line anymore: > > > > # xl sched-sedf -d 3 -p 100 -s 50 > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > > libxl_sched_sedf_domain_set failed. > > > > I''ll try digging a bit more into this ASAP. > > It''s easy: Ian repeated an error he (and I) corrected elsewhere: > He''s always setting period, regardless of the changed parameter... >Not sure, I think Ian''s patch fixes that: --- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100 @@ -627,23 +627,20 @@ static void parse_config_data(const char libxl_domain_build_info_init_type(b_info, c_info->type); - /* the following is the actual config parsing with overriding values in the structures */ + /* the following is the actual config parsing with overriding + * values in the structures */ if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0)) b_info->sched_params.weight = l; if (!xlu_cfg_get_long (config, "cap", &l, 0)) b_info->sched_params.cap = l; - if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0)) - b_info->sched_params.tslice_ms = l; - if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0)) - b_info->sched_params.ratelimit_us = l; if (!xlu_cfg_get_long (config, "period", &l, 0)) b_info->sched_params.period = l; if (!xlu_cfg_get_long (config, "slice", &l, 0)) - b_info->sched_params.period = l; + b_info->sched_params.slice = l; if (!xlu_cfg_get_long (config, "latency", &l, 0)) - b_info->sched_params.period = l; + b_info->sched_params.latency = l; if (!xlu_cfg_get_long (config, "extratime", &l, 0)) - b_info->sched_params.period = l; + b_info->sched_params.extratime = l; So it has to be something else... Thanks and Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Campbell
2012-May-23 07:41 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Wed, 2012-05-23 at 08:22 +0100, Dario Faggioli wrote:> On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote: > > > #define PERIOD_MAX MILLISECS(10000) /* 10s */ > > > #define PERIOD_MIN (MICROSECS(10)) /* 10us */ > > > #define SLICE_MIN (MICROSECS(5)) /* 5us */ > > > > I think this should remain in the hypervisor only. > > > Me too.Agreed.> > > Also, extratime is a flag, so I think 0 and 1 are both meaningful > > > values, maybe we can go for -1 as for cap (I''ll try and let you know). > > > > Why not -1 for all values? > > > Would work, I guess, unless there''s some collision with big weights > represented on short unsigned value (not sure it''s like that, and I''ve > always been bad at this kind of math! :-P).In general in libxl where we can use 0 for the default we do so. It''s not a strong preference though and it''s mainly historical from before the IDL supported init_val.> > >> I''m pretty sure that libxl__sched_set_params needs to get the correct > > >> scheduler for the particular domain, but I''ve no idea how to get that... > > >> > > > Again, I was thinking something like what Juergen did here could help > > > (go getting the scheduler of the cpupool the domain belongs to)... O am > > > I misunderstanding the issue? > > > > > > + poolinfo = libxl_list_cpupool(ctx,&n_pools); > > > + if (!poolinfo) > > > + return ERROR_NOMEM; > > > + > > > + ret = ERROR_INVAL; > > > + for (p = 0; p< n_pools; p++) { > > > + if (poolinfo[p].poolid == poolid) { > > > + scparams->sched = poolinfo[p].sched; > > > + ret = 0; > > > + } > > > + libxl_cpupoolinfo_dispose(poolinfo + p); > > > + } > > > + > > > > This was exactly the purpose of the sniplet. > > > Good to hear that. :-)Great, I''ll make a suitably named function out of it.> > > # xl create vm1.cfg > > > Parsing config from vm1.cfg > > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > > > > > > And I''m getting the above independently on what I put in the config file > > > (valid params, no params at all, etc.). I also can''t change the > > > scheduling parameters of a domain on-line anymore: > > > > > > # xl sched-sedf -d 3 -p 100 -s 50 > > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > > > libxl_sched_sedf_domain_set failed. > > > > > > I''ll try digging a bit more into this ASAP. > > > > It''s easy: Ian repeated an error he (and I) corrected elsewhere: > > He''s always setting period, regardless of the changed parameter... > > > Not sure, I think Ian''s patch fixes that:I thought so to!> --- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100 > @@ -627,23 +627,20 @@ static void parse_config_data(const char > > libxl_domain_build_info_init_type(b_info, c_info->type); > > - /* the following is the actual config parsing with overriding values in the structures */ > + /* the following is the actual config parsing with overriding > + * values in the structures */ > if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0)) > b_info->sched_params.weight = l; > if (!xlu_cfg_get_long (config, "cap", &l, 0)) > b_info->sched_params.cap = l; > - if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0)) > - b_info->sched_params.tslice_ms = l; > - if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0)) > - b_info->sched_params.ratelimit_us = l; > if (!xlu_cfg_get_long (config, "period", &l, 0)) > b_info->sched_params.period = l; > if (!xlu_cfg_get_long (config, "slice", &l, 0)) > - b_info->sched_params.period = l; > + b_info->sched_params.slice = l; > if (!xlu_cfg_get_long (config, "latency", &l, 0)) > - b_info->sched_params.period = l; > + b_info->sched_params.latency = l; > if (!xlu_cfg_get_long (config, "extratime", &l, 0)) > - b_info->sched_params.period = l; > + b_info->sched_params.extratime = l; > > > So it has to be something else... > > Thanks and Regards, > Dario >
Juergen Gross
2012-May-23 08:45 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On 05/23/2012 09:41 AM, Ian Campbell wrote:> # xl create vm1.cfg > Parsing config from vm1.cfg > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > > And I''m getting the above independently on what I put in the config file > (valid params, no params at all, etc.). I also can''t change the > scheduling parameters of a domain on-line anymore: > > # xl sched-sedf -d 3 -p 100 -s 50 > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > libxl_sched_sedf_domain_set failed. > > I''ll try digging a bit more into this ASAP. >>> It''s easy: Ian repeated an error he (and I) corrected elsewhere: >>> He''s always setting period, regardless of the changed parameter... >>> >> Not sure, I think Ian''s patch fixes that: > I thought so to!It fixes the original error.>> --- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100 >> +++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100 >> @@ -627,23 +627,20 @@ static void parse_config_data(const char >> >> libxl_domain_build_info_init_type(b_info, c_info->type); >> >> - /* the following is the actual config parsing with overriding values in the structures */ >> + /* the following is the actual config parsing with overriding >> + * values in the structures */ >> if (!xlu_cfg_get_long (config, "cpu_weight",&l, 0)) >> b_info->sched_params.weight = l; >> if (!xlu_cfg_get_long (config, "cap",&l, 0)) >> b_info->sched_params.cap = l; >> - if (!xlu_cfg_get_long (config, "tslice_ms",&l, 0)) >> - b_info->sched_params.tslice_ms = l; >> - if (!xlu_cfg_get_long (config, "ratelimit_us",&l, 0)) >> - b_info->sched_params.ratelimit_us = l; >> if (!xlu_cfg_get_long (config, "period",&l, 0)) >> b_info->sched_params.period = l; >> if (!xlu_cfg_get_long (config, "slice",&l, 0)) >> - b_info->sched_params.period = l; >> + b_info->sched_params.slice = l; >> if (!xlu_cfg_get_long (config, "latency",&l, 0)) >> - b_info->sched_params.period = l; >> + b_info->sched_params.latency = l; >> if (!xlu_cfg_get_long (config, "extratime",&l, 0)) >> - b_info->sched_params.period = l; >> + b_info->sched_params.extratime = l; >> >> >> So it has to be something else...Oh yes: + + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT) + period = scinfo->period * 1000000; + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT) + period = scinfo->slice * 1000000; + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT) + period = scinfo->latency * 1000000; + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT) + period = scinfo->extratime; + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) + period = scinfo->weight; Regardless which parameter was changed, it is always ''period'' which is set to the changed value. Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
Juergen Gross
2012-May-23 08:48 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On 05/23/2012 09:22 AM, Dario Faggioli wrote:> On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote: >>> #define PERIOD_MAX MILLISECS(10000) /* 10s */ >>> #define PERIOD_MIN (MICROSECS(10)) /* 10us */ >>> #define SLICE_MIN (MICROSECS(5)) /* 5us */ >> I think this should remain in the hypervisor only. >> > Me too. > >>> Also, extratime is a flag, so I think 0 and 1 are both meaningful >>> values, maybe we can go for -1 as for cap (I''ll try and let you know). >> Why not -1 for all values? >> > Would work, I guess, unless there''s some collision with big weights > represented on short unsigned value (not sure it''s like that, and I''ve > always been bad at this kind of math! :-P).The -1 would be set in scparams only, and there it is an integer. It should never be written to the "real" scheduler parameters passed to the hypervisor. Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html
Ian Campbell
2012-May-23 09:17 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Wed, 2012-05-23 at 09:45 +0100, Juergen Gross wrote:> Oh yes: > > + > + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT) > + period = scinfo->period * 1000000; > + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT) > + period = scinfo->slice * 1000000; > + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT) > + period = scinfo->latency * 1000000; > + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT) > + period = scinfo->extratime; > + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) > + period = scinfo->weight; > > Regardless which parameter was changed, it is always ''period'' which is set > to the changed value.Oh balls! Thanks for noticing, I''ll fix this and resend... Ian.
Dario Faggioli
2012-May-23 10:18 UTC
Re: [PATCH 1 of 3] Support of getting scheduler defaults
On Wed, 2012-05-23 at 10:45 +0200, Juergen Gross wrote:> >> Not sure, I think Ian''s patch fixes that: > > I thought so to! > > It fixes the original error. >Yep, that was what I was looking at ...> >> So it has to be something else... > > Oh yes: > > + > + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT) > + period = scinfo->period * 1000000; > + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT) > + period = scinfo->slice * 1000000; > + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT) > + period = scinfo->latency * 1000000; > + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT) > + period = scinfo->extratime; > + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) > + period = scinfo->weight; > > Regardless which parameter was changed, it is always ''period'' which is set > to the changed value. >... And completely ignored this! :-P However, I''m still having some issues (even after fixing this)! I''ll keep looking at this, testing the series Ian just sent and let you know. Thanks and Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel