Re: Should irq_chip->mask disable percpu interrupts to all cpus,or just to this cpu?

From: Ingo Molnar
Date: Wed Sep 24 2008 - 04:46:30 EST




* Jeremy Fitzhardinge <jeremy@xxxxxxxx> 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
--
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/