xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Dario Faggioli <raistlin@linux.it>
To: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian.Jackson@citrix.com,
	Juergen Gross <juergen.gross@ts.fujitsu.com>,
	xen-devel@lists.xen.org, Ian Campbell <ian.campbell@citrix.com>,
	George.Dunlap@citrix.com
Subject: Re: [PATCH 3 of 3] libxl: make it possible to explicitly specify default sched params
Date: Wed, 23 May 2012 23:19:35 +0200	[thread overview]
Message-ID: <1337807975.25349.9.camel@Abyss> (raw)
In-Reply-To: <CAFLBxZZ0mZGTLUr0ONCR3hLQc+6FDN5L6vB4q0kiAn_YosT4CA@mail.gmail.com>


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

On Wed, 2012-05-23 at 20:28 +0100, George Dunlap wrote: 
> Overall the idea of the patch looks good.  There's just the thing
> about shoving all the various schedulers' parameters into one struct.
>
Yep, I really don't like that either.

> One fall-out from it is that if you specify weight in your config file
> (or during domain creation), it will set the weight for credit or
> credit2, but use the defaults for sedf.  This might be nice; but we're
> implicitly baking in an assumption that parameters with the same name
> have to have roughly similar meanings across all schedulers.
> Furthermore, if someone sets a "cap" in the config file, for example,
> but starts the VM in a pool running credit2, should we really just
> silently ignore it, or should we alert the user in some way?
> 
I agree... Some mechanism for providing the user at least with a warning
would be useful.

> In any case, this patch only takes things half-way.  If we're really
> going to have One Struct to Rule Them All, we don't need different
> domain_set/domain_get functions for the different schedulers -- we
> just need a libxl_sched_domain_get(), which will both figure out what
> scheduler the domain is running, and fill in the appropriate
> parameters, and a libxl_sched_domain_set(), which will check to see
> that you've asked for the right scheduler (or marked "unknown" if you
> aren't afraid), and set what it can set.  
>
According to my personal taste, that would be quite ugly, not to mention
that every time we might be adding/removing/modifying a scheduler, we
would need to update this Frankenstein-struct, potentially affecting all
the other ones... :-(

> I'm not really sure which way I think is best.  I can see the
> advantage of not having to know which scheduler is actually running,
> but I'm a bit wary of baking in assumptions about the equivalence of
> parameters; it seems like it could lead to some nasty surprises.
> 
I agree again: the fact that, right now, _almost_ all the existing
schedulers have a parameter called weight with _almost_ the same meaning
shouldn't allow us to assume that to be true now and forever.

> But I think whichever way we choose, we should take it to its logical
> conclusion.  Which in the "One Struct" way, would mean having a single
> domain_get/domain_set function, and in the "separate struct" way would
> probably mean specifying the scheduler -- i.e., "credit_weight",
> "credit2_weight" or something like that.  (Obviously we need xm
> compatibility, but we can throw a warning to encourage people to
> change their config files.)	
> 
For what it counts, I'm all for option #2, i.e., each scheduler with its
own struct, set of helper functions, xl sub-command, etc. Something like
'credit.cap = XX', 'credit2.weight = XX' or 'sedf.period = XXX' would be
nice, for discriminating them in the config file. It'd remain to decide
what to do with things like 'weight = XX', which we need to support for
backward compatibility, but I guess almost anything is fine, provided we
warn the user about what's happening and ask him to update the syntax.

Just my 2 cents. :-)

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

  parent reply	other threads:[~2012-05-23 21:19 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-23  9:26 [PATCH 0 of 3] libxl: make it possible to explicitly specify default sched params Ian Campbell
2012-05-23  9:26 ` [PATCH 1 of 3] libxl: add internal function to get a domain's scheduler Ian Campbell
2012-05-23 19:47   ` George Dunlap
2012-05-24  8:55     ` Ian Campbell
2012-05-24  9:16       ` Dario Faggioli
2012-05-23  9:26 ` [PATCH 2 of 3] libxl: rename libxl_sched_params to libxl_sched_domain_params Ian Campbell
2012-05-23 19:49   ` George Dunlap
2012-05-23  9:26 ` [PATCH 3 of 3] libxl: make it possible to explicitly specify default sched params Ian Campbell
2012-05-23 10:51   ` Ian Jackson
2012-05-23 11:12   ` Dario Faggioli
2012-05-24  8:52     ` Ian Campbell
2012-05-24  9:15       ` Dario Faggioli
2012-05-23 19:28   ` George Dunlap
2012-05-23 19:34     ` George Dunlap
2012-05-23 21:19     ` Dario Faggioli [this message]
2012-05-24  9:13       ` Ian Campbell
2012-05-24  9:36         ` Dario Faggioli
2012-05-24 13:57     ` Ian Jackson
2012-05-24 14:02       ` Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1337807975.25349.9.camel@Abyss \
    --to=raistlin@linux.it \
    --cc=George.Dunlap@citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=Ian.Jackson@citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=juergen.gross@ts.fujitsu.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).