Re: [patch] IRQ threads

[Ingo Molnar]

* Scott Wood <scott@xxxxxxxxxxx> wrote:

> 2. This patch does not disable local interrupts when running a
> threaded handler.  SMP-safe drivers shouldn't be directly bothered by
> this (as the interrupt could as easily have happened on another CPU),
> but there may be some interactions with softirqs and per-cpu data, if
> a softirq thread preempts an IRQ thread, or an IRQ thread gets
> migrated to a different CPU. [...]

hardirqs (unlike softirqs) we might attempt to make preemptable. It
really should work because if they run with interrupts enabled then they
must be prepared to see interrupts coming from a similar device using
the same softirq facilities.

migration between CPUs we can prevent easily. While it's not common for
a driver to rely on per-CPU-ness _outside_ of a spinlocked region, it
could in theory happen.

for now i've gone the more conservative route of keeping irqs atomic
still, and applying lock-break methods to them.

> [...] I'm particularly worried about the network code.  If possible,
> I'd like to find and fix such breakages rather than use
> local_irq_disable(), as that would prevent IRQ proritization from
> working, and prevent IRQ threads from being used to isolate the rest
> of the system from long-running IRQs (such as non-DMA IDE).

this is not enough: DMA-IDE creates big latencies too, and those
latencies happen under a spinlock - so with your patch it would be
non-preemptible still.

> 3. The i8042 driver had to be marked SA_NOTHREAD, as there are
> non-preemptible regions where it spins, waiting for an interrupt.
> Ideally, this driver (and others like it) should be fixed to either do
> a cond_resched() or use a wait queue.

yeah, i fixed it via cond_resched_softirq(), _after_ making sure it's
safe to preempt there - softirqs are not generally preemption safe
because they are fundamentally per-CPU. (meanwhile Dmitry Torokhov has
posted a patch that fixes the atkbd.c problem for real by using
workqueues. The psmouse-base.c problem needs a similar fix.)

> 4. This might be a good time to get around to moving the bulk of the
> arch/whatever/kernel/irq.c into generic code, as the code said was
> supposed to happen in 2.5.  This patch is currently only for x86
> (though we've run IRQ threads on many different platforms in the
> past).

agreed. I punted this one for the time being as it's clearly separate
from the issue of latencies and it's deeply intrusive to 2.6.

> 5. Is there any reason why an IRQ controller might want to have its
> end() called even if IRQ_DISABLED or IRQ_INPROGRESS is set?  It'd be
> nice to merge those checks in with the
> IRQ_THREADPENDING/IRQ_THREADRUNNING checks.

e.g. in the IO-APIC case if we ack the local APIC only in the end()
function then we must do that - an un-acked local APIC prevents other
IRQs from being delivered. We do this for level-triggered IO-APIC irqs.

> 6. This patch causes in_irq() to return true if an IRQ thread is
> running, as some drivers use it in common code to determine how to
> act.  in_interrupt(), however, will return false in such a case. The
> exact meaning of these macros in the presence of IRQ threads isn't
> very well defined, and I hope this results in sane behavior.

this is an incorrect change, just grep for in_interrupt() in
linux/drivers/ ...

I agree with the concept of using multiple threads for interrupts - i'll
add that to the voluntary-preempt patch too. This is an essential
feature to prioritize interrupts.

what do you think about making the i8259A's interrupt priorities
configurable? (a'la rtirq patch) Does it make any sense, given how early
we mask the source irq and ack the interrupt controller [hence giving
all other interrupts a fair chance to arrive ASAP]?

Bernhard Kuhn's rtirq patch is for IO-APIC/APICs, but i think the
latency issues could be equally well fixed by not keeping the local APIC
un-ACK-ed during level triggered irqs, but doing the mask & ack thing.
This will be slightly slower but should make them both redirectable and
more symmetric and fair.

	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/