From owner-freebsd-current@FreeBSD.ORG Tue Aug 26 09:52:04 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C639916A4BF for ; Tue, 26 Aug 2003 09:52:04 -0700 (PDT) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id F27FA43FCB for ; Tue, 26 Aug 2003 09:52:01 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from gamplex.bde.org (katana.zip.com.au [61.8.7.246]) by mailman.zeta.org.au (8.9.3p2/8.8.7) with ESMTP id CAA01629; Wed, 27 Aug 2003 02:50:38 +1000 Date: Wed, 27 Aug 2003 02:50:37 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Mike Jakubik In-Reply-To: Message-ID: <20030827022049.L914@gamplex.bde.org> References: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: current@freebsd.org Subject: Re: stray irq's in systat X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Aug 2003 16:52:04 -0000 On Tue, 26 Aug 2003, Mike Jakubik wrote: > When running 'systat -vmstat 1' on FreeBSD 5.1-CURRENT #1: Mon Aug 25 > 14:54:14 EDT 2003, the interrupts section shows irq's 0 and 6 as stray. I > remember this would happen on 4.x when I took out lpt drivers from the > kernel, and didn't disable lpt in the bios. This however is not the case, > and start irq's do not show up under 4.8 on this box. everything is working > ok, I am just curious why they are showing up. This is caused by a race calling update_intrname(). Most of the patch consists of comments about unfixed races. There are many other bugs in this area. %%% Index: intr_machdep.c =================================================================== RCS file: /home/ncvs/src/sys/i386/isa/intr_machdep.c,v retrieving revision 1.73 diff -u -2 -r1.73 intr_machdep.c --- intr_machdep.c 20 Oct 2002 18:02:46 -0000 1.73 +++ intr_machdep.c 6 Apr 2003 10:14:13 -0000 @@ -663,13 +637,40 @@ ithread_priority(flags), flags, cookiep); - if ((flags & INTR_FAST) == 0 || errcode) + if ((flags & INTR_FAST) == 0 || errcode) { /* * The interrupt process must be in place, but * not necessarily schedulable, before we - * initialize the ICU, since it may cause an + * "initialize" the ICU, since "initializing" it may "cause" an * immediate interrupt. + * + * The pointer to the interrupt counter (which is changed if + * the name is changed) must be in place for the same reason. + * Otherwise, we could and did get normal interrupts bogusly + * counted as stray ones, which mainly messed up systat(8)'s + * layout. + * + * XXX we depend on the interrupt being set up not already + * being enabled here. This is part of the API, and the + * locking for it is hopefully adequate. However, the + * locking is inadequate for other interrupts being set + * up concrrently (we race in update_intrname()) and for + * spurious interrupts (update_intrname() and icu_setup() + * need a common lock). + * + * XXX icu_unset() is only called from isa_defaultirq(), so + * I don't see how bus_teardown_intr() can work. I think + * it leaves a garbage pointer to the interrupt handler. + * In the non-fast case, the pointer is to sched_ithd() so + * which cannot be unloaded, so the only damage is that we + * waste time checking for errors that shouldn't happen. + * In the fast case, the pointer may be into an unloaded + * module. Presumably the interrupt is masked in another + * way, else we would have more problems. However, spurious + * interrupts can't be masked in the ICU. */ + update_intrname(irq, name); if (icu_setup(irq, sched_ithd, arg, flags) != 0) panic("inthand_add: Can't initialize ICU"); + } if (errcode) @@ -677,4 +678,9 @@ if (flags & INTR_FAST) { + /* + * XXX this clause repeats a lot of code, apparently just to + * vary the handler and to avoid panicing if icu_setup() fails. + */ + update_intrname(irq, name); errcode = icu_setup(irq, handler, arg, flags); if (errcode && bootverbose) @@ -685,5 +691,4 @@ } - update_intrname(irq, name); return (0); } %%% Bruce