[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20090226150340.976f4381.akpm@linux-foundation.org>
Date: Thu, 26 Feb 2009 15:03:40 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, mingo@...e.hu, peterz@...radead.org,
arjan@...radead.org, rostedt@...dmis.org, jonathan@...masters.org
Subject: Re: [patch 3/4] genirq: add a quick check handler
On Thu, 26 Feb 2009 13:28:18 -0000
Thomas Gleixner <tglx@...utronix.de> wrote:
> 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.
>
> ...
>
> 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
> @@ -62,6 +62,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;
> @@ -73,8 +74,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,
> - unsigned long, const char *, void *);
> +
> +extern int __must_check
> +request_irq_quickcheck(unsigned int irq, irq_handler_t handler,
> + irq_handler_t quick_check_handler,
> + 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,
> };
The enquiring mind is wondering which of these values the quickcheck
handler can return. IRQ_NEEDS_HANDLING or IRQ_NONE? Or can it
legitimately return IRQ_HANDLED? If so, what would that semantically
mean?
I mean, an IRQ handler could easily have a super-fast-path and a slow
path. It could decide to do the super-fast operation in hard irq
context and return IRQ_HANDLED, and return IRQ_NEEDS_HANDLING if
slow-path handling is needed?
It's all a bit unclear and deserves documenting and thinking about.
> typedef enum irqreturn irqreturn_t;
> 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
> @@ -354,9 +354,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
> @@ -641,9 +641,13 @@ 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 called before the real interrupt
> + * handler. It checks if the interrupt originated
> + * from the device. This can be NULL.
So what semantics are implemented if this pointer is NULL? We just
assume IRQ_NEEDS_HANDLING?
> * @irqflags: Interrupt type flags
> * @devname: An ascii name for the claiming device
> * @dev_id: A cookie passed back to the handler function
> @@ -670,8 +674,10 @@ EXPORT_SYMBOL(free_irq);
> * IRQF_TRIGGER_* Specify active edge(s) or level
> *
> */
> -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;
> struct irq_desc *desc;
> @@ -718,6 +724,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;
> action->name = devname;
> @@ -748,4 +755,4 @@ int request_irq(unsigned int irq, irq_ha
> #endif
> 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@...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