Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Aug 2003 09:57:15 -0700 (PDT)
From:      Julian Elischer <julian@elischer.org>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        current@freebsd.org
Subject:   Re: stray irq's in systat
Message-ID:  <Pine.BSF.4.21.0308260957040.33736-100000@InterJet.elischer.org>
In-Reply-To: <20030827022049.L914@gamplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
any plans to commit this?

On Wed, 27 Aug 2003, Bruce Evans wrote:

> 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
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
> 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0308260957040.33736-100000>