* [PATCH kvm-unit-tests 1/2] arm/arm64: smp: cpu0 is special
2017-06-02 15:12 [PATCH kvm-unit-tests 0/2] arm/arm64: smp: on_cpu improvements Andrew Jones
@ 2017-06-02 15:12 ` Andrew Jones
2017-06-02 15:12 ` [PATCH kvm-unit-tests 2/2] arm/arm64: smp: add deadlock detection Andrew Jones
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Jones @ 2017-06-02 15:12 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar
Unless cpu0 code explicitly calls do_idle(), then it never will,
and thus any on_cpu(0, ...) calls will hang. Let's assume that
if cpu0 hasn't already called do_idle() that the on_cpu(0, ...)
call from a secondary CPU was a programming error and assert.
However, it's possible the developer intends cpu0 to call do_idle
later, so give the developer a chance to disable the assert too.
Also, while cpu0 is special, it's not that special. It shouldn't
be the only CPU that can call on_cpus().
Finally, let's not mess with cpu0's idle state in on_cpus(), as
this could potentially confuse another CPU that's checking it.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
lib/arm/asm/smp.h | 2 ++
lib/arm/smp.c | 16 +++++++++++-----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/lib/arm/asm/smp.h b/lib/arm/asm/smp.h
index 4272e0fd9253..62d14b07bc51 100644
--- a/lib/arm/asm/smp.h
+++ b/lib/arm/asm/smp.h
@@ -10,6 +10,8 @@
#define smp_processor_id() (current_thread_info()->cpu)
+extern bool cpu0_calls_idle;
+
extern void halt(void);
extern void do_idle(void);
diff --git a/lib/arm/smp.c b/lib/arm/smp.c
index 7e3c91ce3b48..b4b43237e32e 100644
--- a/lib/arm/smp.c
+++ b/lib/arm/smp.c
@@ -14,6 +14,8 @@
#include <asm/psci.h>
#include <asm/smp.h>
+bool cpu0_calls_idle;
+
cpumask_t cpu_present_mask;
cpumask_t cpu_online_mask;
cpumask_t cpu_idle_mask;
@@ -81,6 +83,9 @@ void do_idle(void)
{
int cpu = smp_processor_id();
+ if (cpu == 0)
+ cpu0_calls_idle = true;
+
set_cpu_idle(cpu, true);
sev();
@@ -103,6 +108,9 @@ void on_cpu_async(int cpu, void (*func)(void *data), void *data)
return;
}
+ assert_msg(cpu != 0 || cpu0_calls_idle, "Waiting on CPU0, which is unlikely to idle. "
+ "If this is intended set cpu0_calls_idle=1");
+
spin_lock(&lock);
if (!cpu_online(cpu))
__smp_boot_secondary(cpu, do_idle);
@@ -133,17 +141,15 @@ void on_cpu(int cpu, void (*func)(void *data), void *data)
void on_cpus(void (*func)(void))
{
- int cpu;
+ int cpu, me = smp_processor_id();
for_each_present_cpu(cpu) {
- if (cpu == 0)
+ if (cpu == me)
continue;
on_cpu_async(cpu, (on_cpu_func)func, NULL);
}
func();
- set_cpu_idle(0, true);
- while (!cpumask_full(&cpu_idle_mask))
+ while (cpumask_weight(&cpu_idle_mask) < nr_cpus - 1)
wfe();
- set_cpu_idle(0, false);
}
--
2.9.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH kvm-unit-tests 2/2] arm/arm64: smp: add deadlock detection
2017-06-02 15:12 [PATCH kvm-unit-tests 0/2] arm/arm64: smp: on_cpu improvements Andrew Jones
2017-06-02 15:12 ` [PATCH kvm-unit-tests 1/2] arm/arm64: smp: cpu0 is special Andrew Jones
@ 2017-06-02 15:12 ` Andrew Jones
2017-06-02 15:26 ` Andrew Jones
1 sibling, 1 reply; 4+ messages in thread
From: Andrew Jones @ 2017-06-02 15:12 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar
on_cpu() and friends are a bit risky when implemented without IPIs
(no preemption), because we can easily get deadlocks. Luckily, it's
also easy to detect those deadlocks, and assert, to at least make
them easier to debug.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
lib/arm/smp.c | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/lib/arm/smp.c b/lib/arm/smp.c
index b4b43237e32e..34204955a549 100644
--- a/lib/arm/smp.c
+++ b/lib/arm/smp.c
@@ -76,9 +76,25 @@ typedef void (*on_cpu_func)(void *);
struct on_cpu_info {
on_cpu_func func;
void *data;
+ cpumask_t waiters;
};
static struct on_cpu_info on_cpu_info[NR_CPUS];
+static void cpu_wait(int cpu)
+{
+ int me = smp_processor_id();
+
+ if (cpu == me)
+ return;
+
+ assert_msg(!cpumask_test_cpu(cpu, &on_cpu_info[me].waiters), "CPU%d <=> CPU%d deadlock detected", me, cpu);
+
+ cpumask_set_cpu(me, &on_cpu_info[cpu].waiters);
+ while (!cpu_idle(cpu))
+ wfe();
+ cpumask_clear_cpu(me, &on_cpu_info[cpu].waiters);
+}
+
void do_idle(void)
{
int cpu = smp_processor_id();
@@ -117,8 +133,7 @@ void on_cpu_async(int cpu, void (*func)(void *data), void *data)
spin_unlock(&lock);
for (;;) {
- while (!cpu_idle(cpu))
- wfe();
+ cpu_wait(cpu);
spin_lock(&lock);
if ((volatile void *)on_cpu_info[cpu].func == NULL)
break;
@@ -134,9 +149,7 @@ void on_cpu_async(int cpu, void (*func)(void *data), void *data)
void on_cpu(int cpu, void (*func)(void *data), void *data)
{
on_cpu_async(cpu, func, data);
-
- while (!cpu_idle(cpu))
- wfe();
+ cpu_wait(cpu);
}
void on_cpus(void (*func)(void))
@@ -150,6 +163,14 @@ void on_cpus(void (*func)(void))
}
func();
+ for_each_present_cpu(cpu) {
+ if (cpu == me)
+ continue;
+ cpumask_set_cpu(me, &on_cpu_info[cpu].waiters);
+ assert_msg(!cpumask_test_cpu(cpu, &on_cpu_info[me].waiters), "CPU%d <=> CPU%d deadlock detected", me, cpu);
+ }
while (cpumask_weight(&cpu_idle_mask) < nr_cpus - 1)
wfe();
+ for_each_present_cpu(cpu)
+ cpumask_clear_cpu(me, &on_cpu_info[cpu].waiters);
}
--
2.9.4
^ permalink raw reply related [flat|nested] 4+ messages in thread