From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S268954AbUHZNIQ (ORCPT ); Thu, 26 Aug 2004 09:08:16 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S268989AbUHZNCw (ORCPT ); Thu, 26 Aug 2004 09:02:52 -0400 Received: from cantor.suse.de ([195.135.220.2]:56489 "EHLO Cantor.suse.de") by vger.kernel.org with ESMTP id S268985AbUHZMyS (ORCPT ); Thu, 26 Aug 2004 08:54:18 -0400 Date: Thu, 26 Aug 2004 14:50:52 +0200 Message-ID: From: Takashi Iwai To: Andrew Morton Cc: linux-kernel@vger.kernel.org, thomas@undata.org Subject: Re: [PATCH] Fix shared interrupt handling of SA_INTERRUPT and SA_SAMPLE_RANDOM In-Reply-To: <20040825134112.5aefaf8e.akpm@osdl.org> References: <20040824204508.3b31449f.akpm@osdl.org> <20040825134112.5aefaf8e.akpm@osdl.org> User-Agent: Wanderlust/2.10.1 (Watching The Wheels) SEMI/1.14.5 (Awara-Onsen) FLIM/1.14.5 (Demachiyanagi) APEL/10.6 MULE XEmacs/21.4 (patch 15) (Security Through Obscurity) (i386-suse-linux) MIME-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org At Wed, 25 Aug 2004 13:41:12 -0700, Andrew Morton wrote: > > Takashi Iwai wrote: > > > > Anyway, suppressing the unnecessary call of add_interrupt_randomness() > > should be still valid. The reduced patch is below. (snip) > > Shouldn't that be `if (ret == IRQ_HANDLED)'? Yes, it's more strict. > Probably I'll need to pass this to lots of other architecture maintainers > to make the same change. Please write a nice changelog so they understand > what your patch does. Here I patched more architectures. Please pass to arch maintainers if you thinks it's ok. Takashi == [PATCH] Fix the unnecessary entropy call in the irq handler Currently add_interrupt_randomness() is called at each interrupt when one of the handlers has SA_SAMPLE_RANDOM flag, regardless whether the interrupt is processed by that handler or not. This results in the higher latency and perfomance loss. The patch fixes this behavior to avoid the unnecessary call by checking the return value from each handler. Signed-off-by: Takashi Iwai --- linux-2.6.8.1/arch/sh/kernel/irq.c-dist 2004-08-26 14:20:41.536919578 +0200 +++ linux-2.6.8.1/arch/sh/kernel/irq.c 2004-08-26 14:21:00.638579701 +0200 @@ -137,14 +137,16 @@ unlock: int handle_IRQ_event(unsigned int irq, struct pt_regs * regs, struct irqaction * action) { int status = 1; /* Force the "do bottom halves" bit */ - int retval = 0; + int ret, retval = 0; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - retval |= action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; + retval |= ret; action = action->next; } while (action); --- linux-2.6.8.1/arch/um/kernel/irq.c-dist 2004-08-26 14:22:40.692851759 +0200 +++ linux-2.6.8.1/arch/um/kernel/irq.c 2004-08-26 14:23:03.695627509 +0200 @@ -155,13 +155,15 @@ int handle_IRQ_event(unsigned int irq, s struct irqaction * action) { int status = 1; /* Force the "do bottom halves" bit */ + int ret; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/arm/kernel/irq.c-dist 2004-08-26 12:52:34.666247196 +0200 +++ linux-2.6.8.1/arch/arm/kernel/irq.c 2004-08-26 12:53:31.595279800 +0200 @@ -261,7 +261,7 @@ static int __do_irq(unsigned int irq, struct irqaction *action, struct pt_regs *regs) { unsigned int status; - int retval = 0; + int ret, retval = 0; spin_unlock(&irq_controller_lock); @@ -270,8 +270,10 @@ __do_irq(unsigned int irq, struct irqact status = 0; do { - status |= action->flags; - retval |= action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; + retval |= ret; action = action->next; } while (action); --- linux-2.6.8.1/arch/ppc/kernel/irq.c-dist 2004-08-26 14:19:28.275567074 +0200 +++ linux-2.6.8.1/arch/ppc/kernel/irq.c 2004-08-26 14:19:50.988405487 +0200 @@ -414,13 +414,15 @@ static inline void handle_irq_event(int irq, struct pt_regs *regs, struct irqaction *action) { int status = 0; + int ret; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/i386/kernel/irq.c-dist 2004-08-25 13:13:05.000000000 +0200 +++ linux-2.6.8.1/arch/i386/kernel/irq.c 2004-08-26 12:48:25.203108557 +0200 @@ -220,14 +220,16 @@ asmlinkage int handle_IRQ_event(unsigned struct pt_regs *regs, struct irqaction *action) { int status = 1; /* Force the "do bottom halves" bit */ - int retval = 0; + int ret, retval = 0; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - retval |= action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; + retval |= ret; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/cris/kernel/irq.c-dist 2004-08-26 14:14:40.384855612 +0200 +++ linux-2.6.8.1/arch/cris/kernel/irq.c 2004-08-26 14:15:14.701072531 +0200 @@ -125,7 +125,7 @@ asmlinkage void do_IRQ(int irq, struct p { struct irqaction *action; int do_random, cpu; - int retval = 0; + int ret, retval = 0; cpu = smp_processor_id(); irq_enter(); @@ -137,8 +137,10 @@ asmlinkage void do_IRQ(int irq, struct p local_irq_enable(); do_random = 0; do { - do_random |= action->flags; - retval |= action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + do_random |= action->flags; + retval |= ret; action = action->next; } while (action); --- linux-2.6.8.1/arch/ia64/kernel/irq.c-dist 2004-08-26 12:50:25.147796632 +0200 +++ linux-2.6.8.1/arch/ia64/kernel/irq.c 2004-08-26 12:51:06.708311749 +0200 @@ -255,14 +255,16 @@ int handle_IRQ_event(unsigned int irq, struct pt_regs *regs, struct irqaction *action) { int status = 1; /* Force the "do bottom halves" bit */ - int retval = 0; + int ret, retval = 0; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - retval |= action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; + retval |= ret; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/mips/baget/irq.c-dist 2004-08-26 14:17:55.830532962 +0200 +++ linux-2.6.8.1/arch/mips/baget/irq.c 2004-08-26 14:18:34.502764418 +0200 @@ -180,7 +180,7 @@ skip: static void do_IRQ(int irq, struct pt_regs * regs) { struct irqaction *action; - int do_random, cpu; + int ret, do_random, cpu; cpu = smp_processor_id(); irq_enter(); @@ -194,8 +194,9 @@ static void do_IRQ(int irq, struct pt_re action = *(irq + irq_action); do_random = 0; do { - do_random |= action->flags; - action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + do_random |= action->flags; action = action->next; } while (action); if (do_random & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/mips/kernel/irq.c-dist 2004-08-26 14:18:41.770116666 +0200 +++ linux-2.6.8.1/arch/mips/kernel/irq.c 2004-08-26 14:19:08.196125140 +0200 @@ -144,14 +144,16 @@ inline void synchronize_irq(unsigned int int handle_IRQ_event(unsigned int irq, struct pt_regs * regs, struct irqaction * action) { int status = 1; /* Force the "do bottom halves" bit */ - int retval = 0; + int ret, retval = 0; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - retval |= action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; + retval |= ret; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/v850/kernel/irq.c-dist 2004-08-26 14:23:14.792107468 +0200 +++ linux-2.6.8.1/arch/v850/kernel/irq.c 2004-08-26 14:23:34.635601155 +0200 @@ -141,13 +141,15 @@ skip: int handle_IRQ_event(unsigned int irq, struct pt_regs * regs, struct irqaction * action) { int status = 1; /* Force the "do bottom halves" bit */ + int ret; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/sh64/kernel/irq.c-dist 2004-08-26 14:21:14.716381411 +0200 +++ linux-2.6.8.1/arch/sh64/kernel/irq.c 2004-08-26 14:21:36.941332490 +0200 @@ -148,6 +148,7 @@ asmlinkage void do_NMI(unsigned long vec int handle_IRQ_event(unsigned int irq, struct pt_regs * regs, struct irqaction * action) { int status; + int ret; status = 1; /* Force the "do bottom halves" bit */ @@ -155,8 +156,9 @@ int handle_IRQ_event(unsigned int irq, s local_irq_enable(); do { - status |= action->flags; - action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/alpha/kernel/irq.c-dist 2004-08-26 12:51:34.133057570 +0200 +++ linux-2.6.8.1/arch/alpha/kernel/irq.c 2004-08-26 12:52:02.341618752 +0200 @@ -83,6 +83,7 @@ handle_IRQ_event(unsigned int irq, struc struct irqaction *action) { int status = 1; /* Force the "do bottom halves" bit */ + int ret; do { if (!(action->flags & SA_INTERRUPT)) @@ -90,8 +91,9 @@ handle_IRQ_event(unsigned int irq, struc else local_irq_disable(); - status |= action->flags; - action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/arm26/kernel/irq.c-dist 2004-08-26 14:13:44.808461520 +0200 +++ linux-2.6.8.1/arch/arm26/kernel/irq.c 2004-08-26 14:14:08.377115505 +0200 @@ -187,6 +187,7 @@ static void __do_irq(unsigned int irq, struct irqaction *action, struct pt_regs *regs) { unsigned int status; + int ret; spin_unlock(&irq_controller_lock); if (!(action->flags & SA_INTERRUPT)) @@ -194,8 +195,9 @@ __do_irq(unsigned int irq, struct irqact status = 0; do { - status |= action->flags; - action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; action = action->next; } while (action); --- linux-2.6.8.1/arch/ppc64/kernel/irq.c-dist 2004-08-26 14:20:00.603220604 +0200 +++ linux-2.6.8.1/arch/ppc64/kernel/irq.c 2004-08-26 14:20:24.840713164 +0200 @@ -362,14 +362,16 @@ skip: int handle_irq_event(int irq, struct pt_regs *regs, struct irqaction *action) { int status = 0; - int retval = 0; + int ret, retval = 0; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - retval |= action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; + retval |= ret; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) --- linux-2.6.8.1/arch/x86_64/kernel/irq.c-dist 2004-08-26 12:48:38.518091949 +0200 +++ linux-2.6.8.1/arch/x86_64/kernel/irq.c 2004-08-26 12:51:19.752337402 +0200 @@ -213,13 +213,15 @@ inline void synchronize_irq(unsigned int int handle_IRQ_event(unsigned int irq, struct pt_regs * regs, struct irqaction * action) { int status = 1; /* Force the "do bottom halves" bit */ + int ret; if (!(action->flags & SA_INTERRUPT)) local_irq_enable(); do { - status |= action->flags; - action->handler(irq, action->dev_id, regs); + ret = action->handler(irq, action->dev_id, regs); + if (ret == IRQ_HANDLED) + status |= action->flags; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM)