On Tue, 14 Nov 2006, Eric W. Biederman wrote:
>
> The big one I did not set it on is the interrupt if it comes in
> through ExtInt. I assume the 8259 is sane but I may be wrong.
Yes, ExtInt is ok, i fyou actually mask it at the 8259. As mentioned
earlier in the thread, the i8259 has its edge detect logic _after_ the
masking logic, so if the irq is still active, and you unmask it, it will
see an edge, and re-assert the interrupt in hardware.
So the i8259 is a good interrupt controller, and does not need delayed
disable and software logic to re-assert the irq.
> The truth is in practice I don't think it matters because I don't
> think anyone actually disables MSI or hypertransport interrupts.
Fair enough, at least for a 2.6.19 kind of release timeframe (and that is
what I worry about most, at least right now).
> At this point I have two questions.
> - What is the easiest path to get us to a stable 2.6.19 where
> everything works?
If people don't expect HT and MSI interrupts to be masked (and I can well
imagine that), then I think your two-liner patch is good to go. Komuro
seems to have acked it already, and in many ways that's the "minimal
change" for 2.6.19 right now.
I do like Ingo's patch because it seems "safe" (even if I think it might
be a bit _overly_ safe), but it changes semantics enough that I don't like
it for 2.6.19. Even his second version definitely changes semantics for
level-triggered PCI interrupts, even though he fixed ExtInt/i8259 ones.
So I think I'll go with your patch for now, and we can re-visit Ingo's
thing after 2.6.19.
> - What is the sanest thing for long term maintenance, of irqs?
>
> genirq is less code to maintain overall (a plus).
Oh, I absolutely think genirq is the right thing to do. No question at
all. I just think that we might want to refactor the code somewhat, and in
particular I suspect that many irq controller drivers should use separate
"struct irq_chip" entries for edge and level, because they are
fundamentally different.
> My gut feel is that there is room for a lot more cleanup in this
> area but we probably need to stabilize what we have.
Exactly. Baby steps. Make it work. Then clean up. Slowly.
> Since you aren't complaining about what the code actually does but
> rather how the interface looks, I have a proposal. I assert that
> the interface for registering an irq is much to general, and broad.
>
> Instead of having:
>
> irq_desc[irq].status |= IRQ_DELAYED_DISABLE;
> set_irq_chip_and_handler_name(irq, &ioapic_chip,
> handle_edge_irq, "edge");
>
> We should have a set of helper functions one for each common type
> of interrupt.
>
> set_irq_edge_lossy(irq, &ioapic_chip);
> set_irq_edge(irq, &ioapic_chip);
> set_irq_level(irq, &ioapic_chip);
Yeah, that might be a fine way too. That's largely what we do for the IO
schedulers, and it's been fairly successful. Start out by setting common
defaults, and then allow chip drivers to specify particular details
explicitly.
Ingo?
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
