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    [W:0.025 / U:181.576 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site