lkml.org 
[lkml]   [2008]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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


\
 
 \ /
  Last update: 2008-04-24 08:03    [W:0.075 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site