[lkml]   [2003]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [RFC] disable_irq()/enable_irq() semantics and ide-probe.c
    On Wed, Oct 08, 2003 at 07:29:10PM -0700, Linus Torvalds wrote:
    > > If an interrupt comes during that
    > > time, we'll get IRQ_INPROGRESS set and not reset until later register_irq()
    > > (see handle_irq() for details). Note that calling disable_irq() after that
    > > will kill us on SMP - it will spin waiting for IRQ_INPROGRESS to go away.
    > Now _this_ is a bug waiting to happen. I don't think it actually happens
    > now (since anybody who does disable_irq() _will_ either have registered
    > the irq already or will do so soon, but I agree that it's just trouble
    > waiting to happen.

    Ummm... probe_hwif() is a good example of the opposite - it can fail
    past the point where it disables irq and that means no register_irq()
    after enable_irq() call on cleanup.

    > I think the fix to that is to just add a trivial test for "if the handler
    > list is empty, don't bother synchronizing" in disable_irq(), since clearly
    > if the list is empty there is nothing to synchronize _with_. After all,
    > the synchronization is there just to make sure no handler runs
    > concurrently on another CPU.

    How about

    action = NULL;
    if (!(status & (IRQ_DISABLED | IRQ_INPROGRESS))) {
    action = desc->action;
    status &= ~IRQ_PENDING; /* we commit to handling */
    if (likely(action))
    status |= IRQ_INPROGRESS; /* we are handling it */
    desc->status = status;

    in handle_irq()?

    > As far as I can tell, 2.6.x is doing all the right things. Modulo the (not
    > really supported) concurrent device probing, and the (not implemented)
    > atomic irq requesting.
    > Note that the IRQ_INPROGRESS thing was literally the bit that autodetect
    > used to test, it got changed it to IRQ_WAITING to clarify the code and
    > avoid bad interactions with the other uses of IRQ_INPROGRESS.
    > And note that we do _not_ clear IRQ_INPROGRESS on "action == NULL" very
    > much on purpose: that "action == NULL" thing also happens if the IRQ is
    > disabled, and we need to get the edge replay right. This is why
    > request_irq() literally _needs_ to clear that bit in 2.6.x.

    See above - we shouldn't clear it on action == NULL, but we don't
    need to set it, AFAICS.

    > So the fix is to make 2.4.x do what 2.6.x does, methinks.

    ObOtherFun: There's another bogosity in quoted ide-probe.c code, according
    to dwmw2 - he says that there are PCI IDE cards that get IRQ 0, so the
    test for hwif->irq is b0rken. We probably should stop overloading
    ->irq == 0 for "none given", but I'm not sure that we *have* a value
    that would never be used as an IRQ number on all platforms...
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 13:49    [W:0.023 / U:16.668 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site