CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Dec 31 14:44:50 UTC 2021

Modified Files:
src/sys/dev/sysmon: sysmonvar.h

Log Message:
sysmon(9): Document lock order.


To generate a diff of this commit:
cvs rdiff -u -r1.52 -r1.53 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/sysmonvar.h
diff -u src/sys/dev/sysmon/sysmonvar.h:1.52 src/sys/dev/sysmon/sysmonvar.h:1.53
--- src/sys/dev/sysmon/sysmonvar.h:1.52	Fri Dec 31 14:30:04 2021
+++ src/sys/dev/sysmon/sysmonvar.h	Fri Dec 31 14:44:50 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: sysmonvar.h,v 1.52 2021/12/31 14:30:04 riastradh Exp $	*/
+/*	$NetBSD: sysmonvar.h,v 1.53 2021/12/31 14:44:50 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2000 Zembu Labs, Inc.
@@ -217,6 +217,8 @@ struct sysmon_envsys {
 
 	/*
 	 * Locking/synchronization.
+	 *
+	 * Lock order: sme_global_mtx -> sme_mtx -> sme_work_mtx
 	 */
 	int sme_busy;			/* number of items on workqueue,
 	 * sme_work_mtx to read or write */

CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Dec 31 14:44:50 UTC 2021

Modified Files:
src/sys/dev/sysmon: sysmonvar.h

Log Message:
sysmon(9): Document lock order.


To generate a diff of this commit:
cvs rdiff -u -r1.52 -r1.53 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.

CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Dec 31 14:30:04 UTC 2021

Modified Files:
src/sys/dev/sysmon: sysmon_envsys.c sysmon_envsys_events.c sysmonvar.h

Log Message:
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


To generate a diff of this commit:
cvs rdiff -u -r1.149 -r1.150 src/sys/dev/sysmon/sysmon_envsys.c
cvs rdiff -u -r1.122 -r1.123 src/sys/dev/sysmon/sysmon_envsys_events.c
cvs rdiff -u -r1.51 -r1.52 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.

CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Dec 31 14:30:04 UTC 2021

Modified Files:
src/sys/dev/sysmon: sysmon_envsys.c sysmon_envsys_events.c sysmonvar.h

Log Message:
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


To generate a diff of this commit:
cvs rdiff -u -r1.149 -r1.150 src/sys/dev/sysmon/sysmon_envsys.c
cvs rdiff -u -r1.122 -r1.123 src/sys/dev/sysmon/sysmon_envsys_events.c
cvs rdiff -u -r1.51 -r1.52 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.149 src/sys/dev/sysmon/sysmon_envsys.c:1.150
--- src/sys/dev/sysmon/sysmon_envsys.c:1.149	Fri Dec 31 11:05:41 2021
+++ src/sys/dev/sysmon/sysmon_envsys.c	Fri Dec 31 14:30:04 2021
@@ -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 
-__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 
 #include 
@@ -531,7 +531,7 @@ sysmon_envsys_create(void)
 	TAILQ_INIT(>sme_sensors_list);
 	LIST_INIT(>sme_events_list);
 	mutex_init(>sme_mtx, MUTEX_DEFAULT, IPL_NONE);
-	mutex_init(>sme_work_mtx, MUTEX_DEFAULT, IPL_NONE);
+	mutex_init(>sme_work_mtx, MUTEX_DEFAULT, IPL_SOFTCLOCK);
 	cv_init(>sme_condvar, "sme_wait");
 
 	return sme;
@@ -655,11 +655,13 @@ sysmon_envsys_sensor_detach(struct sysmo
 	if (oedata->flags & ENVSYS_FHAS_ENTROPY)
 		rnd_detach_source(>rnd_src);
 	sme_event_unregister_sensor(sme, edata);
+	mutex_enter(>sme_work_mtx);
 	if (LIST_EMPTY(>sme_events_list)) {
 		if (sme->sme_callout_state == SME_CALLOUT_READY)
 			sme_events_halt_callout(sme);
 		destroy = true;
 	}
+	mutex_exit(>sme_work_mtx);
 	TAILQ_REMOVE(>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_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_mtx);
 		sme_schedule_callout(sme);
-		sysmon_envsys_release(sme, true);
-		mutex_exit(>sme_mtx);
+		mutex_exit(>sme_work_mtx);
+
+		sysmon_envsys_release(sme, false);
 	}
 	mutex_exit(_global_mtx);
 }
@@ -1350,6 +1346,7 @@ sme_add_property_dictionary(struct sysmo
 			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 sysmo
 	 * 	...
 	 *
 	 */
+	mutex_enter(>sme_work_mtx);
 	if (sme->sme_events_timeout == 0) {
 		sme->sme_events_timeout = SME_EVENTS_DEFTIMEOUT;
-		mutex_enter(>sme_mtx);
 		sme_schedule_callout(sme);
-		mutex_exit(>sme_mtx);
 	}
+	timo = sme->sme_events_timeout;
+	mutex_exit(>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_envs
 {
 	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_envs
 	/*
 	 * Update the 'refresh-timeout' property.
 	 */
-	if (!prop_dictionary_set_uint64(obj2, "refresh-timeout",
-	sme->sme_events_timeout))
+	mutex_enter(>sme_work_mtx);
+	timo = sme->sme_events_timeout;
+	mutex_exit(>sme_work_mtx);
+	if (!prop_dictionary_set_uint64(obj2, "refresh-timeout", timo))
 		return EINVAL;
 
 	/*
@@ -1852,12 +1852,12 @@ 

CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Dec 31 14:29:14 UTC 2021

Modified Files:
src/sys/dev/sysmon: sysmon_taskq.c

Log Message:
sysmon(9): Fix mistake in sysmon_task_queue_barrier.

Need to allocate the task structure with malloc(9), not on the stack;
the task thread will free it with free(9).  Forgot to squash this
into previous commit.


To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/sysmon/sysmon_taskq.c

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_taskq.c
diff -u src/sys/dev/sysmon/sysmon_taskq.c:1.22 src/sys/dev/sysmon/sysmon_taskq.c:1.23
--- src/sys/dev/sysmon/sysmon_taskq.c:1.22	Fri Dec 31 14:22:11 2021
+++ src/sys/dev/sysmon/sysmon_taskq.c	Fri Dec 31 14:29:14 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: sysmon_taskq.c,v 1.22 2021/12/31 14:22:11 riastradh Exp $	*/
+/*	$NetBSD: sysmon_taskq.c,v 1.23 2021/12/31 14:29:14 riastradh Exp $	*/
 
 /*
  * Copyright (c) 2001, 2003 Wasabi Systems, Inc.
@@ -41,7 +41,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sysmon_taskq.c,v 1.22 2021/12/31 14:22:11 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_taskq.c,v 1.23 2021/12/31 14:29:14 riastradh Exp $");
 
 #include 
 #include 
@@ -280,7 +280,7 @@ tqbarrier_task(void *cookie)
 void
 sysmon_task_queue_barrier(u_int pri)
 {
-	struct sysmon_task st;
+	struct sysmon_task *st;
 	struct tqbarrier bar;
 
 	(void)RUN_ONCE(_tq, tq_preinit);
@@ -292,11 +292,12 @@ sysmon_task_queue_barrier(u_int pri)
 	cv_init(, "sysmontq");
 	bar.done = false;
 
-	st.st_func = _task;
-	st.st_arg = 
-	st.st_pri = pri;
+	st = malloc(sizeof(*st), M_TEMP, M_WAITOK);
+	st->st_func = _task;
+	st->st_arg = 
+	st->st_pri = pri;
 
-	sysmon_task_queue_sched_task();
+	sysmon_task_queue_sched_task(st);
 
 	mutex_enter();
 	while (!bar.done)

CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Dec 31 14:29:14 UTC 2021

Modified Files:
src/sys/dev/sysmon: sysmon_taskq.c

Log Message:
sysmon(9): Fix mistake in sysmon_task_queue_barrier.

Need to allocate the task structure with malloc(9), not on the stack;
the task thread will free it with free(9).  Forgot to squash this
into previous commit.


To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/sysmon/sysmon_taskq.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Dec 31 11:05:41 UTC 2021

Modified Files:
src/sys/dev/sysmon: swsensor.c swwdog.c sysmon.c sysmon_envsys.c
sysmon_envsys_events.c sysmon_envsysvar.h sysmon_power.c
sysmon_taskq.c sysmon_wdog.c sysmonvar.h

Log Message:
sysmon: Delete trailing whitespace.  No functional change intended.


To generate a diff of this commit:
cvs rdiff -u -r1.18 -r1.19 src/sys/dev/sysmon/swsensor.c
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/sysmon/swwdog.c
cvs rdiff -u -r1.30 -r1.31 src/sys/dev/sysmon/sysmon.c
cvs rdiff -u -r1.148 -r1.149 src/sys/dev/sysmon/sysmon_envsys.c
cvs rdiff -u -r1.121 -r1.122 src/sys/dev/sysmon/sysmon_envsys_events.c
cvs rdiff -u -r1.47 -r1.48 src/sys/dev/sysmon/sysmon_envsysvar.h
cvs rdiff -u -r1.68 -r1.69 src/sys/dev/sysmon/sysmon_power.c
cvs rdiff -u -r1.20 -r1.21 src/sys/dev/sysmon/sysmon_taskq.c
cvs rdiff -u -r1.29 -r1.30 src/sys/dev/sysmon/sysmon_wdog.c
cvs rdiff -u -r1.50 -r1.51 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/swsensor.c
diff -u src/sys/dev/sysmon/swsensor.c:1.18 src/sys/dev/sysmon/swsensor.c:1.19
--- src/sys/dev/sysmon/swsensor.c:1.18	Thu Jun 11 02:39:31 2020
+++ src/sys/dev/sysmon/swsensor.c	Fri Dec 31 11:05:41 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: swsensor.c,v 1.18 2020/06/11 02:39:31 thorpej Exp $ */
+/*	$NetBSD: swsensor.c,v 1.19 2021/12/31 11:05:41 riastradh Exp $ */
 /*
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
  * All rights reserved.
@@ -27,7 +27,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: swsensor.c,v 1.18 2020/06/11 02:39:31 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: swsensor.c,v 1.19 2021/12/31 11:05:41 riastradh Exp $");
 
 #include 
 #include 
@@ -139,18 +139,18 @@ swsensor_refresh(struct sysmon_envsys *s
  * Sensor get/set limit routines
  */
 
-static void 
+static void
 swsensor_get_limits(struct sysmon_envsys *sme, envsys_data_t *edata,
-  sysmon_envsys_lim_t *limits, uint32_t *props)  
+  sysmon_envsys_lim_t *limits, uint32_t *props)
 {
 
 	*props = PROP_CRITMIN | PROP_DRIVER_LIMITS;
 	limits->sel_critmin = sw_sensor_limit;
 }
 
-static void 
+static void
 swsensor_set_limits(struct sysmon_envsys *sme, envsys_data_t *edata,
-  sysmon_envsys_lim_t *limits, uint32_t *props)  
+  sysmon_envsys_lim_t *limits, uint32_t *props)
 {
 
 	if (limits == NULL) {
@@ -250,7 +250,7 @@ swsensor_init(void *arg)
 			/* Sensor limit behavior
 			 *	0 - simple sensor, no hw limits
 			 *	1 - simple sensor, hw provides initial limit
-			 *	2 - complex sensor, hw provides settable 
+			 *	2 - complex sensor, hw provides settable
 			 *	limits and does its own limit checking
 			 */
 			if (strcmp(key, "mode") == 0) {

Index: src/sys/dev/sysmon/swwdog.c
diff -u src/sys/dev/sysmon/swwdog.c:1.22 src/sys/dev/sysmon/swwdog.c:1.23
--- src/sys/dev/sysmon/swwdog.c:1.22	Mon Mar 16 21:20:09 2020
+++ src/sys/dev/sysmon/swwdog.c	Fri Dec 31 11:05:41 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: swwdog.c,v 1.22 2020/03/16 21:20:09 pgoyette Exp $	*/
+/*	$NetBSD: swwdog.c,v 1.23 2021/12/31 11:05:41 riastradh Exp $	*/
 
 /*
  * Copyright (c) 2004, 2005 Steven M. Bellovin
@@ -33,7 +33,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: swwdog.c,v 1.22 2020/03/16 21:20:09 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: swwdog.c,v 1.23 2021/12/31 11:05:41 riastradh Exp $");
 
 /*
  *
@@ -286,8 +286,7 @@ SYSCTL_SETUP(swwdog_sysctl_setup, "swwdo
  * Module management
  */
 
-static
-int
+static int
 swwdog_init(void *arg)
 {
 	/*
@@ -314,8 +313,7 @@ swwdog_init(void *arg)
 	return error;
 }
 
-static
-int
+static int
 swwdog_fini(void *arg)
 {
 	int error;
@@ -337,26 +335,22 @@ swwdog_fini(void *arg)
 return error;
 }
 
-static
-int
+static int
 swwdog_modcmd(modcmd_t cmd, void *arg)
 {
 	int ret;
- 
+
 	switch (cmd) {
 	case MODULE_CMD_INIT:
 		ret = swwdog_init(arg);
 		break;
- 
 	case MODULE_CMD_FINI:
 		ret = swwdog_fini(arg);
 		break;
- 
 	case MODULE_CMD_STAT:
 	default:
 		ret = ENOTTY;
 	}
- 
+
 	return ret;
 }
-

Index: src/sys/dev/sysmon/sysmon.c
diff -u src/sys/dev/sysmon/sysmon.c:1.30 src/sys/dev/sysmon/sysmon.c:1.31
--- src/sys/dev/sysmon/sysmon.c:1.30	Mon Aug 26 10:05:34 2019
+++ src/sys/dev/sysmon/sysmon.c	Fri Dec 31 11:05:41 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: sysmon.c,v 1.30 2019/08/26 10:05:34 nakayama Exp $	*/
+/*	$NetBSD: sysmon.c,v 1.31 2021/12/31 11:05:41 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2000 Zembu Labs, Inc.
@@ -39,7 +39,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sysmon.c,v 1.30 2019/08/26 10:05:34 nakayama Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon.c,v 1.31 2021/12/31 11:05:41 riastradh Exp $");
 
 #include 
 #include 
@@ -78,7 +78,7 @@ const struct cdevsw sysmon_cdevsw = {
 	.d_flag = D_OTHER | D_MPSAFE
 };
 
-static int	

CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Dec 31 11:05:41 UTC 2021

Modified Files:
src/sys/dev/sysmon: swsensor.c swwdog.c sysmon.c sysmon_envsys.c
sysmon_envsys_events.c sysmon_envsysvar.h sysmon_power.c
sysmon_taskq.c sysmon_wdog.c sysmonvar.h

Log Message:
sysmon: Delete trailing whitespace.  No functional change intended.


To generate a diff of this commit:
cvs rdiff -u -r1.18 -r1.19 src/sys/dev/sysmon/swsensor.c
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/sysmon/swwdog.c
cvs rdiff -u -r1.30 -r1.31 src/sys/dev/sysmon/sysmon.c
cvs rdiff -u -r1.148 -r1.149 src/sys/dev/sysmon/sysmon_envsys.c
cvs rdiff -u -r1.121 -r1.122 src/sys/dev/sysmon/sysmon_envsys_events.c
cvs rdiff -u -r1.47 -r1.48 src/sys/dev/sysmon/sysmon_envsysvar.h
cvs rdiff -u -r1.68 -r1.69 src/sys/dev/sysmon/sysmon_power.c
cvs rdiff -u -r1.20 -r1.21 src/sys/dev/sysmon/sysmon_taskq.c
cvs rdiff -u -r1.29 -r1.30 src/sys/dev/sysmon/sysmon_wdog.c
cvs rdiff -u -r1.50 -r1.51 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.

CVS commit: src/sys/dev/sysmon

[Jason R Thorpe]

Module Name:src
Committed By:   thorpej
Date:   Sun Sep 26 16:24:21 UTC 2021

Modified Files:
src/sys/dev/sysmon: sysmon_power.c

Log Message:
- Call selnotify() with sysmon_power_event_queue_mtx held, passing the
  correct hints.  Adjust filt_sysmon_power_read() accordingly (assert
  that the mutex is held iff NOTE_SUBMIT).
- Mark sysmon_power_read_filtops as MPSAFE.
- Use seltrue_filtops rather than rolling our own with filt_seltrue.


To generate a diff of this commit:
cvs rdiff -u -r1.67 -r1.68 src/sys/dev/sysmon/sysmon_power.c

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_power.c
diff -u src/sys/dev/sysmon/sysmon_power.c:1.67 src/sys/dev/sysmon/sysmon_power.c:1.68
--- src/sys/dev/sysmon/sysmon_power.c:1.67	Sun Sep 26 01:16:09 2021
+++ src/sys/dev/sysmon/sysmon_power.c	Sun Sep 26 16:24:21 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: sysmon_power.c,v 1.67 2021/09/26 01:16:09 thorpej Exp $	*/
+/*	$NetBSD: sysmon_power.c,v 1.68 2021/09/26 16:24:21 thorpej Exp $	*/
 
 /*-
  * Copyright (c) 2007 Juan Romero Pardines.
@@ -69,7 +69,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sysmon_power.c,v 1.67 2021/09/26 01:16:09 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_power.c,v 1.68 2021/09/26 16:24:21 thorpej Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -419,8 +419,9 @@ sysmon_power_daemon_task(struct power_ev
 		ped->flags |= SYSMON_POWER_DICTIONARY_READY;
 		SIMPLEQ_INSERT_TAIL(_dict_list, ped, pev_dict_head);
 		cv_broadcast(_power_event_queue_cv);
+		selnotify(_power_event_queue_selinfo,
+		POLLIN | POLLRDNORM, NOTE_SUBMIT);
 		mutex_exit(_power_event_queue_mtx);
-		selnotify(_power_event_queue_selinfo, 0, 0);
 	}
 
 out:
@@ -546,27 +547,28 @@ static int
 filt_sysmon_power_read(struct knote *kn, long hint)
 {
 
-	mutex_enter(_power_event_queue_mtx);
+	if (hint & NOTE_SUBMIT) {
+		KASSERT(mutex_owned(_power_event_queue_mtx));
+	} else {
+		mutex_enter(_power_event_queue_mtx);
+	}
+
 	kn->kn_data = sysmon_power_event_queue_count;
-	mutex_exit(_power_event_queue_mtx);
+
+	if ((hint & NOTE_SUBMIT) == 0) {
+		mutex_exit(_power_event_queue_mtx);
+	}
 
 	return kn->kn_data > 0;
 }
 
 static const struct filterops sysmon_power_read_filtops = {
-	.f_flags = FILTEROP_ISFD,
+	.f_flags = FILTEROP_ISFD | FILTEROP_MPSAFE,
 	.f_attach = NULL,
 	.f_detach = filt_sysmon_power_rdetach,
 	.f_event = filt_sysmon_power_read,
 };
 
-static const struct filterops sysmon_power_write_filtops = {
-	.f_flags = FILTEROP_ISFD,
-	.f_attach = NULL,
-	.f_detach = filt_sysmon_power_rdetach,
-	.f_event = filt_seltrue,
-};
-
 /*
  * sysmonkqfilter_power:
  *
@@ -579,20 +581,19 @@ sysmonkqfilter_power(dev_t dev, struct k
 	switch (kn->kn_filter) {
 	case EVFILT_READ:
 		kn->kn_fop = _power_read_filtops;
+		mutex_enter(_power_event_queue_mtx);
+		selrecord_knote(_power_event_queue_selinfo, kn);
+		mutex_exit(_power_event_queue_mtx);
 		break;
 
 	case EVFILT_WRITE:
-		kn->kn_fop = _power_write_filtops;
+		kn->kn_fop = _filtops;
 		break;
 
 	default:
 		return EINVAL;
 	}
 
-	mutex_enter(_power_event_queue_mtx);
-	selrecord_knote(_power_event_queue_selinfo, kn);
-	mutex_exit(_power_event_queue_mtx);
-
 	return 0;
 }
 

CVS commit: src/sys/dev/sysmon

[Jason R Thorpe]

Module Name:src
Committed By:   thorpej
Date:   Sun Sep 26 16:24:21 UTC 2021

Modified Files:
src/sys/dev/sysmon: sysmon_power.c

Log Message:
- Call selnotify() with sysmon_power_event_queue_mtx held, passing the
  correct hints.  Adjust filt_sysmon_power_read() accordingly (assert
  that the mutex is held iff NOTE_SUBMIT).
- Mark sysmon_power_read_filtops as MPSAFE.
- Use seltrue_filtops rather than rolling our own with filt_seltrue.


To generate a diff of this commit:
cvs rdiff -u -r1.67 -r1.68 src/sys/dev/sysmon/sysmon_power.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Takeshi Nakayama]

Module Name:src
Committed By:   nakayama
Date:   Mon Aug 26 10:05:35 UTC 2019

Modified Files:
src/sys/dev/sysmon: sysmon.c

Log Message:
Module class of sysmon_envsys, sysmon_wdog and sysmon_power is
MODULE_CLASS_DRIVER, not MODULE_CLASS_MISC.

Fix that invoking envsys without sysmon_envsys kernel module failes with:
WARNING: module error: incompatible module class for `sysmon_envsys' (1 != 3)


To generate a diff of this commit:
cvs rdiff -u -r1.29 -r1.30 src/sys/dev/sysmon/sysmon.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Takeshi Nakayama]

Module Name:src
Committed By:   nakayama
Date:   Mon Aug 26 10:05:35 UTC 2019

Modified Files:
src/sys/dev/sysmon: sysmon.c

Log Message:
Module class of sysmon_envsys, sysmon_wdog and sysmon_power is
MODULE_CLASS_DRIVER, not MODULE_CLASS_MISC.

Fix that invoking envsys without sysmon_envsys kernel module failes with:
WARNING: module error: incompatible module class for `sysmon_envsys' (1 != 3)


To generate a diff of this commit:
cvs rdiff -u -r1.29 -r1.30 src/sys/dev/sysmon/sysmon.c

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.c
diff -u src/sys/dev/sysmon/sysmon.c:1.29 src/sys/dev/sysmon/sysmon.c:1.30
--- src/sys/dev/sysmon/sysmon.c:1.29	Fri Apr 26 08:38:25 2019
+++ src/sys/dev/sysmon/sysmon.c	Mon Aug 26 10:05:34 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: sysmon.c,v 1.29 2019/04/26 08:38:25 pgoyette Exp $	*/
+/*	$NetBSD: sysmon.c,v 1.30 2019/08/26 10:05:34 nakayama Exp $	*/
 
 /*-
  * Copyright (c) 2000 Zembu Labs, Inc.
@@ -39,7 +39,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sysmon.c,v 1.29 2019/04/26 08:38:25 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon.c,v 1.30 2019/08/26 10:05:34 nakayama Exp $");
 
 #include 
 #include 
@@ -151,7 +151,7 @@ sysmonopen(dev_t dev, int flag, int mode
 		if (sysmon_opvec_table[minor(dev)] == NULL) {
 			mutex_exit(_minor_mtx);
 			error = module_autoload(sysmon_mod[minor(dev)],
-		MODULE_CLASS_MISC);
+			MODULE_CLASS_DRIVER);
 			if (error)
 return error;
 			mutex_enter(_minor_mtx);

CVS commit: src/sys/dev/sysmon

[Maxime Villard]

Module Name:src
Committed By:   maxv
Date:   Tue Aug 20 18:43:57 UTC 2019

Modified Files:
src/sys/dev/sysmon: sysmon_power.c

Log Message:
Fix info leak, not all of 'pev' is initialized.


To generate a diff of this commit:
cvs rdiff -u -r1.60 -r1.61 src/sys/dev/sysmon/sysmon_power.c

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_power.c
diff -u src/sys/dev/sysmon/sysmon_power.c:1.60 src/sys/dev/sysmon/sysmon_power.c:1.61
--- src/sys/dev/sysmon/sysmon_power.c:1.60	Fri Mar  1 11:06:56 2019
+++ src/sys/dev/sysmon/sysmon_power.c	Tue Aug 20 18:43:57 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: sysmon_power.c,v 1.60 2019/03/01 11:06:56 pgoyette Exp $	*/
+/*	$NetBSD: sysmon_power.c,v 1.61 2019/08/20 18:43:57 maxv Exp $	*/
 
 /*-
  * Copyright (c) 2007 Juan Romero Pardines.
@@ -69,7 +69,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sysmon_power.c,v 1.60 2019/03/01 11:06:56 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_power.c,v 1.61 2019/08/20 18:43:57 maxv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -331,6 +331,8 @@ sysmon_power_daemon_task(struct power_ev
 	if (!ped || !ped->dict || !pev_data)
 		return EINVAL;
 
+	memset(, 0, sizeof(pev));
+
 	mutex_enter(_power_event_queue_mtx);
 	
 	switch (event) {

CVS commit: src/sys/dev/sysmon

[Maxime Villard]

Module Name:src
Committed By:   maxv
Date:   Tue Aug 20 18:43:57 UTC 2019

Modified Files:
src/sys/dev/sysmon: sysmon_power.c

Log Message:
Fix info leak, not all of 'pev' is initialized.


To generate a diff of this commit:
cvs rdiff -u -r1.60 -r1.61 src/sys/dev/sysmon/sysmon_power.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Re: CVS commit: src/sys/dev/sysmon

[Jason Thorpe]

Unknown of course

-- thorpej
Sent from my iPhone.

> On May 26, 2018, at 2:15 PM, Jason R Thorpe  wrote:
> 
> Module Name:src
> Committed By:thorpej
> Date:Sat May 26 21:15:46 UTC 2018
> 
> Modified Files:
>src/sys/dev/sysmon: sysmon_envsys.c
> 
> Log Message:
> Avoid dereferencing NULL if we attempt to look up an known unit type.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.142 -r1.143 src/sys/dev/sysmon/sysmon_envsys.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

yeah - done!

thanks!

On Tue, 23 Jun 2015, Christoph Badura wrote:


On Tue, Jun 23, 2015 at 10:41:42AM +, Paul Goyette wrote:

Committed By:   pgoyette
Date:   Tue Jun 23 10:41:42 UTC 2015

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
Fix the KASSERT - we want to make sure that _both_ pointers are non-NULL,
not just that one or the other is non-NULL!


Wouldn't it be better written as:

KASSERT(sme != NULL);
KASSERT(edata != NULL);

Same effect, but when one of the triggers you know immediately which pointer
was NULL.

--chris



-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Christoph Badura]

On Tue, Jun 23, 2015 at 10:41:42AM +, Paul Goyette wrote:
 Committed By: pgoyette
 Date: Tue Jun 23 10:41:42 UTC 2015
 
 Modified Files:
   src/sys/dev/sysmon: sysmon_envsys_events.c
 
 Log Message:
 Fix the KASSERT - we want to make sure that _both_ pointers are non-NULL,
 not just that one or the other is non-NULL!

Wouldn't it be better written as:

KASSERT(sme != NULL);
KASSERT(edata != NULL);

Same effect, but when one of the triggers you know immediately which pointer
was NULL.

--chris

Re: CVS commit: src/sys/dev/sysmon

[Christos Zoulas]

On May 5, 10:22am, p...@vps1.whooppee.com (Paul Goyette) wrote:
-- Subject: Re: CVS commit: src/sys/dev/sysmon

| On Tue, 5 May 2015, Christos Zoulas wrote:
| 
|  In article 20150505002825.9e50...@cvs.netbsd.org,
|  Paul Goyette source-changes-d@NetBSD.org wrote:
|  -=-=-=-=-=-
| 
|  Module Name:   src
|  Committed By:  pgoyette
|  Date:  Tue May  5 00:28:25 UTC 2015
| 
|  Modified Files:
| src/sys/dev/sysmon: sysmon.c
| 
|  Log Message:
|  If module_autoload() returns an error, just return that value instead
|  of overwriting with ENODEV.
| 
|  Thanks, christos!
| 
|  Yes, but break before the mutex_enter...
| 
| Need to hold the mutex so we can release it below...

Return error then?

christos

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

On Tue, 5 May 2015, Christos Zoulas wrote:


|  If module_autoload() returns an error, just return that value instead
|  of overwriting with ENODEV.
| 
| 
|  Yes, but break before the mutex_enter...
|
| Need to hold the mutex so we can release it below...

Return error then?


Yeah, possible. let me have another look.



-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

On Tue, 5 May 2015, Christos Zoulas wrote:


In article 20150505002825.9e50...@cvs.netbsd.org,
Paul Goyette source-changes-d@NetBSD.org wrote:

-=-=-=-=-=-

Module Name:src
Committed By:   pgoyette
Date:   Tue May  5 00:28:25 UTC 2015

Modified Files:
src/sys/dev/sysmon: sysmon.c

Log Message:
If module_autoload() returns an error, just return that value instead
of overwriting with ENODEV.

Thanks, christos!


Yes, but break before the mutex_enter...


Need to hold the mutex so we can release it below...


-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Christos Zoulas]

In article 20150504235036.d534...@cvs.netbsd.org,
Paul Goyette source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  pgoyette
Date:  Mon May  4 23:50:36 UTC 2015

Modified Files:
   src/sys/dev/sysmon: sysmon.c

Log Message:
If autoload of the subcomponent module fails, don't try to call its
open routine.  Just return an error.

Hopefully this will fix the recently reported issues with atf tests
running on xen guest.


To generate a diff of this commit:
cvs rdiff -u -r1.25 -r1.26 src/sys/dev/sysmon/sysmon.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


-=-=-=-=-=-

 #include sys/param.h
 #include sys/conf.h
@@ -153,8 +153,10 @@ sysmonopen(dev_t dev, int flag, int mode
   error = module_autoload(sysmon_mod[minor(dev)],
   MODULE_CLASS_MISC);

I am confused; at this point if autoload returned an error, why bother
continuing and losing the error return from the autoload? Why not add:
if (error)
break;

christos

   mutex_enter(sysmon_minor_mtx);
-  if (sysmon_opvec_table[minor(dev)] == NULL)
+  if (sysmon_opvec_table[minor(dev)] == NULL) {
   error = ENODEV;
+  break;
+  }
   }
   error = (sysmon_opvec_table[minor(dev)]-so_open)(dev, flag,
   mode, l);


-=-=-=-=-=-

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

On Tue, 5 May 2015, Christos Zoulas wrote:


Module Name:src
Committed By:   pgoyette
Date:   Mon May  4 23:50:36 UTC 2015

Modified Files:
src/sys/dev/sysmon: sysmon.c

Log Message:
If autoload of the subcomponent module fails, don't try to call its
open routine.  Just return an error.

Hopefully this will fix the recently reported issues with atf tests
running on xen guest.

-=-=-=-=-=-

#include sys/param.h
#include sys/conf.h
@@ -153,8 +153,10 @@ sysmonopen(dev_t dev, int flag, int mode
error = module_autoload(sysmon_mod[minor(dev)],
MODULE_CLASS_MISC);


I am confused; at this point if autoload returned an error, why bother
continuing and losing the error return from the autoload? Why not add:
if (error)
break;


mutex_enter(sysmon_minor_mtx);
-   if (sysmon_opvec_table[minor(dev)] == NULL)
+   if (sysmon_opvec_table[minor(dev)] == NULL) {
error = ENODEV;
+   break;
+   }
}
error = (sysmon_opvec_table[minor(dev)]-so_open)(dev, flag,
mode, l);


-=-=-=-=-=-



Yeah, makes sense.  I will update accordingly.


-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Christos Zoulas]

In article 20150505002825.9e50...@cvs.netbsd.org,
Paul Goyette source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  pgoyette
Date:  Tue May  5 00:28:25 UTC 2015

Modified Files:
   src/sys/dev/sysmon: sysmon.c

Log Message:
If module_autoload() returns an error, just return that value instead
of overwriting with ENODEV.

Thanks, christos!

Yes, but break before the mutex_enter...

christos



To generate a diff of this commit:
cvs rdiff -u -r1.26 -r1.27 src/sys/dev/sysmon/sysmon.c

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.c
diff -u src/sys/dev/sysmon/sysmon.c:1.26 src/sys/dev/sysmon/sysmon.c:1.27
--- src/sys/dev/sysmon/sysmon.c:1.26   Mon May  4 23:50:36 2015
+++ src/sys/dev/sysmon/sysmon.cTue May  5 00:28:25 2015
@@ -1,4 +1,4 @@
-/*$NetBSD: sysmon.c,v 1.26 2015/05/04 23:50:36 pgoyette Exp $ */
+/*$NetBSD: sysmon.c,v 1.27 2015/05/05 00:28:25 pgoyette Exp $ */
 
 /*-
  * Copyright (c) 2000 Zembu Labs, Inc.
@@ -39,7 +39,7 @@
  */
 
 #include sys/cdefs.h
-__KERNEL_RCSID(0, $NetBSD: sysmon.c,v 1.26 2015/05/04 23:50:36
pgoyette Exp $);
+__KERNEL_RCSID(0, $NetBSD: sysmon.c,v 1.27 2015/05/05 00:28:25
pgoyette Exp $);
 
 #include sys/param.h
 #include sys/conf.h
@@ -153,6 +153,8 @@ sysmonopen(dev_t dev, int flag, int mode
   error = module_autoload(sysmon_mod[minor(dev)],
   MODULE_CLASS_MISC);
   mutex_enter(sysmon_minor_mtx);
+  if (error)
+  break;
   if (sysmon_opvec_table[minor(dev)] == NULL) {
   error = ENODEV;
   break;


-=-=-=-=-=-

Re: CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

   Date: Tue, 28 Apr 2015 17:01:10 +0800 (PHT)
   From: Paul Goyette p...@vps1.whooppee.com

   Well, it really is a (pseudo-)device, and even has its own major device 
   number.

   We cannot avoid all of the device goop, since a non-built-in-module 
   would still have to merge in the devsw.  My goal is to eventually be 
   able to build and run a kernel which has no portion of sysmon built-in, 
   and still be able to load and unload the wdog/power/envsys components, 
   along with the actual drivers for timers/power-switches/sensors.

But what purpose does attaching an *autoconf* device serve?

The devsw is the userland interface, /dev/sysmon, and is not actually
related to autoconf devices.  You can attach a devsw without any
associated autoconf device, and vice versa.

The defpseudo config directive with the associated (often empty)
xyzattach routine allows you to include or exclude the code from a
kernel statically with the pseudo-device directive.

Some pseudo-devices have multiple instances, e.g. cgd(4): each
/dev/cgdN needs its own separate state.  So it uses multiple autoconf
devices to manage the numbered instances.  Some pseudo-devices attach
children, so they also need an autoconf device.

In the case of sysmon, there's only one global state.  Some of it is
put into a softc struct (sc_minor_mtx); the rest of it is in global
variables (sysmon_opvec_table, sysmon_refcnt).  It doesn't seem to
attach any children.  There's no defpseudo, so you can't configure it
statically in your kernel.

I think you could remove the whole cfattach/cfdriver/sysmon_match/c.
business and replace it by initialization in MODULE_CMD_INIT (plus the
RUN_ONCE nonsense because builtin modules get initialized too late).

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Well, it really is a (pseudo-)device, and even has its own major device 
number.


We cannot avoid all of the device goop, since a non-built-in-module 
would still have to merge in the devsw.  My goal is to eventually be 
able to build and run a kernel which has no portion of sysmon built-in, 
and still be able to load and unload the wdog/power/envsys components, 
along with the actual drivers for timers/power-switches/sensors.


It's really not all that complicated.


On Tue, 28 Apr 2015, Taylor R Campbell wrote:


  Date: Thu, 23 Apr 2015 23:22:03 +
  From: Paul Goyette pgoye...@netbsd.org

  Modularize sysmon and its components

Why does this introduce a sysmon(4) autoconf device?  It seems like
needless bookkeeping -- you could just make the mutex global, like all
the state it protects, and skip all the match/attach/detach/cfdriver
autoconf business.



-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Taylor R Campbell]

   Date: Thu, 23 Apr 2015 23:22:03 +
   From: Paul Goyette pgoye...@netbsd.org

   Modularize sysmon and its components

Why does this introduce a sysmon(4) autoconf device?  It seems like
needless bookkeeping -- you could just make the mutex global, like all
the state it protects, and skip all the match/attach/detach/cfdriver
autoconf business.

re: CVS commit: src/sys/dev/sysmon

[matthew green]

Christos Zoulas writes:
 Module Name:  src
 Committed By: christos
 Date: Sat Apr 25 14:06:58 UTC 2015
 
 Modified Files:
   src/sys/dev/sysmon: sysmon_envsys.c
 
 Log Message:
 make things boot again, from martin.

thanks.  updating to test now..

but shouldn't this use RUN_ONCE() and/or a lock/atomic?


.mrg.

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Done.  Doing some testing now but should be ok.

On Sat, 25 Apr 2015, Christos Zoulas wrote:


In article 1723.1429983...@splode.eterna.com.au,
matthew green  m...@eterna.com.au wrote:


Christos Zoulas writes:

Module Name:src
Committed By:   christos
Date:   Sat Apr 25 14:06:58 UTC 2015

Modified Files:
src/sys/dev/sysmon: sysmon_envsys.c

Log Message:
make things boot again, from martin.


thanks.  updating to test now..

but shouldn't this use RUN_ONCE() and/or a lock/atomic?


Yes, I am letting paul fix it properly the way he wants.

christos




-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Thanks.   :)

I'll get this (and the corresponding changes to sysmon_power and 
sysmon_wdog subcomponents) done shortly.




On Sat, 25 Apr 2015, Christos Zoulas wrote:


In article 1723.1429983...@splode.eterna.com.au,
matthew green  m...@eterna.com.au wrote:


Christos Zoulas writes:

Module Name:src
Committed By:   christos
Date:   Sat Apr 25 14:06:58 UTC 2015

Modified Files:
src/sys/dev/sysmon: sysmon_envsys.c

Log Message:
make things boot again, from martin.


thanks.  updating to test now..

but shouldn't this use RUN_ONCE() and/or a lock/atomic?


Yes, I am letting paul fix it properly the way he wants.

christos




-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Jeff Rizzo]

On 4/25/15 3:46 PM, Paul Goyette wrote:

Module Name:src
Committed By:   pgoyette
Date:   Sat Apr 25 22:46:31 UTC 2015


  
+	if (pmf_device_register(sysmon_dev, NULL, NULL))

+   aprintf_error(%s: failed to register with pmf\n,



I think you mean aprint_error.  This doesn't link.

+j

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Ooops - this should have been two commits.

Log message is correct for sysmon.c

For the others, message has been corrected to read:

Handle early initialization requirements - thanks martin@
and others



On Sat, 25 Apr 2015, Paul Goyette wrote:


Module Name:src
Committed By:   pgoyette
Date:   Sat Apr 25 23:40:09 UTC 2015

Modified Files:
src/sys/dev/sysmon: sysmon.c sysmon_envsys.c sysmon_power.c
sysmon_wdog.c

Log Message:
Correctly check return status when registering with pmf


To generate a diff of this commit:
cvs rdiff -u -r1.23 -r1.24 src/sys/dev/sysmon/sysmon.c
cvs rdiff -u -r1.136 -r1.137 src/sys/dev/sysmon/sysmon_envsys.c
cvs rdiff -u -r1.54 -r1.55 src/sys/dev/sysmon/sysmon_power.c
cvs rdiff -u -r1.26 -r1.27 src/sys/dev/sysmon/sysmon_wdog.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.




-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Yup - fixing now...

On Sat, 25 Apr 2015, Jeff Rizzo wrote:


On 4/25/15 3:46 PM, Paul Goyette wrote:

Module Name:src
Committed By:   pgoyette
Date:   Sat Apr 25 22:46:31 UTC 2015



  + if (pmf_device_register(sysmon_dev, NULL, NULL))
+   aprintf_error(%s: failed to register with pmf\n,



I think you mean aprint_error.  This doesn't link.

+j




-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Christos Zoulas]

In article 1723.1429983...@splode.eterna.com.au,
matthew green  m...@eterna.com.au wrote:

Christos Zoulas writes:
 Module Name: src
 Committed By:christos
 Date:Sat Apr 25 14:06:58 UTC 2015
 
 Modified Files:
  src/sys/dev/sysmon: sysmon_envsys.c
 
 Log Message:
 make things boot again, from martin.

thanks.  updating to test now..

but shouldn't this use RUN_ONCE() and/or a lock/atomic?

Yes, I am letting paul fix it properly the way he wants.

christos

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

On Sun, 19 Jun 2011, Martin Husemann wrote:


Module Name:src
Committed By:   martin
Date:   Sun Jun 19 11:40:26 UTC 2011

Modified Files:
src/sys/dev/sysmon: sysmon_envsys.c

Log Message:
Instead of crashing, print a usable error message when a driver eroneously
does not properly initialize sensor state.


Thanks to yourself, Christos, and NONAKA-san for finishing up this work 
while I was sleeping!




-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |  | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

On Wed, 15 Jun 2011, Paul Goyette wrote:


Module Name:src
Committed By:   pgoyette
Date:   Wed Jun 15 13:34:13 UTC 2011

Modified Files:
src/sys/dev/sysmon: sysmon_envsys.c

Log Message:
Previous fix broke the setting of current value for Indicator and
Battery-Charge sensors.  Fix this.


Forgot to add:

Thanks to jruoho@ for finding this.


-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |  | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Jukka Ruohonen]

On Sat, Jun 04, 2011 at 01:24:33PM +, Paul Goyette wrote:
 Since nothing actually records or maintains an average sensor value,
 remove value_avg from the prop_dict.  We can't completely remove it
 because there's one driver that uses this field for its own private
 purposes, so for now we just rename the member.

Wouldn't it be better to just modify the (apparently misbehaving) driver
instead of harness the whole KPI for one quirk?

- Jukka.

Re: CVS commit: src/sys/dev/sysmon

[Paul Goyette]

On Sat, 4 Jun 2011, Jukka Ruohonen wrote:


On Sat, Jun 04, 2011 at 01:24:33PM +, Paul Goyette wrote:

Since nothing actually records or maintains an average sensor value,
remove value_avg from the prop_dict.  We can't completely remove it
because there's one driver that uses this field for its own private
purposes, so for now we just rename the member.


Wouldn't it be better to just modify the (apparently misbehaving) driver
instead of harness the whole KPI for one quirk?


Yes, that driver will be modified.  It is dev/pci/arcmsr.c that does the 
abuse.  Unfortunately there is more abuse in that driver, so clean-up 
will take a little bit longer.


Note that when that modification does happen, it will require a version 
bump, since the ABI for sensor modules will have changed.



-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |  | pgoyette at netbsd.org  |
-

Re: CVS commit: src/sys/dev/sysmon

[Antti Kantee]

On Sat Oct 23 2010 at 01:49:25 +, Jonathan A. Kollasch wrote:
 On Wed, Oct 20, 2010 at 06:50:47PM +, Antti Kantee wrote:
  Module Name:src
  Committed By:   pooka
  Date:   Wed Oct 20 18:50:47 UTC 2010
  
  Modified Files:
  src/sys/dev/sysmon: swsensor.c
  
  Log Message:
  remove unused routine (per comment)
  
  
  To generate a diff of this commit:
  cvs rdiff -u -r1.1 -r1.2 src/sys/dev/sysmon/swsensor.c
 
 This breaks the i386 ALL kernel.

Ok, I put it back apart from the incorrect comment.  Thanks.

 Also, weren't you one of the folks advocating everyone
 run a release build before committing anything?

No, I was advocating running a release build *after* committing and
checking the code out to a clean tree.  I am also advocating on using
good judgment in how to test commits.  In this case believing the comment
and testing only that part was not good judgment, though.

btw, ALL is not part of a release build ;)

Re: CVS commit: src/sys/dev/sysmon

[Antti Kantee]

On Sat Oct 23 2010 at 04:52:17 -0700, Paul Goyette wrote:
 On Sat, 23 Oct 2010, Antti Kantee wrote:
 
 This breaks the i386 ALL kernel.
 
 Ok, I put it back apart from the incorrect comment.  Thanks.
 
 Yes, my bad for the incorrect comment.  Sorry.

If I got a penny every time I wrote an incorrect comment ... ;)

Reminds me, there was one interesting paper in SOSP'07 about verifying
comment correctness against the code (iComment).  Too bad there were no
NetBSD runs.

Re: CVS commit: src/sys/dev/sysmon

[Jonathan A. Kollasch]

On Wed, Oct 20, 2010 at 06:50:47PM +, Antti Kantee wrote:
 Module Name:  src
 Committed By: pooka
 Date: Wed Oct 20 18:50:47 UTC 2010
 
 Modified Files:
   src/sys/dev/sysmon: swsensor.c
 
 Log Message:
 remove unused routine (per comment)
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.1 -r1.2 src/sys/dev/sysmon/swsensor.c

This breaks the i386 ALL kernel.

Also, weren't you one of the folks advocating everyone
run a release build before committing anything?

Jonathan Kollasch

CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Module Name:src
Committed By:   pgoyette
Date:   Sun Feb 14 16:22:09 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
Correct one more instance of checking flags in the wrong place.


To generate a diff of this commit:
cvs rdiff -u -r1.80 -r1.81 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Module Name:src
Committed By:   pgoyette
Date:   Sun Feb 14 23:30:52 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
Replace large amount of replicated code with a macro to improve readability.


To generate a diff of this commit:
cvs rdiff -u -r1.82 -r1.83 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Module Name:src
Committed By:   pgoyette
Date:   Sat Feb 13 19:34:14 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
When checking current sensor value vs limits, check the correct set of
flags to determine which limits are present.


To generate a diff of this commit:
cvs rdiff -u -r1.79 -r1.80 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Jukka Ruohonen]

