Re: [Xen-devel] [PATCH v7 for Xen 4.7 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler

[Chong Li <lichong659@xxxxxxxxx>]

On Thu, Mar 17, 2016 at 5:03 AM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> wrote:
> On Wed, 2016-03-16 at 11:47 -0500, Chong Li wrote:

>
>> --- a/xen/common/sched_rt.c
>> +++ b/xen/common/sched_rt.c
>> @@ -1129,24 +1145,22 @@ rt_dom_cntl(
>>      struct vcpu *v;
>>      unsigned long flags;
>>      int rc = 0;
>> +    xen_domctl_schedparam_vcpu_t local_sched;
>> +    s_time_t period, budget;
>> +    uint32_t index = 0;
>>
>>      switch ( op->cmd )
>>      {
>>      case XEN_DOMCTL_SCHEDOP_getinfo:
>> -        if ( d->max_vcpus > 0 )
>> -        {
>> -            spin_lock_irqsave(&prv->lock, flags);
>> -            svc = rt_vcpu(d->vcpu[0]);
>> -            op->u.rtds.period = svc->period / MICROSECS(1);
>> -            op->u.rtds.budget = svc->budget / MICROSECS(1);
>> -            spin_unlock_irqrestore(&prv->lock, flags);
>> -        }
>> -        else
>> -        {
>> -            /* If we don't have vcpus yet, let's just return the
>> defaults. */
>> -            op->u.rtds.period = RTDS_DEFAULT_PERIOD;
>> -            op->u.rtds.budget = RTDS_DEFAULT_BUDGET;
>> -        }
>> +        /* Return the default parameters.
>> +         * A previous bug fixed here:
>> +         * The default PERIOD or BUDGET should be divided by
>> MICROSECS(1),
>> +         * before returned to upper caller.
>> +         */
> Comment style (missing opening '/*').
>
> Also, putting this kind of things in comments is not at all ideal. If
> doing this in this patch, you should mention it in the changelog. Or
> you do it in a separate patch (that you put before this one in the
> series).
>
> I'd say that, in this specific case, is not a big deal which one of the
> two approaches you take (mentioning or separate patch), but the having
> a separate one is indeed almost always preferable (e.g., the fix can be
> backported).

If I choose mentioning, do I move the comment to the changelog? Or do I keep
it here and say it again in the changelog?

Chong



-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis

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