Re: [Xen-devel] [PATCH v2 1/4] xen: add real time scheduler rt

[Meng Xu <xumengpanda@xxxxxxxxx>]

2014-09-09 7:31 GMT-04:00 George Dunlap <George.Dunlap@xxxxxxxxxxxxx>:
> On Tue, Sep 9, 2014 at 10:42 AM, Dario Faggioli
> <dario.faggioli@xxxxxxxxxx> wrote:
>> On Mon, 2014-09-08 at 19:44 +0100, George Dunlap wrote:
>>> On 09/07/2014 08:40 PM, Meng Xu wrote:
>>
>>> > +/*
>>> > + * update deadline and budget when deadline is in the past,
>>> > + * it need to be updated to the deadline of the current period
>>> > + */
>>> > +static void
>>> > +rt_update_helper(s_time_t now, struct rt_vcpu *svc)
>>> > +{
>>> > +    s_time_t diff = now - svc->cur_deadline;
>>> > +
>>> > +    if ( diff >= 0 )
>>> > +    {
>>> > +        /* now can be later for several periods */
>>> > +        long count = ( diff/svc->period ) + 1;
>>> > +        svc->cur_deadline += count * svc->period;
>>> > +        svc->cur_budget = svc->budget;
>>>
>>> In the common case, don't we expect diff/svc->period to be a small
>>> number, like 0 or 1?
>>>
>> In general, yes. The only exception is when cur_deadline is set for the
>> first time. In that case, now can be arbitrary large and cur_deadline
>> will always be 0, so quite a few iterations may be required, possibly
>> taking longer than the div and the mult.
>
> Right, well we should be able to special-case zero.  Is there any
> reason, if cur_deadline == 0, not to just set cur_deadline=now +
> svc->period?  I can see a reason why after skipping several periods
> you'd want the future periods "lined up with" previous periods.  But
> is there a need to have all the periods lined up from the beginning of
> time?
>

Actually, no need to line up all vcpus from the beginning of time.
This just makes the scheduler more deterministic since everytime when
we boot the system, we know all vcpus are lined up to the beginning of
time. When a vcpu is created, its cur_deadline can be now +
svc->period.

I'm personally fine with either way. (very slightly prefer the line up
way because it is more deterministic, but not so prefer. :-))

>>> And similarly for the other 64-bit division Dario was asking about below?
>>>
>> Hehe, this is, I think, the third or fourth time I say I'd like this to
>> be turned into a while! :-D
>
> Well, if you've asked for it several times, we should probably make it
> a precondition of going in then.

I will modify this for sure in the next version. Didn't realize this
was stressed for many times. Sorry Dario for bothering you. :-(

>
>> If it were me doing this, I'd go for something like this:
>>
>>   static void
>>   rt_update_helper(s_time_t now, struct rt_vcpu *svc)
>>   {
>>       if ( svc->cur_deadline > now )
>>           return;
>>
>>       do
>>           svc->cur_deadline += svc->period;
>>       while ( svc->cur_deadline <= now );
>>       svc->cur_budget = svc->budget;
>>
>>       [tracing]
>>   }
>
> Yes, that looks even cleaner. :-)
>
>>> > +{
>>> > +    struct rt_private *prv = RT_PRIV(ops);
>>> > +    struct list_head *runq = RUNQ(ops);
>>>
>> Oh, BTW, George, what do you think about these? The case, I mean. Since
>> now they're  static inlines, I've been telling Meng to turn the function
>> names lower case.
>>
>> This is, of course, a minor thing, but since we're saying the are not
>> major issues... :-)
>
> Yes, static inlines need to be lower case.

Roger and will change to lower case then.

Thanks,

Meng


-- 


-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania

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