diff options
| author | pgoyette <pgoyette@NetBSD.org> | 2017-09-11 06:02:09 +0000 |
|---|---|---|
| committer | pgoyette <pgoyette@NetBSD.org> | 2017-09-11 06:02:09 +0000 |
| commit | 6c75b1eb5e6359b4d5a689f0807a3bc11d8befe8 (patch) | |
| tree | ab98cb0679ecbfc1751e394c53df9e38bc275aa3 /sys/dev/sysmon | |
| parent | 0362de20498a718aeb5e475eab2d113a1a4860dc (diff) | |
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
XXX Pullup-8 along with the previous fixes from msaitoh@
XXX http://mail-index.netbsd.org/source-changes/2017/09/06/msg088028.html
~
~
Diffstat (limited to 'sys/dev/sysmon')
| -rw-r--r-- | sys/dev/sysmon/sysmon_envsys.c | 45 | ||||
| -rw-r--r-- | sys/dev/sysmon/sysmon_envsys_events.c | 49 | ||||
| -rw-r--r-- | sys/dev/sysmon/sysmonvar.h | 9 |
3 files changed, 71 insertions, 32 deletions
diff --git a/sys/dev/sysmon/sysmon_envsys.c b/sys/dev/sysmon/sysmon_envsys.c index cc9ea02cd68..c4081284af0 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.140 2017/09/06 11:08:53 msaitoh Exp $ */ +/* $NetBSD: sysmon_envsys.c,v 1.141 2017/09/11 06:02:09 pgoyette 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.140 2017/09/06 11:08:53 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.141 2017/09/11 06:02:09 pgoyette 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); @@ -654,7 +656,8 @@ sysmon_envsys_sensor_detach(struct sysmon_envsys *sme, envsys_data_t *edata) 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); @@ -1269,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); @@ -1290,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, @@ -1308,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); } @@ -1350,7 +1375,9 @@ sme_add_property_dictionary(struct sysmon_envsys *sme, prop_array_t array, */ 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", @@ -1824,7 +1851,7 @@ sme_userset_dictionary(struct sysmon_envsys *sme, prop_dictionary_t udict, sme_schedule_callout(sme); } mutex_exit(&sme->sme_mtx); - } + } } return error; diff --git a/sys/dev/sysmon/sysmon_envsys_events.c b/sys/dev/sysmon/sysmon_envsys_events.c index 8b74656bcad..d45be9ee6c6 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.120 2017/09/06 11:08:53 msaitoh Exp $ */ +/* $NetBSD: sysmon_envsys_events.c,v 1.121 2017/09/11 06:02:09 pgoyette 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.120 2017/09/06 11:08:53 msaitoh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys_events.c,v 1.121 2017/09/11 06:02:09 pgoyette Exp $"); #include <sys/param.h> #include <sys/types.h> @@ -313,7 +313,7 @@ sme_event_register(prop_dictionary_t sdict, envsys_data_t *edata, /* * 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_envsys *sme) } 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; } @@ -570,7 +570,7 @@ sme_events_init(struct sysmon_envsys *sme) 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)); @@ -589,8 +589,9 @@ sme_schedule_callout(struct sysmon_envsys *sme) 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) @@ -610,14 +611,18 @@ sme_schedule_callout(struct sysmon_envsys *sme) 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); } @@ -630,11 +635,15 @@ sme_events_halt_callout(struct sysmon_envsys *sme) 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)); @@ -734,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; @@ -748,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); } /* @@ -1003,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); } diff --git a/sys/dev/sysmon/sysmonvar.h b/sys/dev/sysmon/sysmonvar.h index b3dd29ed636..e31901485a2 100644 --- a/sys/dev/sysmon/sysmonvar.h +++ b/sys/dev/sysmon/sysmonvar.h @@ -1,4 +1,4 @@ -/* $NetBSD: sysmonvar.h,v 1.49 2015/04/23 23:22:03 pgoyette Exp $ */ +/* $NetBSD: sysmonvar.h,v 1.50 2017/09/11 06:02:09 pgoyette 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 */ /* |
