From owner-svn-src-all@FreeBSD.ORG Sat Mar 2 14:19:08 2013 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 32A6385F; Sat, 2 Mar 2013 14:19:08 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail09.syd.optusnet.com.au (mail09.syd.optusnet.com.au [211.29.132.190]) by mx1.freebsd.org (Postfix) with ESMTP id A78DCD7A; Sat, 2 Mar 2013 14:19:07 +0000 (UTC) Received: from c211-30-173-106.carlnfd1.nsw.optusnet.com.au (c211-30-173-106.carlnfd1.nsw.optusnet.com.au [211.30.173.106]) by mail09.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r22EJ2tV020291 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 3 Mar 2013 01:19:05 +1100 Date: Sun, 3 Mar 2013 01:19:02 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Marius Strobl Subject: Re: svn commit: r247620 - head/sys/sparc64/pci In-Reply-To: <201303021304.r22D4xoI089886@svn.freebsd.org> Message-ID: <20130303005613.A3532@besplex.bde.org> References: <201303021304.r22D4xoI089886@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=DdhPMYRW c=1 sm=1 a=qBJJwPPf2HcA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=shoyfQpYrMMA:10 a=1nWYaaUoRXj36-mlhGAA:9 a=CjuIK1q_8ugA:10 a=TEtd8y5WR3g2ypngnwZWYw==:117 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 02 Mar 2013 14:19:08 -0000 On Sat, 2 Mar 2013, Marius Strobl wrote: > Log: > Revert the part of r247600 which turned the overtemperature and power fail > interrupt shutdown handlers into filters. Shutdown_nice(9) acquires a sleep > lock, which filters shouldn't do. It also seems that kern_reboot(9) still > may require Giant to be hold. > > Submitted by: bde Thanks. Ithreads shouldn't normally use sleep locks either, but I think that is OK here since the sleeping mainly prevents new interrupts on the same interrupt source being handledm but when this interrupt should shut down that doesn't matter (even if the the interrupt is multiplexed so that the new interrupts may be for another device). Maybe interrupt handlers for urgent events should increase their priority as much as possible. They should start with highest interrupt priority. This is not very high but is hard to change. Then if the interrupt handler can be reached, it is easy for it to raise its priority so that it doesn't get preempted. > Modified: head/sys/sparc64/pci/psycho.c > ... > @@ -643,7 +643,7 @@ psycho_attach(device_t dev) > * The spare hardware interrupt is used for the > * over-temperature interrupt. > */ > - psycho_set_intr(sc, 4, PSR_SPARE_INT_MAP, > + psycho_set_intr(sc, 4, PSR_SPARE_INT_MAP, NULL, > psycho_overtemp); > #ifdef PSYCHO_MAP_WAKEUP > /* I hope this asks for a high hardware interrupt priority. > @@ -722,7 +722,7 @@ psycho_set_intr(struct psycho_softc *sc, > INTVEC(PSYCHO_READ8(sc, intrmap)) != vec || > intr_vectors[vec].iv_ic != &psycho_ic || > bus_setup_intr(sc->sc_dev, sc->sc_irq_res[index], > - INTR_TYPE_MISC | INTR_BRIDGE, handler, NULL, sc, > + INTR_TYPE_MISC | INTR_BRIDGE, filt, intr, sc, > &sc->sc_ihand[index]) != 0) > panic("%s: failed to set up interrupt %d", __func__, index); > } INTR_TYPE_MISC is not very high. In fact, it is mapped to PI_DULL = don't care = lowest hardware interrupt priority. > @@ -858,32 +858,30 @@ psycho_powerdebug(void *arg __unused) > return (FILTER_HANDLED); > } > > -static int > +static void > psycho_powerdown(void *arg __unused) > { > static int shutdown; > > /* As the interrupt is cleared we may be called multiple times. */ > if (shutdown != 0) > - return (FILTER_HANDLED); > + return; > shutdown++; > printf("Power Failure Detected: Shutting down NOW.\n"); > shutdown_nice(RB_POWEROFF); > - return (FILTER_HANDLED); > } Since shutdown_nice() just signals init, raising the priority here wouldn't help run most of the shutdown at high priority or quickly. There are even intentional long delays in normal shutdowns, to let the operator read the messages. So these events need to be not very urgent and allow a few seconds or even minutes to shut down. Bruce