lkml.org 
[lkml]   [2008]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [git patch] free_irq() fixes
Rene Herman wrote:
> On 23-04-08 02:16, Linus Torvalds wrote:
>
>> On Wed, 23 Apr 2008, Adrian Bunk wrote:
>>> If it goes like the regs removal in one big patch around -rc1 into
>>> your tree this shouldn't be a problem.
>>
>> Well, the regs removal had a real upside (it wasn't even sensible for
>> all irq types), and really nobody used it apart from "system users"
>> (ie Sysrq etc).
>>
>> I'm still waiting for anybody mentioning any upside at _all_ on
>> removing "irq".
>
> Saves another 4 bytes of stack? :-/ Seriously, Jeff can probably better
> answer himself but when this was posted before:
>
> http://lkml.org/lkml/2007/5/19/23
>
> Eric Biederman said it fit nicely into his "nefarious plan of making
> everything use a struct irq pointer". A later mention:
>
> http://lkml.org/lkml/2007/10/19/66
>
> got strong ACKs from Thomas Gleixner, Ingo Molnar and Greg KH. Remember
> due to working on a local driver at the time and deleting the "irq"
> argument usage from its handler (unneccesarily used in a debugging
> printk) from it in response.

Thanks. I was hoping that some of the people who expressed interest in
prior threads would appear.

Answering Linus's question, the things I tend to think of are

* it's not used in overwhelming majority of cases

* irq number has morphed over time with MSI-X and APICs and such from a
direct "reference" to a hardware line to a more abstract cookie value.

* the need for a struct [pci_]device everywhere means drivers have ready
access to irq number _anyway_

* it has clearly led to many helpful cleanups and bug fixes, by both me
and others [and yes, for the sake of argument I'm excluding those
discussed in this thread]

* it helps clean up abuses like HPET where it is used to encode data
(ignoring dev_id unnecessarily... I posted a patch to fix this):

if (rtc_int_flag) {
rtc_int_flag |= (RTC_IRQF | (RTC_NUM_INTS << 8));
if (irq_handler)
irq_handler(rtc_int_flag, dev_id);
}
["irq_handler" is a function passed to request_irq, as well as being
called here]

dev_id exists for passing various data to the irq_handler... with some
drivers abusing the 'irq' argument to pass data, that potential opens
holes for bugs whenever the irq numbering (aka cookie) scheme is changed
-- because changing the cookie scheme could potentially trigger code like

if (irq == MAGIC_NUMBER)
this is an internal self-call, do some polling
else
handle real hardware-raised interrupt
When drivers make assumptions about system irq numbering, particularly
on x86, IMO the situation is fragile.

Jeff





\
 
 \ /
  Last update: 2008-04-24 04:13    [from the cache]
©2003-2011 Jasper Spaans