* Linus Torvalds <torva...@linux-foundation.org> wrote:
> On Wed, Feb 13, 2013 at 3:10 AM, Ingo Molnar <mi...@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();
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
