Add a simple trace method to output lock users and detect any sleeping during atomic context. The intention is to provide simple trace data that can be used to pinpoint any deadlocks or attempts to sleep whilst atomic.
Future: The trace output in that patch can be improved to provide easier lookup for lock users rather than line numbers.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- src/arch/xtensa/include/arch/spinlock.h | 3 ++ src/arch/xtensa/init.c | 6 ++++ src/audio/pipeline.c | 1 + src/include/reef/lock.h | 61 +++++++++++++++++++++++++++++---- src/include/reef/wait.h | 11 ++++++ 5 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/src/arch/xtensa/include/arch/spinlock.h b/src/arch/xtensa/include/arch/spinlock.h index 639d7a4..acb1b8e 100644 --- a/src/arch/xtensa/include/arch/spinlock.h +++ b/src/arch/xtensa/include/arch/spinlock.h @@ -37,6 +37,9 @@
typedef struct { volatile uint32_t lock; +#if DEBUG_LOCKS + uint32_t user; +#endif } spinlock_t;
static inline void arch_spinlock_init(spinlock_t *lock) diff --git a/src/arch/xtensa/init.c b/src/arch/xtensa/init.c index 1e52d99..44f7faf 100644 --- a/src/arch/xtensa/init.c +++ b/src/arch/xtensa/init.c @@ -37,8 +37,14 @@ #include <arch/task.h> #include <reef/debug.h> #include <reef/init.h> +#include <reef/lock.h> #include <stdint.h>
+#if DEBUG_LOCKS +uint32_t lock_dbg_atomic = 0; +uint32_t lock_dbg_user[DBG_LOCK_USERS] = {0}; +#endif + /* TODO: this should be fixed by rotating the register Window on the stack and * dumping the saved registers. * TODO: we should also have different handlers for each type where we can take diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c index 636a42b..0ef971e 100644 --- a/src/audio/pipeline.c +++ b/src/audio/pipeline.c @@ -39,6 +39,7 @@ #include <reef/alloc.h> #include <reef/debug.h> #include <reef/ipc.h> +#include <reef/lock.h> #include <platform/timer.h> #include <platform/platform.h> #include <reef/audio/component.h> diff --git a/src/include/reef/lock.h b/src/include/reef/lock.h index 234dff2..8b05ef2 100644 --- a/src/include/reef/lock.h +++ b/src/include/reef/lock.h @@ -34,24 +34,71 @@ #ifndef __INCLUDE_LOCK__ #define __INCLUDE_LOCK__
+#define DEBUG_LOCKS 0 + #include <stdint.h> #include <arch/spinlock.h> #include <reef/interrupt.h> #include <reef/trace.h>
-#define DEBUG_LOCKS 0
#if DEBUG_LOCKS
-#define trace_lock(__e) trace_event(TRACE_CLASS_LOCK, __e) -#define tracev_lock(__e) tracev_event(TRACE_CLASS_LOCK, __e) +#define DBG_LOCK_USERS 8 + +#define trace_lock(__e) trace_event_atomic(TRACE_CLASS_LOCK, __e) +#define tracev_lock(__e) tracev_event_atomic(TRACE_CLASS_LOCK, __e) +#define trace_lock_error(__e) trace_error_atomic(TRACE_CLASS_LOCK, __e) +#define trace_lock_value(__e) _trace_error_atomic(__e) + +extern uint32_t lock_dbg_atomic; +extern uint32_t lock_dbg_user[DBG_LOCK_USERS];
#define spin_lock_dbg() \ - trace_lock("LcE"); \ - trace_value(__LINE__); + trace_lock("LcE"); + #define spin_unlock_dbg() \ trace_lock("LcX");
+/* all SMP spinlocks need init, nothing todo on UP */ +#define spinlock_init(lock) \ + arch_spinlock_init(lock); \ + (lock)->user = __LINE__; + +/* does nothing on UP systems */ +#define spin_lock(lock) \ + spin_lock_dbg(); \ + if (lock_dbg_atomic) { \ + int __i = 0; \ + int __count = lock_dbg_atomic >= DBG_LOCK_USERS \ + ? DBG_LOCK_USERS : lock_dbg_atomic; \ + trace_lock_error("eal"); \ + trace_lock_value(__LINE__); \ + trace_lock_value(lock_dbg_atomic); \ + for (__i = 0; __i < __count; __i++) { \ + trace_lock_value((lock_dbg_atomic << 24) | \ + lock_dbg_user[__i]); \ + } \ + } \ + arch_spin_lock(lock); + +#define spin_unlock(lock) \ + arch_spin_unlock(lock); \ + spin_unlock_dbg(); + +/* disables all IRQ sources and takes lock - enter atomic context */ +#define spin_lock_irq(lock, flags) \ + flags = interrupt_global_disable(); \ + spin_lock(lock); \ + if (++lock_dbg_atomic < DBG_LOCK_USERS) \ + lock_dbg_user[lock_dbg_atomic - 1] = (lock)->user; + +/* re-enables current IRQ sources and releases lock - leave atomic context */ +#define spin_unlock_irq(lock, flags) \ + spin_unlock(lock); \ + lock_dbg_atomic--; \ + interrupt_global_enable(flags); + #else
#define trace_lock(__e) @@ -60,8 +107,6 @@ #define spin_lock_dbg() #define spin_unlock_dbg()
-#endif - /* all SMP spinlocks need init, nothing todo on UP */ #define spinlock_init(lock) \ arch_spinlock_init(lock) @@ -86,3 +131,5 @@ interrupt_global_enable(flags);
#endif + +#endif diff --git a/src/include/reef/wait.h b/src/include/reef/wait.h index 08fc0b0..c5b370e 100644 --- a/src/include/reef/wait.h +++ b/src/include/reef/wait.h @@ -40,8 +40,18 @@ #include <reef/timer.h> #include <reef/interrupt.h> #include <reef/trace.h> +#include <reef/lock.h> #include <platform/interrupt.h>
+#if DEBUG_LOCKS +#define wait_atomic_check \ + if (lock_dbg_atomic) { \ + trace_error_atomic(TRACE_CLASS_WAIT, "atm"); \ + } +#else +#define wait_atomic_check +#endif + typedef struct { uint32_t complete; struct work work; @@ -53,6 +63,7 @@ void arch_wait_for_interrupt(int level); static inline void wait_for_interrupt(int level) { tracev_event(TRACE_CLASS_WAIT, "WFE"); + wait_atomic_check; arch_wait_for_interrupt(level); tracev_event(TRACE_CLASS_WAIT, "WFX"); }