From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH] full support of setting scheduler parameters on domain creation Date: Mon, 21 May 2012 15:48:15 +0200 Message-ID: <4FBA479F.6050208@ts.fujitsu.com> References: <10457bda81c6aea95bbf.1337600810@nehalem1> <1337602071.24660.99.camel@zakaz.uk.xensource.com> <4FBA41C1.30908@ts.fujitsu.com> <1337607296.24660.135.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1337607296.24660.135.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org 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