[lkml]   [2009]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] netfilter: use per-cpu recursive lock (v11)

    On Wed, 22 Apr 2009, Eric Dumazet wrote:
    > I actually sent *one* buggy patch, and you already gave your feedback
    > and NACK.

    Actually, the thing is, I don't even think your original patch was even
    buggy. The bug crept in later. I NAK'ed it not because it was buggy, but
    because of the ad-hoc'ness and the naming. Really.

    And I actually even said so in my original rant:

    'The fact that code "happens to work by mistake" (and I'm not saying that
    your does - but it might just because of the per-cpu'ness of it [..]'

    because your original patch still had the


    in place before the whole

    rl = &__get_cpu_var(arp_tables_lock);
    if (likely(rl->count++ == 0))

    and that should have protected against both BH callers and preemption.

    So I actually believe that your original patch probably worked fine (but
    as I said in my reaction to it, I thought it was almost by mistake and I
    wasn't going to review it)

    So the actual _bug_ crept in later, when the RCU lock was removed, and the
    lock was cleaned up and separated into a function of its own.

    And in fact, that is kind of my point: "uncommented locking with ad-hoc
    semantics is very fragile". Even _correct_ code ends up not being correct
    in the long run, because people don't realize all the subtle issues.

    > I even relayed this to Stephen suggesting him not calling this a recursive lock.
    > (Note how I use 'suggesting' here)
    > So, what do you want from me ? Should I copy 100 times :

    So I consider this thread ended from a technical standpoint.

    [ That said, I will not be at all shocked to hear if people decide later
    that the RCU method was better after all, and that even the per-cpu
    rwlock or spinlock is just too expensive. ]

    My problem today (apart from the relatively minor issue of also wanting to
    get the commit log fixed up) is just that I see emails from you finding my
    reaction shocking and from Jarek Poplawski that seem to still think that
    I'm a troll.

    Just because I pointed out real technical problems? Is that shocking or

    Really - please go back to my _original_ email. No, it was not polite. But
    here's another quote from it:

    "Because even if it works today, it's just a bug waiting to happen."

    and dammit, I sent that out _before_ the very next version of the patch
    that actually _did_ introduce that exact bug.

    So dammit - what part of my email was "shocking" or "trolling"? The part
    where I was right? Or what?


     \ /
      Last update: 2009-04-22 19:33    [W:0.027 / U:9.684 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site