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

[Ingo Molnar <mingo@elte.hu>]

* Johannes Berg <johannes@sipsolutions.net> wrote:

> 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 =
> 					timer->lockdep_map;
> 
> will actually have lockdep_map here with a NULL key. Once it gets into
> 
> 			lock_map_acquire(&lockdep_map);
> 
> 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.

I suspect this invalidates v3 - mind resending a full v4 patch?

	Ingo