Messages in this thread Patch in this message | | | Date | Fri, 15 Feb 2013 12:39:43 +0100 | From | Ingo Molnar <> | Subject | [PATCH] spinlock/debugging: Print out lock name when available |
| |
* Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, Feb 13, 2013 at 3:10 AM, Ingo Molnar <mingo@kernel.org> wrote: > > > > Setting up Logical Volume Management: [ 13.140000] BUG: spinlock lockup suspected on CPU#1, lvm.static/139 > > [ 13.140000] BUG: spinlock lockup suspected on CPU#1, lvm.static/139 > > [ 13.140000] lock: 0x97fe9fc0, .magic: dead4ead, .owner: <none>/-1, .owner_cpu: -1 > > Btw, this is an entirely unrelated thing, but it just struck > me: you have CONFIG_DEBUG_LOCK_ALLOC in your config, so that > spinlock has a _name_, and the not-so-helpful "spin_dump()" is > too stupid to even bother to print it out.
Yeah - the spinlock debug code predates lockdep.
> Now, in this case, it doesn't much matter, since the callchain > really does end up showing pretty unambiguously what the lock > is, but shouldn't we print that lock name when we dump the > lock information? > > I dunno. Maybe the names aren't useful, and the callchain ends > up always making them redundant. But it seems an oversight in > our debug output.
I think it's generally useful, sometimes, for deep crashes we don't get a call-chain at all.
Something like the patch below? (entirely untested)
( Using CPP macros to not have to create trivial wrappers for half a dozen lock types. 'dep_map' is not a very likely source of typos and type confusion in any case. )
We could also do a kallsyms lookup like lockdep does - but I'd rather keep the spinlock debugging code simple and while checking that code I noticed that kernel/lockdep.c:print_lockdep_cache() is probably buggy as it subtly returns an on-stack variable ... Needs a separate fix.
Thanks,
Ingo
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index f05631e..17df33a 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -315,6 +315,8 @@ static inline int lockdep_match_key(struct lockdep_map *lock, return lock->key == key; } +#define lock_name(lock) ((lock)->dep_map.name) + /* * Acquire a lock. * @@ -373,6 +375,8 @@ static inline void lockdep_on(void) { } +# define lock_name(lock) "" + # define lock_acquire(l, s, t, r, c, n, i) do { } while (0) # define lock_release(l, n, i) do { } while (0) # define lock_set_class(l, n, k, s, i) do { } while (0) diff --git a/lib/spinlock_debug.c b/lib/spinlock_debug.c index 0374a59..80d4818 100644 --- a/lib/spinlock_debug.c +++ b/lib/spinlock_debug.c @@ -55,15 +55,19 @@ static void spin_dump(raw_spinlock_t *lock, const char *msg) if (lock->owner && lock->owner != SPINLOCK_OWNER_INIT) owner = lock->owner; - printk(KERN_EMERG "BUG: spinlock %s on CPU#%d, %s/%d\n", + + printk(KERN_EMERG "BUG: spinlock %s on CPU#%d, %s/%d %s\n", msg, raw_smp_processor_id(), - current->comm, task_pid_nr(current)); + current->comm, task_pid_nr(current), + lock_name(lock)); + printk(KERN_EMERG " lock: %pS, .magic: %08x, .owner: %s/%d, " ".owner_cpu: %d\n", lock, lock->magic, owner ? owner->comm : "<none>", owner ? task_pid_nr(owner) : -1, lock->owner_cpu); + dump_stack(); }
| |