Page MenuHomeFreeBSD

Permit timed sleeps before timers are working.
ClosedPublic

Authored by jhb on Nov 23 2016, 12:10 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 8, 11:27 AM
Unknown Object (File)
Sun, Apr 28, 12:06 AM
Unknown Object (File)
Mar 28 2024, 8:59 PM
Unknown Object (File)
Feb 27 2024, 9:26 PM
Unknown Object (File)
Dec 21 2023, 4:15 AM
Unknown Object (File)
Dec 20 2023, 3:27 AM
Unknown Object (File)
Oct 27 2023, 2:22 AM
Unknown Object (File)
Sep 14 2023, 2:35 AM
Subscribers
None

Details

Reviewers
kib
Summary

Permit timed sleeps before timers are working.

The callout subsystem already handles early callouts and schedules
the first clock interrupt appropriately based on the currently pending
callouts.

Test Plan
  • I booted kernels both with and without EARLY_AP_STARTUP with the included test callouts (which I will remove for the actual commit).

    One oddity is that the timers actually fire about 1 second too early. That is, sbinuptime() is about 1 second when the test SYSINIT runs, but the time base that the delta is added to is pulled from DPCPU_GET(hardclocktime) which is 0. So sample output:

CALLOUT: uptime at test start: 4294988770
CALLOUT: time when starting: 4295388202
CALLOUT: 1 second fired at 5417817195
CALLOUT: 2 second fired at 8671043041
CALLOUT: 4 second fired at 17283412636
CALLOUT: 10 second fired at 42952326079

If you examine the values in hex you get 0x1000053e2 for the test start
and 0x100066c2a when the timers start.  The first callout fires about a
quarter of a second later at 0x142ed546b.  The remaining callouts are
all about a second early: 0x204d59de1, 0x4062bf29c, and 0xa00287bbf.

I could "fix" this by patching callout_when() to use sbinuptime() as
the time base instead of DPCPU_GET() before timer interrupts are
initialized, or we could just let it be?

Also, I had originally suggested to still panic in the sleepq code
for thread0, so if that is desired I can restrict the panic that this
removes to only be for thread0.

Finally, callout_reset() that is too early should panic due to
cc_inited == 0 check in callout_reset_sbt_on(), so need to add a new
check for that.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6054
Build 6319: arc lint + arc unit

Event Timeline

jhb retitled this revision from to Permit timed sleeps before timers are working..
jhb updated this object.
jhb edited the test plan for this revision. (Show Details)
jhb added a reviewer: kib.

My current feel is that it is fine to leave the 1 sec inaccuracy as is, until proven harmful. As I understand the current state, early callouts are guaranteed to fire not earlier than timer starts, which make the attempt to handle this rather ineffective.

OTOH, I do think that panic for thread0 sleeping too early is the good thing to have, since this is the feature that would allow to detect deadlocks with non-functional callouts required to make the progress.

jhb edited edge metadata.
  • Bring back the timed sleep panic, but only for thread0.
  • Handle callouts scheduled before DPCPU_GET(hardclocktime) is set.
  • Print times in hex.

Latest output (now in hex since that is simpler to work with):

CALLOUT: uptime at test start: 1000064a9
CALLOUT: time when starting: 10007570c
CALLOUT: 1 second fired at 236c02328
CALLOUT: 2 second fired at 32214bee0
CALLOUT: 4 second fired at 500287315
CALLOUT: 10 second fired at b162dd900
sys/kern/kern_timeout.c
1009

This proved simpler to fix than I had imagined. This fixes "off by 1 second" problem.

kib edited edge metadata.
This revision is now accepted and ready to land.Nov 24 2016, 9:11 AM

Forgot to include reference in commit rS309148.