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

From: Benjamin Herrenschmidt
Date: Thu Oct 09 2003 - 11:59:13 EST


On Thu, 2003-10-09 at 18:38, Jeff Garzik wrote:
> Manfred Spraul wrote:
> > I'd like to use that for nic shutdown for natsemi:
> >
> > disable_irq();
> > shutdown_nic();
> > free_irq();
> > enable_irq();
>
>
> Why not just shutdown the NIC inside spin_lock_irqsave or disable_irq,
> and then free_irq separately?
>
> If you can't stop the NIC hardware from generating interrupts, that's a
> driver bug. And if the driver cannot handle its interrupt handler
> between the spin_unlock_irqrestore() and free_irq() (shared irq case),
> it's also buggy.

Actually you may still get a stale irq ;) The problem is that IRQs are
typically an asynchronous event, and an irq can be sort of "queued" up
(especially if it's a level one) in the PIC... though at least this
won't be a stale level irq so you won't deadlock in an irq handler that
can do nothing...

Anyway, I quite like the idea. I've been trying to avoid taking a lock
in some similar shutdown routine for sungem, because some bits in there
need a few ms delay to workaround a chip bug (or machine sleep will
break) and I want to schedule. Breaking the lock makes things ugly,
beeing able to just disable_irq before/after is nice.

The problem of course is when that irq is shared... you are suddently
shutting off for a potentially long time a neighbour irq, bad bad...
(at least I know that on pmac, sungem irq is never shared).

Ben.


-
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/