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:
#0 timeout_add+0x43
#1 random_start+0xb7
#2 main+0x96
#3 longmode_hi+0x9c
#0 timeout_add+0x43
#1 thinkpad_attach+0x1f9
#2 config_attach+0x1f4
#3 acpi_foundhid+0x326
#4 aml_find_node+0x74
#5 aml_find_node+0xa1
#6 aml_find_node+0xa1
#7 aml_find_node+0xa1
#8 aml_find_node+0xa1
#9 aml_find_node+0xa1
#10 acpi_attach_common+0x6f4
#11 config_attach+0x1f4
#12 bios_attach+0x74f
#13 config_attach+0x1f4
#14 mainbus_attach+0x7b
#15 config_attach+0x1f4
#16 config_rootfound+0xd2
#17 cpu_configure+0x2a
#18 main+0x3a8
#0 timeout_add+0x43
#1 acpibat_attach+0x171
#2 config_attach+0x1f4
#3 acpi_foundhid+0x326
#4 aml_find_node+0x74
#5 aml_find_node+0xa1
#6 aml_find_node+0xa1
#7 aml_find_node+0xa1
#8 aml_find_node+0xa1
#9 aml_find_node+0xa1
#10 acpi_attach_common+0x6f4
#11 config_attach+0x1f4
#12 bios_attach+0x74f
#13 config_attach+0x1f4
#14 mainbus_attach+0x7b
#15 config_attach+0x1f4
#16 config_rootfound+0xd2
#17 cpu_configure+0x2a
#18 main+0x3a8
#0 timeout_add+0x43
#1 acpitz_attach+0x559
#2 config_attach+0x1f4
#3 acpi_add_device+0x147
#4 aml_walknodes+0x3b
#5 aml_walknodes+0x61
#6 aml_walknodes+0x61
#7 acpi_attach_common+0x712
#8 config_attach+0x1f4
#9 bios_attach+0x74f
#10 config_attach+0x1f4
#11 mainbus_attach+0x7b
#12 config_attach+0x1f4
#13 config_rootfound+0xd2
#14 cpu_configure+0x2a
#15 main+0x3a8
#0 timeout_add+0x43
#1 if_attachsetup+0x102
#2 if_attach+0x4e
#3 iwm_attach+0xe5a
#4 config_attach+0x1f4
#5 pci_probe_device+0x515
#6 pci_enumerate_bus+0x189
#7 config_attach+0x1f4
#8 ppbattach+0x790
#9 config_attach+0x1f4
#10 pci_probe_device+0x515
#11 pci_enumerate_bus+0x189
#12 config_attach+0x1f4
#13 acpipci_attach_bus+0x1b3
#14 acpipci_attach_busses+0x4d
#15 mainbus_attach+0x1c6
#16 config_attach+0x1f4
#17 config_rootfound+0xd2
#18 cpu_configure+0x2a
#0 timeout_add+0x43
#1 if_attachsetup+0x102
#2 if_attach+0x4e
#3 em_setup_interface+0x1c6
#4 em_attach+0x401
#5 config_attach+0x1f4
#6 pci_probe_device+0x515
#7 pci_enumerate_bus+0x189
#8 config_attach+0x1f4
#9 acpipci_attach_bus+0x1b3
#10 acpipci_attach_busses+0x4d
#11 mainbus_attach+0x1c6
#12 config_attach+0x1f4
#13 config_rootfound+0xd2
#14 cpu_configure+0x2a
#15 main+0x3a8
#0 timeout_add+0x43
#1 pckbdattach+0x172
#2 config_attach+0x1f4
#3 pckbc_attach+0x239
#4 pckbc_isa_attach+0x17c
#5 config_attach+0x1f4
#6 isascan+0x309
#7 config_scan+0xab
#8 config_attach+0x1f4
#9 pcib_callback+0x62
#10 config_process_deferred_children+0x83
#11 config_attach+0x1fc
#12 acpipci_attach_bus+0x1b3
#13 acpipci_attach_busses+0x4d
#14 mainbus_attach+0x1c6
#15 config_attach+0x1f4
#16 config_rootfound+0xd2
#17 cpu_configure+0x2a
#18 main+0x3a8
#0 timeout_add+0x43
#1 azalia_rirb_intr+0x67
#2 azalia_intr+0xe1
#3 intr_handler+0x67
#4 Xintr_ioapic_edge22_untramp+0x18f
#5 Xspllower+0x17
#6 cpu_configure+0x76
#7 main+0x3a8
#0 timeout_add+0x43
#1 rdrand+0xca
#2 cpu_configure+0xe1
#3 main+0x3a8
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"?
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 27 Feb 2023 22:55:24 -0000
@@ -86,21 +86,15 @@ int stathz;
int schedhz;
int profhz;
int profprocs;
-int ticks;
static int psdiv, pscnt; /* prof => stat divider */
int psratio; /* ratio: prof / stat */
-volatile unsigned long jiffies; /* XXX Linux API for drm(4) */
-
/*
* Initialize clock frequencies and start both clocks running.
*/
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.
@@ -171,7 +165,8 @@ hardclock(struct clockframe *frame)
tc_ticktock();
ticks++;
- jiffies++;
+ extern volatile unsigned long jiffies;
+ jiffies++; /* XXX drm(4) */
/*
* Update the timeout wheel.
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 27 Feb 2023 22:55:24 -0000
@@ -73,6 +73,8 @@
#define HZ 100
#endif
int hz = HZ;
+int ticks = INT_MAX - (15 * 60 * HZ);
+volatile unsigned long jiffies = ULONG_MAX - (10 * 60 * HZ); /* drm(4) */
int tick = 1000000 / HZ;
int tick_nsec = 1000000000 / HZ;
int utc_offset = 0;
