lkml.org 
[lkml]   [1999]   [May]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch] SMP race fix [was Re: SMP lockup & 3c509 on 2.2.x [aka. the Deadly 'ping -f']]


    On Wed, 5 May 1999, Andrea Arcangeli wrote:
    >
    > Ah! The code above is showing some bad design of the IRQ_INPROGRESS stuff.

    Andrea, I think you're just guessing at what's going on again. Don't
    just guess.

    IRQ_INPROGRESS works the way it works on purpose: it's only cleared after
    a successful execution of the irq handler - and it is left pending if
    there was no irq handler etc..

    Your "while ()" was not reliable, and that's the part that needs fixing.
    For example, your while patch will loop forever if an irq tries to
    synchronize with itself (instead of just returning like the original code
    did). Don't try to fix it up by changing code that works fine.

    > This my second incremental patch make sure to have a relialable
    > IRQ_INPROGRESS value, to allow us to safely loop while(in_progress) at the
    > end of disable_irq().

    See above why it's not safe at all, even with your changes.

    > Really with this new patch I removed the need to clear the IRQINPROGRESS
    > bit upon irq_enable() and so I think this patch alone would just fix the
    > SMP race (the problem is that we was doing a enable_irq while the irq was
    > running and so we was allowing a second irq to reenter nested in the irq
    > handler).

    No.

    If people are doing random enable_irq() calls, then everything will break
    anyway.

    "enable_irq()" is only valid after a "disable_irq()" has been called.
    That's basically enforced by the "disable counter" setup, but it also
    makes sense.

    And "disable_irq()" will synchronously wait for the irq handler to have
    exited (it has to - otherwise you'd see the case where somebody calls
    disable_irq(), yet another CPU is still busy running that irq). So you
    wouldn't see any nested irq handlers. Think about it.

    Now, I fully expect that there can be (and is) a bug somewhere there, but
    the way to fix it is not to make the above kind of guesses.

    > Now I have to shutdown the computer (I had to go to University ...), I'll
    > continue to think about the problem in the late evening.

    Please do.

    I generally have a problem with your patches: a lot of them are really
    good. But at the same time a lot of them are changing things because
    you're in "random walk" mode, and you just walk over the code without
    really thinking about them. In the end, you then have a huge patch, and
    nobody really knows which part of your patch is really the worthwhile
    part. Which is then why it takes so long for some of your _real_ fixes to
    get into a stable kernel..

    Linus


    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.rutgers.edu
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 13:51    [W:0.022 / U:0.832 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site