MPSAFE locking

* Restructure the event handling path.  acpi_tz_thread() now calls
  acpi_tz_timeout() any time an event occurs.  acpi_tz_timeout() checks
  the flags and calls acpi_tz_power_profile(), acpi_tz_establish(), and
  acpi_tz_monitor() as appropriate.  Notifies only do a wakeup and let
  acpi_tz_thread() do the actual work.  This path is cleaner and allows
  locking since the call path is now always a D.A.G.
* Add the acpi_tz_signal() function to set flags and wake the thread.
* Remove the tz_tmp_updating flag since calls are serialized by
  acpi_tz_thread().
* Remove Giant locking.

1 files changed, 108 insertions, 120 deletions

diff --git a/sys/dev/acpica/acpi_thermal.c b/sys/dev/acpica/acpi_thermal.c
index 9efbddb..0a26ee0 100644
--- a/sys/dev/acpica/acpi_thermal.c
+++ b/sys/dev/acpica/acpi_thermal.c
@@ -31,7 +31,9 @@ __FBSDID("$FreeBSD$");
 #include "opt_acpi.h"
 #include <sys/param.h>
 #include <sys/kernel.h>
+#include <sys/bus.h>
 #include <sys/kthread.h>
+#include <sys/malloc.h>
 #include <sys/module.h>
 #include <sys/bus.h>
 #include <sys/proc.h>
@@ -94,14 +96,15 @@ struct acpi_tz_softc {
     int				tz_flags;
 #define TZ_FLAG_NO_SCP		(1<<0)		/*No _SCP method*/
 #define TZ_FLAG_GETPROFILE	(1<<1)		/*Get power_profile in timeout*/
+#define TZ_FLAG_GETSETTINGS	(1<<2)		/*Get devs/setpoints*/
     struct timespec		tz_cooling_started;
 					/*Current cooling starting time*/
 
     struct sysctl_ctx_list	tz_sysctl_ctx;
     struct sysctl_oid		*tz_sysctl_tree;
-    
+    eventhandler_tag		tz_event;
+
     struct acpi_tz_zone 	tz_zone;	/*Thermal zone parameters*/
-    int				tz_tmp_updating;
     int				tz_validchecks;
 };
 
@@ -118,7 +121,8 @@ static void	acpi_tz_sanity(struct acpi_tz_softc *sc, int *val, char *what);
 static int	acpi_tz_active_sysctl(SYSCTL_HANDLER_ARGS);
 static void	acpi_tz_notify_handler(ACPI_HANDLE h, UINT32 notify,
 				       void *context);
-static void	acpi_tz_timeout(struct acpi_tz_softc *sc);
+static void	acpi_tz_signal(struct acpi_tz_softc *sc, int flags);
+static void	acpi_tz_timeout(struct acpi_tz_softc *sc, int flags);
 static void	acpi_tz_power_profile(void *arg);
 static void	acpi_tz_thread(void *arg);
 
@@ -149,33 +153,21 @@ static int			acpi_tz_polling_rate = TZ_POLLRATE;
 
 /* Timezone polling thread */
 static struct proc		*acpi_tz_proc;
+ACPI_LOCK_DECL(thermal, "ACPI thermal zone");
 
-/*
- * Match an ACPI thermal zone.
- */
 static int
 acpi_tz_probe(device_t dev)
 {
     int		result;
-    ACPI_LOCK_DECL;
-    
-    ACPI_LOCK;
     
-    /* No FUNCTION_TRACE - too noisy */
-
     if (acpi_get_type(dev) == ACPI_TYPE_THERMAL && !acpi_disabled("thermal")) {
 	device_set_desc(dev, "Thermal Zone");
 	result = -10;
-    } else {
+    } else
 	result = ENXIO;
-    }
-    ACPI_UNLOCK;
     return (result);
 }
 
-/*
- * Attach to an ACPI thermal zone.
- */
 static int
 acpi_tz_attach(device_t dev)
 {
@@ -183,25 +175,22 @@ acpi_tz_attach(device_t dev)
     struct acpi_softc		*acpi_sc;
     int				error;
     char			oidname[8];
-    ACPI_LOCK_DECL;
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    ACPI_LOCK;
-
     sc = device_get_softc(dev);
     sc->tz_dev = dev;
     sc->tz_handle = acpi_get_handle(dev);
     sc->tz_requested = TZ_ACTIVE_NONE;
-    sc->tz_tmp_updating = 0;
 
     /*
      * Parse the current state of the thermal zone and build control
-     * structures.
+     * structures.  We don't need to worry about interference with the
+     * control thread since we haven't fully attached this device yet.
      */
     if ((error = acpi_tz_establish(sc)) != 0)
-	goto out;
-    
+	return (error);
+
     /*
      * Register for any Notify events sent to this zone.
      */
@@ -258,35 +247,35 @@ acpi_tz_attach(device_t dev)
 		      sizeof(sc->tz_zone.ac), "I", "");
 
     /*
-     * Register our power profile event handler, and flag it for a manual
-     * invocation by our timeout.  We defer it like this so that the rest
-     * of the subsystem has time to come up.
+     * Create our thread; we only need one, it will service all of the
+     * thermal zones.  Register our power profile event handler.
      */
-    EVENTHANDLER_REGISTER(power_profile_change, acpi_tz_power_profile, sc, 0);
-    sc->tz_flags |= TZ_FLAG_GETPROFILE;
+    sc->tz_event = EVENTHANDLER_REGISTER(power_profile_change,
+	acpi_tz_power_profile, sc, 0);
+    if (acpi_tz_proc == NULL) {
+	error = kthread_create(acpi_tz_thread, NULL, &acpi_tz_proc,
+	    RFHIGHPID, 0, "acpi_thermal");
+	if (error != 0) {
+	    device_printf(sc->tz_dev, "could not create thread - %d", error);
+	    goto out;
+	}
+    }
 
     /*
-     * Don't bother evaluating/printing the temperature at this point;
-     * on many systems it'll be bogus until the EC is running.
+     * Flag the event handler for a manual invocation by our timeout.
+     * We defer it like this so that the rest of the subsystem has time
+     * to come up.  Don't bother evaluating/printing the temperature at
+     * this point; on many systems it'll be bogus until the EC is running.
      */
+    sc->tz_flags |= TZ_FLAG_GETPROFILE;
 
-    /*
-     * Create our thread; we only need one, it will service all of the
-     * thermal zones.
-     */
-    if (acpi_tz_proc == NULL) {
-	    error = kthread_create(acpi_tz_thread, NULL, &acpi_tz_proc,
-				   RFHIGHPID, 0, "acpi_thermal");
-	    if (error != 0) {
-		    device_printf(sc->tz_dev, "could not create thread - %d",
-				  error);
-		    goto out;
-	    }
+out:
+    if (error != 0) {
+	EVENTHANDLER_DEREGISTER(power_profile_change, sc->tz_event);
+	AcpiRemoveNotifyHandler(sc->tz_handle, ACPI_DEVICE_NOTIFY,
+	    acpi_tz_notify_handler);
+	sysctl_ctx_free(&sc->tz_sysctl_ctx);
     }
-	
- out:
-    ACPI_UNLOCK;
-
     return_VALUE (error);
 }
 
@@ -304,8 +293,6 @@ acpi_tz_establish(struct acpi_tz_softc *sc)
     
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    ACPI_ASSERTLOCK;
-
     /* Power everything off and erase any existing state. */
     acpi_tz_all_off(sc);
     for (i = 0; i < TZ_NUMLEVELS; i++)
@@ -364,17 +351,18 @@ acpi_tz_establish(struct acpi_tz_softc *sc)
     return_VALUE (0);
 }
 
-static char	*aclevel_string[] =	{
-	"NONE", "_AC0", "_AC1", "_AC2", "_AC3", "_AC4",
-	"_AC5", "_AC6", "_AC7", "_AC8", "_AC9" };
+static char *aclevel_string[] = {
+    "NONE", "_AC0", "_AC1", "_AC2", "_AC3", "_AC4",
+    "_AC5", "_AC6", "_AC7", "_AC8", "_AC9"
+};
 
 static __inline const char *
 acpi_tz_aclevel_string(int active)
 {
-	if (active < -1 || active >= TZ_NUMLEVELS)
-		return (aclevel_string[0]);
+    if (active < -1 || active >= TZ_NUMLEVELS)
+	return (aclevel_string[0]);
 
-	return (aclevel_string[active+1]);
+    return (aclevel_string[active + 1]);
 }
 
 /*
@@ -392,12 +380,7 @@ acpi_tz_monitor(void *Context)
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    ACPI_ASSERTLOCK;
-
     sc = (struct acpi_tz_softc *)Context;
-    if (sc->tz_tmp_updating)
-	goto out;
-    sc->tz_tmp_updating = 1;
 
     /* Get the current temperature. */
     status = acpi_GetInteger(sc->tz_handle, "_TMP", &temp);
@@ -406,7 +389,7 @@ acpi_tz_monitor(void *Context)
 	    "error fetching current temperature -- %s\n",
 	     AcpiFormatException(status));
 	/* XXX disable zone? go to max cooling? */
