Date: Sun, 3 Mar 2013 01:19:02 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Marius Strobl <marius@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r247620 - head/sys/sparc64/pci Message-ID: <20130303005613.A3532@besplex.bde.org> In-Reply-To: <201303021304.r22D4xoI089886@svn.freebsd.org> References: <201303021304.r22D4xoI089886@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130303005613.A3532>