Messages in this thread | | | Date | Wed, 24 Sep 2008 10:45:58 +0200 | From | Ingo Molnar <> | Subject | Re: Should irq_chip->mask disable percpu interrupts to all cpus, or just to this cpu? |
| |
* Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> Hi, > > I'm reworking Xen's interrupt handling to isolate it a bit from the > workings of the apic-based code, as Eric suggested a while back. > > As I've mentioned before, Xen represents interrupts as event channels. > There are two major classes of event channels: per-cpu and, erm, not > percpu. Per-cpu event channels are for things like timers and IPI > function calls which are inherently per-cpu; it's meaningless to > consider, for example, migrating them from cpu to cpu. I guess > they're analogous to the the local apic vectors. > > (Non-percpu event channels can be bound to a particular cpu, and > rebound at will; I'm not worried about them here.) > > Previously I allocated an irq per percpu event channel per cpu. This > was pretty wasteful, since I need about 5-6 of them per cpu, so the > number of interrupts increases quite quickly as cpus does. There's no > deep problem with that, but it gets fairly ugly in /proc/interrupts, > and there's some tricky corners to manage in suspend/resume. > > This time around I'm allocating a single irq for each percpu interrupt > source (so one for timers, one for IPI, etc), and mapping each per-cpu > event channel to each. But I'm wondering what the correct behaviour > of irq_chip->mask/unmask should be in this case. Each event channel > is individually maskable, so when ->mask gets called, I can either > mask all the event channels associated with that irq, or just the one > for this cpu. The latter makes most sense for me, but I don't quite > understand the irq infrastructure enough to know if it will have bad > effects globally. > > When I request the irq, I pass IRQF_PERCPU in the flags, but aside > from preventing migration, this only seems to have an effect on > __do_IRQ(), which looks like a legacy path anyway. It seems to me > that by setting it that I'm giving the interrupt subsystem fair > warning that ->mask() is only going to disable the interrupt on this > cpu. > > Are there any other ill-effects of sharing an irq across all cpus like > this? I guess there's some risk of contention on the irq_desc lock.
You should be a pretty special case: both the producer and consumer of those IRQs. So if you change the semantics of ->mask()/->unmask() you'll only affect your own drivers: you might get irqs even after you disable_irq_nosync(). [but the genirq layer wont pass them down]
The genirq layer should be robust enough all across - as stray IRQs are commonplace on real hw anyway. Sometimes we have ->unmask() methods that opportunistically do not unmask the hw itself (but hope for an irq to not occur) - edge handlers for example. And you probably wont use disable_irq_nosync() anyway, you just want genirq to prevent irq handler self-reentry, right?
So i _think_ in theory with your scheme you should get enough concurrency and no arbitrary limitations/serialization/etc. - but you should check whether Miss Practice agrees with that ;)
Ingo
| |