Re: [Xen-devel] [PATCH] xen: enable per-VCPU parameter for RTDS

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 05/04/16 00:14, Andrew Cooper wrote:
> On 04/04/2016 23:45, Chong Li wrote:
>> From: Chong-Li <lichong659@xxxxxxxxx>
>>
>> Commit f7b87b0745b4 ("enable per-VCPU parameter for RTDS") introduced
>> a bug: it made it possible, in Credit and Credit2, when doing domain 
>> or vcpu parameters' manipulation, to leave the hypervisor with a 
>> spinlock held.
> 
> And interrupts disabled (which is far more of a problem than just the
> spinlock).
> 
>>
>> Fix it.
>>
>> Signed-off-by: Chong Li <chong.li@xxxxxxxxx>
>> Signed-off-by: Meng Xu <mengxu@xxxxxxxxxxxxx>
>> Signed-off-by: Sisu Xi <xisisu@xxxxxxxxx>
> 
> This patch is not SoB by anyone other than you.
> 
>>
>> Acked-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
>>
>> ---
>> CC: <dario.faggioli@xxxxxxxxxx>
>> CC: <george.dunlap@xxxxxxxxxxxxx>
>> CC: <dgolomb@xxxxxxxxxxxxxx>
>> CC: <mengxu@xxxxxxxxxxxxx>
>> CC: <jbeulich@xxxxxxxx>
>> CC: <lichong659@xxxxxxxxx>
>> ---
>>  xen/common/sched_credit.c  | 1 +
>>  xen/common/sched_credit2.c | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
>> index e5d15d8..fa6b7f0 100644
>> --- a/xen/common/sched_credit.c
>> +++ b/xen/common/sched_credit.c
>> @@ -1101,6 +1101,7 @@ csched_dom_cntl(
>>              sdom->cap = op->u.credit.cap;
>>          break;
>>      default:
>> +        spin_unlock_irqrestore(&prv->lock, flags);
>>          return -EINVAL;
>>      }
>>  
> 
> While Dario didn't care too much how you fixed the issue, I do.
> 
> Please use an "int rc = 0" and remove remove the return statement
> (instead, assigning rc = -EINVAL; and a break;).  It makes far more
> readable and understandable code, which is better in the long run.

Well Dario's a scheduler maintainer and you're not.

But in any case, I also prefer the rc / break (or goto out) pattern
enough to ask for a re-send.  (But I now see that a v3 has already been
sent.)

 -George

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