Re: [Xen-users] xl doesn't honour the parameter cpu_weight from my config file while xm does honour it

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4060 bytes --]

On Mon, 2012-04-23 at 13:04 +0100, Ian Campbell wrote: 
> > I've made a little patch to set the cpu_weight for credit, credit2 and
> > sedf scheduler from the config file.
> 
> Awesome, thank you!
> 
Agreed, nice work and thank you for doing it!

> > Btw.: for the sedf scheduler there seems to be some type mismatch.  
> > The functions xc_sedf_domain_set and xc_sedf_domain_get expect the type
> > 'uint16_t' for variables 'extratime' and 'weight' while the structure
> > 'xen_domctl_sched_sedf' defines the type uint32_t for them.
> > I think they should be the same, or not ?
> 
> I'm not clear enough on the scheduling stuff to be able to say one way
> or another. I'd be inclined to suggest that if this needs to change for
> some reason then we should leave it for post 4.2.
> 
In Xen's SEDF code weight is a 'short', while extratime is nothing more
than a boolean flag (it can store only the EXTRA_AWARE flag). So I think
it would be worth looking at this, but I agree we ca defer that for now,
especially considering the whole SEDF interface might need some
restructuring! :-/

> >      xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
> >      libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus,
> > &info->cpumap);
> >      xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> > LIBXL_MAXMEM_CONSTANT);
> > +
> > +    sched = libxl_get_scheduler (ctx);
> > +    
> > +    switch (sched) {
> > +    case LIBXL_SCHEDULER_SEDF:
> > +      xc_sedf_domain_get (ctx->xch, domid, &(sched_op.u.sedf.period),
> > &(sched_op.u.sedf.slice), &(sched_op.u.sedf.latency), (uint16_t *)
> > &(sched_op.u.sedf.extratime), (uint16_t *) &(sched_op.u.sedf.weight));
> > +      sched_op.u.sedf.weight = info->weight;
> > +      xc_sedf_domain_set (ctx->xch, domid, sched_op.u.sedf.period,
> > sched_op.u.sedf.slice, sched_op.u.sedf.latency, (uint16_t)
> > sched_op.u.sedf.extratime, (uint16_t) sched_op.u.sedf.weight);
> 
> Wow, that's a pretty sucky interface at the libxc level! Anyway no need
> for you to fix it here but please do wrap your lines to <80 columns for
> readability.
> 
Indeed! I really think we have to do something about this, but that
definitely will happen post-release. :-)

> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 5cf9708..f185d4c 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -232,6 +232,8 @@ MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
> >  libxl_domain_build_info = Struct("domain_build_info",[
> >      ("max_vcpus",       integer),
> >      ("cur_vcpus",       integer),
> > +    ("weight",          integer),
> > +    ("cap",             integer),
> 
> Are these values common parameters to all schedulers? Do they always
> have the same meaning/units/etc?
> 
Maybe weight is, but still, I think having some mechanism for specifying
the full set of parameter of a specific scheduler is to be preferred...

> I wonder if perhaps including each of libxl_sched_*_params in build_info
> might be a preferable interface? I would probably cleanup the above code
> in build_pre too since you could just call the appropriate
> libxl_sched_*_params_set.
> 
... Exactly, that's much better looking to me.

> Ideally libxl_sched_*_params would be in a union in
> libxl_domain_build_info but that would require that xl could easily
> determine which scheduler was going to be used for the domain, having a
> non-union here would keep things somewhat simpler from that PoV.
> 
Yes, again, I agree that a union will be even better, but maybe not so
much a big deal for now (we can turn it into an union later, right? Or
you think there will be some API implications?)

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)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel