xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Questions about the use of idle_vcpu[]
@ 2016-01-15  1:04 Tianyang Chen
  2016-01-18 10:47 ` George Dunlap
  0 siblings, 1 reply; 11+ messages in thread
From: Tianyang Chen @ 2016-01-15  1:04 UTC (permalink / raw)
  To: xen-devel@lists.xen.org; +Cc: Meng Xu

Hello,

I'm confused by the use of idle_vcpu[] in Xen schedulers. In credit, 
credit2 and rtds schedulers, they are used in conjunction with variable 
"tasklet_work_scheduled" like this:

if ( tasklet_work_scheduled )
{
     snext = #_VCPU(idle_vcpu[cpu]);
     ....
}

The idle_vcpu array is initialized in cpu_schedule_up() and indeed they 
are set in corresponding alloc_vdata() to have IDLE_CREDIT and 0 weight 
or DEFAULT_PERIOD and 0 budget.

The value of tasklet_work_scheduled comes from scheduler.c interface. 
According to the comments in sched_credit2.c, if there's tasklet work to 
do, we want to chose the idle vcpu for this processor, and mark the 
current for delayed runqueue add. Can someone elaborate the reason why 
an idle vcpu should be picked? What does an idle_vcpu do to a tasklet? I 
feel like I'm missing something here.

If an idle vcpu is picked, the ret.time is set accordingly in both 
credit and credit2 by checking whether snext is idle. if so, credit 
returns -1 and credit2 returns 2ms. However, there is no corresponding 
code in the RTDS scheduler to handle this. When an idle_vcpu is picked, 
the value of ret.time would be 0 and the scheduler would be invoked 
again. What is the logic behind this?

I'd appreciate it if anyone could point me to the right direction.

Thanks,
Tianyang Chen

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Questions about the use of idle_vcpu[]
  2016-01-15  1:04 Questions about the use of idle_vcpu[] Tianyang Chen
@ 2016-01-18 10:47 ` George Dunlap
  2016-01-18 11:00   ` Dario Faggioli
  0 siblings, 1 reply; 11+ messages in thread
From: George Dunlap @ 2016-01-18 10:47 UTC (permalink / raw)
  To: Tianyang Chen; +Cc: Dario Faggioli, Meng Xu, xen-devel@lists.xen.org

On Fri, Jan 15, 2016 at 1:04 AM, Tianyang Chen <tiche@seas.upenn.edu> wrote:
> Hello,
>
> I'm confused by the use of idle_vcpu[] in Xen schedulers. In credit, credit2
> and rtds schedulers, they are used in conjunction with variable
> "tasklet_work_scheduled" like this:
>
> if ( tasklet_work_scheduled )
> {
>     snext = #_VCPU(idle_vcpu[cpu]);
>     ....
> }
>
> The idle_vcpu array is initialized in cpu_schedule_up() and indeed they are
> set in corresponding alloc_vdata() to have IDLE_CREDIT and 0 weight or
> DEFAULT_PERIOD and 0 budget.
>
> The value of tasklet_work_scheduled comes from scheduler.c interface.
> According to the comments in sched_credit2.c, if there's tasklet work to do,
> we want to chose the idle vcpu for this processor, and mark the current for
> delayed runqueue add. Can someone elaborate the reason why an idle vcpu
> should be picked? What does an idle_vcpu do to a tasklet? I feel like I'm
> missing something here.

do_tasklet() is called from idle_loop() (see xen/arch/{arm,x86}/domain.c).

Xen doesn't have kernel threads, so when it has tasks which are
prompted by an interrupt but need to be done in a non-interrupt
context, it switches to the "idle" vcpu and does them there.

> If an idle vcpu is picked, the ret.time is set accordingly in both credit
> and credit2 by checking whether snext is idle. if so, credit returns -1 and
> credit2 returns 2ms. However, there is no corresponding code in the RTDS
> scheduler to handle this. When an idle_vcpu is picked, the value of ret.time
> would be 0 and the scheduler would be invoked again. What is the logic
> behind this?

No real logic, as far as I can tell. :-)  The ret.time return value
tells the generic scheduling code when to set the next scheduler
timer.  According to the comment in xen/common/schedule.c:schedule(),
returning a negative value means "don't bother setting a timer" (e.g.,
no time limit).  So credit1 does the right thing.

It looks like credit2's behavior will probably prevent the processor
from going into deeper power-saving states, and rtds' behavior might
cause it to essentially busy-wait.

Dario / Meng, am I reading things right?  If so, we should probably fix that...

 -George

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Questions about the use of idle_vcpu[]
  2016-01-18 10:47 ` George Dunlap
@ 2016-01-18 11:00   ` Dario Faggioli
  2016-01-18 12:37     ` George Dunlap
  2016-01-18 16:07     ` Questions about the use of idle_vcpu[] Meng Xu
  0 siblings, 2 replies; 11+ messages in thread
From: Dario Faggioli @ 2016-01-18 11:00 UTC (permalink / raw)
  To: George Dunlap, Tianyang Chen; +Cc: Meng Xu, xen-devel@lists.xen.org


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

On Mon, 2016-01-18 at 10:47 +0000, George Dunlap wrote:
> On Fri, Jan 15, 2016 at 1:04 AM, Tianyang Chen <tiche@seas.upenn.edu>
> 
> > If an idle vcpu is picked, the ret.time is set accordingly in both
> > credit
> > and credit2 by checking whether snext is idle. if so, credit
> > returns -1 and
> > credit2 returns 2ms. However, there is no corresponding code in the
> > RTDS
> > scheduler to handle this. When an idle_vcpu is picked, the value of
> > ret.time
> > would be 0 and the scheduler would be invoked again. What is the
> > logic
> > behind this?
> 
> No real logic, as far as I can tell. :-)  The ret.time return value
> tells the generic scheduling code when to set the next scheduler
> timer.  According to the comment in xen/common/schedule.c:schedule(),
> returning a negative value means "don't bother setting a timer"
> (e.g.,
> no time limit).  So credit1 does the right thing.
> 
It does.

> It looks like credit2's behavior will probably prevent the processor
> from going into deeper power-saving states, and rtds' behavior might
> cause it to essentially busy-wait.
> 
RTDS behavior is broken in many respect, including this, and in fact,
Meng and Tianyang are sending patches already to fix it (I'll let you
guys have my comments shortly :-P).

Credit2, AFAICR, could also avoid _always_ re-setting the timer, but it
does need to do that at least a few times, even when idle is selected,
because of the dynamic load tracking mechanism it includes. In fact,
that is based on a 'decaying average', which in turns relies on
csched2_schedule() to run and update the statistics, even when the cpu
is idle. If we don't do that, the load tracking mechanism will never
see that the cpu (well, it's actually the runqueue) is idle, and the
load will never go down! :-/

I've got patches for:
 - extending the dynamic load tracking logic to all scheduler (iff
   they want to use it, of course)
 - only call {csched,csched2,rtds}_schedule() if necessary. That
   means, if the pcpu/runqueue is idle (and if the dynamic tracking is
   in use by the specific scheduler), only return a positive value 
   and set the timer if the dynamic load is >0, to allow it to decay
   (if the pcpu/runqueue stays idle enough).

But they are on hold, while I'm finishing some other work.

> Dario / Meng, am I reading things right?  If so, we should probably
> fix that...
> 
Yep, and in fact, we're on it already. I hope to be able to post my
patches soon. :-)

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Questions about the use of idle_vcpu[]
  2016-01-18 11:00   ` Dario Faggioli
@ 2016-01-18 12:37     ` George Dunlap
  2016-01-18 12:47       ` Dario Faggioli
  2016-01-18 16:07     ` Questions about the use of idle_vcpu[] Meng Xu
  1 sibling, 1 reply; 11+ messages in thread
From: George Dunlap @ 2016-01-18 12:37 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: Tianyang Chen, Meng Xu, xen-devel@lists.xen.org

On Mon, Jan 18, 2016 at 11:00 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
>> It looks like credit2's behavior will probably prevent the processor
>> from going into deeper power-saving states, and rtds' behavior might
>> cause it to essentially busy-wait.
>>
> RTDS behavior is broken in many respect, including this, and in fact,
> Meng and Tianyang are sending patches already to fix it (I'll let you
> guys have my comments shortly :-P).
>
> Credit2, AFAICR, could also avoid _always_ re-setting the timer, but it
> does need to do that at least a few times, even when idle is selected,
> because of the dynamic load tracking mechanism it includes. In fact,
> that is based on a 'decaying average', which in turns relies on
> csched2_schedule() to run and update the statistics, even when the cpu
> is idle. If we don't do that, the load tracking mechanism will never
> see that the cpu (well, it's actually the runqueue) is idle, and the
> load will never go down! :-/

I don't think that's true -- it looks like balance_load() will call
__update_runq_load() on the "other" runqueue before considering it,
and will also call __update_svc_load() on each vcpu before considering
it.  Shouldn't that suffice?

 -George

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Questions about the use of idle_vcpu[]
  2016-01-18 12:37     ` George Dunlap
@ 2016-01-18 12:47       ` Dario Faggioli
  2016-01-18 14:12         ` Load calculation refresh in credit2 (was in Re: Questions about the use of idle_vcpu[]) George Dunlap
  0 siblings, 1 reply; 11+ messages in thread
From: Dario Faggioli @ 2016-01-18 12:47 UTC (permalink / raw)
  To: George Dunlap; +Cc: Tianyang Chen, Meng Xu, xen-devel@lists.xen.org


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

On Mon, 2016-01-18 at 12:37 +0000, George Dunlap wrote:
> On Mon, Jan 18, 2016 at 11:00 AM, Dario Faggioli
> <dario.faggioli@citrix.com> wrote:
> > > 
> > Credit2, AFAICR, could also avoid _always_ re-setting the timer,
> > but it
> > does need to do that at least a few times, even when idle is
> > selected,
> > because of the dynamic load tracking mechanism it includes. In
> > fact,
> > that is based on a 'decaying average', which in turns relies on
> > csched2_schedule() to run and update the statistics, even when the
> > cpu
> > is idle. If we don't do that, the load tracking mechanism will
> > never
> > see that the cpu (well, it's actually the runqueue) is idle, and
> > the
> > load will never go down! :-/
> 
> I don't think that's true -- it looks like balance_load() will call
> __update_runq_load() on the "other" runqueue before considering it,
> and will also call __update_svc_load() on each vcpu before
> considering
> it.  Shouldn't that suffice?
> 
Mm... It looks like it should.

And yet, I observed that 'load not going down' behavior while doing
development for the patch I mentioned, both on Credit2 and Credit (with
patches for extending the load tracking to Credit applied).

I was, in the same series, also trying to optimize the Credit2's load
balancer a little bit, though, so what I saw may be the effect of some
other change of mine...

I'll have a go at just disabling the periodic scheduler activation when
cpu is idle in the current code, and if it works, I'll send a patch.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Load calculation refresh in credit2 (was in Re: Questions about the use of idle_vcpu[])
  2016-01-18 12:47       ` Dario Faggioli
@ 2016-01-18 14:12         ` George Dunlap
  2016-01-19  9:23           ` Dario Faggioli
  0 siblings, 1 reply; 11+ messages in thread
From: George Dunlap @ 2016-01-18 14:12 UTC (permalink / raw)
  To: Dario Faggioli, George Dunlap
  Cc: Tianyang Chen, Meng Xu, xen-devel@lists.xen.org

[Changing the title to align with the current topic]

On 18/01/16 12:47, Dario Faggioli wrote:
> On Mon, 2016-01-18 at 12:37 +0000, George Dunlap wrote:
>> On Mon, Jan 18, 2016 at 11:00 AM, Dario Faggioli
>> <dario.faggioli@citrix.com> wrote:
>>>>  
>>> Credit2, AFAICR, could also avoid _always_ re-setting the timer,
>>> but it
>>> does need to do that at least a few times, even when idle is
>>> selected,
>>> because of the dynamic load tracking mechanism it includes. In
>>> fact,
>>> that is based on a 'decaying average', which in turns relies on
>>> csched2_schedule() to run and update the statistics, even when the
>>> cpu
>>> is idle. If we don't do that, the load tracking mechanism will
>>> never
>>> see that the cpu (well, it's actually the runqueue) is idle, and
>>> the
>>> load will never go down! :-/
>>
>> I don't think that's true -- it looks like balance_load() will call
>> __update_runq_load() on the "other" runqueue before considering it,
>> and will also call __update_svc_load() on each vcpu before
>> considering
>> it.  Shouldn't that suffice?
>>
> Mm... It looks like it should.
> 
> And yet, I observed that 'load not going down' behavior while doing
> development for the patch I mentioned, both on Credit2 and Credit (with
> patches for extending the load tracking to Credit applied).
> 
> I was, in the same series, also trying to optimize the Credit2's load
> balancer a little bit, though, so what I saw may be the effect of some
> other change of mine...

Hmm... Did you see it when the system was under load, or mostly idle?

Load balancing only happens on a reset event; and the frequency of reset
events will be CREDIT_INIT / (% utilization); so for a system at 1%
utilization that would be once every second.  Is that the kind of number
you were seeing?  Or were you actually seeing idle runqueues not having
anything pushed to them *during* a balance for some reason?

 -George

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Questions about the use of idle_vcpu[]
  2016-01-18 11:00   ` Dario Faggioli
  2016-01-18 12:37     ` George Dunlap
@ 2016-01-18 16:07     ` Meng Xu
  2016-01-18 16:32       ` Dario Faggioli
  2016-01-19 22:59       ` Tianyang Chen
  1 sibling, 2 replies; 11+ messages in thread
From: Meng Xu @ 2016-01-18 16:07 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: George Dunlap, Tianyang Chen, xen-devel@lists.xen.org

On Mon, Jan 18, 2016 at 6:00 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
>
> On Mon, 2016-01-18 at 10:47 +0000, George Dunlap wrote:
> > On Fri, Jan 15, 2016 at 1:04 AM, Tianyang Chen <tiche@seas.upenn.edu>
> >
> > > If an idle vcpu is picked, the ret.time is set accordingly in both
> > > credit
> > > and credit2 by checking whether snext is idle. if so, credit
> > > returns -1 and
> > > credit2 returns 2ms. However, there is no corresponding code in the
> > > RTDS
> > > scheduler to handle this. When an idle_vcpu is picked, the value of
> > > ret.time
> > > would be 0 and the scheduler would be invoked again. What is the
> > > logic
> > > behind this?
> >
> > No real logic, as far as I can tell. :-)  The ret.time return value
> > tells the generic scheduling code when to set the next scheduler
> > timer.  According to the comment in xen/common/schedule.c:schedule(),
> > returning a negative value means "don't bother setting a timer"
> > (e.g.,
> > no time limit).  So credit1 does the right thing.
> >
> It does.


Then the RTDS is doing *incorrectly* right now. :-(

>
>
> > It looks like credit2's behavior will probably prevent the processor
> > from going into deeper power-saving states, and rtds' behavior might
> > cause it to essentially busy-wait.
> >
> RTDS behavior is broken in many respect, including this,
>
> and in fact,
> Meng and Tianyang are sending patches already to fix it (I'll let you
> guys have my comments shortly :-P).


Right. Tianyang and I are working on changing it from quantum driven
model to event-driven (or called timer-driven) model. Tianyang sent
out the first-version patch, but that version has some problems. He is
working on the second version now.

Hi Dario,
Tianyang is working on the second version right now.
If you could have a quick look at our discussion in that thread and
points out the "serious" issues in the decision, that will be great!
We won't repeat the error again and again in the following versions.
As to the minor issues, we could refine it in the second version.
(I'm just thinking about how to save your time to have this done. For
the obvious things that I can handle, I will do it and avoid "wasting"
you time. For the design choices that we are unclear, we definitely
need your insights/commands. ;-) )

>
> Credit2, AFAICR, could also avoid _always_ re-setting the timer, but it
> does need to do that at least a few times, even when idle is selected,
> because of the dynamic load tracking mechanism it includes. In fact,
> that is based on a 'decaying average', which in turns relies on
> csched2_schedule() to run and update the statistics, even when the cpu
> is idle. If we don't do that, the load tracking mechanism will never
> see that the cpu (well, it's actually the runqueue) is idle, and the
> load will never go down! :-/
>
> I've got patches for:
>  - extending the dynamic load tracking logic to all scheduler (iff
>    they want to use it, of course)
>  - only call {csched,csched2,rtds}_schedule() if necessary. That
>    means, if the pcpu/runqueue is idle (and if the dynamic tracking is
>    in use by the specific scheduler), only return a positive value
>    and set the timer if the dynamic load is >0, to allow it to decay
>    (if the pcpu/runqueue stays idle enough).
>
> But they are on hold, while I'm finishing some other work.
>
> > Dario / Meng, am I reading things right?  If so, we should probably
> > fix that...
> >
> Yep, and in fact, we're on it already. I hope to be able to post my
> patches soon. :-)


Hi George,
Yes, you are right. The current RTDS should not return 0 when the idle
VCPU is picked. I think it should do as what the credit does, i.e.,
returning a negative value to avoid arming the timer.
Right now, we are working on changing RTDS to event-driven model. We
will fix this in the next version of the patch.

If needed, we can send out a separate patch to fix this specific issue
(i.e. it should return negative value when idle vcpu is picked.) I'm
ok with either way. Which way do you guys prefer?

Best,

Meng



-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Questions about the use of idle_vcpu[]
  2016-01-18 16:07     ` Questions about the use of idle_vcpu[] Meng Xu
@ 2016-01-18 16:32       ` Dario Faggioli
  2016-01-18 16:41         ` Meng Xu
  2016-01-19 22:59       ` Tianyang Chen
  1 sibling, 1 reply; 11+ messages in thread
From: Dario Faggioli @ 2016-01-18 16:32 UTC (permalink / raw)
  To: Meng Xu; +Cc: George Dunlap, Tianyang Chen, xen-devel@lists.xen.org


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

On Mon, 2016-01-18 at 11:07 -0500, Meng Xu wrote:
> On Mon, Jan 18, 2016 at 6:00 AM, Dario Faggioli
> <dario.faggioli@citrix.com> wrote:
> > 
> > RTDS behavior is broken in many respect, including this,
> > 
> > and in fact,
> > Meng and Tianyang are sending patches already to fix it (I'll let
> > you
> > guys have my comments shortly :-P).
> 
> 
> Right. Tianyang and I are working on changing it from quantum driven
> model to event-driven (or called timer-driven) model. Tianyang sent
> out the first-version patch, but that version has some problems. He
> is
> working on the second version now.
> 
> Hi Dario,
> Tianyang is working on the second version right now.
> If you could have a quick look at our discussion in that thread and
> points out the "serious" issues in the decision, that will be great!
>
Ok, that's useful to know... I'll do that way.

> We won't repeat the error again and again in the following versions.
> As to the minor issues, we could refine it in the second version.
> (I'm just thinking about how to save your time to have this done. For
> the obvious things that I can handle, I will do it and avoid
> "wasting"
> you time. For the design choices that we are unclear, we definitely
> need your insights/commands. ;-) )
> 
Thanks for all this. :-)

> Hi George,
> Yes, you are right. The current RTDS should not return 0 when the
> idle
> VCPU is picked. I think it should do as what the credit does, i.e.,
> returning a negative value to avoid arming the timer.
> Right now, we are working on changing RTDS to event-driven model. We
> will fix this in theNah, the rework you're doing is big enough, that this change can very well find its place in there.
 next version of the patch.
> 
> If needed, we can send out a separate patch to fix this specific
> issue
> (i.e. it should return negative value when idle vcpu is picked.) I'm
> ok with either way. Which way do you guys prefer?
> 
Nah, the rework you're doing is big enough, that this change can very
well find its place in there.

Doing it right now will need efforts on both our sides, only for the
sake of putting something from "broken" to "just a little bit less
broken" state, and I don't think that's worth. :-P

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Questions about the use of idle_vcpu[]
  2016-01-18 16:32       ` Dario Faggioli
@ 2016-01-18 16:41         ` Meng Xu
  0 siblings, 0 replies; 11+ messages in thread
From: Meng Xu @ 2016-01-18 16:41 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: George Dunlap, Tianyang Chen, xen-devel@lists.xen.org

Hi Dario,

On Mon, Jan 18, 2016 at 11:32 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Mon, 2016-01-18 at 11:07 -0500, Meng Xu wrote:
>> On Mon, Jan 18, 2016 at 6:00 AM, Dario Faggioli
>> <dario.faggioli@citrix.com> wrote:
>> >
>> > RTDS behavior is broken in many respect, including this,
>> >
>> > and in fact,
>> > Meng and Tianyang are sending patches already to fix it (I'll let
>> > you
>> > guys have my comments shortly :-P).
>>
>>
>> Right. Tianyang and I are working on changing it from quantum driven
>> model to event-driven (or called timer-driven) model. Tianyang sent
>> out the first-version patch, but that version has some problems. He
>> is
>> working on the second version now.
>>
>> Hi Dario,
>> Tianyang is working on the second version right now.
>> If you could have a quick look at our discussion in that thread and
>> points out the "serious" issues in the decision, that will be great!
>>
> Ok, that's useful to know... I'll do that way.
>
>> We won't repeat the error again and again in the following versions.
>> As to the minor issues, we could refine it in the second version.
>> (I'm just thinking about how to save your time to have this done. For
>> the obvious things that I can handle, I will do it and avoid
>> "wasting"
>> you time. For the design choices that we are unclear, we definitely
>> need your insights/commands. ;-) )
>>
> Thanks for all this. :-)
>
>> Hi George,
>> Yes, you are right. The current RTDS should not return 0 when the
>> idle
>> VCPU is picked. I think it should do as what the credit does, i.e.,
>> returning a negative value to avoid arming the timer.
>> Right now, we are working on changing RTDS to event-driven model. We
>> will fix this in theNah, the rework you're doing is big enough, that this change can very well find its place in there.
>  next version of the patch.
>>
>> If needed, we can send out a separate patch to fix this specific
>> issue
>> (i.e. it should return negative value when idle vcpu is picked.) I'm
>> ok with either way. Which way do you guys prefer?
>>
> Nah, the rework you're doing is big enough, that this change can very
> well find its place in there.
>
> Doing it right now will need efforts on both our sides, only for the
> sake of putting something from "broken" to "just a little bit less
> broken" state, and I don't think that's worth. :-P
>

I see and got it.

BTW,
please let me know if I should "ack" the decision. If not, I will not
send this kind of email in the future to avoid spamming your email
folders. ;-)

Best,

Meng


-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Load calculation refresh in credit2 (was in Re: Questions about the use of idle_vcpu[])
  2016-01-18 14:12         ` Load calculation refresh in credit2 (was in Re: Questions about the use of idle_vcpu[]) George Dunlap
@ 2016-01-19  9:23           ` Dario Faggioli
  0 siblings, 0 replies; 11+ messages in thread
From: Dario Faggioli @ 2016-01-19  9:23 UTC (permalink / raw)
  To: George Dunlap, George Dunlap
  Cc: Tianyang Chen, Meng Xu, xen-devel@lists.xen.org


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

On Mon, 2016-01-18 at 14:12 +0000, George Dunlap wrote:
> [Changing the title to align with the current topic]
> 
> Load balancing only happens on a reset event; and the frequency of
> reset
> events will be CREDIT_INIT / (% utilization); so for a system at 1%
> utilization that would be once every second.  Is that the kind of
> number
> you were seeing?  Or were you actually seeing idle runqueues not
> having
> anything pushed to them *during* a balance for some reason?
> 
As I said, I need to recheck... but yes, this could be the cause of my
"issue".

In fact, since I was reading the load from the toolstack (that was one
of the purposes of the whole thing), it's quite likely that I was
seeing non-updated values because the load balance hadn't run since a
while.

I'll keep this in mind when revisiting that work, and try to cook a
patch that avoid setting the timer when the idle vcpu is returned, and
see how it goes.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Questions about the use of idle_vcpu[]
  2016-01-18 16:07     ` Questions about the use of idle_vcpu[] Meng Xu
  2016-01-18 16:32       ` Dario Faggioli
