lkml.org 
[lkml]   [2003]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:49    [from the cache]
©2003-2011 Jasper Spaans