|
On 28/10/13 16:05, Jan Beulich wrote:
So commit eedd6039 ("scheduler: adjust internal locking interface")
uncovered - by now using proper spin lock constructs - a bug after all:
When bringing down a CPU, cpu_disable_scheduler() gets called with
interrupts disabled, and hence the use of vcpu_schedule_lock_irq() was
never really correct (i.e. the caller ended up with interrupts enabled
despite having disabled them explicitly).
Fixing this however surfaced another problem: The call path
vcpu_migrate() -> evtchn_move_pirqs() wants to acquire the event lock,
which however is a non-IRQ-safe once, and hence check_lock() doesn't
like this lock to be acquired when interrupts are already off. As we're
in stop-machine context here, getting things wrong wrt interrupt state
management during lock acquire/release is out of question though, so
the simple solution to this appears to be to just suppress spin lock
debugging for the period of time while the stop machine callback gets
run.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
One comment however. With the unlock_irqrestore functions, having
the cpu and flags parameters reversed wrt the lock_irqsave looks
funny. As this new interface is still very new, would it be worth
moving the flags parameter to the end of unlock_irqrestore() for
consistency ?
~Andrew
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -601,7 +601,8 @@ int cpu_disable_scheduler(unsigned int c
{
for_each_vcpu ( d, v )
{
- spinlock_t *lock = vcpu_schedule_lock_irq(v);
+ unsigned long flags;
+ spinlock_t *lock = vcpu_schedule_lock_irqsave(v, &flags);
cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid);
if ( cpumask_empty(&online_affinity) &&
@@ -622,14 +623,12 @@ int cpu_disable_scheduler(unsigned int c
if ( v->processor == cpu )
{
set_bit(_VPF_migrating, &v->pause_flags);
- vcpu_schedule_unlock_irq(lock, v);
+ vcpu_schedule_unlock_irqrestore(lock, flags, v);
vcpu_sleep_nosync(v);
vcpu_migrate(v);
}
else
- {
- vcpu_schedule_unlock_irq(lock, v);
- }
+ vcpu_schedule_unlock_irqrestore(lock, flags, v);
/*
* A vcpu active in the hypervisor will not be migratable.
--- a/xen/common/stop_machine.c
+++ b/xen/common/stop_machine.c
@@ -110,6 +110,7 @@ int stop_machine_run(int (*fn)(void *),
local_irq_disable();
stopmachine_set_state(STOPMACHINE_DISABLE_IRQ);
stopmachine_wait_state();
+ spin_debug_disable();
stopmachine_set_state(STOPMACHINE_INVOKE);
if ( (cpu == smp_processor_id()) || (cpu == NR_CPUS) )
@@ -117,6 +118,7 @@ int stop_machine_run(int (*fn)(void *),
stopmachine_wait_state();
ret = stopmachine_data.fn_result;
+ spin_debug_enable();
stopmachine_set_state(STOPMACHINE_EXIT);
stopmachine_wait_state();
local_irq_enable();
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
