From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S266170AbUHWRNa (ORCPT ); Mon, 23 Aug 2004 13:13:30 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S266183AbUHWRNa (ORCPT ); Mon, 23 Aug 2004 13:13:30 -0400 Received: from cantor.suse.de ([195.135.220.2]:31873 "EHLO Cantor.suse.de") by vger.kernel.org with ESMTP id S266170AbUHWRNL (ORCPT ); Mon, 23 Aug 2004 13:13:11 -0400 Date: Mon, 23 Aug 2004 19:10:11 +0200 Message-ID: From: Takashi Iwai To: linux-kernel@vger.kernel.org Cc: Thomas Charbonnel Subject: [PATCH] Fix shared interrupt handling of SA_INTERRUPT and SA_SAMPLE_RANDOM 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 Hi, while the recent investation of latency issues, Thomas Charbonne suggested that there is a long-standing bug in the irq handler. When the irq is shared, SA_INTERRUPT flag is checked only for the first registered handler. When it's without SA_INTERRUPT, always local_irq_enable() is called even if the second or later handler has SA_INTERRUPT. Also, handle_IRQ_event() always calls add_interrupt_randomness() even if the irq is not for the handler with SA_SAMPLE_RANDOM. This is a performance loss. The patch below fixes these problems by adding the SA_INTERRUPT handler always to the head of the irq list, and by checking the return value of each handler. The patch is for i386 and x86-64 only. Similar patches will be needed for other architectures, too (or more better, making an arch-independent generic handler as in voluntary-preemptive patch). -- Takashi Iwai ALSA Developer - www.alsa-project.org --- linux/arch/i386/kernel/irq.c 2004-08-18 15:15:18.000000000 +0200 +++ linux/arch/i386/kernel/irq.c 2004-08-20 14:54:14.000000000 +0200 @@ -221,13 +221,21 @@ asmlinkage int handle_IRQ_event(unsigned { int status = 1; /* Force the "do bottom halves" bit */ int retval = 0; - - if (!(action->flags & SA_INTERRUPT)) - local_irq_enable(); + int irq_off = 1; do { - status |= action->flags; - retval |= action->handler(irq, action->dev_id, regs); + int ret; + /* Assume that all SA_INTERRUPT handlers are at the head + * of the irq queue + */ + if (irq_off && ! (action->flags & SA_INTERRUPT)) { + local_irq_enable(); + irq_off = 0; + } + ret = action->handler(irq, action->dev_id, regs); + if (ret) + status |= action->flags; + retval |= ret; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) @@ -955,11 +963,16 @@ int setup_irq(unsigned int irq, struct i return -EBUSY; } - /* add new interrupt at end of irq queue */ - do { - p = &old->next; - old = *p; - } while (old); + if (new->flags & SA_INTERRUPT) + /* add interrupt at the start of the queue */ + new->next = old; + else + /* add new interrupt at end of irq queue */ + do { + p = &old->next; + old = *p; + } while (old); + shared = 1; } --- linux/arch/x86_64/kernel/irq.c 2004-08-20 15:04:26.000000000 +0200 +++ linux/arch/x86_64/kernel/irq.c 2004-08-20 15:07:06.000000000 +0200 @@ -213,13 +213,20 @@ 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 */ - - if (!(action->flags & SA_INTERRUPT)) - local_irq_enable(); + int irq_off = 1; do { - status |= action->flags; - action->handler(irq, action->dev_id, regs); + int ret; + /* Assume that all SA_INTERRUPT handlers are at the head + * of the irq queue + */ + if (irq_off && ! (action->flags & SA_INTERRUPT)) { + local_irq_enable(); + irq_off = 0; + } + ret = action->handler(irq, action->dev_id, regs); + if (ret) + status |= action->flags; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM) @@ -794,11 +801,16 @@ int setup_irq(unsigned int irq, struct i return -EBUSY; } - /* add new interrupt at end of irq queue */ - do { - p = &old->next; - old = *p; - } while (old); + if (new->flags & SA_INTERRUPT) + /* add interrupt at the start of the queue */ + new->next = old; + else + /* add new interrupt at end of irq queue */ + do { + p = &old->next; + old = *p; + } while (old); + shared = 1; }