Hi,
We have one comment that locking for ratecheck(9) is missing. In
all other places locking status of the struct timeval *lasttime
is unclear.
The easiest fix is a global mutex for all lasttime in ratecheck().
This covers the usual usecase of the function.
Same for ppsratecheck(9), lasttime and curpps are protected.
Remove a useless #if 1 while there.
ok?
bluhm
Index: kern/kern_malloc.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_malloc.c,v
retrieving revision 1.146
diff -u -p -r1.146 kern_malloc.c
--- kern/kern_malloc.c 16 May 2021 15:10:20 -0000 1.146
+++ kern/kern_malloc.c 3 May 2022 21:51:21 -0000
@@ -188,7 +188,6 @@ malloc(size_t size, int type, int flags)
if (size > 65535 * PAGE_SIZE) {
if (flags & M_CANFAIL) {
#ifndef SMALL_KERNEL
- /* XXX lock */
if (ratecheck(&malloc_lasterr, &malloc_errintvl))
printf("malloc(): allocation too large, "
"type = %d, size = %lu\n", type, size);
Index: kern/kern_time.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.154
diff -u -p -r1.154 kern_time.c
--- kern/kern_time.c 18 Jun 2021 15:59:14 -0000 1.154
+++ kern/kern_time.c 3 May 2022 21:51:21 -0000
@@ -782,11 +782,13 @@ itimerdecr(struct itimerspec *itp, long
int
ratecheck(struct timeval *lasttime, const struct timeval *mininterval)
{
+ static struct mutex mtx = MUTEX_INITIALIZER(IPL_HIGH);
struct timeval tv, delta;
int rv = 0;
getmicrouptime(&tv);
+ mtx_enter(&mtx);
timersub(&tv, lasttime, &delta);
/*
@@ -798,6 +800,7 @@ ratecheck(struct timeval *lasttime, cons
*lasttime = tv;
rv = 1;
}
+ mtx_leave(&mtx);
return (rv);
}
@@ -808,11 +811,13 @@ ratecheck(struct timeval *lasttime, cons
int
ppsratecheck(struct timeval *lasttime, int *curpps, int maxpps)
{
+ static struct mutex mtx = MUTEX_INITIALIZER(IPL_HIGH);
struct timeval tv, delta;
int rv;
microuptime(&tv);
+ mtx_enter(&mtx);
timersub(&tv, lasttime, &delta);
/*
@@ -837,20 +842,11 @@ ppsratecheck(struct timeval *lasttime, i
else
rv = 0;
-#if 1 /*DIAGNOSTIC?*/
/* be careful about wrap-around */
if (*curpps + 1 > *curpps)
*curpps = *curpps + 1;
-#else
- /*
- * assume that there's not too many calls to this function.
- * not sure if the assumption holds, as it depends on *caller's*
- * behavior, not the behavior of this function.
- * IMHO it is wrong to make assumption on the caller's behavior,
- * so the above #if is #if 1, not #ifdef DIAGNOSTIC.
- */
- *curpps = *curpps + 1;
-#endif
+
+ mtx_leave(&mtx);
return (rv);
}
