Re: [Xen-devel] [PATCH v2 for Xen 4.6 4/4] xl: enabling XL to set per-VCPU parameters of a domain for RTDS scheduler

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Mon, 2015-06-08 at 16:18 -0500, Chong Li wrote:
> On Mon, Jun 8, 2015 at 11:21 AM, Dario Faggioli
> <dario.faggioli@xxxxxxxxxx> wrote:
> > On Mon, 2015-05-25 at 19:11 -0500, Chong Li wrote:

> > I appreciate just now that this probably affect other bits of the
> > interface, as well as other interfaces, and I think we should handle it
> > consistently...
> >
> > What do you think?
> > For example (although this belong to patch 3's review) what
> > libxl_domain_sched_params_get() does in the case I jus described?
> 
> Here, "-o/--output" is used for the users who only do per-dom
> settings. In those cases, "-o" is provided to show per-dom parameters,
> and the output is just the same as what the old RTDS tool does.
> 
Right, I saw that. And as far as xl is concerned, this is ok... I just
do not like the name "-o/--output". I'd rather go with something like
"-a/--all", or implement something like this:

 # xl sched-rtds -d 2 -v all

This is a perhaps a bit more difficult to implement (but not so much,
unless I'm overlooking something), but I personally like it better,
considering it's similar to the syntax we already have for `xl
vcpu-pin'.

> When "-o" is set, libxl_domain_sched_params_get() and
> sched_rtds_domain_get() (both two functions in libxl.c) are called.
> 
OTOH, while in libxl, the thing is different. I mean, you can't assume
that a specific libxl function would be called _only_ in the way and
from the places where you're calling it in xl: other toolstack building
on top of libxl can try to do things differently!

So, IIRC, in patch 3 you are just not touching
libxl_domain_sched_params_get(), nor sched_rtds_domain_get() and
(perhaps in another patch) xc_sched_rtds_domain_get(). What I'm asking
is, what does it mean "users who only do per-dom settings"? What happens
if I, for instance, do a bunch of:

 libxl_vcpu_sched_params_set(...)

for various vcpus, each one with different parameters, and then call:

 libxl_domain_sched_params_get(...)

Have you tested this case? What's the output?

> Without "-o", then sched_rtds_domain_get() should be deleted because
> it will never be touched. I'm also fine with that.
> 
Deleting stuff may or not may be an option, depending on what components
and what interfaces we're talking about. If we're talking about libxl,
internal functions can well go, of course. Public API calls can't.

Anyway, what I think is that, for a scheduler that supports per-vcpu
parameters, getting and setting per-domain parameters should be
converted to "do a vcpu_param_set() on all vcpus", and that applied
everywhere: libxl, libxc and xen.

That does not necessarily mean ripping off the entire per-domain params
handling logic, or convert it as above always and everywhere, because:
 1) e.g., in libxl, you just can't get rid of the public API call (not
    to mention that I think it's actually useful to have it there and
    behave as described above)
 2) even after this patch, there still are schedulers that wants 
    per-domain and not per-vcpu parameters. When (if?) that won't be
    true any longer, at least at the hypervisor and xc levels (for
    libxl, see 1)), we could refactor things, but not now.

Let me know what you think.

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

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