linux-kernel - [PATCH] spinlock/debugging: Print out lock name when available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130215113943.GC26955@gmail.com>
Date:	Fri, 15 Feb 2013 12:39:43 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jens Axboe <axboe@...nel.dk>,
	Thomas Gleixner <tglx@...utronix.de>,
	Alexander Viro <viro@....linux.org.uk>,
	Theodore Ts'o <tytso@....edu>, "H. Peter Anvin" <hpa@...or.com>
Subject: [PATCH] spinlock/debugging: Print out lock name when available


* Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Wed, Feb 13, 2013 at 3:10 AM, Ingo Molnar <mingo@...nel.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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/