lkml.org 
[lkml]   [2013]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
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();
}


\
 
 \ /
  Last update: 2013-02-15 13:21    [W:0.087 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site