All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Dario Faggioli <dario.faggioli@citrix.com>
To: Dagaen Golomb <dgolomb@seas.upenn.edu>
Cc: Wei Liu <wei.liu2@citrix.com>, Sisu Xi <xisisu@gmail.com>,
	George Dunlap <george.dunlap@eu.citrix.com>,
	xen-devel@lists.xen.org, Meng Xu <mengxu@cis.upenn.edu>,
	Jan Beulich <jbeulich@suse.com>, Chong Li <lichong659@gmail.com>
Subject: Re: [PATCH] Modified RTDS scheduler to use an event-driven model instead of polling.
Date: Mon, 22 Jun 2015 11:11:40 +0200	[thread overview]
Message-ID: <1434964300.25170.65.camel@citrix.com> (raw)
In-Reply-To: <CALcuvTjRhG=PV8HW20Vr4vFsUdg42_wk4NQYBvFfgavv_QZRbg@mail.gmail.com>


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

On Thu, 2015-06-18 at 14:07 -0400, Dagaen Golomb wrote:

> > Anyway, I've zero interest in turning this into a fight over
> > terminology... If you want to call runq_tickle() "the scheduler", go
> > ahead, it would just make communication a bit more difficult, but I'm up
> > for the challenge! :-)
> >
> > Oh, BTW, while we're here, __runq_pick() being called from a bunch of
> > places, outside of rt_schedule(), is also something I never liked (you
> > can go check that in the archives), and I also blame the confusion
> > between scheduling and replenishmets, for the fact that such a thing is
> > necessary. I seriously hope this can be fixed too.
> 
> I have no interest in a terminology war either. I just figured that if we want
> the replenishment function to only tickle when necessary, then it should
> check that the replenished vCPU is a greater priority than current ones.
>
Exactly, we need a preemption check, which is part of the 'scheduling
logic'... but everything in the file is part of the scheduling logic (or
it will be in another one), and we don't want to put everything that is
in the file in one only function! :-)

An in fact, a preemption check is not the full scheduling logic it's...
well... a preemption check. For example, if there are idle pCPUs, it's
something very quick.

Note that, right now, this preemption check is done by acquiring the
global lock and checking the deadlines of currently running vCPUs on all
pCPUs. In future, this can be modified/improved, by using a dedicate
data structure, and a locking scheme that would reduce the pressure on
the global lock. I'm not saying that you should do this as part of the
work you're doing now. Rather, I'm saying that the work being done now
should go in the direction of making this easier, not harder.

> > And in fact, I want __runq_pick() and related logic to be in
> > rt_schedule(), and nowhere else, while I want runq_tickle() to be done
> > from replenishment (and wakeup), and nowhere else.
> 
> I guess the last remaining question is this: when the scheduler is
> enforcing a budget and times out, should it check for replenishment
> before kicking the vCPU? Or assume that any relevant replenishments
> have occurred?
>
This is an implementation detail that is quite hard to discuss without
seeing the code. In theory, if there wasn't any overhead, etc., you
shouldn't, because it should not be necessary. In practise, yes, it is
possible that various sources of overhead prevent a replenishment to be
notified in time, and that you end up with a vCPU running, and consuming
the last bits of its budget _after_ a scheduled replenishment instant as
it is, I guess, possible that you figure during a replenishment that the
budget was exhausted and the vCPU had overrun a bit, I guess (or not, it
again depends on the actual implementation).

So, yes, you certainly need to take care of this corner cases, and you
can do it in the way you think it's best, basing on how the code ends up
looking like. The important thing is that they're treated for what they
are, i.e., we should handle them, not design the code around them.

> This is the issue I mentioned before about two timers
> armed at the same time - I'm not sure what convention Xen uses to
> order them. 
>
No, I don't think this has much to do with the internals of timers'
implementation, it's a corner case that, due to overhead, long critical
section (or, in general, interrupts-off sections), etc., will always be
present and will need to be taken care of, no mater how the scheduler is
implemented.

> I would assume from your very reason for mentioning this
> change that you don't want any replenishment in rt_schedule, but then
> it may kick a vCPU who at that very instant is supposed to be replenished
> as well, and should actually stay on the pCPU.
>
That depends of the vCPU, on it's budget and deadline, and on what's
running on other pCPUs. And that is something unlikely, although
possible, and it should be the exception rather than the rule.

Anyway, I think it would be correct, for instance, to check during
rt_schedule() whether the vCPU running on the pCPU is already beyond
it's deadline/replenishment time. If yes, log/count a deadline miss
(because that's what just happened!), replenish it and continue with
rt_schedule().

>  This is assuming you
> still want rt_schedule to be timer-triggered to enforce budget, is this
> correct?
> 
Of course.

> However, I think this is a more minor issue that we can sort out via
> inspecting the default Xen behavior, or allowing a single replenishment
> call before kicking (I don't expect you to like that option :P), or some
> other method.
> 
Exactly.

Thanks and Regards,
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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

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

  reply	other threads:[~2015-06-22  9:11 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-08 11:46 [PATCH] Modified RTDS scheduler to use an event-driven model instead of polling Dagaen Golomb
2015-06-09 12:53 ` Dario Faggioli
2015-06-10  4:18   ` Dagaen Golomb
2015-06-10 22:30     ` Dario Faggioli
2015-06-13 20:33       ` Dagaen Golomb
2015-06-16 17:07         ` Dagaen Golomb
2015-06-17  0:20           ` Dario Faggioli
2015-06-17  5:24             ` Dagaen Golomb
2015-06-17  5:45               ` Meng Xu
2015-06-17  6:03                 ` Dagaen Golomb
2015-06-17  6:09                   ` Meng Xu
2015-06-17  9:20                 ` Dario Faggioli
2015-06-17  8:27               ` Dario Faggioli
2015-06-18 18:07                 ` Dagaen Golomb
2015-06-22  9:11                   ` Dario Faggioli [this message]
2015-06-22 11:58                     ` Dagaen Golomb
2015-06-10  5:54   ` Meng Xu
2015-06-10 17:46     ` Dario Faggioli
2015-06-11  5:50       ` Meng Xu
2015-06-12  9:28         ` Dario Faggioli
2015-06-13 17:21           ` Meng Xu

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=1434964300.25170.65.camel@citrix.com \
    --to=dario.faggioli@citrix.com \
    --cc=dgolomb@seas.upenn.edu \
    --cc=george.dunlap@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=lichong659@gmail.com \
    --cc=mengxu@cis.upenn.edu \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    --cc=xisisu@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.