Re: [RFC] disable_irq()/enable_irq() semantics and ide-probe.c

[viro]

On Thu, Oct 09, 2003 at 11:03:14AM -0700, Linus Torvalds wrote:
> 
> On Thu, 9 Oct 2003 viro@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx wrote:
> >
> > a) on x86:
> > static void end_8259A_irq (unsigned int irq)
> > {
> >         if (!(irq_desc[irq].status & (IRQ_DISABLED|IRQ_INPROGRESS)) &&
> >				 irq_desc[irq].action)
				^^^^^^^^^^^^^^^^^^^^^^^
> >                enable_8259A_irq(irq);
> > }
> 
> This matches the "if IRQ is disabled for whatever reason" test in irq.c, 
> and as such it makes some amount of sense. However, from a logical 
> standpoint it is indeed not very sensible. It's hard to see why the code 
> does what it does.

The underlined bit is absent on alpha version of the same function.

Note that this piece is bogus - if .action is NULL, we are already caught
by IRQ_INPROGRESS check.  So it's not exactly a bug, but considering
your arguments about exact same check slightly earlier in handle_irq()...

It's from cset1.437.22.19 by mingo; the same changeset had done unconditional
removal of IRQ_INPROGRESS, so there it made sense.  After the irq.c part
had been reverted (1.497.61.30 from you), i8259.c one should be killed
too, AFAICS...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/