@ 2016-01-19 22:59       ` Tianyang Chen
  1 sibling, 0 replies; 11+ messages in thread
From: Tianyang Chen @ 2016-01-19 22:59 UTC (permalink / raw)
  To: Dario Faggioli, George Dunlap; +Cc: Meng Xu, xen-devel@lists.xen.org



On 1/18/2016 11:07 AM, Meng Xu wrote:
> On Mon, Jan 18, 2016 at 6:00 AM, Dario Faggioli
> <dario.faggioli@citrix.com> wrote:
>>
>> On Mon, 2016-01-18 at 10:47 +0000, George Dunlap wrote:
>>> On Fri, Jan 15, 2016 at 1:04 AM, Tianyang Chen <tiche@seas.upenn.edu>
>>>
>>>> If an idle vcpu is picked, the ret.time is set accordingly in both
>>>> credit
>>>> and credit2 by checking whether snext is idle. if so, credit
>>>> returns -1 and
>>>> credit2 returns 2ms. However, there is no corresponding code in the
>>>> RTDS
>>>> scheduler to handle this. When an idle_vcpu is picked, the value of
>>>> ret.time
>>>> would be 0 and the scheduler would be invoked again. What is the
>>>> logic
>>>> behind this?
>>>
>>> No real logic, as far as I can tell. :-)  The ret.time return value
>>> tells the generic scheduling code when to set the next scheduler
>>> timer.  According to the comment in xen/common/schedule.c:schedule(),
>>> returning a negative value means "don't bother setting a timer"
>>> (e.g.,
>>> no time limit).  So credit1 does the right thing.
>>>
>> It does.
>
>
> Then the RTDS is doing *incorrectly* right now. :-(
>

George: Thanks. After looking at idle_loop() it makes sense now. Even 
though an idle vcpu won't tell scheduler timer when to fire next time, 
do_tasklet() checks if all tasklets on the list are finished and then 
raise SCHEDULE_SOFTIRQ.

>>
>>
>>> It looks like credit2's behavior will probably prevent the processor
>>> from going into deeper power-saving states, and rtds' behavior might
>>> cause it to essentially busy-wait.
>>>
>> RTDS behavior is broken in many respect, including this,
>>
>> and in fact,
>> Meng and Tianyang are sending patches already to fix it (I'll let you
>> guys have my comments shortly :-P).
>
>
> Right. Tianyang and I are working on changing it from quantum driven
> model to event-driven (or called timer-driven) model. Tianyang sent
> out the first-version patch, but that version has some problems. He is
> working on the second version now.
>
> Hi Dario,
> Tianyang is working on the second version right now.
> If you could have a quick look at our discussion in that thread and
> points out the "serious" issues in the decision, that will be great!
> We won't repeat the error again and again in the following versions.
> As to the minor issues, we could refine it in the second version.
> (I'm just thinking about how to save your time to have this done. For
> the obvious things that I can handle, I will do it and avoid "wasting"
> you time. For the design choices that we are unclear, we definitely
> need your insights/commands. ;-) )
>
Dario: I had some discussion with Meng recently and the second version 
will soon come out. You can directly comment on it if that saves you 
some time.

Thanks,
Tianyang

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-01-19 22:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-15  1:04 Questions about the use of idle_vcpu[] Tianyang Chen
2016-01-18 10:47 ` George Dunlap
2016-01-18 11:00   ` Dario Faggioli
2016-01-18 12:37     ` George Dunlap
2016-01-18 12:47       ` Dario Faggioli
2016-01-18 14:12         ` Load calculation refresh in credit2 (was in Re: Questions about the use of idle_vcpu[]) George Dunlap
2016-01-19  9:23           ` Dario Faggioli
2016-01-18 16:07     ` Questions about the use of idle_vcpu[] Meng Xu
2016-01-18 16:32       ` Dario Faggioli
2016-01-18 16:41         ` Meng Xu
2016-01-19 22:59       ` Tianyang Chen

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).