-	goto out;
+	return_VOID;
     }
 
     ACPI_DEBUG_PRINT((ACPI_DB_VALUES, "got %d.%dC\n", TZ_KELVTOC(temp)));
@@ -504,8 +487,6 @@ acpi_tz_monitor(void *Context)
     }
     sc->tz_thflags = newflags;
 
-out:
-    sc->tz_tmp_updating = 0;
     return_VOID;
 }
 
@@ -519,8 +500,6 @@ acpi_tz_all_off(struct acpi_tz_softc *sc)
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    ACPI_ASSERTLOCK;
-    
     /* Scan all the _ALx objects and turn them all off. */
     for (i = 0; i < TZ_NUMLEVELS; i++) {
 	if (sc->tz_zone.al[i].Pointer == NULL)
@@ -550,8 +529,6 @@ acpi_tz_switch_cooler_off(ACPI_OBJECT *obj, void *arg)
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    ACPI_ASSERTLOCK;
-
     cooler = acpi_GetReference(NULL, obj);
     if (cooler == NULL) {
 	ACPI_DEBUG_PRINT((ACPI_DB_OBJECTS, "can't get handle\n"));
@@ -580,8 +557,6 @@ acpi_tz_switch_cooler_on(ACPI_OBJECT *obj, void *arg)
     
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    ACPI_ASSERTLOCK;
-
     cooler = acpi_GetReference(NULL, obj);
     if (cooler == NULL) {
 	ACPI_DEBUG_PRINT((ACPI_DB_OBJECTS, "can't get handle\n"));
@@ -609,8 +584,6 @@ acpi_tz_getparam(struct acpi_tz_softc *sc, char *node, int *data)
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    ACPI_ASSERTLOCK;
-
     if (ACPI_FAILURE(acpi_GetInteger(sc->tz_handle, node, data))) {
 	*data = -1;
     } else {
@@ -644,9 +617,6 @@ acpi_tz_active_sysctl(SYSCTL_HANDLER_ARGS)
     struct acpi_tz_softc	*sc;
     int				active;
     int		 		error;
-    ACPI_LOCK_DECL;
-
-    ACPI_LOCK;
 
     sc = (struct acpi_tz_softc *)oidp->oid_arg1;
     active = sc->tz_active;
@@ -654,19 +624,14 @@ acpi_tz_active_sysctl(SYSCTL_HANDLER_ARGS)
 
     /* Error or no new value */
     if (error != 0 || req->newptr == NULL)
-	goto out;
-    if (active < -1 || active >= TZ_NUMLEVELS) {
-	error = EINVAL;
-	goto out;
-    }
+	return (error);
+    if (active < -1 || active >= TZ_NUMLEVELS)
+	return (EINVAL);
 
     /* Set new preferred level and re-switch */
     sc->tz_requested = active;
-    acpi_tz_monitor(sc);
-
- out:
-    ACPI_UNLOCK;
-    return (error);
+    acpi_tz_signal(sc, 0);
+    return (0);
 }
 
 /*
@@ -679,18 +644,15 @@ acpi_tz_notify_handler(ACPI_HANDLE h, UINT32 notify, void *context)
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
-    ACPI_ASSERTLOCK;
-
-    switch(notify) {
+    switch (notify) {
     case TZ_NOTIFY_TEMPERATURE:
 	/* Temperature change occurred */
-	AcpiOsQueueForExecution(OSD_PRIORITY_HIGH, acpi_tz_monitor, sc);
+	acpi_tz_signal(sc, 0);
 	break;
     case TZ_NOTIFY_DEVICES:
     case TZ_NOTIFY_LEVELS:
 	/* Zone devices/setpoints changed */
-	AcpiOsQueueForExecution(OSD_PRIORITY_HIGH,
-				(OSD_EXECUTION_CALLBACK)acpi_tz_establish, sc);
+	acpi_tz_signal(sc, TZ_FLAG_GETSETTINGS);
 	break;
     default:
 	ACPI_VPRINT(sc->tz_dev, acpi_device_get_parent_softc(sc->tz_dev),
@@ -703,19 +665,28 @@ acpi_tz_notify_handler(ACPI_HANDLE h, UINT32 notify, void *context)
     return_VOID;
 }
 
+static void
+acpi_tz_signal(struct acpi_tz_softc *sc, int flags)
+{
+    ACPI_LOCK(thermal);
+    sc->tz_flags |= flags;
+    ACPI_UNLOCK(thermal);
+    wakeup(&acpi_tz_proc);
+}
+
 /*
  * Poll the thermal zone.
  */
 static void
-acpi_tz_timeout(struct acpi_tz_softc *sc)
+acpi_tz_timeout(struct acpi_tz_softc *sc, int flags)
 {
-    /* Do we need to get the power profile settings? */
-    if (sc->tz_flags & TZ_FLAG_GETPROFILE) {
-	acpi_tz_power_profile((void *)sc);
-	sc->tz_flags &= ~TZ_FLAG_GETPROFILE;
-    }
+    /* If requested, get the power profile settings. */
+    if (flags & TZ_FLAG_GETPROFILE)
+	acpi_tz_power_profile(sc);
 
-    ACPI_ASSERTLOCK;
+    /* If requested, check for new devices/setpoints. */
+    if (flags & TZ_FLAG_GETSETTINGS)
+	acpi_tz_establish(sc);
 
     /* Check the current temperature and take action based on it */
     acpi_tz_monitor(sc);
@@ -736,14 +707,11 @@ acpi_tz_power_profile(void *arg)
     ACPI_STATUS			status;
     struct acpi_tz_softc	*sc = (struct acpi_tz_softc *)arg;
     int				state;
-    ACPI_LOCK_DECL;
 
     state = power_profile_get_state();
     if (state != POWER_PROFILE_PERFORMANCE && state != POWER_PROFILE_ECONOMY)
 	return;
 
-    ACPI_LOCK;
-
     /* check that we haven't decided there's no _SCP method */
     if ((sc->tz_flags & TZ_FLAG_NO_SCP) == 0) {
 
@@ -760,13 +728,9 @@ acpi_tz_power_profile(void *arg)
 	    sc->tz_flags |= TZ_FLAG_NO_SCP;
 	} else {
 	    /* We have to re-evaluate the entire zone now */
-	    AcpiOsQueueForExecution(OSD_PRIORITY_HIGH,
-				    (OSD_EXECUTION_CALLBACK)acpi_tz_establish,
-				    sc);
+	    acpi_tz_signal(sc, TZ_FLAG_GETSETTINGS);
 	}
     }
-
-    ACPI_UNLOCK;
 }
 
 /*
@@ -777,30 +741,54 @@ acpi_tz_thread(void *arg)
 {
     device_t	*devs;
     int		devcount, i;
-    ACPI_LOCK_DECL;
+    int		flags;
+    struct acpi_tz_softc **sc;
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
     devs = NULL;
     devcount = 0;
+    sc = NULL;
 
     for (;;) {
-	tsleep(&acpi_tz_proc, PZERO, "tzpoll", hz * acpi_tz_polling_rate);
-
-#if __FreeBSD_version >= 500000
-	mtx_lock(&Giant);
-#endif
-
-	if (devcount == 0)
+	/* If the number of devices has changed, re-evaluate. */
+	if (devclass_get_maxunit(acpi_tz_devclass) != devcount) {
+	    if (devs != NULL) {
+		free(devs, M_TEMP);
+		free(sc, M_TEMP);
+	    }
 	    devclass_get_devices(acpi_tz_devclass, &devs, &devcount);
+	    sc = malloc(sizeof(struct acpi_tz_softc *) * devcount, M_TEMP,
+			M_WAITOK | M_ZERO);
+	    for (i = 0; i < devcount; i++)
+		sc[i] = device_get_softc(devs[i]);
+	}
 
-	ACPI_LOCK;
-	for (i = 0; i < devcount; i++)
-	    acpi_tz_timeout(device_get_softc(devs[i]));
-	ACPI_UNLOCK;
+	/* Check for temperature events and act on them. */
+	for (i = 0; i < devcount; i++) {
+	    ACPI_LOCK(thermal);
+	    flags = sc[i]->tz_flags;
+	    sc[i]->tz_flags &= TZ_FLAG_NO_SCP;
+	    ACPI_UNLOCK(thermal);
+	    acpi_tz_timeout(sc[i], flags);
+	}
+
+	/* If more work to do, don't go to sleep yet. */
+	ACPI_LOCK(thermal);
+	for (i = 0; i < devcount; i++) {
+	    if (sc[i]->tz_flags & ~TZ_FLAG_NO_SCP)
+		break;
+	}
 
-#if __FreeBSD_version >= 500000
-	mtx_unlock(&Giant);
-#endif
+	/*
+	 * If we have no more work, sleep for a while, setting PDROP so that
+	 * the mutex will not be reacquired.  Otherwise, drop the mutex and
+	 * loop to handle more events.
+	 */
+	if (i == devcount)
+	    msleep(&acpi_tz_proc, &thermal_mutex, PZERO | PDROP, "tzpoll",
+		hz * acpi_tz_polling_rate);
+	else
+	    ACPI_UNLOCK(thermal);
     }
 }