Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[.joined/src/trunk]: .joined/src/sys/dev/sysmon sysmon(9): Fix callout/thread...
details: https://anonhg.NetBSD.org/.joined/src/rev/18b18c8019b3
branches: trunk
changeset: 359350:18b18c8019b3
user: riastradh <riastradh%NetBSD.org@localhost>
date: Fri Dec 31 14:30:04 2021 +0000
description:
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:
sys/dev/sysmon/sysmon_envsys.c | 42 +++++++++++++++++-----------------
sys/dev/sysmon/sysmon_envsys_events.c | 29 ++++++++++++++++++-----
sys/dev/sysmon/sysmonvar.h | 17 +++++++++-----
3 files changed, 54 insertions(+), 34 deletions(-)
diffs (truncated from 314 to 300 lines):
diff -r 24b8e699d8c0 -r 18b18c8019b3 sys/dev/sysmon/sysmon_envsys.c
--- a/sys/dev/sysmon/sysmon_envsys.c Fri Dec 31 14:29:14 2021 +0000
+++ b/sys/dev/sysmon/sysmon_envsys.c Fri Dec 31 14:30:04 2021 +0000
@@ -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 @@
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 @@
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 @@
/*
* Restore default timeout value.
*/
+ mutex_enter(&sme->sme_work_mtx);
sme->sme_events_timeout = SME_EVENTS_DEFTIMEOUT;
+ sme_schedule_callout(sme);
+ mutex_exit(&sme->sme_work_mtx);
- /*
- * 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);
+ sysmon_envsys_release(sme, false);
}
mutex_exit(&sme_global_mtx);
}
@@ -1350,6 +1346,7 @@
prop_dictionary_t dict)
{
prop_dictionary_t pdict;
+ uint64_t timo;
const char *class;
int error = 0;
@@ -1374,15 +1371,15 @@
* ...
*
*/
+ 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 @@
{
envsys_data_t *edata;
prop_object_t array, dict, obj, obj2;
+ uint64_t timo;
int error = 0;
/*
@@ -1634,8 +1632,10 @@
/*
* 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 @@
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 -r 24b8e699d8c0 -r 18b18c8019b3 sys/dev/sysmon/sysmon_envsys_events.c
--- a/sys/dev/sysmon/sysmon_envsys_events.c Fri Dec 31 14:29:14 2021 +0000
+++ b/sys/dev/sysmon/sysmon_envsys_events.c Fri Dec 31 14:30:04 2021 +0000
@@ -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 @@
&(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 @@
}
}
+ 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_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 @@
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 @@
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 @@
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 @@
else
timo = SME_EVTIMO;
- callout_stop(&sme->sme_callout);
callout_schedule(&sme->sme_callout, timo);
}
@@ -743,7 +756,6 @@
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 @@
}
if (!sysmon_low_power)
sme_schedule_callout(sme);
- mutex_exit(&sme->sme_mtx);
mutex_exit(&sme->sme_work_mtx);
}
@@ -779,11 +790,15 @@
* 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 -r 24b8e699d8c0 -r 18b18c8019b3 sys/dev/sysmon/sysmonvar.h
--- a/sys/dev/sysmon/sysmonvar.h Fri Dec 31 14:29:14 2021 +0000
+++ b/sys/dev/sysmon/sysmonvar.h Fri Dec 31 14:30:04 2021 +0000
@@ -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 @@
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 @@
/*
* 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.
Home |
Main Index |
Thread Index |
Old Index