On 05/04/16 00:14, Andrew Cooper wrote:
> On 04/04/2016 23:45, Chong Li wrote:
>> From: Chong-Li <lichong...@gmail.com>
>>
>> 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...@wustl.edu>
>> Signed-off-by: Meng Xu <men...@cis.upenn.edu>
>> Signed-off-by: Sisu Xi <xis...@gmail.com>
>
> This patch is not SoB by anyone other than you.
>
>>
>> Acked-by: Dario Faggioli <dario.faggi...@citrix.com>
>>
>> ---
>> CC: <dario.faggi...@citrix.com>
>> CC: <george.dun...@eu.citrix.com>
>> CC: <dgol...@seas.upenn.edu>
>> CC: <men...@cis.upenn.edu>
>> CC: <jbeul...@suse.com>
>> CC: <lichong...@gmail.com>
>> ---
>> 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@lists.xen.org
http://lists.xen.org/xen-devel
