From 70a37300e49df77e9d68d28b38745b7e49488c51 Mon Sep 17 00:00:00 2001 From: xtraeme Date: Sat, 8 Sep 2007 15:25:18 +0000 Subject: [PATCH] Use only one single condvar(9) for devices and events, they are protected by the same mutex (sme_mtx) and there's no point in using two of them. Also add a comment mentioning some locking notes. Reviewed and ok by rmind. --- sys/dev/sysmon/sysmon_envsys.c | 34 +++++++++++++++++++-------- sys/dev/sysmon/sysmon_envsys_events.c | 12 +++++----- sys/dev/sysmon/sysmon_envsysvar.h | 12 ++++------ 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/sys/dev/sysmon/sysmon_envsys.c b/sys/dev/sysmon/sysmon_envsys.c index 9d452847266d..8084131345dd 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.60 2007/09/08 03:41:28 xtraeme Exp $ */ +/* $NetBSD: sysmon_envsys.c,v 1.61 2007/09/08 15:25:18 xtraeme Exp $ */ /*- * Copyright (c) 2007 The NetBSD Foundation, Inc. @@ -75,7 +75,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.60 2007/09/08 03:41:28 xtraeme Exp $"); +__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.61 2007/09/08 15:25:18 xtraeme Exp $"); #include #include @@ -93,12 +93,27 @@ __KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.60 2007/09/08 03:41:28 xtraeme E #include #include -static prop_dictionary_t sme_propd; -static kcondvar_t sme_list_cv; +/* + * Notes about locking: + * + * There's a global lock 'sme_mtx' to protect access to 'sysmon_envsys_list' + * (devices linked list), 'struct sysmon_envsys' (device), 'sme_events_list' + * (events linked list), 'sme_event_t' (event) and the global counter + * 'sysmon_envsys_next_sensor_index'. + * + * Another lock 'sme_init_mtx' is used to protect initialization and + * finalization of the events framework (the callout(9) and workqueue(9) + * that is used to check for conditions and sending events to powerd(8)). + * + * The global 'sme_cv' condition variable is used to wait for state changes + * on the 'sysmon_envsys_list' and 'sme_events_list' linked lists. + * + */ kmutex_t sme_mtx, sme_event_init_mtx; -kcondvar_t sme_event_cv; +kcondvar_t sme_cv; +static prop_dictionary_t sme_propd; static uint32_t sysmon_envsys_next_sensor_index = 0; static struct sysmon_envsys *sysmon_envsys_find_40(u_int); @@ -118,8 +133,7 @@ sysmon_envsys_init(void) LIST_INIT(&sme_events_list); mutex_init(&sme_mtx, MUTEX_DRIVER, IPL_NONE); mutex_init(&sme_event_init_mtx, MUTEX_DRIVER, IPL_NONE); - cv_init(&sme_list_cv, "smefind"); - cv_init(&sme_event_cv, "smeevent"); + cv_init(&sme_cv, "smework"); sme_propd = prop_dictionary_create(); } @@ -577,7 +591,7 @@ sysmon_envsys_unregister(struct sysmon_envsys *sme) mutex_enter(&sme_mtx); while (sme->sme_flags & SME_FLAG_BUSY) { sme->sme_flags |= SME_FLAG_WANTED; - cv_wait(&sme_list_cv, &sme_mtx); + cv_wait(&sme_cv, &sme_mtx); } sysmon_envsys_next_sensor_index -= sme->sme_nsensors; /* @@ -621,7 +635,7 @@ again: if (strcmp(sme->sme_name, name) == 0) { if (sme->sme_flags & SME_FLAG_BUSY) { sme->sme_flags |= SME_FLAG_WANTED; - cv_wait(&sme_list_cv, &sme_mtx); + cv_wait(&sme_cv, &sme_mtx); goto again; } sme->sme_flags |= SME_FLAG_BUSY; @@ -642,7 +656,7 @@ sysmon_envsys_release(struct sysmon_envsys *sme) { mutex_enter(&sme_mtx); if (sme->sme_flags & SME_FLAG_WANTED) - cv_broadcast(&sme_list_cv); + cv_broadcast(&sme_cv); sme->sme_flags &= ~(SME_FLAG_BUSY | SME_FLAG_WANTED); mutex_exit(&sme_mtx); } diff --git a/sys/dev/sysmon/sysmon_envsys_events.c b/sys/dev/sysmon/sysmon_envsys_events.c index 6602c22110ca..0b09b2b20e23 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.32 2007/09/08 03:41:28 xtraeme Exp $ */ +/* $NetBSD: sysmon_envsys_events.c,v 1.33 2007/09/08 15:25:18 xtraeme Exp $ */ /*- * Copyright (c) 2007 The NetBSD Foundation, Inc. @@ -41,7 +41,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys_events.c,v 1.32 2007/09/08 03:41:28 xtraeme Exp $"); +__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys_events.c,v 1.33 2007/09/08 15:25:18 xtraeme Exp $"); #include #include @@ -78,7 +78,7 @@ static struct workqueue *seewq; static struct callout seeco; static bool sme_events_initialized = false; kmutex_t sme_mtx, sme_event_init_mtx; -kcondvar_t sme_event_cv; +kcondvar_t sme_cv; /* 10 seconds of timeout for the callout */ static int sme_events_timeout = 10; @@ -275,7 +275,7 @@ sme_event_unregister_all(const char *sme_name) see->pes.pes_sensname, see->type, sme_name)); while (see->see_flags & SME_EVENT_WORKING) - cv_wait(&sme_event_cv, &sme_mtx); + cv_wait(&sme_cv, &sme_mtx); LIST_REMOVE(see, see_list); kmem_free(see, sizeof(*see)); @@ -320,7 +320,7 @@ sme_event_unregister(const char *sensor, int type) } while (see->see_flags & SME_EVENT_WORKING) - cv_wait(&sme_event_cv, &sme_mtx); + cv_wait(&sme_cv, &sme_mtx); DPRINTF(("%s: removing dev=%s sensor=%s type=%d\n", __func__, see->pes.pes_dvname, sensor, type)); @@ -632,6 +632,6 @@ do { \ } out: see->see_flags &= ~SME_EVENT_WORKING; - cv_broadcast(&sme_event_cv); + cv_broadcast(&sme_cv); mutex_exit(&sme_mtx); } diff --git a/sys/dev/sysmon/sysmon_envsysvar.h b/sys/dev/sysmon/sysmon_envsysvar.h index 833c636aae7d..ff362cd4b150 100644 --- a/sys/dev/sysmon/sysmon_envsysvar.h +++ b/sys/dev/sysmon/sysmon_envsysvar.h @@ -1,4 +1,4 @@ -/* $NetBSD: sysmon_envsysvar.h,v 1.18 2007/09/08 03:17:38 xtraeme Exp $ */ +/* $NetBSD: sysmon_envsysvar.h,v 1.19 2007/09/08 15:25:19 xtraeme Exp $ */ /*- * Copyright (c) 2007 The NetBSD Foundation, Inc. @@ -105,13 +105,9 @@ struct sme_description_table { }; /* common */ -extern kmutex_t sme_mtx; /* mutex for the sysmon envsys devices/events */ - -/* mutex to intialize/destroy the sysmon envsys events framework */ -extern kmutex_t sme_event_init_mtx; - -/* condition variable to wait for the worker thread to finish */ -extern kcondvar_t sme_event_cv; +extern kmutex_t sme_mtx; /* mutex for devices/events */ +extern kmutex_t sme_event_init_mtx; /* init/destroy the events framework */ +extern kcondvar_t sme_cv; /* to wait for devices/events working */ /* linked list for the sysmon envsys devices */ LIST_HEAD(, sysmon_envsys) sysmon_envsys_list;