Juergen Gross
2012-May-21 11:46 UTC
[PATCH] full support of setting scheduler parameters on domain creation
Obtains current scheduler parameters before modifying any settings specified via domain config. Only specified settings must be modified, of course! Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com> 3 files changed, 37 insertions(+), 24 deletions(-) tools/libxl/libxl_dom.c | 25 +++++++++++++++++-------- tools/libxl/libxl_types.idl | 8 ++++++++ tools/libxl/xl_cmdimpl.c | 28 ++++++++++++---------------- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Campbell
2012-May-21 12:07 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On Mon, 2012-05-21 at 12:46 +0100, Juergen Gross wrote:> Obtains current scheduler parameters before modifying any settings specified > via domain config. Only specified settings must be modified, of course! >I presume this will fix the libxl: error: libxl.c:3208:libxl_sched_credit_domain_set: Cpu weight out of range, valid values are within range from 1 to 65535 warning we are currently seeing? Thanks!> @@ -233,6 +233,14 @@ libxl_sched_params = Struct("sched_param > ("slice", integer), > ("latency", integer), > ("extratime", integer), > + ("set_weight", bool), > + ("set_cap", bool), > + ("set_tslice_ms", bool), > + ("set_ratelimit_us", bool), > + ("set_period", bool), > + ("set_slice", bool), > + ("set_latency", bool), > + ("set_extratime", bool),Rather than doing this it would be preferable to identify some specific value which means "default" for each of these fields. Generally this would be either 0 (preferred if possible) or ~0 or -1. You can then describe this in the IDL using the "init_val" property on each field. e.g.: @@ -225,7 +225,7 @@ libxl_domain_create_info = Struct("domai MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT") libxl_sched_params = Struct("sched_params",[ - ("weight", integer), + ("weight", integer, {''init_val'': -1}), ("cap", integer), ("tslice_ms", integer), ("ratelimit_us", integer), (just as an illustration of a non-zero default, I suspect 0 would actually be a fine default value for weight, and 0 is the default init_val) Then libxl_sched_params_init, which xl must call (perhaps indirectly, e.g. via libxl_domain_build_info_init) would set these defaults and xl would override them from the config and libxl would only set those which were non-default. Ian.
Juergen Gross
2012-May-21 13:23 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On 05/21/2012 02:07 PM, Ian Campbell wrote:> On Mon, 2012-05-21 at 12:46 +0100, Juergen Gross wrote: >> Obtains current scheduler parameters before modifying any settings specified >> via domain config. Only specified settings must be modified, of course! >> > I presume this will fix the > libxl: error: libxl.c:3208:libxl_sched_credit_domain_set: Cpu weight out > of range, valid values are within range from 1 to 65535 > warning we are currently seeing? Thanks! > >> @@ -233,6 +233,14 @@ libxl_sched_params = Struct("sched_param >> ("slice", integer), >> ("latency", integer), >> ("extratime", integer), >> + ("set_weight", bool), >> + ("set_cap", bool), >> + ("set_tslice_ms", bool), >> + ("set_ratelimit_us", bool), >> + ("set_period", bool), >> + ("set_slice", bool), >> + ("set_latency", bool), >> + ("set_extratime", bool), > Rather than doing this it would be preferable to identify some specific > value which means "default" for each of these fields. Generally this > would be either 0 (preferred if possible) or ~0 or -1. You can then > describe this in the IDL using the "init_val" property on each field. > e.g.: > > @@ -225,7 +225,7 @@ libxl_domain_create_info = Struct("domai > MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT") > > libxl_sched_params = Struct("sched_params",[ > - ("weight", integer), > + ("weight", integer, {''init_val'': -1}), > ("cap", integer), > ("tslice_ms", integer), > ("ratelimit_us", integer), > > (just as an illustration of a non-zero default, I suspect 0 would > actually be a fine default value for weight, and 0 is the default > init_val) > > Then libxl_sched_params_init, which xl must call (perhaps indirectly, > e.g. via libxl_domain_build_info_init) would set these defaults and xl > would override them from the config and libxl would only set those which > were non-default.Hmm. Scheduling parameters are handled in the hypervisor. I don''t want to export the knowledge about semantics to the tools. If this is no problem, why can''t I just set the defaults in the tools and omit asking the hypervisor for the current settings? Additionally there are parameters (well, one parameter) which apply to multiple schedulers. Just by chance -1 would be invalid for all of them, but I don''t like to hard code this coincidence. 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-21 13:34 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On Mon, 2012-05-21 at 14:23 +0100, Juergen Gross wrote:> On 05/21/2012 02:07 PM, Ian Campbell wrote: > > On Mon, 2012-05-21 at 12:46 +0100, Juergen Gross wrote: > >> Obtains current scheduler parameters before modifying any settings specified > >> via domain config. Only specified settings must be modified, of course! > >> > > I presume this will fix the > > libxl: error: libxl.c:3208:libxl_sched_credit_domain_set: Cpu weight out > > of range, valid values are within range from 1 to 65535 > > warning we are currently seeing? Thanks! > > > >> @@ -233,6 +233,14 @@ libxl_sched_params = Struct("sched_param > >> ("slice", integer), > >> ("latency", integer), > >> ("extratime", integer), > >> + ("set_weight", bool), > >> + ("set_cap", bool), > >> + ("set_tslice_ms", bool), > >> + ("set_ratelimit_us", bool), > >> + ("set_period", bool), > >> + ("set_slice", bool), > >> + ("set_latency", bool), > >> + ("set_extratime", bool), > > Rather than doing this it would be preferable to identify some specific > > value which means "default" for each of these fields. Generally this > > would be either 0 (preferred if possible) or ~0 or -1. You can then > > describe this in the IDL using the "init_val" property on each field. > > e.g.: > > > > @@ -225,7 +225,7 @@ libxl_domain_create_info = Struct("domai > > MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT") > > > > libxl_sched_params = Struct("sched_params",[ > > - ("weight", integer), > > + ("weight", integer, {''init_val'': -1}), > > ("cap", integer), > > ("tslice_ms", integer), > > ("ratelimit_us", integer), > > > > (just as an illustration of a non-zero default, I suspect 0 would > > actually be a fine default value for weight, and 0 is the default > > init_val) > > > > Then libxl_sched_params_init, which xl must call (perhaps indirectly, > > e.g. via libxl_domain_build_info_init) would set these defaults and xl > > would override them from the config and libxl would only set those which > > were non-default. > > Hmm. Scheduling parameters are handled in the hypervisor. I don''t want to > export the knowledge about semantics to the tools. If this is no problem, > why can''t I just set the defaults in the tools and omit asking the > hypervisor for the current settings?Exporting the idea that 0 is not a valid weight is (IMHO at least) better than exporting the fact that the default weight is (e.g.) 200 and hard coding that in multiple places.> Additionally there are parameters (well, one parameter) which apply to > multiple schedulers. Just by chance -1 would be invalid for all of them, > but I don''t like to hard code this coincidence.Which parameter is it? Is there any reasonable possibility that a scheduler would ever use -1 (or +4 billion) for it? You could define a symbolic name if that would make you more comfortable (that would allow us to change the specific value without changing the API) Ian.
Juergen Gross
2012-May-21 13:48 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On 05/21/2012 03:34 PM, Ian Campbell wrote:> On Mon, 2012-05-21 at 14:23 +0100, Juergen Gross wrote: >> On 05/21/2012 02:07 PM, Ian Campbell wrote: >>> On Mon, 2012-05-21 at 12:46 +0100, Juergen Gross wrote: >>>> Obtains current scheduler parameters before modifying any settings specified >>>> via domain config. Only specified settings must be modified, of course! >>>> >>> I presume this will fix the >>> libxl: error: libxl.c:3208:libxl_sched_credit_domain_set: Cpu weight out >>> of range, valid values are within range from 1 to 65535 >>> warning we are currently seeing? Thanks! >>> >>>> @@ -233,6 +233,14 @@ libxl_sched_params = Struct("sched_param >>>> ("slice", integer), >>>> ("latency", integer), >>>> ("extratime", integer), >>>> + ("set_weight", bool), >>>> + ("set_cap", bool), >>>> + ("set_tslice_ms", bool), >>>> + ("set_ratelimit_us", bool), >>>> + ("set_period", bool), >>>> + ("set_slice", bool), >>>> + ("set_latency", bool), >>>> + ("set_extratime", bool), >>> Rather than doing this it would be preferable to identify some specific >>> value which means "default" for each of these fields. Generally this >>> would be either 0 (preferred if possible) or ~0 or -1. You can then >>> describe this in the IDL using the "init_val" property on each field. >>> e.g.: >>> >>> @@ -225,7 +225,7 @@ libxl_domain_create_info = Struct("domai >>> MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT") >>> >>> libxl_sched_params = Struct("sched_params",[ >>> - ("weight", integer), >>> + ("weight", integer, {''init_val'': -1}), >>> ("cap", integer), >>> ("tslice_ms", integer), >>> ("ratelimit_us", integer), >>> >>> (just as an illustration of a non-zero default, I suspect 0 would >>> actually be a fine default value for weight, and 0 is the default >>> init_val) >>> >>> Then libxl_sched_params_init, which xl must call (perhaps indirectly, >>> e.g. via libxl_domain_build_info_init) would set these defaults and xl >>> would override them from the config and libxl would only set those which >>> were non-default. >> Hmm. Scheduling parameters are handled in the hypervisor. I don''t want to >> export the knowledge about semantics to the tools. If this is no problem, >> why can''t I just set the defaults in the tools and omit asking the >> hypervisor for the current settings? > Exporting the idea that 0 is not a valid weight is (IMHO at least) > better than exporting the fact that the default weight is (e.g.) 200 and > hard coding that in multiple places.Agreed.>> Additionally there are parameters (well, one parameter) which apply to >> multiple schedulers. Just by chance -1 would be invalid for all of them, >> but I don''t like to hard code this coincidence. > Which parameter is it? Is there any reasonable possibility that a > scheduler would ever use -1 (or +4 billion) for it?It''s the cpu_weight. :-) I don''t object using an invalid value instead of a boolean, I just thought it would be cleaner to say "parameter xy was specified". This would include the case when a user specifies 0 for the cpu_weight: it would be rejected instead of just ignored...> You could define a symbolic name if that would make you more comfortable > (that would allow us to change the specific value without changing the > API)As the "invalid" value would be used at least twice this is a must. 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-21 13:48 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On Mon, May 21, 2012 at 2:34 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:>> Hmm. Scheduling parameters are handled in the hypervisor. I don''t want to >> export the knowledge about semantics to the tools. If this is no problem, >> why can''t I just set the defaults in the tools and omit asking the >> hypervisor for the current settings? > > Exporting the idea that 0 is not a valid weight is (IMHO at least) > better than exporting the fact that the default weight is (e.g.) 200 and > hard coding that in multiple places.I agree.> You could define a symbolic name if that would make you more comfortable > (that would allow us to change the specific value without changing the > API)That is, as long as the "reasonable value" is the same for all of the parameters. I half wonder if having an "init schedule params" function which would set each value to the default for that value would be useful, or if it would be overkill. Of course, if we''re doing that, it''s only one step further to just reading the actual scheduler parameters... -George
Ian Campbell
2012-May-21 13:53 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On Mon, 2012-05-21 at 14:48 +0100, George Dunlap wrote:> On Mon, May 21, 2012 at 2:34 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > >> Hmm. Scheduling parameters are handled in the hypervisor. I don''t want to > >> export the knowledge about semantics to the tools. If this is no problem, > >> why can''t I just set the defaults in the tools and omit asking the > >> hypervisor for the current settings? > > > > Exporting the idea that 0 is not a valid weight is (IMHO at least) > > better than exporting the fact that the default weight is (e.g.) 200 and > > hard coding that in multiple places. > > I agree. > > > You could define a symbolic name if that would make you more comfortable > > (that would allow us to change the specific value without changing the > > API) > > That is, as long as the "reasonable value" is the same for all of the > parameters.I actually meant a symbolic name for the default of each, rather than one for all of them.> I half wonder if having an "init schedule params" function which would > set each value to the default for that value would be useful, or if it > would be overkill. > > Of course, if we''re doing that, it''s only one step further to just > reading the actual scheduler parameters...I suppose we could make the autogenerated libxl_sched_params_init instead be a hand-coded thing which actually reads them. Ian.
Juergen Gross
2012-May-22 05:43 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On 05/21/2012 03:53 PM, Ian Campbell wrote:> On Mon, 2012-05-21 at 14:48 +0100, George Dunlap wrote: >> On Mon, May 21, 2012 at 2:34 PM, Ian Campbell<Ian.Campbell@citrix.com> wrote: >>>> Hmm. Scheduling parameters are handled in the hypervisor. I don''t want to >>>> export the knowledge about semantics to the tools. If this is no problem, >>>> why can''t I just set the defaults in the tools and omit asking the >>>> hypervisor for the current settings? >>> Exporting the idea that 0 is not a valid weight is (IMHO at least) >>> better than exporting the fact that the default weight is (e.g.) 200 and >>> hard coding that in multiple places. >> I agree. >> >>> You could define a symbolic name if that would make you more comfortable >>> (that would allow us to change the specific value without changing the >>> API) >> That is, as long as the "reasonable value" is the same for all of the >> parameters. > I actually meant a symbolic name for the default of each, rather than > one for all of them.-1 would fit for all parameters. This value is either invalid or "don''t care".>> I half wonder if having an "init schedule params" function which would >> set each value to the default for that value would be useful, or if it >> would be overkill. >> >> Of course, if we''re doing that, it''s only one step further to just >> reading the actual scheduler parameters... > I suppose we could make the autogenerated libxl_sched_params_init > instead be a hand-coded thing which actually reads them.Reading the scheduler parameters would require a new hypervisor interface (e.g. a new sub-command of XEN_DOMCTL_scheduler_op) which will have to be implemented by all schedulers supporting parameter changes. I think this would be the cleanest solution. If this is the way to go, I can prepare a patch. BTW: I just discovered the first patch was not complete, as the original coding to select the scheduler didn''t take cpupools into account. It just selected the default scheduler instead of the cpupool specific one. 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-22 07:42 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On Tue, 2012-05-22 at 07:43 +0200, Juergen Gross wrote:> Reading the scheduler parameters would require a new hypervisor interface > (e.g. a new sub-command of XEN_DOMCTL_scheduler_op) which will have to be > implemented by all schedulers supporting parameter changes. > > I think this would be the cleanest solution. If this is the way to go, I > can prepare a patch. >For what it counts, this is what I''d like most.> BTW: I just discovered the first patch was not complete, as the original > coding to select the scheduler didn''t take cpupools into account. It just > selected the default scheduler instead of the cpupool specific one. >Yep, I noticed it too and mentioned on the list, but didn''t have time yet to cook a patch for that... I guess you''re going to cover it while reposting, aren''t you? If not, let me know, I can put something together. :-) 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
Juergen Gross
2012-May-22 07:47 UTC
Re: [PATCH] full support of setting scheduler parameters on domain creation
On 05/22/2012 09:42 AM, Dario Faggioli wrote:> On Tue, 2012-05-22 at 07:43 +0200, Juergen Gross wrote: >> Reading the scheduler parameters would require a new hypervisor interface >> (e.g. a new sub-command of XEN_DOMCTL_scheduler_op) which will have to be >> implemented by all schedulers supporting parameter changes. >> >> I think this would be the cleanest solution. If this is the way to go, I >> can prepare a patch. >> > For what it counts, this is what I''d like most.Okay, already two of us :-) Stay tuned, patch(es) in progress...>> BTW: I just discovered the first patch was not complete, as the original >> coding to select the scheduler didn''t take cpupools into account. It just >> selected the default scheduler instead of the cpupool specific one. >> > Yep, I noticed it too and mentioned on the list, but didn''t have time > yet to cook a patch for that... I guess you''re going to cover it while > reposting, aren''t you? If not, let me know, I can put something > together. :-)Finished already :-) 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