summaryrefslogtreecommitdiff
path: root/sys/dev/sysmon
diff options
context:
space:
mode:
authorriastradh <riastradh@NetBSD.org>2021-12-31 14:30:04 +0000
committerriastradh <riastradh@NetBSD.org>2021-12-31 14:30:04 +0000
commit1bf8e094ee54209cabab072a29628c8574f64927 (patch)
treeeef50dfa74adf2da947a0d3bdc53649ab8ea375a /sys/dev/sysmon
parentaa0eba2b8fb0524522739522d8f59ce8b5114c4d (diff)
sysmon(9): Fix callout/thread synchronization.
Callout may ONLY take sme_work_mtx, at IPL_SOFTCLOCK; MUST NOT touch sme_mtx at IPL_NONE. All state the callout needs is serialized by sme_work_mtx now: - calls to sme_schedule_callout - calls to sme_schedule_halt - struct sysmon_envsys::sme_events_timeout - struct sysmon_envsys::sme_events_list - struct sysmon_envsys::sme_callout_state - struct envsys_data::flags => yes, this is a little silly -- used for ENVSYS_FNEED_REFRESH => should maybe separate the static driver-defined features from the state flags needed by sysmon_envsys but not important now Sleeping under sme_work_mtx (except on other adaptive locks at IPL_SOFTCLOCK) is forbidden. Calling out to the driver under sme_work_mtx is forbidden. This should properly fix: https://mail-index.netbsd.org/tech-kern/2015/10/14/msg019511.html PR kern/56592
Diffstat (limited to 'sys/dev/sysmon')
-rw-r--r--sys/dev/sysmon/sysmon_envsys.c42
-rw-r--r--sys/dev/sysmon/sysmon_envsys_events.c29
-rw-r--r--sys/dev/sysmon/sysmonvar.h17
3 files changed, 54 insertions, 34 deletions
diff --git a/sys/dev/sysmon/sysmon_envsys.c b/sys/dev/sysmon/sysmon_envsys.c
index 04cd95b929a..012ad4d531c 100644
--- a/sys/dev/sysmon/sysmon_envsys.c
+++ b/sys/dev/sysmon/sysmon_envsys.c
@@ -1,4 +1,4 @@
-/* $NetBSD: sysmon_envsys.c,v 1.149 2021/12/31 11:05:41 riastradh Exp $ */
+/* $NetBSD: sysmon_envsys.c,v 1.150 2021/12/31 14:30:04 riastradh 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.149 2021/12/31 11:05:41 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.150 2021/12/31 14:30:04 riastradh Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -531,7 +531,7 @@ sysmon_envsys_create(void)
TAILQ_INIT(&sme->sme_sensors_list);
LIST_INIT(&sme->sme_events_list);
mutex_init(&sme->sme_mtx, MUTEX_DEFAULT, IPL_NONE);
- mutex_init(&sme->sme_work_mtx, MUTEX_DEFAULT, IPL_NONE);
+ mutex_init(&sme->sme_work_mtx, MUTEX_DEFAULT, IPL_SOFTCLOCK);
cv_init(&sme->sme_condvar, "sme_wait");
return sme;
@@ -655,11 +655,13 @@ sysmon_envsys_sensor_detach(struct sysmon_envsys *sme, envsys_data_t *edata)
if (oedata->flags & ENVSYS_FHAS_ENTROPY)
rnd_detach_source(&oedata->rnd_src);
sme_event_unregister_sensor(sme, edata);
+ mutex_enter(&sme->sme_work_mtx);
if (LIST_EMPTY(&sme->sme_events_list)) {
if (sme->sme_callout_state == SME_CALLOUT_READY)
sme_events_halt_callout(sme);
destroy = true;
}
+ mutex_exit(&sme->sme_work_mtx);
TAILQ_REMOVE(&sme->sme_sensors_list, edata, sensors_head);
sme->sme_nsensors--;
sysmon_envsys_release(sme, true);
@@ -1324,18 +1326,12 @@ sme_remove_userprops(void)
/*
* Restore default timeout value.
*/
+ mutex_enter(&sme->sme_work_mtx);
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, true);
- mutex_exit(&sme->sme_mtx);
+ mutex_exit(&sme->sme_work_mtx);
+
+ sysmon_envsys_release(sme, false);
}
mutex_exit(&sme_global_mtx);
}
@@ -1350,6 +1346,7 @@ sme_add_property_dictionary(struct sysmon_envsys *sme, prop_array_t array,
prop_dictionary_t dict)
{
prop_dictionary_t pdict;
+ uint64_t timo;
const char *class;
int error = 0;
@@ -1374,15 +1371,15 @@ sme_add_property_dictionary(struct sysmon_envsys *sme, prop_array_t array,
* ...
*
*/
+ mutex_enter(&sme->sme_work_mtx);
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);
}
+ timo = sme->sme_events_timeout;
+ mutex_exit(&sme->sme_work_mtx);
- if (!prop_dictionary_set_uint64(pdict, "refresh-timeout",
- sme->sme_events_timeout)) {
+ if (!prop_dictionary_set_uint64(pdict, "refresh-timeout", timo)) {
error = EINVAL;
goto out;
}
@@ -1606,6 +1603,7 @@ sme_update_dictionary(struct sysmon_envsys *sme)
{
envsys_data_t *edata;
prop_object_t array, dict, obj, obj2;
+ uint64_t timo;
int error = 0;
/*
@@ -1634,8 +1632,10 @@ sme_update_dictionary(struct sysmon_envsys *sme)
/*
* Update the 'refresh-timeout' property.
*/
- if (!prop_dictionary_set_uint64(obj2, "refresh-timeout",
- sme->sme_events_timeout))
+ mutex_enter(&sme->sme_work_mtx);
+ timo = sme->sme_events_timeout;
+ mutex_exit(&sme->sme_work_mtx);
+ if (!prop_dictionary_set_uint64(obj2, "refresh-timeout", timo))
return EINVAL;
/*
@@ -1852,12 +1852,12 @@ sme_userset_dictionary(struct sysmon_envsys *sme, prop_dictionary_t udict,
if (refresh_timo < 1)
error = EINVAL;
else {
- mutex_enter(&sme->sme_mtx);
+ mutex_enter(&sme->sme_work_mtx);
if (sme->sme_events_timeout != refresh_timo) {
sme->sme_events_timeout = refresh_timo;
sme_schedule_callout(sme);
}
- mutex_exit(&sme->sme_mtx);
+ mutex_exit(&sme->sme_work_mtx);
}
}
return error;
diff --git a/sys/dev/sysmon/sysmon_envsys_events.c b/sys/dev/sysmon/sysmon_envsys_events.c
index 1f1917ba633..ec6a691319e 100644
--- a/sys/dev/sysmon/sysmon_envsys_events.c
+++ b/sys/dev/sysmon/sysmon_envsys_events.c
@@ -1,4 +1,4 @@
-/* $NetBSD: sysmon_envsys_events.c,v 1.122 2021/12/31 11:05:41 riastradh Exp $ */
+/* $NetBSD: sysmon_envsys_events.c,v 1.123 2021/12/31 14:30:04 riastradh 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.122 2021/12/31 11:05:41 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys_events.c,v 1.123 2021/12/31 14:30:04 riastradh Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -325,8 +325,11 @@ sme_event_register(prop_dictionary_t sdict, envsys_data_t *edata,
&(edata->upropset));
out:
- if ((error == 0 || error == EEXIST) && osee == NULL)
+ if ((error == 0 || error == EEXIST) && osee == NULL) {
+ mutex_enter(&sme->sme_work_mtx);
LIST_INSERT_HEAD(&sme->sme_events_list, see, see_list);
+ mutex_exit(&sme->sme_work_mtx);
+ }
mutex_exit(&sme->sme_mtx);
@@ -373,11 +376,13 @@ sme_event_unregister_all(struct sysmon_envsys *sme)
}
}
+ mutex_enter(&sme->sme_work_mtx);
if (LIST_EMPTY(&sme->sme_events_list) &&
sme->sme_callout_state == SME_CALLOUT_READY) {
sme_events_halt_callout(sme);
destroy = true;
}
+ mutex_exit(&sme->sme_work_mtx);
mutex_exit(&sme->sme_mtx);
if (destroy)
@@ -425,10 +430,12 @@ sme_event_unregister(struct sysmon_envsys *sme, const char *sensor, int type)
sme_remove_event(see, sme);
+ mutex_enter(&sme->sme_work_mtx);
if (LIST_EMPTY(&sme->sme_events_list)) {
sme_events_halt_callout(sme);
destroy = true;
}
+ mutex_exit(&sme->sme_work_mtx);
mutex_exit(&sme->sme_mtx);
if (destroy)
@@ -480,7 +487,10 @@ sme_remove_event(sme_event_t *see, struct sysmon_envsys *sme)
KASSERT(mutex_owned(&sme->sme_mtx));
+ mutex_enter(&sme->sme_work_mtx);
LIST_REMOVE(see, see_list);
+ mutex_exit(&sme->sme_work_mtx);
+
kmem_free(see, sizeof(*see));
}
@@ -570,8 +580,12 @@ sme_events_init(struct sysmon_envsys *sme)
callout_init(&sme->sme_callout, CALLOUT_MPSAFE);
callout_setfunc(&sme->sme_callout, sme_events_check, sme);
+
+ mutex_enter(&sme->sme_work_mtx);
sme->sme_callout_state = SME_CALLOUT_READY;
sme_schedule_callout(sme);
+ mutex_exit(&sme->sme_work_mtx);
+
DPRINTF(("%s: events framework initialized for '%s'\n",
__func__, sme->sme_name));
@@ -589,7 +603,7 @@ sme_schedule_callout(struct sysmon_envsys *sme)
uint64_t timo;
KASSERT(sme != NULL);
- KASSERT(mutex_owned(&sme->sme_mtx));
+ KASSERT(mutex_owned(&sme->sme_work_mtx));
if (sme->sme_callout_state != SME_CALLOUT_READY)
return;
@@ -599,7 +613,6 @@ sme_schedule_callout(struct sysmon_envsys *sme)
else
timo = SME_EVTIMO;
- callout_stop(&sme->sme_callout);
callout_schedule(&sme->sme_callout, timo);
}
@@ -743,7 +756,6 @@ sme_events_check(void *arg)
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;
@@ -751,7 +763,6 @@ sme_events_check(void *arg)
}
if (!sysmon_low_power)
sme_schedule_callout(sme);
- mutex_exit(&sme->sme_mtx);
mutex_exit(&sme->sme_work_mtx);
}
@@ -779,11 +790,15 @@ sme_events_worker(struct work *wk, void *arg)
* sme_envsys_refresh_sensor will not call the driver if the driver
* does its own setting of the sensor value.
*/
+ mutex_enter(&sme->sme_work_mtx);
if ((edata->flags & ENVSYS_FNEED_REFRESH) != 0) {
/* refresh sensor in device */
+ mutex_exit(&sme->sme_work_mtx);
sysmon_envsys_refresh_sensor(sme, edata);
+ mutex_enter(&sme->sme_work_mtx);
edata->flags &= ~ENVSYS_FNEED_REFRESH;
}
+ mutex_exit(&sme->sme_work_mtx);
DPRINTFOBJ(("%s: (%s) desc=%s sensor=%d type=%d state=%d units=%d "
"value_cur=%d upropset=0x%04x\n", __func__, sme->sme_name, edata->desc,
diff --git a/sys/dev/sysmon/sysmonvar.h b/sys/dev/sysmon/sysmonvar.h
index 8a60b7fff8b..69af7673de4 100644
--- a/sys/dev/sysmon/sysmonvar.h
+++ b/sys/dev/sysmon/sysmonvar.h
@@ -1,4 +1,4 @@
-/* $NetBSD: sysmonvar.h,v 1.51 2021/12/31 11:05:41 riastradh Exp $ */
+/* $NetBSD: sysmonvar.h,v 1.52 2021/12/31 14:30:04 riastradh Exp $ */
/*-
* Copyright (c) 2000 Zembu Labs, Inc.
@@ -185,14 +185,17 @@ struct sysmon_envsys {
sysmon_envsys_lim_t *, uint32_t *);
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 */
+ struct callout sme_callout; /* for the events
+ * sme_work_mtx to schedule or halt */
+ int sme_callout_state; /* state of the event's callout
+ * sme_work_mtx to read or write */
#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 */
+ uint64_t sme_events_timeout; /* the timeout used in the callout
+ * sme_work_mtx to read or write */
/*
* linked list for the sysmon envsys devices.
@@ -201,11 +204,14 @@ struct sysmon_envsys {
/*
* linked list for the events that a device maintains.
+ * - sme_work_mtx OR sme_mtx to read
+ * - sme_work_mtx AND sme_mtx to write
*/
LIST_HEAD(, sme_event) sme_events_list;
/*
* tailq for the sensors that a device maintains.
+ * - sme_mtx to read or write
*/
TAILQ_HEAD(, envsys_data) sme_sensors_list;
@@ -213,8 +219,7 @@ struct sysmon_envsys {
* Locking/synchronization.
*/
int sme_busy; /* number of items on workqueue,
- sme_mtx or sme_work_mtx to read,
- both to write */
+ * sme_work_mtx to read or write */
kmutex_t sme_mtx;
kmutex_t sme_work_mtx;
kcondvar_t sme_condvar;