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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

~Andrew

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