Re: [PATCH] workaround minor lockdep bug triggered by mm_take_all_locks

Andrea Arcangeli
2008-08-04T22:13:28Z

On Mon, Aug 04, 2008 at 02:41:14PM -0700, Andrew Morton wrote:
> lockdep will warn here about the ranking violation. As soon as it
> occurs. Even though the system never deadlocked.

Yes I wasn't aware of the AB BA feature that doesn't require the
inversion to happen on both cpus at the same time, despite having
spent weeks when it was kernel crashing systems at boot (which shows
it's not immediately easy to understand how that thing works by
reading the code).

static DEFINE_MUTEX(a);
static DEFINE_MUTEX(b);
static DEFINE_MUTEX(c);

void f1(void)
{
mutex_lock(c);
mutex_lock(a);
mutex_lock(b);
mutex_unlock(c);
mutex_unlock(a);
mutex_unlock(b);
}

void f2(void)
{
mutex_lock(c);
mutex_lock(b);
mutex_lock(a);
mutex_unlock(c);
mutex_unlock(b);
mutex_unlock(a);
}

void f3(void)
{
mutex_lock(a);
mutex_lock(b);
mutex_unlock(a);
mutex_unlock(b);
}

void f4(void)
{
mutex_lock(b);
mutex_lock(a);
mutex_unlock(b);
mutex_unlock(a);
}

void doit(void)
{
f1();
f2();
stop_machine(f3);
stop_machine(f4);
}

> It's very very clever and powerful.

I'm curious, does it also handle the above? I mean not a big deal to
refactor the code to shutdown lockdep warnings, but it doesn't look
that clever to me. It seems just clever the minimum enough to be
useful with its AB BA memory.

Now I see lockdep has a good point to exist, for the lock inversion
"memory" but this is by no mean a feature I can remotely like in
general given how inaccurate and prone for false positives it is. It
looks more a necessary evil to deal with, like I tried to do with my
patch (even before understanding it was more useful than what I
thought initially).