[Xen-devel] [PATCH v4 1/6] xen: sched: fix locking of remove_vcpu() in credit1

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

In fact, csched_vcpu_remove() (i.e., the credit1
implementation of remove_vcpu()) manipulates runqueues,
so holding the runqueue lock is necessary.

However, the vCPU just can't be on the runqueue, when
the function is called. We can therefore ASSERT() that,
and avoid doing any runqueue manipulations (rather than
adding the runqueue locking around it).

Also, while there, *_lock_irq() (for the private lock) is
enough, there is no need to *_lock_irqsave().

Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
---
Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Jan Beulich <JBeulich@xxxxxxxx>
---
Changes from v3:
 * instead of adding locking, get rid of __runq_remove(),
   and add an ASSERT() about vCPU not being in runq already,
   as suggested during review.

Changes from the other series:
 * split the patch (wrt the original patch, in the original
   series), and take care, in this one, only of remove_vcpu();
 * removed pointless parentheses.
---
The fact that vCPU can't be in runqueue when calling remove_vcpu() is true for
other schedulers as well. In them, though, there isn't any race condition to
fix. Therefore, taking care of the other schedulers will happen in a followup
series.
---
 xen/common/sched_credit.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 1b30e67..6dfcff6 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -934,28 +934,25 @@ csched_vcpu_remove(const struct scheduler *ops, struct 
vcpu *vc)
     struct csched_private *prv = CSCHED_PRIV(ops);
     struct csched_vcpu * const svc = CSCHED_VCPU(vc);
     struct csched_dom * const sdom = svc->sdom;
-    unsigned long flags;
 
     SCHED_STAT_CRANK(vcpu_remove);
 
+    ASSERT(!__vcpu_on_runq(svc));
+
     if ( test_and_clear_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
     {
         SCHED_STAT_CRANK(vcpu_unpark);
         vcpu_unpause(svc->vcpu);
     }
 
-    if ( __vcpu_on_runq(svc) )
-        __runq_remove(svc);
-
-    spin_lock_irqsave(&(prv->lock), flags);
+    spin_lock_irq(&prv->lock);
 
     if ( !list_empty(&svc->active_vcpu_elem) )
         __csched_vcpu_acct_stop_locked(prv, svc);
 
-    spin_unlock_irqrestore(&(prv->lock), flags);
+    spin_unlock_irq(&prv->lock);
 
     BUG_ON( sdom == NULL );
-    BUG_ON( !list_empty(&svc->runq_elem) );
 }
 
 static void


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