lkml.org 
[lkml]   [2020]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC] Are you good with Lockdep?
    On Wed, 11 Nov 2020 11:54:41 +0100
    Ingo Molnar <mingo@kernel.org> wrote:

    > * Byungchul Park <byungchul.park@lge.com> wrote:
    >
    > > PROBLEM 1) First of all, Lockdep gets disabled on the first detection.
    >
    > Lockdep disabling itself after the first warning was an intentional
    > and deliberate design decision. (See more details below.)
    >

    [..]

    > Making the code simpler is always welcome, but I disagree with
    > enabling multiple warnings, for the technical reasons outlined above.

    I 100% agree with Ingo. I wish we could stop *all* warnings after the first
    one. The number of times people sent me bug reports with warnings without
    showing me previous warnings that caused me to go on wild goose chases is
    too many to count. The first warning is the *only* thing I look at.
    Anything after that is likely to be caused by the integrity of the system
    being compromised by the first bug.

    And this is especially true with lockdep, because lockdep only detects the
    deadlock, it doesn't tell you which lock was the incorrect locking.

    For example. If we have a locking chain of:

    A -> B -> D

    A -> C -> D

    Which on a correct system looks like this:

    lock(A)
    lock(B)
    unlock(B)
    unlock(A)

    lock(B)
    lock(D)
    unlock(D)
    unlock(B)

    lock(A)
    lock(C)
    unlock(C)
    unlock(A)

    lock(C)
    lock(D)
    unlock(D)
    unlock(C)

    which creates the above chains in that order.

    But, lets say we have a bug and the system boots up doing:

    lock(D)
    lock(A)
    unlock(A)
    unlock(D)

    which creates the incorrect chain.

    D -> A


    Now you do the correct locking:

    lock(A)
    lock(B)

    Creates A -> B

    lock(A)
    lock(C)

    Creates A -> C

    lock(B)
    lock(D)

    Creates B -> D and lockdep detects:

    D -> A -> B -> D

    and gives us the lockdep splat!!!

    But we don't disable lockdep. We let it continue...

    lock(C)
    lock(D)

    Which creates C -> D

    Now it explodes with D -> A -> C -> D

    Which it already reported. And it can be much more complex when dealing
    with interrupt contexts and longer chains. That is, perhaps a different
    chain had a missing irq disable, now you might get 5 or 6 more lockdep
    splats because of that one bug.

    The point I'm making is that the lockdep splats after the first one may
    just be another version of the same bug and not a new one. Worse, if you
    only look at the later lockdep splats, it may be much more difficult to
    find the original bug than if you just had the first one. Believe me, I've
    been down that road too many times!

    And it can be very difficult to know if new lockdep splats are not the same
    bug, and this will waste a lot of developers time!

    This is why the decision to disable lockdep after the first splat was made.
    There were times I wanted to check locking somewhere, but is was using
    linux-next which had a lockdep splat that I didn't care about. So I
    made it not disable lockdep. And then I hit this exact scenario, that the
    one incorrect chain was causing reports all over the place. To solve it, I
    had to patch the incorrect chain to do raw locking to have lockdep ignore
    it ;-) Then I was able to test the code I was interested in.

    >
    > > PROBLEM 2) Lockdep forces us to emulate lock acquisition for non-lock.
    >
    > > I have the patch set. Let me share it with you in a few days.
    >
    > Not sure I understand the "problem 2)" outlined here, but I'm looking
    > forward to your patchset!
    >

    I think I understand it. For things like completions and other "wait for
    events" we have lockdep annotation, but it is rather awkward to implement.
    Having something that says "lockdep_wait_event()" and
    "lockdep_exec_event()" wrappers would be useful.

    -- Steve

    \
     
     \ /
      Last update: 2020-11-11 15:37    [W:4.194 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site