[RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers - Kernel

This is a discussion on [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers - Kernel ; This patch series implements support for threaded irq handlers for the generic IRQ layer. Threaded interrupt handlers are not only interesting in the preempt-rt context. Threaded interrupt handlers can help to address common problems in the interrupt handling code: - ...

+ Reply to Thread
Page 1 of 3 1 2 3 LastLast
Results 1 to 20 of 47

Thread: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

  1. [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    This patch series implements support for threaded irq handlers for the
    generic IRQ layer.

    Threaded interrupt handlers are not only interesting in the preempt-rt
    context. Threaded interrupt handlers can help to address common
    problems in the interrupt handling code:

    - move long running handlers out of the hard interrupt context

    - avoid complex hardirq -> tasklet/softirq interaction and locking
    problems by integration of this functionality into the threaded
    handler code

    - improved debugability of the kernel: faulty handlers do not take
    down the system.

    - allows prioritizing of the handlers which share an interrupt line

    The implementation provides an opt-in mechanism to convert drivers to
    the threaded interrupt handler model contrary to the preempt-rt patch
    where the threaded handlers are enabled by a brute force switch. The
    brute force switch is suboptimal as it does not change the interrupt
    handler -> tasklet/softirq interaction problems, but was the only way
    which was possible for the limited man power of the preempt-rt
    developers.

    Converting an interrupt to threaded makes only sense when the handler
    code takes advantage of it by integrating tasklet/softirq
    functionality and simplifying the locking.

    When a driver wants to use threaded interrupt handlers it needs to
    provide a quick check handler function, which checks whether the
    interrupt was originated from the device or not.

    In case it was originated from the device the quick check handler must
    disable the interrupt at the device level and return
    IRQ_WAKE_THREAD. The generic interrupt handling core then sets the
    IRQF_RUNTHREAD in the irqaction of the handler and wakes the
    associated thread.

    The irqaction is referenced in the threads task_struct so handlers can
    check for newly arrived interrupts in action flags. Aside of that the
    reference is used to prevent further referencing of the thread in the
    interrupt code in the case of segfault to make sure that the system
    (minus the now dead interrupt handler) survives and debug information
    can be retrieved. In the best case the driver can be restarted, but
    dont expect that as a given.

    I hope to have a real converted driver (aside of the dummy module used
    for testing) ready real soon - as far as bugs/regressions stay out of
    my way for a while.

    Comments, reviews, flames as usual.

    Thanks,

    tglx

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [RFC patch 5/5] genirq: make irq threading robust

    To make sure that a crashed irq thread does not cause more trouble
    when the irq code tries to wake up a gone thread or the device code
    calling free_irq and trying to kthread_stop the dead thread, we plug a
    pointer to irqaction into task_struct, which is evaluated in
    do_exit(). When the thread crashes the do_exit code marks the thread
    as DIED in irqaction->flags to prevent further wakeups from the
    interrupt handler code.

    On thread creation we get a reference to task_struct so it stays
    around until the free_irq code releases it again.

    The procedure vs. the crashed irq handler thread is slightly racy, but
    we do not want to have additional locking in the hard interrupt code
    path. The worst things which can happen are a warning that we tried to
    wakeup a dead task and a hung kthread_stop in free_irq. I'm not
    worried about that at all, as removing a module which had a crashed
    interrupt handler is critical anyway.

    The main purpose of this is to keep the system alive w/o the affected
    device working.

    Signed-off-by: Thomas Gleixner
    Reviewed-by: Ingo Molnar
    ---
    include/linux/interrupt.h | 3 +++
    include/linux/sched.h | 1 +
    kernel/exit.c | 2 ++
    kernel/irq/handle.c | 13 +++++++++++--
    kernel/irq/manage.c | 44 +++++++++++++++++++++++++++++++++++++++++---
    5 files changed, 58 insertions(+), 5 deletions(-)

    Index: linux-2.6-tip/include/linux/interrupt.h
    ================================================== =================
    --- linux-2.6-tip.orig/include/linux/interrupt.h
    +++ linux-2.6-tip/include/linux/interrupt.h
    @@ -61,6 +61,7 @@
    #define IRQF_THREADED 0x00002000
    #define IRQF_RUNTHREAD 0x00004000
    #define IRQF_WARNED_THREADED 0x00008000
    +#define IRQF_THREAD_DIED 0x00010000

    typedef irqreturn_t (*irq_handler_t)(int, void *);

    @@ -114,6 +115,8 @@ static inline int irq_thread_should_run(
    return test_and_clear_bit(IRQF_RUNTHREAD, &action->flags);
    }

    +extern void exit_irq_thread(struct task_struct *tsk);
    +
    /*
    * On lockdep we dont want to enable hardirqs in hardirq
    * context. Use local_irq_enable_in_hardirq() to annotate
    Index: linux-2.6-tip/include/linux/sched.h
    ================================================== =================
    --- linux-2.6-tip.orig/include/linux/sched.h
    +++ linux-2.6-tip/include/linux/sched.h
    @@ -1301,6 +1301,7 @@ struct task_struct {
    int latency_record_count;
    struct latency_record latency_record[LT_SAVECOUNT];
    #endif
    + struct irqaction *irqaction;
    };

    /*
    Index: linux-2.6-tip/kernel/exit.c
    ================================================== =================
    --- linux-2.6-tip.orig/kernel/exit.c
    +++ linux-2.6-tip/kernel/exit.c
    @@ -1030,6 +1030,8 @@ NORET_TYPE void do_exit(long code)
    schedule();
    }

    + exit_irq_thread(tsk);
    +
    exit_signals(tsk); /* sets PF_EXITING */
    /*
    * tsk->flags are checked in the futex code to protect against
    Index: linux-2.6-tip/kernel/irq/handle.c
    ================================================== =================
    --- linux-2.6-tip.orig/kernel/irq/handle.c
    +++ linux-2.6-tip/kernel/irq/handle.c
    @@ -161,8 +161,17 @@ irqreturn_t handle_IRQ_event(unsigned in
    set_bit(IRQF_WARNED_THREADED, &action->flags);

    case IRQ_WAKE_THREAD:
    - set_bit(IRQF_RUNTHREAD, &action->flags);
    - wake_up_process(action->thread);
    + /*
    + * In case the thread crashed and was killed
    + * we just pretend that we handled the
    + * interrupt. The quick check handler has
    + * disabled the device interrupt, so no irq
    + * storm is lurking.
    + */
    + if (likely(!(action->flags & IRQF_THREAD_DIED))) {
    + set_bit(IRQF_RUNTHREAD, &action->flags);
    + wake_up_process(action->thread);
    + }
    /*
    * Set it to handled so the spurious check
    * does not trigger.
    Index: linux-2.6-tip/kernel/irq/manage.c
    ================================================== =================
    --- linux-2.6-tip.orig/kernel/irq/manage.c
    +++ linux-2.6-tip/kernel/irq/manage.c
    @@ -338,6 +338,8 @@ static int irq_thread(void *data)
    {
    struct irqaction *action = data;

    + current->irqaction = action;
    +
    set_current_state(TASK_INTERRUPTIBLE);

    while (!kthread_should_stop()) {
    @@ -351,11 +353,36 @@ static int irq_thread(void *data)
    action->handler(action->irq, action->dev_id);
    set_current_state(TASK_INTERRUPTIBLE);
    }
    + /*
    + * Clear irqaction. Otherwise exit_irq_thread() would make
    + * fuzz about an active irq thread going into nirvana.
    + */
    + current->irqaction = NULL;
    __set_current_state(TASK_RUNNING);
    return 0;
    }

    /*
    + * Called from do_exit()
    + */
    +void exit_irq_thread(struct task_struct *tsk)
    +{
    + if (!tsk->irqaction)
    + return;
    +
    + printk(KERN_ERR
    + "exiting task \"%s\" (%d) is an active IRQ thread (irq %d)\n",
    + tsk->comm ? tsk->comm : "", tsk->pid, tsk->irqaction->irq);
    +
    + /*
    + * Set the THREAD DIED flag to prevent further wakeups of the
    + * soon to be gone threaded handler.
    + */
    + set_bit(IRQF_THREAD_DIED, &tsk->irqaction->flags);
    + tsk->irqaction = NULL;
    +}
    +
    +/*
    * Internal function to register an irqaction - typically used to
    * allocate special interrupts that are part of the architecture.
    */
    @@ -439,7 +466,12 @@ int setup_irq(unsigned int irq, struct i
    new->name);
    if (IS_ERR(t))
    return PTR_ERR(t);
    -
    + /*
    + * We keep the reference to the task struct even if
    + * the thread dies to avoid that the interrupt code
    + * references an already gone task_struct.
    + */
    + get_task_struct(t);
    new->thread = t;
    }

    @@ -565,8 +597,14 @@ void free_irq(unsigned int irq, void *de
    if (desc->chip->release)
    desc->chip->release(irq, dev_id);
    #endif
    - if (action->thread)
    - kthread_stop(action->thread);
    + if (action->thread) {
    + struct task_struct *t = action->thread;
    +
    + action->thread = NULL;
    + if (likely(!(action->flags & IRQF_THREAD_DIED)))
    + kthread_stop(t);
    + put_task_struct(t);
    + }

    if (!desc->action) {
    desc->status |= IRQ_DISABLED;


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. [RFC patch 2/5] genirq: add a quick check handler

    Preparatory patch for threaded interrupt handlers.

    Adds a quick check handler which is called before the real handler.
    The quick check handler can decide whether the interrupt was originated
    from the device or not. It can also declare the interrupt as handled
    and subsequently avoid that the real handler is called.

    Signed-off-by: Thomas Gleixner
    Reviewed-by: Ingo Molnar
    ---
    include/linux/interrupt.h | 14 +++++++++++++-
    include/linux/irqreturn.h | 2 ++
    kernel/irq/handle.c | 18 +++++++++++++++---
    kernel/irq/manage.c | 19 +++++++++++++------
    4 files changed, 43 insertions(+), 10 deletions(-)

    Index: linux-2.6-tip/include/linux/interrupt.h
    ================================================== =================
    --- linux-2.6-tip.orig/include/linux/interrupt.h
    +++ linux-2.6-tip/include/linux/interrupt.h
    @@ -58,6 +58,7 @@
    typedef irqreturn_t (*irq_handler_t)(int, void *);

    struct irqaction {
    + irq_handler_t quick_check_handler;
    irq_handler_t handler;
    unsigned long flags;
    cpumask_t mask;
    @@ -69,8 +70,19 @@ struct irqaction {
    };

    extern irqreturn_t no_action(int cpl, void *dev_id);
    -extern int __must_check request_irq(unsigned int, irq_handler_t handler,
    +
    +extern int __must_check
    +request_irq_quickcheck(unsigned int, irq_handler_t handler,
    + irq_handler_t quick_check_handler,
    unsigned long, const char *, void *);
    +
    +static inline int __must_check
    +request_irq(unsigned int irq, irq_handler_t handler, unsigned long flags,
    + const char *name, void *dev)
    +{
    + return request_irq_quickcheck(irq, handler, NULL, flags, name, dev);
    +}
    +
    extern void free_irq(unsigned int, void *);

    struct device;
    Index: linux-2.6-tip/include/linux/irqreturn.h
    ================================================== =================
    --- linux-2.6-tip.orig/include/linux/irqreturn.h
    +++ linux-2.6-tip/include/linux/irqreturn.h
    @@ -5,10 +5,12 @@
    * enum irqreturn
    * @IRQ_NONE interrupt was not from this device
    * @IRQ_HANDLED interrupt was handled by this device
    + * @IRQ_NEEDS_HANDLING quick check handler requests to run real handler
    */
    enum irqreturn {
    IRQ_NONE,
    IRQ_HANDLED,
    + IRQ_NEEDS_HANDLING,
    };

    #define irqreturn_t enum irqreturn
    Index: linux-2.6-tip/kernel/irq/handle.c
    ================================================== =================
    --- linux-2.6-tip.orig/kernel/irq/handle.c
    +++ linux-2.6-tip/kernel/irq/handle.c
    @@ -137,9 +137,21 @@ irqreturn_t handle_IRQ_event(unsigned in
    local_irq_enable_in_hardirq();

    do {
    - ret = action->handler(irq, action->dev_id);
    - if (ret == IRQ_HANDLED)
    - status |= action->flags;
    + if (action->quick_check_handler)
    + ret = action->quick_check_handler(irq, action->dev_id);
    + else
    + ret = IRQ_NEEDS_HANDLING;
    +
    + switch (ret) {
    + default:
    + break;
    +
    + case IRQ_NEEDS_HANDLING:
    + ret = action->handler(irq, action->dev_id);
    + if (ret == IRQ_HANDLED)
    + status |= action->flags;
    + break;
    + }
    retval |= ret;
    action = action->next;
    } while (action);
    Index: linux-2.6-tip/kernel/irq/manage.c
    ================================================== =================
    --- linux-2.6-tip.orig/kernel/irq/manage.c
    +++ linux-2.6-tip/kernel/irq/manage.c
    @@ -563,9 +563,14 @@ void free_irq(unsigned int irq, void *de
    EXPORT_SYMBOL(free_irq);

    /**
    - * request_irq - allocate an interrupt line
    + * request_irq_quickcheck - allocate an interrupt line
    * @irq: Interrupt line to allocate
    - * @handler: Function to be called when the IRQ occurs
    + * @handler: Function to be called when the IRQ occurs.
    + * Primary handler for threaded interrupts
    + * @quick_check_handler: Function to be called when the interrupt
    + * before the real handler is called to check
    + * whether the interrupt originated from the device
    + * can be NULL
    * @irqflags: Interrupt type flags
    * @devname: An ascii name for the claiming device
    * @dev_id: A cookie passed back to the handler function
    @@ -589,10 +594,11 @@ EXPORT_SYMBOL(free_irq);
    * IRQF_SHARED Interrupt is shared
    * IRQF_DISABLED Disable local interrupts while processing
    * IRQF_SAMPLE_RANDOM The interrupt can be used for entropy
    - *
    */
    -int request_irq(unsigned int irq, irq_handler_t handler,
    - unsigned long irqflags, const char *devname, void *dev_id)
    +int request_irq_quickcheck(unsigned int irq, irq_handler_t handler,
    + irq_handler_t quick_check_handler,
    + unsigned long irqflags, const char *devname,
    + void *dev_id)
    {
    struct irqaction *action;
    int retval;
    @@ -622,6 +628,7 @@ int request_irq(unsigned int irq, irq_ha
    if (!action)
    return -ENOMEM;

    + action->quick_check_handler = quick_check_handler;
    action->handler = handler;
    action->flags = irqflags;
    cpus_clear(action->mask);
    @@ -651,4 +658,4 @@ int request_irq(unsigned int irq, irq_ha

    return retval;
    }
    -EXPORT_SYMBOL(request_irq);
    +EXPORT_SYMBOL(request_irq_quickcheck);


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. [RFC patch 1/5] genirq: make irqreturn_t an enum

    Remove the 2.4 compabiliy cruft

    Signed-off-by: Thomas Gleixner
    Reviewed-by: Ingo Molnar
    ---
    include/linux/irq.h | 4 ++--
    include/linux/irqreturn.h | 28 ++++++++++------------------
    2 files changed, 12 insertions(+), 20 deletions(-)

    Index: linux-2.6-tip/include/linux/irq.h
    ================================================== =================
    --- linux-2.6-tip.orig/include/linux/irq.h
    +++ linux-2.6-tip/include/linux/irq.h
    @@ -252,7 +252,7 @@ static inline int irq_balancing_disabled
    }

    /* Handle irq action chains: */
    -extern int handle_IRQ_event(unsigned int irq, struct irqaction *action);
    +extern irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction *action);

    /*
    * Built-in IRQ handlers for various IRQ types,
    @@ -294,7 +294,7 @@ static inline void generic_handle_irq(un

    /* Handling of unhandled and spurious interrupts: */
    extern void note_interrupt(unsigned int irq, struct irq_desc *desc,
    - int action_ret);
    + irqreturn_t action_ret);

    /* Resending of interrupts :*/
    void check_irq_resend(struct irq_desc *desc, unsigned int irq);
    Index: linux-2.6-tip/include/linux/irqreturn.h
    ================================================== =================
    --- linux-2.6-tip.orig/include/linux/irqreturn.h
    +++ linux-2.6-tip/include/linux/irqreturn.h
    @@ -1,25 +1,17 @@
    -/* irqreturn.h */
    #ifndef _LINUX_IRQRETURN_H
    #define _LINUX_IRQRETURN_H

    -/*
    - * For 2.4.x compatibility, 2.4.x can use
    - *
    - * typedef void irqreturn_t;
    - * #define IRQ_NONE
    - * #define IRQ_HANDLED
    - * #define IRQ_RETVAL(x)
    - *
    - * To mix old-style and new-style irq handler returns.
    - *
    - * IRQ_NONE means we didn't handle it.
    - * IRQ_HANDLED means that we did have a valid interrupt and handled it.
    - * IRQ_RETVAL(x) selects on the two depending on x being non-zero (for handled)
    +/**
    + * enum irqreturn
    + * @IRQ_NONE interrupt was not from this device
    + * @IRQ_HANDLED interrupt was handled by this device
    */
    -typedef int irqreturn_t;
    +enum irqreturn {
    + IRQ_NONE,
    + IRQ_HANDLED,
    +};

    -#define IRQ_NONE (0)
    -#define IRQ_HANDLED (1)
    -#define IRQ_RETVAL(x) ((x) != 0)
    +#define irqreturn_t enum irqreturn
    +#define IRQ_RETVAL(x) ((x) != IRQ_NONE)

    #endif


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    On Wed, 01 Oct 2008 23:02:08 -0000
    Thomas Gleixner wrote:

    > This patch series implements support for threaded irq handlers for the
    > generic IRQ layer.
    >
    > ...
    >
    > I hope to have a real converted driver (aside of the dummy module used
    > for testing) ready real soon


    That would be nice


    I'm a bit surprised to see that there is no facility for per-cpu
    interrupt threads?

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    On Wed, 1 Oct 2008 16:23:33 -0700
    Andrew Morton wrote:

    >
    > I'm a bit surprised to see that there is no facility for per-cpu
    > interrupt threads?
    >


    per handler is the right approach (that way, if one dies, all other
    interrupts will likely keep working)

    now.. normally an interrupt only goes to one cpu, so effectively it is
    per cpu already anyway

    we should however make the irq threads follow the affinity masks of the
    irq... that'd be an easy add-on and probably worthwhile.


    --
    Arjan van de Ven Intel Open Source Technology Centre
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    On Wed, 1 Oct 2008 16:29:50 -0700
    Arjan van de Ven wrote:

    > On Wed, 1 Oct 2008 16:23:33 -0700
    > Andrew Morton wrote:
    >
    > >
    > > I'm a bit surprised to see that there is no facility for per-cpu
    > > interrupt threads?
    > >

    >
    > per handler is the right approach (that way, if one dies, all other
    > interrupts will likely keep working)
    >
    > now.. normally an interrupt only goes to one cpu, so effectively it is
    > per cpu already anyway


    Yes, if a) the thread was asleep when it was woken up and b) if the
    scheduler does the right thing and wakes the thread on the CPU which
    called wake_up().

    The ongoing sagas of tbench/mysql/volanomark regressions make me think
    that any behaviour which we "expect" of the scheduler should be
    triple-checked daily

    > we should however make the irq threads follow the affinity masks of the
    > irq... that'd be an easy add-on and probably worthwhile.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    On Wed, 1 Oct 2008, Andrew Morton wrote:

    > On Wed, 1 Oct 2008 16:29:50 -0700
    > Arjan van de Ven wrote:
    >
    > > On Wed, 1 Oct 2008 16:23:33 -0700
    > > Andrew Morton wrote:
    > >
    > > >
    > > > I'm a bit surprised to see that there is no facility for per-cpu
    > > > interrupt threads?
    > > >

    > >
    > > per handler is the right approach (that way, if one dies, all other
    > > interrupts will likely keep working)
    > >
    > > now.. normally an interrupt only goes to one cpu, so effectively it is
    > > per cpu already anyway

    >
    > Yes, if a) the thread was asleep when it was woken up and b) if the
    > scheduler does the right thing and wakes the thread on the CPU which
    > called wake_up().
    >
    > The ongoing sagas of tbench/mysql/volanomark regressions make me think
    > that any behaviour which we "expect" of the scheduler should be
    > triple-checked daily


    Yup. I missed that detail when I dusted off the moldy patches.

    Of course we need to pin the thread to the affinity mask of the
    hardware interrupt.

    /me goes back to do home work

    Thanks,

    tglx
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [RFC patch 2/5] genirq: add a quick check handler

    On Wed, 2008-10-01 at 23:02 +0000, Thomas Gleixner wrote:

    > struct irqaction {
    > + irq_handler_t quick_check_handler;
    > irq_handler_t handler;


    When we originally discussed this, there was an idea to modify the
    request_irq API to take this handler and an IRQF_THREADED type to mark
    the interrupt accordingly. I understand why it's a separate function in
    this implementation for ease of migration, but what do you think should
    happen in the end? Also, I suggest calling this something like
    "quiesce_device" because the quickcheck also needs to do that.

    We probably need some documentation eventually so people realize what
    this "quickcheck" handler is for and what it's not for - under no
    circumstances should anything more than the bare minimum be done.
    Otherwise it breaks the benefit of deferred threaded handling. It's hard
    to enforce that - but this is *not* a return of top/bottom half handling
    where you can do whatever crap you like in the quickcheck bit.

    Jon.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    On Wed, 2008-10-01 at 23:02 +0000, Thomas Gleixner wrote:

    > I hope to have a real converted driver (aside of the dummy module used
    > for testing) ready real soon - as far as bugs/regressions stay out of
    > my way for a while.


    Sven and I started poking at the various USB host drivers on the flight
    back from Plumbers. I'll see if I can convert over a few here on the
    systems that I have and send over some patches.

    Jon.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [RFC patch 5/5] genirq: make irq threading robust

    On Wed, 2008-10-01 at 23:02 +0000, Thomas Gleixner wrote:

    > To make sure that a crashed irq thread does not cause more trouble
    > when the irq code tries to wake up a gone thread or the device code
    > calling free_irq and trying to kthread_stop the dead thread, we plug a
    > pointer to irqaction into task_struct, which is evaluated in
    > do_exit(). When the thread crashes the do_exit code marks the thread
    > as DIED in irqaction->flags to prevent further wakeups from the
    > interrupt handler code.


    > @@ -1301,6 +1301,7 @@ struct task_struct {
    > int latency_record_count;
    > struct latency_record latency_record[LT_SAVECOUNT];
    > #endif
    > + struct irqaction *irqaction;
    > };


    Is that going to fly? For the vast majority of task_structs this is now
    a wasted 4/8 bytes that won't be used.

    Jon.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [RFC patch 2/5] genirq: add a quick check handler


    Thomas,

    Nice work, thanks for doing this.

    Little comments below.

    On Wed, 1 Oct 2008, Thomas Gleixner wrote:
    >
    > extern irqreturn_t no_action(int cpl, void *dev_id);
    > -extern int __must_check request_irq(unsigned int, irq_handler_t handler,
    > +
    > +extern int __must_check
    > +request_irq_quickcheck(unsigned int, irq_handler_t handler,
    > + irq_handler_t quick_check_handler,
    > unsigned long, const char *, void *);


    It would be nice to still keep the name of the parameters here.
    unsigned long flags, const char *name, void *dev

    > +
    > +static inline int __must_check
    > +request_irq(unsigned int irq, irq_handler_t handler, unsigned long flags,
    > + const char *name, void *dev)
    > +{
    > + return request_irq_quickcheck(irq, handler, NULL, flags, name, dev);
    > +}
    > +
    > extern void free_irq(unsigned int, void *);
    >
    > struct device;
    > Index: linux-2.6-tip/include/linux/irqreturn.h
    > ================================================== =================
    > --- linux-2.6-tip.orig/include/linux/irqreturn.h
    > +++ linux-2.6-tip/include/linux/irqreturn.h
    > @@ -5,10 +5,12 @@
    > * enum irqreturn
    > * @IRQ_NONE interrupt was not from this device
    > * @IRQ_HANDLED interrupt was handled by this device
    > + * @IRQ_NEEDS_HANDLING quick check handler requests to run real handler
    > */
    > enum irqreturn {
    > IRQ_NONE,
    > IRQ_HANDLED,
    > + IRQ_NEEDS_HANDLING,
    > };
    >
    > #define irqreturn_t enum irqreturn
    > Index: linux-2.6-tip/kernel/irq/handle.c
    > ================================================== =================
    > --- linux-2.6-tip.orig/kernel/irq/handle.c
    > +++ linux-2.6-tip/kernel/irq/handle.c
    > @@ -137,9 +137,21 @@ irqreturn_t handle_IRQ_event(unsigned in
    > local_irq_enable_in_hardirq();
    >
    > do {
    > - ret = action->handler(irq, action->dev_id);
    > - if (ret == IRQ_HANDLED)
    > - status |= action->flags;
    > + if (action->quick_check_handler)
    > + ret = action->quick_check_handler(irq, action->dev_id);
    > + else
    > + ret = IRQ_NEEDS_HANDLING;
    > +
    > + switch (ret) {
    > + default:
    > + break;
    > +
    > + case IRQ_NEEDS_HANDLING:
    > + ret = action->handler(irq, action->dev_id);
    > + if (ret == IRQ_HANDLED)
    > + status |= action->flags;
    > + break;
    > + }
    > retval |= ret;
    > action = action->next;
    > } while (action);
    > Index: linux-2.6-tip/kernel/irq/manage.c
    > ================================================== =================
    > --- linux-2.6-tip.orig/kernel/irq/manage.c
    > +++ linux-2.6-tip/kernel/irq/manage.c
    > @@ -563,9 +563,14 @@ void free_irq(unsigned int irq, void *de
    > EXPORT_SYMBOL(free_irq);
    >
    > /**
    > - * request_irq - allocate an interrupt line
    > + * request_irq_quickcheck - allocate an interrupt line
    > * @irq: Interrupt line to allocate
    > - * @handler: Function to be called when the IRQ occurs
    > + * @handler: Function to be called when the IRQ occurs.
    > + * Primary handler for threaded interrupts
    > + * @quick_check_handler: Function to be called when the interrupt
    > + * before the real handler is called to check
    > + * whether the interrupt originated from the device
    > + * can be NULL



    The above reads funny. How about something like:

    @quick_check_handler: Function called before the real interrupt
    handler. It checks if the interrupt originated
    from the device. This can be NULL.

    -- Steve


    > * @irqflags: Interrupt type flags
    > * @devname: An ascii name for the claiming device
    > * @dev_id: A cookie passed back to the handler function
    > @@ -589,10 +594,11 @@ EXPORT_SYMBOL(free_irq);
    > * IRQF_SHARED Interrupt is shared
    > * IRQF_DISABLED Disable local interrupts while processing
    > * IRQF_SAMPLE_RANDOM The interrupt can be used for entropy
    > - *
    > */
    > -int request_irq(unsigned int irq, irq_handler_t handler,
    > - unsigned long irqflags, const char *devname, void *dev_id)
    > +int request_irq_quickcheck(unsigned int irq, irq_handler_t handler,
    > + irq_handler_t quick_check_handler,
    > + unsigned long irqflags, const char *devname,
    > + void *dev_id)
    > {
    > struct irqaction *action;
    > int retval;
    > @@ -622,6 +628,7 @@ int request_irq(unsigned int irq, irq_ha
    > if (!action)
    > return -ENOMEM;
    >
    > + action->quick_check_handler = quick_check_handler;
    > action->handler = handler;
    > action->flags = irqflags;
    > cpus_clear(action->mask);
    > @@ -651,4 +658,4 @@ int request_irq(unsigned int irq, irq_ha
    >
    > return retval;
    > }
    > -EXPORT_SYMBOL(request_irq);
    > +EXPORT_SYMBOL(request_irq_quickcheck);
    >
    >
    >

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [RFC patch 3/5] genirq: add threaded interrupt handler support


    On Wed, 1 Oct 2008, Thomas Gleixner wrote:
    > #define irqreturn_t enum irqreturn
    > Index: linux-2.6-tip/kernel/irq/handle.c
    > ================================================== =================
    > --- linux-2.6-tip.orig/kernel/irq/handle.c
    > +++ linux-2.6-tip/kernel/irq/handle.c
    > @@ -143,13 +143,34 @@ irqreturn_t handle_IRQ_event(unsigned in
    > ret = IRQ_NEEDS_HANDLING;
    >
    > switch (ret) {
    > - default:
    > + case IRQ_NEEDS_HANDLING:
    > + if (!(action->flags & IRQF_THREADED)) {
    > + ret = action->handler(irq, action->dev_id);
    > + if (ret == IRQ_HANDLED)
    > + status |= action->flags;
    > + break;
    > + }
    > + /*
    > + * Warn once when a quick check handler asked
    > + * for invoking the threaded (main) handler
    > + * directly
    > + */
    > + WARN(!(action->flags & IRQF_WARNED_THREADED),
    > + "IRQ %d requested to run threaded handler "
    > + "in hard interrupt context\n", irq);
    > + set_bit(IRQF_WARNED_THREADED, &action->flags);


    Do you purposely fall through to the next case statement here?
    If so, could you please add a comment.

    /* fall through */

    or something similar so we know it's not a bug.

    > +
    > + case IRQ_WAKE_THREAD:
    > + set_bit(IRQF_RUNTHREAD, &action->flags);
    > + wake_up_process(action->thread);
    > + /*
    > + * Set it to handled so the spurious check
    > + * does not trigger.
    > + */
    > + ret = IRQ_HANDLED;
    > break;
    >
    > - case IRQ_NEEDS_HANDLING:
    > - ret = action->handler(irq, action->dev_id);
    > - if (ret == IRQ_HANDLED)
    > - status |= action->flags;
    > + default:
    > break;
    > }
    > retval |= ret;



    -- Steve

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [RFC patch 2/5] genirq: add a quick check handler


    On Wed, 1 Oct 2008, Jon Masters wrote:
    >
    > We probably need some documentation eventually so people realize what
    > this "quickcheck" handler is for and what it's not for - under no
    > circumstances should anything more than the bare minimum be done.
    > Otherwise it breaks the benefit of deferred threaded handling. It's hard
    > to enforce that - but this is *not* a return of top/bottom half handling
    > where you can do whatever crap you like in the quickcheck bit.
    >


    We could always implement something similar to what I was told Microsoft
    does (I was just told this, I don't know for fact). Time this function and
    if it takes longer than, say 50us, print a warning and kill the device
    ;-)

    -- Steve

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [RFC patch 5/5] genirq: make irq threading robust


    On Wed, 1 Oct 2008, Jon Masters wrote:

    > On Wed, 2008-10-01 at 23:02 +0000, Thomas Gleixner wrote:
    >
    > > To make sure that a crashed irq thread does not cause more trouble
    > > when the irq code tries to wake up a gone thread or the device code
    > > calling free_irq and trying to kthread_stop the dead thread, we plug a
    > > pointer to irqaction into task_struct, which is evaluated in
    > > do_exit(). When the thread crashes the do_exit code marks the thread
    > > as DIED in irqaction->flags to prevent further wakeups from the
    > > interrupt handler code.

    >
    > > @@ -1301,6 +1301,7 @@ struct task_struct {
    > > int latency_record_count;
    > > struct latency_record latency_record[LT_SAVECOUNT];
    > > #endif
    > > + struct irqaction *irqaction;
    > > };

    >
    > Is that going to fly? For the vast majority of task_structs this is now
    > a wasted 4/8 bytes that won't be used.


    Perhaps we could convert parts of the task_struct into a union.
    There is quite a lot of things that I'm not sure kernel threads use.

    fpu_counter? well, it is only one byte.
    binfmt?

    Do kernel threads use group_leader?

    What about the ptraced items?

    group ids/info on kernel threads?

    do kernel threads need robust futex info?

    This was just a quick look at the task struct. Perhaps we could separate
    out kernel only and user space only info and bring the total size down?

    Although I'm not sure how much is there that is kernel_thread only :-/
    Of course this irqaction will be.

    -- Steve
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [RFC patch 2/5] genirq: add a quick check handler

    On Thu, 2008-10-02 at 01:09 -0400, Steven Rostedt wrote:
    > On Wed, 1 Oct 2008, Jon Masters wrote:
    > >
    > > We probably need some documentation eventually so people realize what
    > > this "quickcheck" handler is for and what it's not for - under no
    > > circumstances should anything more than the bare minimum be done.
    > > Otherwise it breaks the benefit of deferred threaded handling. It's hard
    > > to enforce that - but this is *not* a return of top/bottom half handling
    > > where you can do whatever crap you like in the quickcheck bit.
    > >

    >
    > We could always implement something similar to what I was told Microsoft
    > does (I was just told this, I don't know for fact). Time this function and
    > if it takes longer than, say 50us, print a warning and kill the device
    > ;-)


    You know, it's funny you suggested that because I thought about going
    there. But there's probably some silly patent on that groundshattering
    Microsoft solution to the halting problem.

    Anyway, I like to think we in the Linux community trust developers to do
    the right thing more than Microsoft does

    Jon.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    On Wed, 2008-10-01 at 23:02 +0000, Thomas Gleixner wrote:
    > The implementation provides an opt-in mechanism to convert drivers to
    > the threaded interrupt handler model contrary to the preempt-rt patch
    > where the threaded handlers are enabled by a brute force switch. The
    > brute force switch is suboptimal as it does not change the interrupt
    > handler -> tasklet/softirq interaction problems, but was the only way
    > which was possible for the limited man power of the preempt-rt
    > developers.
    >
    > Converting an interrupt to threaded makes only sense when the handler
    > code takes advantage of it by integrating tasklet/softirq
    > functionality and simplifying the locking.


    I'm not clear on your direction here.. I don't have a problem with a
    mass driver audit, which I think is what your suggesting with this patch
    set .. However, a mass audit like that would push a fully real time
    system out for quite some time..

    I also don't see a clear connection between these changes and ultimately
    removing spinlock level latency in the kernel. I realize you don't
    address that in your comments, but this is part of the initiative to
    remove spinlock level latency..

    So with this set of changes and in terms of real time, I'm wonder your
    going with this ?

    Daniel

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    Thomas Gleixner writes:
    >
    > - move long running handlers out of the hard interrupt context


    I'm not sure I'm really looking forward to this brave new world
    of very long running interrupt handlers. e.g. what do you
    do for example when some handler blocks for a very long time?

    > - improved debugability of the kernel: faulty handlers do not take
    > down the system.


    I had an old patch to handle this without threaded interrupts.

    What normally happens is when a interrupt oopses it tries to kill the
    idle process which panics. My fix was to just restart another idle
    process instead of panicing.

    But back then it was rejected by Linus with the argument that
    a crashing interrupt handler will typically hold some lock
    and the next time the interrupt happens it will deadlock on that
    lock.

    Has that changed with your threaded interrupts?

    If it has changed I suspect the restart idle change could
    be made to work to be equivalent in debuggability.

    > Comments, reviews, flames as usual.


    To be honest my opinion is that it will encourage badly written interrupt
    code longer term.

    -Andi

    --
    ak@linux.intel.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  19. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers


    On Wed, 1 Oct 2008, Daniel Walker wrote:
    > >
    > > Converting an interrupt to threaded makes only sense when the handler
    > > code takes advantage of it by integrating tasklet/softirq
    > > functionality and simplifying the locking.

    >
    > I'm not clear on your direction here.. I don't have a problem with a
    > mass driver audit, which I think is what your suggesting with this patch
    > set .. However, a mass audit like that would push a fully real time
    > system out for quite some time..


    This has nothing to do with real time, although it helps.

    >
    > I also don't see a clear connection between these changes and ultimately
    > removing spinlock level latency in the kernel. I realize you don't
    > address that in your comments, but this is part of the initiative to
    > remove spinlock level latency..


    This is a completely different topic.

    >
    > So with this set of changes and in terms of real time, I'm wonder your
    > going with this ?


    This helps with latencies and locking. With the current scheme of hardirq,
    softirq/tasklets, there are a lot of craziness with spin_locks and
    spin_lock_irqs and mutexes.

    By creating an interrupt thread, we can skip the softirq/tasklet
    altogether, and this simplifies locking.

    There are other cases where threaded interrupt handlers also improve
    performance. But we will get to those in due time.

    -- Steve

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [RFC patch 0/5] genirq: add infrastructure for threaded interrupt handlers

    On Thu, 2008-10-02 at 11:02 -0400, Steven Rostedt wrote:
    > On Wed, 1 Oct 2008, Daniel Walker wrote:
    > > >
    > > > Converting an interrupt to threaded makes only sense when the handler
    > > > code takes advantage of it by integrating tasklet/softirq
    > > > functionality and simplifying the locking.

    > >
    > > I'm not clear on your direction here.. I don't have a problem with a
    > > mass driver audit, which I think is what your suggesting with this patch
    > > set .. However, a mass audit like that would push a fully real time
    > > system out for quite some time..

    >
    > This has nothing to do with real time, although it helps.


    Clearly threading irq handlers does have something to do with real time,
    unless this patch isn't actually threading anything ..

    > >
    > > I also don't see a clear connection between these changes and ultimately
    > > removing spinlock level latency in the kernel. I realize you don't
    > > address that in your comments, but this is part of the initiative to
    > > remove spinlock level latency..

    >
    > This is a completely different topic.


    It's all connected to the removal of latency .. One part depending on
    the other parts, so you can't disconnect this from the rest of it.

    > >
    > > So with this set of changes and in terms of real time, I'm wonder your
    > > going with this ?

    >
    > This helps with latencies and locking. With the current scheme of hardirq,
    > softirq/tasklets, there are a lot of craziness with spin_locks and
    > spin_lock_irqs and mutexes.
    >
    > By creating an interrupt thread, we can skip the softirq/tasklet
    > altogether, and this simplifies locking.


    How does this simplify locking ?

    Daniel

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 1 of 3 1 2 3 LastLast