Messages in this thread | | | From | (Eric W. Biederman) | Date | Wed, 23 Apr 2008 22:59:39 -0700 | Subject | Re: [git patch] free_irq() fixes |
| |
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, 23 Apr 2008, Jeff Garzik wrote: >> >> When drivers make assumptions about system irq numbering, particularly on x86, >> IMO the situation is fragile. > > And when people make changes to long-standing and stable infrastructure, > the situation also gets fragile. > > The fact is, stability of interfaces is a really worthy goal in itself. > Making a change for its own sake is not a good thing. This fixes > *nothing*, and the driver changes I objected to I objected to because they > were ugly as sin. > > And I want to point out that your patches made it *much* uglier. > > So "cleanup" it sure as hell wasn't. That irq number may not be worth all > that much in itself, but it has no subtle implementation problems (we > _need_ that irq number for registration and irq handler lookup anyway, so > it is meaningful from a driver perspective, and is well-defined from a irq > core standpoint as well). > > I don't mind cleanups, but this is "churn". Change for its own sake. If it > doesn't lead to any _improvement_, it's pointless. > > If drivers don't need it, let them ignore it. But let them ignore it in > ways that work across versions, and in ways that don't cause ridiculous > and ugly work-arounds for when they do want it (even if it's just for a > printk() or similar).
I haven't looked at what Jeff's patches in particular so I can not comment there. I do remember looking at the drivers in question and yes there were indeed bugs with the handful of drivers that used the irq parameter. So fixing and cleaning up those drivers so they use the same idioms as the rest of the kernel should be a maintenance win. Even if we do keep the irq parameter to the interrupt handler.
I can comment on where there seems to be a real need for change. The hard coded NR_IRQS parameter and the arrays of size NR_IRQS are a kernel scaling bottle neck. They prevents us from building one kernel that works well on a large ranges of machines sizes. Having a single array prevents us from allocating the irq structures with NUMA affinity which slows down irq processing. Having a small number for NR_IRQS to keep the table compact keeps the irq number from being readable/useful in the case of MSI and occasionally in the case of IO_APICs.
... Since there does seem to be a real win it is my intention to kill all arrays of NR_IRQs size in the generic code. I did a quick search for [NR_IRQS] and have only found about 7 of them.
Then modify the genirq code to allow architectures to provide an alternative to todays irq_desc array.
To export the struct irq_desc to the rest of the kernel generically as a struct irq, with a new API irq_request,irq_free etc.
To build the existing API on top of the new API with a potentially slow function that maps an irq number to the struct irq. Irq actions are not fast path so we should be ok with even if we do a linear walk through the set of irqs.
After that I will see about removing irq number references from the bulk of kernel drivers. Essentially transferring the drivers to the new API in some stepwise fashion. Which is where I see synergy with Jeff's effort.
For old ISA drivers the irq number has real meaning and I don't intend to loose that, and I don't fundamentally intend to loose the irq number, instead reserving it for talking to users. Which will allow us to use a stable mapping for MSI irqs where we simply do not have enough bits below NR_IRQS today.
My goal is for an API upgrade that is a slight variation of todays API that will be stable into the future as machines grow larger and larger and we get more and more irq sources. I believe the trend line is for the number of irqs in a machine to grow linearly with the number of cores, which means I expect them to start going exponential. Not having an integer which can and has been abused in some hideous ways but instead a structure pointer is likely to keep things more robust into the future.
Eric
| |