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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.09.14 at 10:42, <dario.faggioli@xxxxxxxxxx> wrote:
> On Sun, 2014-09-14 at 17:37 -0400, Meng Xu wrote:
>> --- /dev/null
>> +++ b/xen/common/sched_rt.c
> 
>> +/*
>> + * Init/Free related code
>> + */
>> +static int
>> +rt_init(struct scheduler *ops)
>> +{
>> +    struct rt_private *prv = xzalloc(struct rt_private);
>> +
>> +    printk("Initializing RTDS scheduler\n" \
>> +           "WARNING: This is experimental software in development.\n" \
>> +           "Use at your own risk.\n");
>>
> I don't think you need the '\' in this case...

Definitely not. Please drop.

>> +    list_for_each_safe(iter, tmp, runq)
>> +    {
>> +        svc = __q_elem(iter);
>> +
>> +        if ( now >= svc->cur_deadline )
>> +        {
>> +            rt_update_deadline(now, svc);
>> +            /* reinsert the vcpu if its deadline is updated */
>> +            __q_remove(svc);
>> +            __runq_insert(ops, svc);
>> +        }
>> +        else
>> +            break;
>>
> Just from an aesthetic perspective, I think I'd have inverted the
> condition and, hence, the two brances (i.e., "if ( < ) break; else {}").

In which case the "else" and with it one level of indentation
should go away.

>> +/*
>> + * Pick a cpu where to run a vcpu, possibly kicking out the vcpu running 
>> there
>> + * Called by wake() and context_saved()
>> + * We have a running candidate here, the kick logic is:
>> + * Among all the cpus that are within the cpu affinity
>> + * 1) if the new->cpu is idle, kick it. This could benefit cache hit
>> + * 2) if there are any idle vcpu, kick it.                                  
>> + * 3) now all pcpus are busy, among all the running vcpus, pick lowest 
>> priority one
>>
> Long line.

This ...

>> +    /* 3) candicate has higher priority, kick out the lowest priority vcpu 
>> */
>> +    if ( latest_deadline_vcpu != NULL && new->cur_deadline < 
>> latest_deadline_vcpu->cur_deadline )
>>
> Long line again.

... and even more so this warrant re-sending, but perhaps only
after George also had a chance to give review feedback.

Jan


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