From: "Emilio G. Cota" <c...@braap.org> The few direct users of &cpu->lock will be converted soon.
The per-thread bitmap introduced here might seem unnecessary, since a bool could just do. However, once we complete the conversion to per-vCPU locks, we will need to cover the use case where all vCPUs are locked by the same thread, which explains why the bitmap is introduced here. Reviewed-by: Richard Henderson <richard.hender...@linaro.org> Signed-off-by: Emilio G. Cota <c...@braap.org> [RF: Add assert in smp_parse. Make cpu_mutex_lock stub empty] Signed-off-by: Robert Foley <robert.fo...@linaro.org> --- cpus.c | 45 +++++++++++++++++++++++++++++++++++++++++-- hw/core/machine.c | 1 + include/hw/core/cpu.h | 36 ++++++++++++++++++++++++++++++++++ stubs/Makefile.objs | 1 + stubs/cpu-lock.c | 20 +++++++++++++++++++ 5 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 stubs/cpu-lock.c diff --git a/cpus.c b/cpus.c index 3c011ec452..ca271411a4 100644 --- a/cpus.c +++ b/cpus.c @@ -92,6 +92,44 @@ static unsigned int throttle_percentage; #define CPU_THROTTLE_PCT_MAX 99 #define CPU_THROTTLE_TIMESLICE_NS 10000000 +/* + * Note: we index the bitmap with cpu->cpu_index + 1 so that the logic + * also works during early CPU initialization, when cpu->cpu_index is set to + * UNASSIGNED_CPU_INDEX == -1. + */ +static __thread DECLARE_BITMAP(cpu_lock_bitmap, CPU_LOCK_BITMAP_SIZE); + +bool no_cpu_mutex_locked(void) +{ + return bitmap_empty(cpu_lock_bitmap, CPU_LOCK_BITMAP_SIZE); +} + +void cpu_mutex_lock_impl(CPUState *cpu, const char *file, int line) +{ +/* coverity gets confused by the indirect function call */ +#ifdef __COVERITY__ + qemu_mutex_lock_impl(&cpu->lock, file, line); +#else + QemuMutexLockFunc f = atomic_read(&qemu_mutex_lock_func); + + g_assert(!cpu_mutex_locked(cpu)); + set_bit(cpu->cpu_index + 1, cpu_lock_bitmap); + f(&cpu->lock, file, line); +#endif +} + +void cpu_mutex_unlock_impl(CPUState *cpu, const char *file, int line) +{ + g_assert(cpu_mutex_locked(cpu)); + qemu_mutex_unlock_impl(&cpu->lock, file, line); + clear_bit(cpu->cpu_index + 1, cpu_lock_bitmap); +} + +bool cpu_mutex_locked(const CPUState *cpu) +{ + return test_bit(cpu->cpu_index + 1, cpu_lock_bitmap); +} + bool cpu_is_stopped(CPUState *cpu) { return cpu->stopped || !runstate_is_running(); @@ -101,9 +139,9 @@ static inline bool cpu_work_list_empty(CPUState *cpu) { bool ret; - qemu_mutex_lock(&cpu->lock); + cpu_mutex_lock(cpu); ret = QSIMPLEQ_EMPTY(&cpu->work_list); - qemu_mutex_unlock(&cpu->lock); + cpu_mutex_unlock(cpu); return ret; } @@ -1865,6 +1903,9 @@ void qemu_mutex_lock_iothread_impl(const char *file, int line) { QemuMutexLockFunc bql_lock = atomic_read(&qemu_bql_mutex_lock_func); + /* enforce locking order */ + g_assert(no_cpu_mutex_locked()); + g_assert(!qemu_mutex_iothread_locked()); bql_lock(&qemu_global_mutex, file, line); iothread_locked = true; diff --git a/hw/core/machine.c b/hw/core/machine.c index 1d80ab0e1d..74edd3c86d 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -769,6 +769,7 @@ static void smp_parse(MachineState *ms, QemuOpts *opts) ms->smp.threads = threads; ms->smp.sockets = sockets; } + assert(ms->smp.max_cpus <= CPU_LOCK_BITMAP_SIZE); if (ms->smp.cpus > 1) { Error *blocker = NULL; diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index afc8081ca8..4d4006f1fb 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -457,6 +457,39 @@ extern CPUTailQ cpus; extern __thread CPUState *current_cpu; +/** + * cpu_mutex_lock - lock a CPU's mutex + * @cpu: the CPU whose mutex is to be locked + * + * To avoid deadlock, a CPU's mutex must be acquired after the BQL. + */ +#define cpu_mutex_lock(cpu) \ + cpu_mutex_lock_impl(cpu, __FILE__, __LINE__) +void cpu_mutex_lock_impl(CPUState *cpu, const char *file, int line); + +/** + * cpu_mutex_unlock - unlock a CPU's mutex + * @cpu: the CPU whose mutex is to be unlocked + */ +#define cpu_mutex_unlock(cpu) \ + cpu_mutex_unlock_impl(cpu, __FILE__, __LINE__) +void cpu_mutex_unlock_impl(CPUState *cpu, const char *file, int line); + +/** + * cpu_mutex_locked - check whether a CPU's mutex is locked + * @cpu: the CPU of interest + * + * Returns true if the calling thread is currently holding the CPU's mutex. + */ +bool cpu_mutex_locked(const CPUState *cpu); + +/** + * no_cpu_mutex_locked - check whether any CPU mutex is held + * + * Returns true if the calling thread is not holding any CPU mutex. + */ +bool no_cpu_mutex_locked(void); + static inline void cpu_tb_jmp_cache_clear(CPUState *cpu) { unsigned int i; @@ -1184,4 +1217,7 @@ extern const VMStateDescription vmstate_cpu_common; #define UNASSIGNED_CPU_INDEX -1 #define UNASSIGNED_CLUSTER_INDEX -1 +/* XXX: is this really the max number of CPUs? */ +#define CPU_LOCK_BITMAP_SIZE 2048 + #endif diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 28e48171d1..becf8f6986 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -1,6 +1,7 @@ stub-obj-y += blk-commit-all.o stub-obj-y += cpu-get-clock.o stub-obj-y += cpu-get-icount.o +stub-obj-y += cpu-lock.o stub-obj-y += dump.o stub-obj-y += error-printf.o stub-obj-y += fdset.o diff --git a/stubs/cpu-lock.c b/stubs/cpu-lock.c new file mode 100644 index 0000000000..1981a00fb3 --- /dev/null +++ b/stubs/cpu-lock.c @@ -0,0 +1,20 @@ +#include "qemu/osdep.h" +#include "hw/core/cpu.h" + +void cpu_mutex_lock_impl(CPUState *cpu, const char *file, int line) +{ +} + +void cpu_mutex_unlock_impl(CPUState *cpu, const char *file, int line) +{ +} + +bool cpu_mutex_locked(const CPUState *cpu) +{ + return true; +} + +bool no_cpu_mutex_locked(void) +{ + return true; +} -- 2.17.1