From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S266888AbUHYLUX (ORCPT ); Wed, 25 Aug 2004 07:20:23 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S266890AbUHYLUW (ORCPT ); Wed, 25 Aug 2004 07:20:22 -0400 Received: from cantor.suse.de ([195.135.220.2]:55447 "EHLO Cantor.suse.de") by vger.kernel.org with ESMTP id S266891AbUHYLTw (ORCPT ); Wed, 25 Aug 2004 07:19:52 -0400 Date: Wed, 25 Aug 2004 13:17:07 +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: <20040824204508.3b31449f.akpm@osdl.org> References: <20040824204508.3b31449f.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 Tue, 24 Aug 2004 20:45:08 -0700, Andrew Morton wrote: > > Takashi Iwai wrote: > > > > 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. > > That's because SA_INTERRUPT interrupts shouldn't be shared. The grey cell > which remembered why this is so seems to have died, but I've put the email > thread here: http://www.zip.com.au/~akpm/linux/patches/stuff/x.txt Oh thanks that helps to understand what happened. (BTW, regarding the atomicity in that discussion: could the code like rtc.c (assuming it has also SA_SHIRQ) really cause deadlock?) Anyway, suppressing the unnecessary call of add_interrupt_randomness() should be still valid. The reduced patch is below. Takashi --- linux-2.6.8.1/arch/i386/kernel/irq.c-dist 2004-08-25 13:13:05.153227112 +0200 +++ linux-2.6.8.1/arch/i386/kernel/irq.c 2004-08-25 13:13:34.760726088 +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) + status |= action->flags; + retval |= ret; action = action->next; } while (action); if (status & SA_SAMPLE_RANDOM)