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

From: Ingo Molnar
Date: Thu Jan 29 2009 - 07:38:29 EST



* Johannes Berg <johannes@xxxxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/