LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Linux Kernel list <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@elte.hu>
Subject: [PATCH 3/2] tasklet: implement lockdep deadlock detection
Date: Thu, 23 Oct 2008 22:37:14 +0200	[thread overview]
Message-ID: <1224794234.6002.70.camel@johannes.berg> (raw)
In-Reply-To: <1224790541.6002.53.camel@johannes.berg>

The tasklet code can deadlock when you try to disable a tasklet
under a lock that the tasklet requires. This patch implements
lockdep checking for this situation.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
I don't have an actual instance of this in the kernel either, but my
test case triggers correctly.

This was easy, but I think it's probably unlikely that somebody will do
this mistake.

 include/linux/interrupt.h |   41 ++++++++++++++++++++++++++++++++++++-----
 kernel/softirq.c          |   10 +++++++---
 2 files changed, 43 insertions(+), 8 deletions(-)

--- wireless-testing.orig/include/linux/interrupt.h	2008-10-23 21:42:58.495547519 +0200
+++ wireless-testing/include/linux/interrupt.h	2008-10-23 22:09:18.911743812 +0200
@@ -299,13 +299,31 @@ struct tasklet_struct
 	atomic_t count;
 	void (*func)(unsigned long);
 	unsigned long data;
+#ifdef CONFIG_LOCKDEP
+	struct lockdep_map lock_map;
+#endif
 };
 
-#define DECLARE_TASKLET(name, func, data) \
-struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(0), func, data }
+#ifdef CONFIG_LOCKDEP
+#define __TASKLET_LOCK_INIT(n, k) .lock_map = STATIC_LOCKDEP_MAP_INIT(n, k),
+#else
+#define __TASKLET_LOCK_INIT(n, k)
+#endif
 
+#define __DECLARE_TASKLET(name, _func, _data, c)\
+struct tasklet_struct name = {			\
+	.next = NULL,				\
+	.state = 0,				\
+	.count = ATOMIC_INIT(c),		\
+	.func = _func,				\
+	.data = _data,				\
+	__TASKLET_LOCK_INIT(#name, &name)	\
+}
+
+#define DECLARE_TASKLET(name, func, data)	\
+	__DECLARE_TASKLET(name, func, data, 0)
 #define DECLARE_TASKLET_DISABLED(name, func, data) \
-struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(1), func, data }
+	__DECLARE_TASKLET(name, func, data, 1)
 
 
 enum
@@ -328,6 +346,8 @@ static inline void tasklet_unlock(struct
 
 static inline void tasklet_unlock_wait(struct tasklet_struct *t)
 {
+	lock_map_acquire_noirq(&t->lock_map);
+	lock_map_release(&t->lock_map);
 	while (test_bit(TASKLET_STATE_RUN, &(t)->state)) { barrier(); }
 }
 #else
@@ -380,8 +400,19 @@ static inline void tasklet_hi_enable(str
 
 extern void tasklet_kill(struct tasklet_struct *t);
 extern void tasklet_kill_immediate(struct tasklet_struct *t, unsigned int cpu);
-extern void tasklet_init(struct tasklet_struct *t,
-			 void (*func)(unsigned long), unsigned long data);
+extern void tasklet_init_key(struct tasklet_struct *t,
+			     const char *name, struct lock_class_key *key,
+			     void (*func)(unsigned long), unsigned long data);
+
+#ifdef CONFIG_LOCKDEP
+#define tasklet_init(t, f, d)					\
+	do {							\
+		static struct lock_class_key __key;		\
+		tasklet_init_key((t), #t, &__key, (f), (d));	\
+	} while (0)
+#else
+#define tasklet_init(t, f, d) tasklet_init_key((t), NULL, NULL, (f), (d))
+#endif
 
 /*
  * Autoprobing for irqs:
--- wireless-testing.orig/kernel/softirq.c	2008-10-23 21:46:26.808921693 +0200
+++ wireless-testing/kernel/softirq.c	2008-10-23 22:01:34.382734952 +0200
@@ -383,7 +383,9 @@ static void tasklet_action(struct softir
 			if (!atomic_read(&t->count)) {
 				if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state))
 					BUG();
+				lock_map_acquire_noirq(&t->lock_map);
 				t->func(t->data);
+				lock_map_release(&t->lock_map);
 				tasklet_unlock(t);
 				continue;
 			}
@@ -435,17 +437,19 @@ static void tasklet_hi_action(struct sof
 }
 
 
-void tasklet_init(struct tasklet_struct *t,
-		  void (*func)(unsigned long), unsigned long data)
+void tasklet_init_key(struct tasklet_struct *t,
+		      const char *name, struct lock_class_key *key,
+		      void (*func)(unsigned long), unsigned long data)
 {
 	t->next = NULL;
 	t->state = 0;
 	atomic_set(&t->count, 0);
 	t->func = func;
 	t->data = data;
+	lockdep_init_map(&t->lock_map, name, key, 0);
 }
 
-EXPORT_SYMBOL(tasklet_init);
+EXPORT_SYMBOL(tasklet_init_key);
 
 void tasklet_kill(struct tasklet_struct *t)
 {



      parent reply	other threads:[~2008-10-24  9:46 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-23 19:35 [PATCH 0/2] Timer sync lock checking Johannes Berg
2008-10-23 19:44 ` [PATCH 1/2] lockdep: implement full check without irq checking Johannes Berg
2008-10-29 15:23   ` Peter Zijlstra
2008-10-30 10:36     ` Johannes Berg
2008-10-30 11:15       ` Peter Zijlstra
2008-10-30 11:25         ` Johannes Berg
2008-10-23 19:46 ` [PATCH 2/2] timer: implement lockdep deadlock detection Johannes Berg
2008-10-23 20:37 ` Johannes Berg [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1224794234.6002.70.camel@johannes.berg \
    --to=johannes@sipsolutions.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH 3/2] tasklet: implement lockdep deadlock detection' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).