lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m1od7zkd2s.fsf@frodo.ebiederm.org>
Date:	Wed, 23 Apr 2008 22:59:39 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Jeff Garzik <jeff@...zik.org>,
	Rene Herman <rene.herman@...access.nl>,
	Adrian Bunk <bunk@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, rmk@....linux.org.uk,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>
Subject: Re: [git patch] free_irq() fixes

Linus Torvalds <torvalds@...ux-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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