Re: [Xen-devel] [PATCH 03/24] xen: credit1: return the 'time remaining to the limit' as next timeslice.

[Dario Faggioli]

On Wed, 2016-09-14 at 10:34 +0100, George Dunlap wrote:
> On 12/09/16 18:00, Dario Faggioli wrote:
> > 
> > I also agree on the fact that most of the times ratelimit_us and
> > MIN_TIMER will be close enough (like in the example above) that it
> > won't probably matter much... but if someone set ratelimit_us to
> > something higher (say, 10ms --we accept values as big as the
> > timeslice,
> > which is 30ms b default) it may matter a bit.
> > 
> > What do you think?
> > 
> > If we decide not to care, and leave things as they are, I'd add a
> > comment saying that code is like that on purpose, so we won't trip
> > over
> > this again in 1 or 2 years. :-)
> Yeah, I think while we're thinking about it we might as well add in
> the
> MIN_TIMER thing you mention above (if you don't mind doing it).
> 
Sure I don't mind. I'll do it.

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 03/24] xen: credit1: return the 'time remaining to the limit' as next timeslice.

[George Dunlap]

On 12/09/16 18:00, Dario Faggioli wrote:
> On Mon, 2016-09-12 at 16:14 +0100, George Dunlap wrote:
>> On 17/08/16 18:17, Dario Faggioli wrote:
>>>
>>> If vcpu x has run for 200us, and sched_ratelimit_us is
>>> 1000us, continue running x _but_ return 1000us-200us as
>>> the next time slice. This way, next scheduling point will
>>> happen in 800us, i.e., exactly at the point when x crosses
>>> the threshold, and can be descheduled (if appropriate).
>>>
>>> Right now (without this patch), we're always returning
>>> sched_ratelimit_us (1000us, in the example above), which
>>> means we're (potentially) allowing x to run more than
>>> it should have been able to (even when considering rate
>>> limiting into account).
>> Part of the reason I went with this in the first place was because I
>> wanted to avoid really short timers.  Part of the reason for the
>> ratelimit was actually to limit the amount of time spent in the
>> scheduler.  Since we expect ratelimit to normally be pretty short,
>> waiting for the whole ratelimit time seemed like a good idea.
>>
> I see what you mean.
> 
> I personally am not a fan of ratelimit, because of how it acts behind
> the algorithm's back and mess with and partly invalidates the
> algorithms own assumptions and properties (not that these assumptions
> and properties are very clear in Credit1, even before ratelimiting,
> but, anyway :-)), and this is an example of that.
> 
> Nevertheless, I see that this patch may, up to some extent, re-
> introduce some of the "small timers" that it was ratelemiting's own
> purpose to mitigate.
> 
> So, I guess, either we make this a three way condition, introducing an
> absolute minimum runtime value, under which we don't ever want to go,
> e.g.:
> 
>  tsclice = MICROSECS(prv->runtime_us) - runtime > CSCHED_MIN_TIMER :
>MICROSECS(prv->runtime_us) - runtime : CSCHED_MIN_TIMER;
> 
> or we leave things as they are now.
> 
> The MIN_TIMER option would let at least the cases where the vcpu has
> run for a few, but not too much, time to be more precisely scheduled.
> E.g., if ratelimit_us is 1000us, MIN_TIMER is 500us, and the vcpu run
> for 400us, we let it be preempted after 600us more (i.e., 1000us in
> total == ratelimit_us), instead of after 1000us more (i.e., 1400us in
> total).
> 
> I also agree on the fact that most of the times ratelimit_us and
> MIN_TIMER will be close enough (like in the example above) that it
> won't probably matter much... but if someone set ratelimit_us to
> something higher (say, 10ms --we accept values as big as the timeslice,
> which is 30ms b default) it may matter a bit.
> 
> What do you think?
> 
> If we decide not to care, and leave things as they are, I'd add a
> comment saying that code is like that on purpose, so we won't trip over
> this again in 1 or 2 years. :-)

Yeah, I think while we're thinking about it we might as well add in the
MIN_TIMER thing you mention above (if you don't mind doing it).

Thanks,
 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 03/24] xen: credit1: return the 'time remaining to the limit' as next timeslice.

[George Dunlap]

On 17/08/16 18:17, Dario Faggioli wrote:
> If vcpu x has run for 200us, and sched_ratelimit_us is
> 1000us, continue running x _but_ return 1000us-200us as
> the next time slice. This way, next scheduling point will
> happen in 800us, i.e., exactly at the point when x crosses
> the threshold, and can be descheduled (if appropriate).
> 
> Right now (without this patch), we're always returning
> sched_ratelimit_us (1000us, in the example above), which
> means we're (potentially) allowing x to run more than
> it should have been able to (even when considering rate
> limiting into account).

Part of the reason I went with this in the first place was because I
wanted to avoid really short timers.  Part of the reason for the
ratelimit was actually to limit the amount of time spent in the
scheduler.  Since we expect ratelimit to normally be pretty short,
waiting for the whole ratelimit time seemed like a good idea.

Thoughts?

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 03/24] xen: credit1: return the 'time remaining to the limit' as next timeslice.

[Dario Faggioli]

If vcpu x has run for 200us, and sched_ratelimit_us is
1000us, continue running x _but_ return 1000us-200us as
the next time slice. This way, next scheduling point will
happen in 800us, i.e., exactly at the point when x crosses
the threshold, and can be descheduled (if appropriate).

Right now (without this patch), we're always returning
sched_ratelimit_us (1000us, in the example above), which
means we're (potentially) allowing x to run more than
it should have been able to (even when considering rate
limiting into account).

Signed-off-by: Dario Faggioli 
---
Cc: George Dunlap 
---
 xen/common/sched_credit.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 3d4f223..3f439a0 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1782,7 +1782,7 @@ csched_schedule(
 snext = scurr;
 snext->start_time += now;
 perfc_incr(delay_ms);
-tslice = MICROSECS(prv->ratelimit_us);
+tslice = MICROSECS(prv->ratelimit_us) - runtime;
 ret.migrated = 0;
 goto out;
 }


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel