[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] Shutdown panic in disable_nonboot_cpus after cpupool-numa-split
On 07/07/14 12:33, Stefan Bader wrote:
> I recently noticed that I get a
panic (rebooting the system) on shutdown in some
> cases. This happened only on my AMD system and also not all
the time. Finally
> realized that it is related to the use of using
cpupool-numa-split
> (libxl with xen-4.4 maybe, but not 100% sure 4.3 as well).
>
> What happens is that on shutdown the hypervisor runs
disable_nonboot_cpus which
> call cpu_down for each online cpu. There is a BUG_ON in the
code for the case of
> cpu_down returning -EBUSY. This happens in my case as soon as
the first cpu that
> has been moved to pool-1 by cpupool-numa-split is attempted.
The error is
> returned by running the notifier_call_chain and I suspect
that ends up calling
> cpupool_cpu_remove which always returns EBUSY for cpus not in
pool0.
>
> I am not sure which end needs to be fixed but looping over
all online cpus in
> disable_nonboot_cpus sounds plausible. So maybe the check for
pool-0 in
> cpupool_cpu_remove is wrong...?
>
> -Stefan
Hmm yes - this looks completely broken.
cpupool_cpu_remove() only has a single caller which is from
cpu_down(), and will unconditionally fail for cpus outside of the
default pool.
It is not obvious at all how this is supposed to work, and the
comment beside cpupool_cpu_remove() doesn't help.
Can you try the following (only compile tested) patch, which looks
plausibly like it might DTRT. The for_each_cpupool() is a little
nastly but there appears to be no cpu_to_cpupool mapping available.
~Andrew
---8<-----
diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c
index 4a0e569..a1af1ea 100644
--- a/xen/common/cpupool.c
+++ b/xen/common/cpupool.c
@@ -472,12 +472,12 @@ static void cpupool_cpu_add(unsigned int cpu)
static int cpupool_cpu_remove(unsigned int cpu)
{
int ret = 0;
+ struct cpupool **c;
spin_lock(&cpupool_lock);
- if ( !cpumask_test_cpu(cpu, cpupool0->cpu_valid))
- ret = -EBUSY;
- else
- cpumask_set_cpu(cpu, &cpupool_locked_cpus);
+ for_each_cpupool(c)
+ cpumask_clear_cpu(cpu, (*c)->cpu_valid);
+ cpumask_set_cpu(cpu, &cpupool_locked_cpus);
spin_unlock(&cpupool_lock);
return ret;
|
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|