Module Name:src
Committed By:   jruoho
Date:   Fri Feb  5 19:22:25 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
Fix a typo in one DPRINTF (s/edata-des/edata-desc/).


To generate a diff of this commit:
cvs rdiff -u -r1.78 -r1.79 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Module Name:src
Committed By:   pgoyette
Date:   Thu Feb  4 18:06:53 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
If the AC adapter is flagged with SME_DISABLE_REFRESH, don't try to call
its refresh() callback.  Just assume that its value has been properly
updated by other means.


To generate a diff of this commit:
cvs rdiff -u -r1.77 -r1.78 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Antti Kantee]

Module Name:src
Committed By:   pooka
Date:   Sun Jan 31 02:54:56 UTC 2010

Modified Files:
src/sys/dev/sysmon: swwdog.c

Log Message:
print newline after timer expiry message


To generate a diff of this commit:
cvs rdiff -u -r1.8 -r1.9 src/sys/dev/sysmon/swwdog.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Module Name:src
Committed By:   pgoyette
Date:   Sat Jan 30 02:46:53 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys.c sysmon_envsys_events.c

Log Message:
Validate limit-based monitoring in one place, so we can apply the
constraints to both driver-requested and user-requested monitoring.

While here, clean up some of the debugging DPRINTFs.


