Download raw body.
Please test: mpsafe sensors
I'm not happy with sensor pointers dereference after sleeping malloc(9)
in sysctl_sensors(), so let's start making them mpsafe.
New `sensordev_mtx' protects `sensordev_count' and `sensordev_list'.
Reference counters are used to make sensor device and attached sensors
dereference safe. Not all sensors do sensor_detach(), so sensor
'ksensordev' and 'ksensor' share reference counter.
Each 'ksensordev' has it's own mutex(9) to protect mutable data. Each
'ksensor' has mutex too, but since we have many of them, only
SENSOR_MPSAFE marked sensors rely on it. This diff doesn't introduce any
of them, but it could be easily made on top. Anyway, we have many of
sensors so they need to be unlocked separately.
Please note, except sparc64 only sysctl_sensors() use sensordev_get()
and sensor_find(), drivers have their instances and we don't need to
acquire and release them.
I'm not asking for OK's, this diff was shared to receive feedback and
help with testing. That's why sensors manual page left unmodified.
Index: sys/arch/sparc64/dev/tda.c
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/dev/tda.c,v
diff -u -p -r1.9 tda.c
--- sys/arch/sparc64/dev/tda.c 24 Oct 2021 17:05:04 -0000 1.9
+++ sys/arch/sparc64/dev/tda.c 7 Aug 2024 14:33:53 -0000
@@ -115,10 +115,12 @@ tda_attach(struct device *parent, struct
tda_setspeed(sc);
/* Get the number of sensor devices. */
+ mtx_enter(&sensordev_mtx);
for (i = 0; ; i++) {
- if (sensordev_get(i, &ksens) == ENOENT)
+ if (sensordev_get_locked(i, &ksens) == ENOENT)
break;
}
+ mtx_leave(&sensordev_mtx);
sc->sc_nsensors = i;
if (!sc->sc_nsensors) {
@@ -199,6 +201,7 @@ tda_adjust(void *v)
goto out;
}
ctemp = MAX(ctemp, ks->value);
+ sensor_rele(ks);
err = sensor_find(i, SENSOR_TEMP, SENSOR_TEMP_INT, &ks);
if (err) {
@@ -207,6 +210,7 @@ tda_adjust(void *v)
goto out;
}
stemp = MAX(stemp, ks->value);
+ sensor_rele(ks);
}
if (ctemp < CPU_TEMP_MIN)
Index: sys/kern/kern_sensors.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sensors.c,v
diff -u -p -r1.40 kern_sensors.c
--- sys/kern/kern_sensors.c 5 Dec 2022 23:18:37 -0000 1.40
+++ sys/kern/kern_sensors.c 7 Aug 2024 14:33:57 -0000
@@ -31,18 +31,27 @@
#include <sys/sensors.h>
#include "hotplug.h"
+/*
+ * Locks used to protect data:
+ * D sensordev_mtx
+ */
+
+struct mutex sensordev_mtx = MUTEX_INITIALIZER(IPL_MPFLOOR);
+
struct taskq *sensors_taskq;
-int sensordev_count;
+int sensordev_count; /* [D] */
SLIST_HEAD(, ksensordev) sensordev_list =
- SLIST_HEAD_INITIALIZER(sensordev_list);
+ SLIST_HEAD_INITIALIZER(sensordev_list); /* [D] */
void
sensordev_install(struct ksensordev *sensdev)
{
struct ksensordev *v, *nv;
- int s;
- s = splhigh();
+ refcnt_init(&sensdev->refcnt);
+ mtx_init(&sensdev->mtx, IPL_MPFLOOR);
+
+ mtx_enter(&sensordev_mtx);
if (sensordev_count == 0) {
sensdev->num = 0;
SLIST_INSERT_HEAD(&sensordev_list, sensdev, list);
@@ -54,8 +63,9 @@ sensordev_install(struct ksensordev *sen
sensdev->num = v->num + 1;
SLIST_INSERT_AFTER(v, sensdev, list);
}
+
sensordev_count++;
- splx(s);
+ mtx_leave(&sensordev_mtx);
#if NHOTPLUG > 0
hotplug_device_attach(DV_DULL, "sensordev");
@@ -67,9 +77,12 @@ sensor_attach(struct ksensordev *sensdev
{
struct ksensor *v, *nv;
struct ksensors_head *sh;
- int s, i;
+ int i;
+
+ sens->parent = sensdev;
+ mtx_init(&sens->mtx, IPL_MPFLOOR);
- s = splhigh();
+ mtx_enter(&sensdev->mtx);
sh = &sensdev->sensors_list;
if (sensdev->sensors_count == 0) {
for (i = 0; i < SENSOR_MAX_TYPES; i++)
@@ -95,31 +108,30 @@ sensor_attach(struct ksensordev *sensdev
if (sensdev->maxnumt[sens->type] == sens->numt)
sensdev->maxnumt[sens->type]++;
sensdev->sensors_count++;
- splx(s);
+ mtx_leave(&sensdev->mtx);
}
void
sensordev_deinstall(struct ksensordev *sensdev)
{
- int s;
-
- s = splhigh();
+ mtx_enter(&sensordev_mtx);
sensordev_count--;
SLIST_REMOVE(&sensordev_list, sensdev, ksensordev, list);
- splx(s);
+ mtx_leave(&sensordev_mtx);
#if NHOTPLUG > 0
hotplug_device_detach(DV_DULL, "sensordev");
#endif
+
+ refcnt_finalize(&sensdev->refcnt, "sensordev");
}
void
sensor_detach(struct ksensordev *sensdev, struct ksensor *sens)
{
struct ksensors_head *sh;
- int s;
- s = splhigh();
+ mtx_enter(&sensdev->mtx);
sh = &sensdev->sensors_list;
sensdev->sensors_count--;
SLIST_REMOVE(sh, sens, ksensor, list);
@@ -128,14 +140,16 @@ sensor_detach(struct ksensordev *sensdev
*/
if (sens->numt == sensdev->maxnumt[sens->type] - 1)
sensdev->maxnumt[sens->type]--;
- splx(s);
+ mtx_leave(&sensdev->mtx);
}
int
-sensordev_get(int num, struct ksensordev **sensdev)
+sensordev_get_locked(int num, struct ksensordev **sensdev)
{
struct ksensordev *sd;
+ MUTEX_ASSERT_LOCKED(&sensordev_mtx);
+
SLIST_FOREACH(sd, &sensordev_list, list) {
if (sd->num == num) {
*sensdev = sd;
@@ -148,25 +162,76 @@ sensordev_get(int num, struct ksensordev
}
int
+sensordev_get(int num, struct ksensordev **sensdev)
+{
+ int error;
+
+ mtx_enter(&sensordev_mtx);
+ if ((error = sensordev_get_locked(num, sensdev)) == 0)
+ refcnt_take(&(*sensdev)->refcnt);
+ mtx_leave(&sensordev_mtx);
+
+ return (error);
+}
+
+void
+sensordev_put(struct ksensordev *sensdev)
+{
+ refcnt_rele_wake(&sensdev->refcnt);
+}
+
+int
sensor_find(int dev, enum sensor_type type, int numt, struct ksensor **ksensorp)
{
struct ksensor *s;
struct ksensordev *sensdev;
struct ksensors_head *sh;
- int ret;
+ int error;
- ret = sensordev_get(dev, &sensdev);
- if (ret)
- return (ret);
+ error = sensordev_get(dev, &sensdev);
+ if (error)
+ return (error);
+ mtx_enter(&sensdev->mtx);
sh = &sensdev->sensors_list;
- SLIST_FOREACH(s, sh, list)
- if (s->type == type && s->numt == numt) {
- *ksensorp = s;
- return (0);
- }
+ SLIST_FOREACH(s, sh, list) {
+ if (s->type == type && s->numt == numt)
+ break;
+ }
- return (ENOENT);
+ if (s)
+ *ksensorp = s;
+ else {
+ sensordev_put(sensdev);
+ error = ENOENT;
+ }
+ mtx_leave(&sensdev->mtx);
+
+ return (error);
+}
+
+void
+sensor_rele(struct ksensor *sens)
+{
+ refcnt_rele_wake(&sens->parent->refcnt);
+}
+
+void
+sensor_lock(struct ksensor *sens)
+{
+ if (sens->flags & SENSOR_MPSAFE)
+ mtx_enter(&sens->mtx);
+ else
+ KERNEL_LOCK();
+}
+
+void
+sensor_unlock(struct ksensor *sens)
+{
+ if (sens->flags & SENSOR_MPSAFE)
+ mtx_leave(&sens->mtx);
+ else
+ KERNEL_UNLOCK();
}
struct sensor_task {
Index: sys/kern/kern_sysctl.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
diff -u -p -r1.434 kern_sysctl.c
--- sys/kern/kern_sysctl.c 6 Aug 2024 12:36:54 -0000 1.434
+++ sys/kern/kern_sysctl.c 7 Aug 2024 14:33:58 -0000
@@ -790,7 +790,6 @@ hw_sysctl(int *name, u_int namelen, void
case HW_DISKSTATS:
case HW_CPUSPEED:
#ifndef SMALL_KERNEL
- case HW_SENSORS:
case HW_SETPERF:
case HW_PERFPOLICY:
case HW_BATTERY:
@@ -809,6 +808,11 @@ hw_sysctl(int *name, u_int namelen, void
sysctl_vsunlock(oldp, savelen);
return (err);
}
+#ifndef SMALL_KERNEL
+ case HW_SENSORS:
+ return (sysctl_sensors(name + 1, namelen - 1, oldp, oldlenp,
+ newp, newlen));
+#endif /* !SMALL_KERNEL */
case HW_VENDOR:
if (hw_vendor)
return (sysctl_rdstring(oldp, oldlenp, newp,
@@ -879,9 +883,6 @@ hw_sysctl_locked(int *name, u_int namele
return err;
return (sysctl_rdint(oldp, oldlenp, newp, cpuspeed));
#ifndef SMALL_KERNEL
- case HW_SENSORS:
- return (sysctl_sensors(name + 1, namelen - 1, oldp, oldlenp,
- newp, newlen));
case HW_SETPERF:
return (sysctl_hwsetperf(oldp, oldlenp, newp, newlen));
case HW_PERFPOLICY:
@@ -2656,10 +2657,15 @@ sysctl_sensors(int *name, u_int namelen,
/* Grab a copy, to clear the kernel pointers */
usd = malloc(sizeof(*usd), M_TEMP, M_WAITOK|M_ZERO);
+
+ mtx_enter(&ksd->mtx);
usd->num = ksd->num;
strlcpy(usd->xname, ksd->xname, sizeof(usd->xname));
memcpy(usd->maxnumt, ksd->maxnumt, sizeof(usd->maxnumt));
usd->sensors_count = ksd->sensors_count;
+ mtx_leave(&ksd->mtx);
+
+ sensordev_put(ksd);
ret = sysctl_rdstruct(oldp, oldlenp, newp, usd,
sizeof(struct sensordev));
@@ -2677,13 +2683,18 @@ sysctl_sensors(int *name, u_int namelen,
/* Grab a copy, to clear the kernel pointers */
us = malloc(sizeof(*us), M_TEMP, M_WAITOK|M_ZERO);
+
+ sensor_lock(ks);
memcpy(us->desc, ks->desc, sizeof(us->desc));
us->tv = ks->tv;
us->value = ks->value;
us->type = ks->type;
us->status = ks->status;
us->numt = ks->numt;
- us->flags = ks->flags;
+ us->flags = ks->flags & ~SENSOR_MPSAFE;
+ sensor_unlock(ks);
+
+ sensor_rele(ks);
ret = sysctl_rdstruct(oldp, oldlenp, newp, us,
sizeof(struct sensor));
Index: sys/sys/sensors.h
===================================================================
RCS file: /cvs/src/sys/sys/sensors.h,v
diff -u -p -r1.37 sensors.h
--- sys/sys/sensors.h 15 Jul 2020 07:13:57 -0000 1.37
+++ sys/sys/sensors.h 7 Aug 2024 14:33:58 -0000
@@ -119,6 +119,7 @@ struct sensor {
int flags; /* sensor flags */
#define SENSOR_FINVALID 0x0001 /* sensor is invalid */
#define SENSOR_FUNKNOWN 0x0002 /* sensor value is unknown */
+#define SENSOR_MPSAFE 0x0004 /* sensor is mpsafe */
};
/* Sensor device data:
@@ -133,13 +134,29 @@ struct sensordev {
#ifdef _KERNEL
+#include <sys/refcnt.h>
+
+/*
+ * Locks used to protect data:
+ * I immutable after creation
+ * D sensordev_mtx
+ * d ksensordev's mtx
+ */
+
+struct ksensordev;
+
+extern struct mutex sensordev_mtx;
+
/* Sensor data */
struct ksensor {
- SLIST_ENTRY(ksensor) list; /* device-scope list */
- char desc[32]; /* sensor description, may be empty */
+ struct mutex mtx;
+ struct ksensordev *parent; /* [I] parent ksensordev */
+ SLIST_ENTRY(ksensor) list; /* [d] device-scope list */
+ char desc[32]; /* [I] sensor description, may be
+ empty */
struct timeval tv; /* sensor value last change time */
int64_t value; /* current value */
- enum sensor_type type; /* sensor type */
+ enum sensor_type type; /* [I] sensor type */
enum sensor_status status; /* sensor status */
int numt; /* sensor number of .type type */
int flags; /* sensor flags, ie. SENSOR_FINVALID */
@@ -148,23 +165,31 @@ SLIST_HEAD(ksensors_head, ksensor);
/* Sensor device data */
struct ksensordev {
- SLIST_ENTRY(ksensordev) list;
- int num; /* sensordev number */
- char xname[16]; /* unix device name */
- int maxnumt[SENSOR_MAX_TYPES];
- int sensors_count;
- struct ksensors_head sensors_list;
+ struct refcnt refcnt;
+ struct mutex mtx;
+ SLIST_ENTRY(ksensordev) list; /* [D] */
+ int num; /* [I] sensordev number */
+ char xname[16]; /* [I] unix device name */
+ int maxnumt[SENSOR_MAX_TYPES]; /* [d] */
+ int sensors_count; /* [d] */
+ struct ksensors_head sensors_list; /* [d] */
};
/* struct ksensordev */
void sensordev_install(struct ksensordev *);
void sensordev_deinstall(struct ksensordev *);
+int sensordev_get_locked(int, struct ksensordev **);
int sensordev_get(int, struct ksensordev **);
+void sensordev_put(struct ksensordev *);
+void sensor_lock(struct ksensor *);
+void sensor_unlock(struct ksensor *);
/* struct ksensor */
void sensor_attach(struct ksensordev *, struct ksensor *);
void sensor_detach(struct ksensordev *, struct ksensor *);
-int sensor_find(int, enum sensor_type, int, struct ksensor **);
+int sensor_find(int, enum sensor_type, int,
+ struct ksensor **);
+void sensor_rele(struct ksensor *);
/* task scheduling */
struct sensor_task;
Please test: mpsafe sensors