Re: [PATCH v3] timer: implement lockdep deadlock detection

From: Johannes Berg
Date: Wed Jan 28 2009 - 13:48:47 EST

On Wed, 2009-01-28 at 11:59 +0100, Johannes Berg wrote:

> > > I actually got "trying to register non-static key" on my powerpc64
> > > machine. Is there a possibility that functions are not static??
> >
> > Hmm, weird, afaict static_obj() includes both text and data, for the
> > core kernel as well as modules.
> Yeah, I'd think it should. I'll run it by the powerpc list when I get it
> again, it only seems to happen very rarely.

It's actually a generic bug.

delayed work structs are initialised like this:

#define __DELAYED_WORK_INITIALIZER(n, f) { \
.work = __WORK_INITIALIZER((n).work, (f)), \
.timer = TIMER_INITIALIZER(NULL, 0, 0), \

#define DECLARE_DELAYED_WORK(n, f) \
struct delayed_work n = __DELAYED_WORK_INITIALIZER(n, f)

Note the NULL function, which I used for the key of timers. Thus, the
key is NULL, and the name is "NULL".

Now, this means that my code for the run timer:

struct lockdep_map lockdep_map =

will actually have lockdep_map here with a NULL key. Once it gets into


it'll try to register &lockdep_map as the key.

Interestingly, that doesn't seem to be a problem on x86_64, which would
appear to be a bug, the stack certainly isn't a static location.

The patch below fixes it by using the file/lineno of the static
definition as both the name and the key -- using it as the name means
you have a good chance of finding it if something goes wrong, and using
it as the key means we have a good key for it. The patch looks horrible
though. Any better ideas? If not, I think we should roll this into the
original patch.

include/linux/timer.h | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)

--- wireless-testing.orig/include/linux/timer.h 2009-01-28 18:55:03.270071595 +0100
+++ wireless-testing/include/linux/timer.h 2009-01-28 19:00:41.326947619 +0100
@@ -32,23 +32,35 @@ extern struct tvec_base boot_tvec_bases;
* NB: because we have to copy the lockdep_map, setting _key
* here is required, otherwise it could get initialised to the
- * copy of the lockdep_map! We use the function pointer as the
- * lockdep key here.
+ * copy of the lockdep_map! We use the pointer to and the string
+ * "<file>:<line>" as the key resp. the name of the lockdep_map.
-#define __TIMER_LOCKDEP_MAP_INITIALIZER(fn, name) \
- .lockdep_map = STATIC_LOCKDEP_MAP_INIT(name, fn),
+#define __TIMER_LOCKDEP_MAP_INITIALIZER(name, key) \
+ .lockdep_map = STATIC_LOCKDEP_MAP_INIT(name, key),

-#define TIMER_INITIALIZER(_function, _expires, _data) { \
+#define ____TIMER_INITIALIZER(_function, _expires, _data, _name, _key) {\
.entry = { .prev = TIMER_ENTRY_STATIC }, \
.function = (_function), \
.expires = (_expires), \
.data = (_data), \
.base = &boot_tvec_bases, \
- __TIMER_LOCKDEP_MAP_INITIALIZER((_function), #_function)\
+ __TIMER_LOCKDEP_MAP_INITIALIZER((_name), (_key)) \
+ * All the indirection here is required to build the
+ * "<file>:<line>" string and the pointer to it.
+ */
+#define ___TIMER_INITIALIZER(_fn, _exp, _dat, _kn) \
+ ____TIMER_INITIALIZER(_fn, _exp, _dat, (_kn), &(_kn))
+#define __TIMER_INITIALIZER(_fn, _exp, _dat, _f, _c, _l) \
+ ___TIMER_INITIALIZER(_fn, _exp, _dat, _f # _c # _l)
+#define _TIMER_INITIALIZER(_fn, _exp, _dat, _f, _l) \
+ __TIMER_INITIALIZER(_fn, _exp, _dat, _f, :, _l)
+#define TIMER_INITIALIZER(_function, _expires, _data) \
+ _TIMER_INITIALIZER(_function, _expires, _data, __FILE__, __LINE__)

#define DEFINE_TIMER(_name, _function, _expires, _data) \
struct timer_list _name = \

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at