Skip to contentSkip to conversation

disable_irq()/enable_irq() semantics and ide-probe.c

RFC Manfred Spraul 4 participants 8 messages Oct 9, 2003

lore
Share & tools
lore
#0 lore raw
Quoted 9 lines · Linus

Nobody has ever really complained, but if anybody ever wants to do this, then the way to do it would be to

- find out the irq
- disable it
- request the irq
- enable the PCI routing for it
- set up the device
- enable the irq

I'd like to use that for nic shutdown for natsemi:

   disable_irq();
   shutdown_nic();
   free_irq();
   enable_irq();

The irq handler touches registers that restart the nic. Right now I use a np->hands_off variable to avoid that.

But I don't know if all systems can support atomic request_irq/free_irq calls. request_irq creates /proc/irq/x/cpu_affinity, and I could imagine that on some archs it might have perform IPIs to reconfigure the irq controller of a remote node.

--
Manfred

#1 lore raw
Quoted 6 lines · Manfred Spraul

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.

Jeff

#2 lore raw
Quoted 9 lines · Jeff Garzik
Quoted 6 lines · Manfred Spraul

I'd like to use that for nic shutdown for natsemi:

   disable_irq();
   shutdown_nic();
   free_irq();
   enable_irq();
Quoted 7 lines

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.

#3 lore raw
Quoted 14 lines · Benjamin Herrenschmidt
Quoted 12 lines · Jeff Garzik
Quoted 6 lines · Manfred Spraul

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?

Quoted 8 lines
Quoted 1 line

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

Easily solved with a synchronize_irq() ;-)

As a slight tangent, with MSI coming up, as well as the presence of hardware that queues IRQ events asynchronously, we'll want synchronize_irq() to handle those cases, even for uniprocess (often synchronize_irq is only defined for SMP).

Quoted 9 lines

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).

You want to disable_irq() then sleep? ewww... :) I think we could figure out a better way...

Jeff

#4 lore raw
Quoted 1 line · Jeff Garzik

Easily solved with a synchronize_irq() ;-)

No. synchronize_irq will do nothing to an irq that is still somewhere in the HW path from the device to the core, and even in the core it may be queued for some cycles before actually delivered.

#5 lore raw
Quoted 10 lines · Benjamin Herrenschmidt
Quoted 1 line · Jeff Garzik

Easily solved with a synchronize_irq() ;-)

No. synchronize_irq will do nothing to an irq that is still somewhere in the HW path from the device to the core, and even in the core it may be queued for some cycles before actually delivered.

hmmm, ok :)

Well my main objection is disable_irq+free_irq+enable_irq. Seems to me we'll wind up coding around that. Maybe a free_and_enable_irq is appropriate to avoid such a situation? Oh well, just thinking out loud...

Jeff

#6 lore raw
In reply to Jeff Garzik
Quoted 3 lines · Jeff Garzik

If you can't stop the NIC hardware from generating interrupts, that's a driver bug.

No it's not.

Think shared interrupts here.

Linus

#7 lore raw
Quoted 8 lines · Linus Torvalds
Quoted 1 line · Jeff Garzik

If you can't stop the NIC hardware from generating interrupts, that's a driver bug.

No it's not.

Think shared interrupts here.

I was being specific in what I said :)

Sure, shared interrupts can still call a driver's handler.

But if the driver doesn't stop _its own_ hardware from generating interrupts, you've got screaming interrupts the minute it issues free_irq and signals it doesn't care anymore. The driver damn well better be able to stop the _NIC hardware_ from generating interrupts :)

Jeff