Module Name: src
Committed By: snj
Date: Sat Sep 23 17:22:49 UTC 2017
Modified Files:
src/sys/dev/sysmon [netbsd-8]: sysmon_envsys.c sysmon_envsys_events.c
sysmonvar.h
Log Message:
Pull up following revision(s) (requested by pgoyette in ticket #281):
sys/dev/sysmon/sysmon_envsys.c: 1.140-1.141
sys/dev/sysmon/sysmon_envsys_events.c: 1.120-1.121
sys/dev/sysmon/sysmonvar.h: 1.50
Fixes a problem that some driver(e.g. acpitz(4) or coretemp(5)) which
use sysmon_envsys sleep waiting at "rndsrc" when "drvctl -d".
Don't call rnd_detach_source() in sme_remove_event() which is called
from sme_event_unregister_all(). Instead, call rnd_detach_source() in
sysmon_envsys_sensor_detach() and call sysmon_envsys_sensor_detach()
before sme_event_unregister_sensor(). Each sensor(envsys_data) has each
rnd_src, but some sme_events point to the same rnd_src in a sensor.
Calling rnd_detach_souce() twice with the same rnd_src brokes a reference
count in rnd_src. OK'd by pgoyette@.
--
Improve tracking of the state of an event's callout, and protect all
queries or modifications of the state with the sme_mtx mutex.
Detach the rndsrc before re-attaching it (with potentially new values).
Clean up some lock-ordering issues.
And a couple of KNF issues for good measure!
Should address PR kern/52533
To generate a diff of this commit:
cvs rdiff -u -r1.139 -r1.139.10.1 src/sys/dev/sysmon/sysmon_envsys.c
cvs rdiff -u -r1.119 -r1.119.2.1 src/sys/dev/sysmon/sysmon_envsys_events.c
cvs rdiff -u -r1.49 -r1.49.10.1 src/sys/dev/sysmon/sysmonvar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/sysmon/sysmon_envsys.c
diff -u src/sys/dev/sysmon/sysmon_envsys.c:1.139 src/sys/dev/sysmon/sysmon_envsys.c:1.139.10.1
--- src/sys/dev/sysmon/sysmon_envsys.c:1.139 Mon Dec 14 01:08:47 2015
+++ src/sys/dev/sysmon/sysmon_envsys.c Sat Sep 23 17:22:48 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: sysmon_envsys.c,v 1.139 2015/12/14 01:08:47 pgoyette Exp $ */
+/* $NetBSD: sysmon_envsys.c,v 1.139.10.1 2017/09/23 17:22:48 snj Exp $ */
/*-
* Copyright (c) 2007, 2008 Juan Romero Pardines.
@@ -64,7 +64,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.139 2015/12/14 01:08:47 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.139.10.1 2017/09/23 17:22:48 snj Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -525,6 +525,8 @@ sysmon_envsys_create(void)
{
struct sysmon_envsys *sme;
+ CTASSERT(SME_CALLOUT_INVALID == 0);
+
sme = kmem_zalloc(sizeof(*sme), KM_SLEEP);
TAILQ_INIT(&sme->sme_sensors_list);
LIST_INIT(&sme->sme_events_list);
@@ -650,9 +652,12 @@ sysmon_envsys_sensor_detach(struct sysmo
/*
* remove it, unhook from rnd(4), and decrement the sensors count.
*/
+ if (oedata->flags & ENVSYS_FHAS_ENTROPY)
+ rnd_detach_source(&oedata->rnd_src);
sme_event_unregister_sensor(sme, edata);
if (LIST_EMPTY(&sme->sme_events_list)) {
- sme_events_halt_callout(sme);
+ if (sme->sme_callout_state == SME_CALLOUT_READY)
+ sme_events_halt_callout(sme);
destroy = true;
}
TAILQ_REMOVE(&sme->sme_sensors_list, edata, sensors_head);
@@ -970,6 +975,7 @@ sysmon_envsys_unregister(struct sysmon_e
{
prop_array_t array;
struct sysmon_envsys *osme;
+ envsys_data_t *edata;
KASSERT(sme != NULL);
@@ -987,6 +993,10 @@ sysmon_envsys_unregister(struct sysmon_e
LIST_REMOVE(sme, sme_list);
mutex_exit(&sme_global_mtx);
+ TAILQ_FOREACH(edata, &sme->sme_sensors_list, sensors_head) {
+ sysmon_envsys_sensor_detach(sme, edata);
+ }
+
/*
* Unregister all events associated with device.
*/
@@ -1262,15 +1272,23 @@ sme_remove_userprops(void)
}
/*
- * Finally, remove any old limits event, then
- * install a new event (which will update the
- * dictionary)
+ * If the sensor is providing entropy data,
+ * get rid of the rndsrc; we'll provide a new
+ * one shortly.
+ */
+ if (edata->flags & ENVSYS_FHAS_ENTROPY)
+ rnd_detach_source(&edata->rnd_src);
+
+ /*
+ * Remove the old limits event, if any
*/
sme_event_unregister(sme, edata->desc,
PENVSYS_EVENT_LIMITS);
/*
- * Find the correct units for this sensor.
+ * Create and install a new event (which will
+ * update the dictionary) with the correct
+ * units.
*/
sdt_units = sme_find_table_entry(SME_DESC_UNITS,
edata->units);
@@ -1283,6 +1301,11 @@ sme_remove_userprops(void)
&lims, props, PENVSYS_EVENT_LIMITS,
sdt_units->crittype);
}
+
+ /* Finally, if the sensor provides entropy,
+ * create an additional event entry and attach
+ * the rndsrc
+ */
if (edata->flags & ENVSYS_FHAS_ENTROPY) {
sme_event_register(sdict, edata, sme,
&lims, props, PENVSYS_EVENT_NULL,
@@ -1301,8 +1324,17 @@ sme_remove_userprops(void)
* Restore default timeout value.
*/
sme->sme_events_timeout = SME_EVENTS_DEFTIMEOUT;
+
+ /*
+ * Note that we need to hold the sme_mtx while calling
+ * sme_schedule_callout(). Thus to avoid dropping,
+ * reacquiring, and dropping it again, we just tell
+ * sme_envsys_release() that the mutex is already owned.
+ */
+ mutex_enter(&sme->sme_mtx);
sme_schedule_callout(sme);
- sysmon_envsys_release(sme, false);
+ sysmon_envsys_release(sme, true);
+ mutex_exit(&sme->sme_mtx);
}
mutex_exit(&sme_global_mtx);
}
@@ -1343,7 +1375,9 @@ sme_add_property_dictionary(struct sysmo
*/
if (sme->sme_events_timeout == 0) {
sme->sme_events_timeout = SME_EVENTS_DEFTIMEOUT;
+ mutex_enter(&sme->sme_mtx);
sme_schedule_callout(sme);
+ mutex_exit(&sme->sme_mtx);
}
if (!prop_dictionary_set_uint64(pdict, "refresh-timeout",
@@ -1817,7 +1851,7 @@ sme_userset_dictionary(struct sysmon_env
sme_schedule_callout(sme);
}
mutex_exit(&sme->sme_mtx);
- }
+ }
}
return error;
Index: src/sys/dev/sysmon/sysmon_envsys_events.c
diff -u src/sys/dev/sysmon/sysmon_envsys_events.c:1.119 src/sys/dev/sysmon/sysmon_envsys_events.c:1.119.2.1
--- src/sys/dev/sysmon/sysmon_envsys_events.c:1.119 Thu Jun 1 02:45:11 2017
+++ src/sys/dev/sysmon/sysmon_envsys_events.c Sat Sep 23 17:22:48 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: sysmon_envsys_events.c,v 1.119 2017/06/01 02:45:11 chs Exp $ */
+/* $NetBSD: sysmon_envsys_events.c,v 1.119.2.1 2017/09/23 17:22:48 snj Exp $ */
/*-
* Copyright (c) 2007, 2008 Juan Romero Pardines.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys_events.c,v 1.119 2017/06/01 02:45:11 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys_events.c,v 1.119.2.1 2017/09/23 17:22:48 snj Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -313,7 +313,7 @@ sme_event_register(prop_dictionary_t sdi
/*
* Initialize the events framework if it wasn't initialized before.
*/
- if ((sme->sme_flags & SME_CALLOUT_INITIALIZED) == 0)
+ if (sme->sme_callout_state == SME_CALLOUT_INVALID)
error = sme_events_init(sme);
/*
@@ -374,7 +374,7 @@ sme_event_unregister_all(struct sysmon_e
}
if (LIST_EMPTY(&sme->sme_events_list) &&
- sme->sme_flags & SME_CALLOUT_INITIALIZED) {
+ sme->sme_callout_state == SME_CALLOUT_READY) {
sme_events_halt_callout(sme);
destroy = true;
}
@@ -480,8 +480,6 @@ sme_remove_event(sme_event_t *see, struc
KASSERT(mutex_owned(&sme->sme_mtx));
- if (see->see_edata->flags & ENVSYS_FHAS_ENTROPY)
- rnd_detach_source(&see->see_edata->rnd_src);
LIST_REMOVE(see, see_list);
kmem_free(see, sizeof(*see));
}
@@ -572,7 +570,7 @@ sme_events_init(struct sysmon_envsys *sm
callout_init(&sme->sme_callout, CALLOUT_MPSAFE);
callout_setfunc(&sme->sme_callout, sme_events_check, sme);
- sme->sme_flags |= SME_CALLOUT_INITIALIZED;
+ sme->sme_callout_state = SME_CALLOUT_READY;
sme_schedule_callout(sme);
DPRINTF(("%s: events framework initialized for '%s'\n",
__func__, sme->sme_name));
@@ -591,8 +589,9 @@ sme_schedule_callout(struct sysmon_envsy
uint64_t timo;
KASSERT(sme != NULL);
+ KASSERT(mutex_owned(&sme->sme_mtx));
- if ((sme->sme_flags & SME_CALLOUT_INITIALIZED) == 0)
+ if (sme->sme_callout_state != SME_CALLOUT_READY)
return;
if (sme->sme_events_timeout)
@@ -612,14 +611,18 @@ sme_schedule_callout(struct sysmon_envsy
void
sme_events_halt_callout(struct sysmon_envsys *sme)
{
+
KASSERT(mutex_owned(&sme->sme_mtx));
+ KASSERT(sme->sme_callout_state == SME_CALLOUT_READY);
/*
- * Unset before callout_halt to ensure callout is not scheduled again
- * during callout_halt.
+ * Set HALTED before callout_halt to ensure callout is not
+ * scheduled again during callout_halt. (callout_halt()
+ * can potentially release the mutex, so an active callout
+ * could reschedule itself if it grabs the mutex.)
*/
- sme->sme_flags &= ~SME_CALLOUT_INITIALIZED;
+ sme->sme_callout_state = SME_CALLOUT_HALTED;
callout_halt(&sme->sme_callout, &sme->sme_mtx);
}
@@ -632,11 +635,15 @@ sme_events_halt_callout(struct sysmon_en
void
sme_events_destroy(struct sysmon_envsys *sme)
{
+
KASSERT(!mutex_owned(&sme->sme_mtx));
- KASSERT((sme->sme_flags & SME_CALLOUT_INITIALIZED) == 0);
- callout_destroy(&sme->sme_callout);
- workqueue_destroy(sme->sme_wq);
+ if (sme->sme_callout_state == SME_CALLOUT_HALTED) {
+ callout_destroy(&sme->sme_callout);
+ sme->sme_callout_state = SME_CALLOUT_INVALID;
+ workqueue_destroy(sme->sme_wq);
+ }
+ KASSERT(sme->sme_callout_state == SME_CALLOUT_INVALID);
DPRINTF(("%s: events framework destroyed for '%s'\n",
__func__, sme->sme_name));
@@ -736,13 +743,7 @@ sme_events_check(void *arg)
mutex_exit(&sme->sme_work_mtx);
return;
}
- if (!mutex_tryenter(&sme->sme_mtx)) {
- /* can't get lock - try again later */
- if (!sysmon_low_power)
- sme_schedule_callout(sme);
- mutex_exit(&sme->sme_work_mtx);
- return;
- }
+ mutex_enter(&sme->sme_mtx);
LIST_FOREACH(see, &sme->sme_events_list, see_list) {
workqueue_enqueue(sme->sme_wq, &see->see_wk, NULL);
see->see_edata->flags |= ENVSYS_FNEED_REFRESH;
@@ -750,8 +751,8 @@ sme_events_check(void *arg)
}
if (!sysmon_low_power)
sme_schedule_callout(sme);
- mutex_exit(&sme->sme_work_mtx);
mutex_exit(&sme->sme_mtx);
+ mutex_exit(&sme->sme_work_mtx);
}
/*
@@ -1005,12 +1006,16 @@ sme_deliver_event(sme_event_t *see)
*/
if (!sysmon_low_power && sme_event_check_low_power()) {
struct penvsys_state pes;
+ struct sysmon_envsys *sme = see->see_sme;
/*
* Stop the callout and send the 'low-power' event.
*/
sysmon_low_power = true;
- callout_stop(&see->see_sme->sme_callout);
+ KASSERT(mutex_owned(&sme->sme_mtx));
+ KASSERT(sme->sme_callout_state == SME_CALLOUT_READY);
+ callout_stop(&sme->sme_callout);
+ sme->sme_callout_state = SME_CALLOUT_HALTED;
pes.pes_type = PENVSYS_TYPE_BATTERY;
sysmon_penvsys_event(&pes, PENVSYS_EVENT_LOW_POWER);
}
Index: src/sys/dev/sysmon/sysmonvar.h
diff -u src/sys/dev/sysmon/sysmonvar.h:1.49 src/sys/dev/sysmon/sysmonvar.h:1.49.10.1
--- src/sys/dev/sysmon/sysmonvar.h:1.49 Thu Apr 23 23:22:03 2015
+++ src/sys/dev/sysmon/sysmonvar.h Sat Sep 23 17:22:48 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: sysmonvar.h,v 1.49 2015/04/23 23:22:03 pgoyette Exp $ */
+/* $NetBSD: sysmonvar.h,v 1.49.10.1 2017/09/23 17:22:48 snj Exp $ */
/*-
* Copyright (c) 2000 Zembu Labs, Inc.
@@ -163,7 +163,6 @@ struct sysmon_envsys {
int sme_flags; /* additional flags */
#define SME_FLAG_BUSY 0x00000001 /* device busy */
#define SME_DISABLE_REFRESH 0x00000002 /* disable sme_refresh */
-#define SME_CALLOUT_INITIALIZED 0x00000004 /* callout was initialized */
#define SME_INIT_REFRESH 0x00000008 /* call sme_refresh() after
interrupts are enabled in
the autoconf(9) process. */
@@ -187,6 +186,12 @@ struct sysmon_envsys {
struct workqueue *sme_wq; /* the workqueue for the events */
struct callout sme_callout; /* for the events */
+ int sme_callout_state; /* state of the event's callout */
+
+#define SME_CALLOUT_INVALID 0x0 /* callout is not initialized */
+#define SME_CALLOUT_READY 0x1 /* callout is ready for use */
+#define SME_CALLOUT_HALTED 0x2 /* callout can be destroyed */
+
uint64_t sme_events_timeout; /* the timeout used in the callout */
/*
