On Mon, Feb 27, 2023 at 08:48:53PM -0600, Scott Cheloha wrote:
> On Tue, Feb 28, 2023 at 01:01:32PM +1100, Jonathan Gray wrote:
> > On Mon, Feb 27, 2023 at 06:26:00PM -0600, Scott Cheloha wrote:
> > > On Tue, Feb 28, 2023 at 10:18:16AM +1100, Jonathan Gray wrote:
> > > > On Mon, Feb 27, 2023 at 04:57:04PM -0600, Scott Cheloha wrote:
> > > > > ticks and jiffies start at zero. During boot in initclocks(), we
> > > > > reset them:
> > > > >
> > > > > /* sys/kern/kern_clock.c */
> > > > >
> > > > > 89 int ticks;
> > > > > 90 static int psdiv, pscnt; /* prof => stat
> > > > > divider */
> > > > > 91 int psratio; /* ratio: prof
> > > > > / stat */
> > > > > 92
> > > > > 93 volatile unsigned long jiffies; /* XXX Linux
> > > > > API for drm(4) */
> > > > > 94
> > > > > 95 /*
> > > > > 96 * Initialize clock frequencies and start both clocks
> > > > > running.
> > > > > 97 */
> > > > > 98 void
> > > > > 99 initclocks(void)
> > > > > 100 {
> > > > > 101 ticks = INT_MAX - (15 * 60 * hz);
> > > > > 102 jiffies = ULONG_MAX - (10 * 60 * hz);
> > > > > 103
> > > > > 104 /* [... ] */
> > > > >
> > > > > The idea here (committed by dlg@) is sound. We reset ticks and
> > > > > jiffies to near-rollover values to catch buggy code misusing them.
> > > > >
> > > > > But! That jump from zero to whatever violates valid assumptions made
> > > > > by correct code, too.
> > > >
> > > > Assumptions made by what code? Does it exist in the tree?
> > >
> > > First, even if the code did not exist, wouldn't it be simpler to not
> > > do the jump? No?
> >
> > There are enough problems to fix without chasing ones that
> > don't exist.
> >
> > > Second, with rare exception, all kernel code using ticks/jiffies
> > > assumes ticks/jiffies does not advance more than once every 1/hz
> > > seconds on average.
> > >
> > > In timeout_add(9), we assign an absolute expiration time relative
> > > to the current value of ticks. Code calling timeout_add(9) before
> > > initclocks() cannot account for the jump in initclocks().
> >
> > What code calling timeout_add() before initclocks()?
>
> I count 8 calls on my laptop:
>
> [...]
>
> Jumping ticks/jiffies as we do in initclocks() violates assumptions
> about how ticks/jiffies work. If we initialize ticks/jiffies to
> values near rollover we can keep the test of correct behavior intended
> by dlg's patch without surprising other code.
>
> ok?
>
> Is there a better place to put "jiffies"?
kettenis@ suggested finding a way to initialize ticks/jiffies without
moving them.
HZ is not visible outside of param.c. If we move the conditional
definition of HZ from param.c into sys/kernel.h (where hz(9) is
extern'd), kern_clock.c can use it. I don't see a more obvious place
to put HZ than sys/kernel.h.
This fixes the problem. The timeouts no longer fire early.
Can I go with this?
Index: kern/kern_clock.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_clock.c,v
retrieving revision 1.106
diff -u -p -r1.106 kern_clock.c
--- kern/kern_clock.c 4 Feb 2023 19:33:03 -0000 1.106
+++ kern/kern_clock.c 1 Mar 2023 16:34:50 -0000
@@ -86,11 +86,11 @@ int stathz;
int schedhz;
int profhz;
int profprocs;
-int ticks;
+int ticks = INT_MAX - (15 * 60 * HZ);
static int psdiv, pscnt; /* prof => stat divider */
int psratio; /* ratio: prof / stat */
-volatile unsigned long jiffies; /* XXX Linux API for drm(4) */
+volatile unsigned long jiffies = ULONG_MAX - (10 * 60 * HZ);
/*
* Initialize clock frequencies and start both clocks running.
@@ -98,9 +98,6 @@ volatile unsigned long jiffies; /* XXX
void
initclocks(void)
{
- ticks = INT_MAX - (15 * 60 * hz);
- jiffies = ULONG_MAX - (10 * 60 * hz);
-
/*
* Set divisors to 1 (normal case) and let the machine-specific
* code do its bit.
Index: sys/kernel.h
===================================================================
RCS file: /cvs/src/sys/sys/kernel.h,v
retrieving revision 1.25
diff -u -p -r1.25 kernel.h
--- sys/kernel.h 13 Jan 2021 16:28:50 -0000 1.25
+++ sys/kernel.h 1 Mar 2023 16:34:50 -0000
@@ -56,3 +56,7 @@ extern int hz; /* system clock's frequ
extern int stathz; /* statistics clock's frequency */
extern int profhz; /* profiling clock's frequency */
extern int lbolt; /* once a second sleep address */
+
+#ifndef HZ
+#define HZ 100
+#endif
Index: conf/param.c
===================================================================
RCS file: /cvs/src/sys/conf/param.c,v
retrieving revision 1.47
diff -u -p -r1.47 param.c
--- conf/param.c 13 Apr 2022 10:08:10 -0000 1.47
+++ conf/param.c 1 Mar 2023 16:34:50 -0000
@@ -69,9 +69,6 @@
* Compiled with -DHZ=xx -DMAXUSERS=xx
*/
-#ifndef HZ
-#define HZ 100
-#endif
int hz = HZ;
int tick = 1000000 / HZ;
int tick_nsec = 1000000000 / HZ;