To generate a diff of this commit:
cvs rdiff -u -r1.90 -r1.91 src/sys/dev/sysmon/sysmon_envsys.c
cvs rdiff -u -r1.76 -r1.77 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Module Name:src
Committed By:   pgoyette
Date:   Mon Jan 18 21:48:15 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
If no limits are exceeded, make sure we set state to SVALID


To generate a diff of this commit:
cvs rdiff -u -r1.74 -r1.75 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Module Name:src
Committed By:   pgoyette
Date:   Sat Jan  2 19:02:40 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
Flags for existence of limit values need to be propagated to the
event data.  Otherwise, they're not processed.

XXX This needs to be rethought.  While fixing it this way makes things
XXX work, we really shouldn't need to replicate these flags.  The
XXX limits and their flags should be kept as part of the sensor data,
XXX and not part of the event-monitor data.  I'll work on this in the
XXX near future.


To generate a diff of this commit:
cvs rdiff -u -r1.73 -r1.74 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/sysmon

[Paul Goyette]

Module Name:src
Committed By:   pgoyette
Date:   Fri Jan  1 15:41:25 UTC 2010

Modified Files:
src/sys/dev/sysmon: sysmon_envsys_events.c

Log Message:
Don't infer a sensor driver's ability to handle limit checking itself.


To generate a diff of this commit:
cvs rdiff -u -r1.72 -r1.73 src/sys/dev/sysmon/sysmon_envsys_events.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.