diff options
| author | Merlin Scholz <merlin@scholz.ruhr> | 2023-03-16 03:03:49 +0100 |
|---|---|---|
| committer | Merlin Scholz <merlin@scholz.ruhr> | 2023-03-16 03:03:49 +0100 |
| commit | bf1ce4ce265b9f38a71f0bca82d8f0e8c055233a (patch) | |
| tree | 35484e5bd2cfb7f8f1dc93d5b310e1870459fc50 | |
| parent | 1096504a73ba2e1c619ee0a8c8a5f091d17a71db (diff) | |
Implement file and line logging for mutex locks
This commit implements proper file and line logging as a proof of concept for mutex locks. The previous versions only logged the location inside the mutex_enter function, which was useless.
To get correct filenames, calls to mutex_enter (which usually go directly to ASM) are replaced through the macro defined in lockdoc.h. This logs the according information and passes on the function call to the old ASM code.
Furthermore, some other things like apm.c or patch.c had to be modified, to keep the code compilable, even with this new replacement macro.
Also, the old logging methods in lockdebug.h have been removed, since we can not get the proper file and line by going through there.
This commit only implements these changes for mutex_enter, mutex_spin_enter, mutex_exit and mutex_spin_exit, and serves as a template for changes in the other lock functions and types.
| -rw-r--r-- | .gitignore | 3 | ||||
| -rw-r--r-- | sys/arch/i386/i386/lock_stubs.S | 32 | ||||
| -rw-r--r-- | sys/arch/x86/x86/patch.c | 4 | ||||
| -rw-r--r-- | sys/dev/apm/apm.c | 11 | ||||
| -rw-r--r-- | sys/kern/kern_mutex.c | 10 | ||||
| -rw-r--r-- | sys/lockdoc/log.c | 48 | ||||
| -rw-r--r-- | sys/lockdoc/test.c | 4 | ||||
| -rw-r--r-- | sys/sys/lockdebug.h | 59 | ||||
| -rw-r--r-- | sys/sys/lockdoc.h | 43 | ||||
| -rw-r--r-- | sys/sys/mutex.h | 11 |
10 files changed, 108 insertions, 117 deletions
diff --git a/.gitignore b/.gitignore index 41d4b849aa6..f8d64d7d426 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /obj/** /.idea/** -/.vscode/**
\ No newline at end of file +/.vscode/** +/netbsd.code-workspace
\ No newline at end of file diff --git a/sys/arch/i386/i386/lock_stubs.S b/sys/arch/i386/i386/lock_stubs.S index 80fa3c87f0b..554a675e96e 100644 --- a/sys/arch/i386/i386/lock_stubs.S +++ b/sys/arch/i386/i386/lock_stubs.S @@ -65,7 +65,11 @@ __KERNEL_RCSID(0, "$NetBSD: lock_stubs.S,v 1.30 2019/02/11 14:59:32 cherry Exp $ */ ALIGN64 +#ifdef LOCKDOC +ENTRY(_mutex_enter) +#else ENTRY(mutex_enter) +#endif movl 4(%esp), %edx xorl %eax, %eax movl %fs:CPU_INFO_CURLWP(%eax), %ecx @@ -75,7 +79,11 @@ ENTRY(mutex_enter) RET(1) 1: jmp _C_LABEL(mutex_vector_enter) +#ifdef LOCKDOC +END(_mutex_enter) +#else END(mutex_enter) +#endif /* * void mutex_exit(kmutex_t *mtx); @@ -86,7 +94,11 @@ END(mutex_enter) * on multiprocessor systems, and comments in arch/x86/include/lock.h about * memory ordering on Intel x86 systems. */ +#ifdef LOCKDOC +ENTRY(_mutex_exit) +#else ENTRY(mutex_exit) +#endif movl 4(%esp), %edx xorl %ecx, %ecx movl %fs:CPU_INFO_CURLWP(%ecx), %eax @@ -95,7 +107,11 @@ ENTRY(mutex_exit) ret 1: jmp _C_LABEL(mutex_vector_exit) +#ifdef LOCKDOC +END(_mutex_exit) +#else END(mutex_exit) +#endif /* * void rw_enter(krwlock_t *rwl, krw_t op); @@ -230,7 +246,11 @@ END(rw_tryenter) * * Acquire a spin mutex and post a load fence. */ +#ifdef LOCKDOC +ENTRY(_mutex_spin_enter) +#else ENTRY(mutex_spin_enter) +#endif movl 4(%esp), %edx movb CPUVAR(ILEVEL), %cl movb MTX_IPL(%edx), %ch @@ -252,12 +272,20 @@ ENTRY(mutex_spin_enter) ALIGN64 LABEL(mutex_spin_enter_end) +#ifdef LOCKDOC +END(_mutex_spin_enter) +#else END(mutex_spin_enter) +#endif /* * Release a spin mutex and post a store fence. */ +#ifdef LOCKDOC +ENTRY(_mutex_spin_exit) +#else ENTRY(mutex_spin_exit) +#endif movl 4(%esp), %edx movl CPUVAR(MTX_OLDSPL), %ecx incl CPUVAR(MTX_COUNT) @@ -274,7 +302,11 @@ ENTRY(mutex_spin_exit) .space 32 .align 32 LABEL(mutex_spin_exit_end) +#ifdef LOCKDOC +END(_mutex_spin_exit) +#else END(mutex_spin_exit) +#endif /* * Patch for i686 CPUs where cli/sti is prohibitively expensive. diff --git a/sys/arch/x86/x86/patch.c b/sys/arch/x86/x86/patch.c index 9ddf6f13d39..3d1609c7f2e 100644 --- a/sys/arch/x86/x86/patch.c +++ b/sys/arch/x86/x86/patch.c @@ -261,13 +261,13 @@ x86_patch(bool early) spllower, spllower_end, cx8_spllower_patch ); -#if defined(i386) && !defined(LOCKDEBUG) +#if defined(i386) && !defined(LOCKDEBUG) && !defined(LOCKDOC) patchfunc( i686_mutex_spin_exit, i686_mutex_spin_exit_end, mutex_spin_exit, mutex_spin_exit_end, i686_mutex_spin_exit_patch ); -#endif /* i386 && !LOCKDEBUG */ +#endif /* i386 && !LOCKDEBUG && !LOCKDOC */ } #endif /* !SPLDEBUG */ diff --git a/sys/dev/apm/apm.c b/sys/dev/apm/apm.c index 04e5c541308..9819afff6be 100644 --- a/sys/dev/apm/apm.c +++ b/sys/dev/apm/apm.c @@ -87,11 +87,22 @@ int apmdebug = 0; * assert an exclusive lock any time thread context enters the * APM module. This is both the APM thread itself, as well as * user context. + * + * LOCKDOC Update: I don't know why this is implemented as a + * macro with (void) cast here, but it breaks when LOCKDOC replaces + * mutex_enter with a macro. So we circumvent that. */ +#ifdef LOCKDOC +#define APM_LOCK(apmsc) \ + mutex_enter(&(apmsc)->sc_lock) +#define APM_UNLOCK(apmsc) \ + mutex_exit(&(apmsc)->sc_lock) +#else #define APM_LOCK(apmsc) \ (void) mutex_enter(&(apmsc)->sc_lock) #define APM_UNLOCK(apmsc) \ (void) mutex_exit(&(apmsc)->sc_lock) +#endif static void apm_event_handle(struct apm_softc *, u_int, u_int); static void apm_periodic_check(struct apm_softc *); diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c index 7fb68cf2d8f..bf40a48246f 100644 --- a/sys/kern/kern_mutex.c +++ b/sys/kern/kern_mutex.c @@ -260,14 +260,24 @@ MUTEX_RELEASE(kmutex_t *mtx) #endif #ifndef __HAVE_MUTEX_STUBS +#ifdef LOCKDOC +__strong_alias(_mutex_enter,mutex_vector_enter); +__strong_alias(_mutex_exit,mutex_vector_exit); +#else __strong_alias(mutex_enter,mutex_vector_enter); __strong_alias(mutex_exit,mutex_vector_exit); #endif +#endif #ifndef __HAVE_SPIN_MUTEX_STUBS +#ifdef LOCKDOC +__strong_alias(_mutex_spin_enter,mutex_vector_enter); +__strong_alias(_mutex_spin_exit,mutex_vector_exit); +#else __strong_alias(mutex_spin_enter,mutex_vector_enter); __strong_alias(mutex_spin_exit,mutex_vector_exit); #endif +#endif static void mutex_abort(const char *, size_t, const kmutex_t *, const char *); diff --git a/sys/lockdoc/log.c b/sys/lockdoc/log.c index 84aba0893f5..56815b93d75 100644 --- a/sys/lockdoc/log.c +++ b/sys/lockdoc/log.c @@ -79,9 +79,9 @@ void trace_irqs_on(struct trapframe *frame) { cur_eflags = x86_read_flags(); if ((frame->tf_eflags & (1 << 9)) && !(cur_eflags & (1 << 9))) { #ifdef DEBUG_TRAPS - log_lock(V_WRITE, (struct lock_object*)PSEUDOLOCK_ADDR_HARDIRQ, __FILE__, __LINE__, frame->tf_trapno); + lockdoc_log_lock(V_WRITE, (struct lock_object*)PSEUDOLOCK_ADDR_HARDIRQ, __FILE__, __LINE__, frame->tf_trapno); #else - log_lock(V_WRITE, (struct lock_object*)PSEUDOLOCK_ADDR_HARDIRQ, __FILE__, __LINE__, LOCK_NONE); + lockdoc_log_lock(V_WRITE, (struct lock_object*)PSEUDOLOCK_ADDR_HARDIRQ, __FILE__, __LINE__, LOCK_NONE); #endif } } @@ -91,55 +91,19 @@ void trace_irqs_off(struct trapframe *frame) { cur_eflags = x86_read_flags(); if ((frame->tf_eflags & (1 << 9)) && !(cur_eflags & (1 << 9))) { #ifdef DEBUG_TRAPS - log_lock(P_WRITE, (struct lock_object*)PSEUDOLOCK_ADDR_HARDIRQ, __FILE__, __LINE__, frame->tf_trapno); + lockdoc_log_lock(P_WRITE, (struct lock_object*)PSEUDOLOCK_ADDR_HARDIRQ, __FILE__, __LINE__, frame->tf_trapno); #else - log_lock(P_WRITE, (struct lock_object*)PSEUDOLOCK_ADDR_HARDIRQ, __FILE__, __LINE__, LOCK_NONE); + lockdoc_log_lock(P_WRITE, (struct lock_object*)PSEUDOLOCK_ADDR_HARDIRQ, __FILE__, __LINE__, LOCK_NONE); #endif } } -void lockdoc_alloc(const char *func, const char *file, size_t line, volatile void *lock, lockops_t *lo, uintptr_t initaddr){ -} - -void lockdoc_free(const char *func, const char *file, size_t line, volatile void *lock){ -} - -void lockdoc_wantlock(const char *func, const char *file, size_t line, const volatile void *lock, uintptr_t where, int shared){ -} - -void lockdoc_locked(const char *func, const char *file, size_t line, volatile void *lock, void *cvlock, uintptr_t where, int shared){ - // shared == 0 means exclusive (write) lock; shared > 0 means shared (read) lock - if(shared > 0) { - // TODO Check if LOCK_NONE is appropriate - log_lock(P_READ, lock, file, line, LOCK_NONE); - } else if (shared == 0){ - log_lock(P_WRITE, lock, file, line, LOCK_NONE); - } -} -void lockdoc_unlocked(const char *func, const char *file, size_t line, volatile void *lock, uintptr_t where, int shared){ - // shared == 0 means exclusive (write) lock; shared > 0 means shared (read) lock - if(shared > 0) { - // TODO Check if LOCK_NONE is appropriate - log_lock(V_READ, lock, file, line, LOCK_NONE); - } else if (shared == 0){ - log_lock(V_WRITE, lock, file, line, LOCK_NONE); - } -} -void lockdoc_barrier(const char *func, const char *file, size_t line, volatile void *spinlock, int slplocks){ -} - -void lockdoc_mem_check(const char *func, const char *file, size_t line, void *base, size_t sz){ -} - -void lockdoc_wakeup(const char *func, const char *file, size_t line, volatile void *lock, uintptr_t where){ -} - void __x86_disable_intr(const char *file, int line, const char *func){ u_long eflags; eflags = x86_read_flags(); if (eflags & (1 << 9)){ // Check if interrupts were enabled in the first place - log_lock(P_WRITE, (void*)PSEUDOLOCK_ADDR_HARDIRQ, file, line, LOCK_NONE); + lockdoc_log_lock(P_WRITE, (void*)PSEUDOLOCK_ADDR_HARDIRQ, file, line, LOCK_NONE); } lockdoc_x86_disable_intr(); } @@ -158,7 +122,7 @@ void __x86_enable_intr(const char *file, int line, const char *func){ eflags = x86_read_flags(); if (!(eflags & (1 << 9))){ // Check if interrupts were disabled in the first place - log_lock(V_WRITE, (void*)PSEUDOLOCK_ADDR_HARDIRQ, file, line, LOCK_NONE); + lockdoc_log_lock(V_WRITE, (void*)PSEUDOLOCK_ADDR_HARDIRQ, file, line, LOCK_NONE); } lockdoc_x86_enable_intr(); } diff --git a/sys/lockdoc/test.c b/sys/lockdoc/test.c index 710f861f71d..48c4595c9c5 100644 --- a/sys/lockdoc/test.c +++ b/sys/lockdoc/test.c @@ -382,7 +382,7 @@ static void control_thread_work(void *data) { kthread_exit(1); } ring_buffer->size = RING_BUFFER_SIZE_REAL; - log_memory(1, "lockdoc_ring_buffer", ring_buffer, sizeof(*ring_buffer)); + lockdoc_log_memory(1, "lockdoc_ring_buffer", ring_buffer, sizeof(*ring_buffer)); START_AND_WAIT_THREAD(producer_thread_work); START_AND_WAIT_THREAD(consumer_thread_work); @@ -391,7 +391,7 @@ static void control_thread_work(void *data) { START_AND_WAIT_THREAD(dirty_alllocks_thread_work); START_AND_WAIT_THREAD(dirty_order_thread_work); - log_memory(0, "lockdoc_ring_buffer", ring_buffer, sizeof(*ring_buffer)); + lockdoc_log_memory(0, "lockdoc_ring_buffer", ring_buffer, sizeof(*ring_buffer)); free(ring_buffer, M_LOCKDOC); mutex_destroy(&rb_lock); diff --git a/sys/sys/lockdebug.h b/sys/sys/lockdebug.h index 020a44d8ee8..514bfd06911 100644 --- a/sys/sys/lockdebug.h +++ b/sys/sys/lockdebug.h @@ -52,10 +52,6 @@ typedef struct lockops { void (*lo_dump)(const volatile void *, lockop_printer_t); } lockops_t; -#ifdef LOCKDOC -#include <sys/lockdoc.h> // Has to be included *after* lockops_t definition -#endif /* LOCKDOC */ - #define LOCKDEBUG_ABORT(f, ln, l, o, m) \ lockdebug_abort(f, ln, l, o, m) @@ -84,37 +80,6 @@ void lockdebug_barrier(const char *, size_t, volatile void *, int); void lockdebug_mem_check(const char *, size_t, void *, size_t); void lockdebug_wakeup(const char *, size_t, volatile void *, uintptr_t); -#endif - -#if defined(LOCKDEBUG) && defined(LOCKDOC) - -#define LOCKDEBUG_ALLOC(lock, ops, addr) \ - lockdebug_alloc(__func__, __LINE__, lock, ops, addr); \ - lockdoc_alloc(__func__, __FILE__, __LINE__, lock, ops, addr) -#define LOCKDEBUG_FREE(dodebug, lock) \ - if (dodebug) lockdebug_free(__func__, __LINE__, lock); \ - lockdoc_free(__func__, __FILE__, __LINE__, lock) -#define LOCKDEBUG_WANTLOCK(dodebug, lock, where, s) \ - if (dodebug) lockdebug_wantlock(__func__, __LINE__, lock, where, s); \ - lockdoc_wantlock(__func__, __FILE__, __LINE__, lock, where, s) -#define LOCKDEBUG_LOCKED(dodebug, lock, al, where, s) \ - if (dodebug) lockdebug_locked(__func__, __LINE__, lock, al, where, s); \ - lockdoc_locked(__func__, __FILE__, __LINE__, lock, al, where, s) -#define LOCKDEBUG_UNLOCKED(dodebug, lock, where, s) \ - if (dodebug) lockdebug_unlocked(__func__, __LINE__, lock, where, s); \ - lockdoc_unlocked(__func__, __FILE__, __LINE__, lock, where, s) -#define LOCKDEBUG_BARRIER(lock, slp) \ - lockdebug_barrier(__func__, __LINE__, lock, slp); \ - lockdoc_barrier(__func__, __FILE__, __LINE__, lock, slp) -#define LOCKDEBUG_MEM_CHECK(base, sz) \ - lockdebug_mem_check(__func__, __LINE__, base, sz); \ - lockdoc_mem_check(__func__, __FILE__, __LINE__, base, sz) -#define LOCKDEBUG_WAKEUP(dodebug, lock, where) \ - if (dodebug) lockdebug_wakeup(__func__, __LINE__, lock, where); \ - lockdoc_wakeup(__func__, __FILE__, __LINE__, lock, where) - -#elif defined(LOCKDEBUG) && !defined(LOCKDOC) - #define LOCKDEBUG_ALLOC(lock, ops, addr) \ lockdebug_alloc(__func__, __LINE__, lock, ops, addr) #define LOCKDEBUG_FREE(dodebug, lock) \ @@ -132,27 +97,7 @@ void lockdebug_wakeup(const char *, size_t, volatile void *, uintptr_t); #define LOCKDEBUG_WAKEUP(dodebug, lock, where) \ if (dodebug) lockdebug_wakeup(__func__, __LINE__, lock, where) -#elif !defined(LOCKDEBUG) && defined(LOCKDOC) - -#define LOCKDEBUG_ALLOC(lock, ops, addr) \ - false; \ - lockdoc_alloc(__func__, __FILE__, __LINE__, lock, ops, addr) -#define LOCKDEBUG_FREE(dodebug, lock) \ - lockdoc_free(__func__, __FILE__, __LINE__, lock) -#define LOCKDEBUG_WANTLOCK(dodebug, lock, where, s) \ - lockdoc_wantlock(__func__, __FILE__, __LINE__, lock, where, s) -#define LOCKDEBUG_LOCKED(dodebug, lock, al, where, s) \ - lockdoc_locked(__func__, __FILE__, __LINE__, lock, al, where, s) -#define LOCKDEBUG_UNLOCKED(dodebug, lock, where, s) \ - lockdoc_unlocked(__func__, __FILE__, __LINE__, lock, where, s) -#define LOCKDEBUG_BARRIER(lock, slp) \ - lockdoc_barrier(__func__, __FILE__, __LINE__, lock, slp) -#define LOCKDEBUG_MEM_CHECK(base, sz) \ - lockdoc_mem_check(__func__, __FILE__, __LINE__, base, sz) -#define LOCKDEBUG_WAKEUP(dodebug, lock, where) \ - lockdoc_wakeup(__func__, __FILE__, __LINE__, lock, where) - -#else /* !LOCKDEBUG && !LOCKDOC */ +#else /* LOCKDEBUG */ #define LOCKDEBUG_ALLOC(lock, ops, addr) false #define LOCKDEBUG_FREE(dodebug, lock) /* nothing */ @@ -163,6 +108,6 @@ void lockdebug_wakeup(const char *, size_t, volatile void *, uintptr_t); #define LOCKDEBUG_MEM_CHECK(base, sz) /* nothing */ #define LOCKDEBUG_WAKEUP(dodebug, lock, where) /* nothing */ -#endif /* LOCKDEBUG && LOCKDOC */ +#endif /* LOCKDEBUG */ #endif /* __SYS_LOCKDEBUG_H__ */ diff --git a/sys/sys/lockdoc.h b/sys/sys/lockdoc.h index 5d933bae237..84fc5895680 100644 --- a/sys/sys/lockdoc.h +++ b/sys/sys/lockdoc.h @@ -2,6 +2,7 @@ #define __LOCKDOC_H__ #include <arch/x86/include/cpufunc.h> +#include <arch/i386/include/frame.h> #include <sys/types.h> #include <sys/systm.h> @@ -19,22 +20,38 @@ int32_t lockdoc_get_ctx(void); void lockdoc_x86_restore_intr(u_long eflags); +void _mutex_enter(kmutex_t *); +void _mutex_exit(kmutex_t *); +void _mutex_spin_enter(kmutex_t *); +void _mutex_spin_exit(kmutex_t *); + void trace_irqs_on(struct trapframe *frame); void trace_irqs_off(struct trapframe *frame); extern struct log_action la_buffer; - -// For DEFINEs in lockdebug.h -// Implemented in lockdoc/log.c -void lockdoc_alloc(const char *, const char *, size_t, volatile void *, lockops_t *, uintptr_t); -void lockdoc_free(const char *, const char *, size_t, volatile void *); -void lockdoc_wantlock(const char *, const char *, size_t, const volatile void *, uintptr_t, int); -void lockdoc_locked(const char *, const char *, size_t, volatile void *, void *, uintptr_t, int); -void lockdoc_unlocked(const char *, const char *, size_t, volatile void *, uintptr_t, int); -void lockdoc_barrier(const char *, const char *, size_t, volatile void *, int); -void lockdoc_mem_check(const char *, const char *, size_t, void *, size_t); -void lockdoc_wakeup(const char *, const char *, size_t, volatile void *, uintptr_t); +/* + * Redefine lock function as macros instead of normal functions, so that __FILE__ etc. will work properly + */ +#define mutex_enter(lock) do { \ + lockdoc_log_lock(P_WRITE, lock, __FILE__, __LINE__, LOCK_NONE); \ + _mutex_enter((lock)); \ +} while(/* CONSTCOND */ 0) + +#define mutex_exit(lock) do { \ + lockdoc_log_lock(V_WRITE, lock, __FILE__, __LINE__, LOCK_NONE); \ + _mutex_exit((lock)); \ +} while(/* CONSTCOND */ 0) + +#define mutex_spin_enter(lock) do { \ + lockdoc_log_lock(P_WRITE, lock, __FILE__, __LINE__, LOCK_NONE); \ + _mutex_spin_enter((lock)); \ +} while(/* CONSTCOND */ 0) + +#define mutex_spin_exit(lock) do { \ + lockdoc_log_lock(V_WRITE, lock, __FILE__, __LINE__, LOCK_NONE); \ + _mutex_spin_exit((lock)); \ +} while(/* CONSTCOND */ 0) /* Basic port I/O */ static inline void outb_(u_int8_t v, u_int16_t port) @@ -42,7 +59,7 @@ static inline void outb_(u_int8_t v, u_int16_t port) __asm __volatile("outb %0,%1" : : "a" (v), "dN" (port)); } -static inline void log_memory(int alloc, const char *datatype, const void *ptr, size_t size) { +static inline void lockdoc_log_memory(int alloc, const char *datatype, const void *ptr, size_t size) { u_long flags; flags = lockdoc_x86_disable_intr(); @@ -69,7 +86,7 @@ static inline void log_memory(int alloc, const char *datatype, const void *ptr, lockdoc_x86_restore_intr(flags); } -static inline void log_lock(int lock_op, const volatile void* ptr, const char *file, int line, int irq_sync) { +static inline void lockdoc_log_lock(int lock_op, const volatile void* ptr, const char *file, int line, int irq_sync) { u_long eflags; eflags = x86_read_psl(); lockdoc_x86_disable_intr(); diff --git a/sys/sys/mutex.h b/sys/sys/mutex.h index b37b5a97e04..eb202d79dde 100644 --- a/sys/sys/mutex.h +++ b/sys/sys/mutex.h @@ -184,6 +184,15 @@ void mutex_wakeup(kmutex_t *); #include <machine/mutex.h> /* + * This import is needed so that the mutex_ are being defined + * without having to add the lockdoc.h import to every single+ + * file where mutex_functions are being called. + */ +#ifdef LOCKDOC +#include <sys/lockdoc.h> +#endif + +/* * Return true if no spin mutexes are held by the current CPU. */ #ifndef MUTEX_NO_SPIN_ACTIVE_P @@ -195,11 +204,13 @@ void mutex_wakeup(kmutex_t *); void mutex_init(kmutex_t *, kmutex_type_t, int); void mutex_destroy(kmutex_t *); +#ifndef LOCKDOC void mutex_enter(kmutex_t *); void mutex_exit(kmutex_t *); void mutex_spin_enter(kmutex_t *); void mutex_spin_exit(kmutex_t *); +#endif int mutex_tryenter(kmutex_t *); |
