Page MenuHomeFreeBSD

calibrate lapic timer in native_lapic_setup
ClosedPublic

Authored by avg on May 14 2018, 9:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 10, 8:47 PM
Unknown Object (File)
Mon, Dec 9, 1:14 PM
Unknown Object (File)
Thu, Dec 5, 10:26 PM
Unknown Object (File)
Mon, Dec 2, 1:39 PM
Unknown Object (File)
Sat, Nov 30, 8:28 PM
Unknown Object (File)
Sat, Nov 30, 7:39 PM
Unknown Object (File)
Nov 18 2024, 3:30 PM
Unknown Object (File)
Nov 17 2024, 11:50 AM
Subscribers

Details

Summary

The idea is to calibrate the LAPIC timer just once and only on boot,
given that [at present] the timer constants are global and shared
between all processors.

My primary motivation is to fix a panic that can happen when dynamically
switching to lapic timer. The panic is caused by a recursion on
et_hw_mtx when printing the calibration results to console. Also, the
code should become slightly simpler and easier to read. The previous
code was racy too. Multiple processors could start calibrating the
global constants concurrently, although that seems to have been benign.

The panic mentioned earlier:

spin lock 0xffffffff817ced00 (et_hw_mtx) held by 0xfffff80010314000 (tid 100172) too long
exclusive spin mutex et_hw_mtx (et_hw_mtx) r = 0 (0xffffffff817ced00) locked @ /usr/src/sys/kern/kern_clocksource.c:489
panic: spin lock held too long
cpuid = 1
time = 1525709205
KDB: stack backtrace:
db_trace_self_wrapper() at 0xffffffff804665ab = db_trace_self_wrapper+0x2b/frame 0xfffffe00df7cf180
kdb_backtrace() at 0xffffffff806ce099 = kdb_backtrace+0x39/frame 0xfffffe00df7cf230
vpanic() at 0xffffffff80693246 = vpanic+0x166/frame 0xfffffe00df7cf270
panic() at 0xffffffff806932e3 = panic+0x43/frame 0xfffffe00df7cf2d0
_mtx_lock_spin_failed() at 0xffffffff8067ac67 = _mtx_lock_spin_failed+0x57/frame 0xfffffe00df7cf2f0
_mtx_lock_spin_cookie() at 0xffffffff8067a438 = _mtx_lock_spin_cookie+0x168/frame 0xfffffe00df7cf370
__mtx_lock_spin_flags() at 0xffffffff8067a1f6 = __mtx_lock_spin_flags+0xe6/frame 0xfffffe00df7cf3d0
cpu_new_callout() at 0xffffffff8091d492 = cpu_new_callout+0xb2/frame 0xfffffe00df7cf410
callout_cc_add() at 0xffffffff806ab475 = callout_cc_add+0x165/frame 0xfffffe00df7cf450
callout_reset_sbt_on() at 0xffffffff806aa71b = callout_reset_sbt_on+0x23b/frame 0xfffffe00df7cf4c0
callout_schedule() at 0xffffffff806ab50e = callout_schedule+0x2e/frame 0xfffffe00df7cf4e0
vt_schedule_flush() at 0xffffffff8057cbf5 = vt_schedule_flush+0x35/frame 0xfffffe00df7cf4f0
vt_resume_flush_timer() at 0xffffffff8057cb98 = vt_resume_flush_timer+0x38/frame 0xfffffe00df7cf510
vtterm_putchar() at 0xffffffff8057b44d = vtterm_putchar+0x1d/frame 0xfffffe00df7cf530
termteken_putchar() at 0xffffffff806e1502 = termteken_putchar+0x92/frame 0xfffffe00df7cf580
teken_funcs_putchar() at 0xffffffff80869da1 = teken_funcs_putchar+0x41/frame 0xfffffe00df7cf5b0
teken_subr_do_putchar() at 0xffffffff80869c87 = teken_subr_do_putchar+0x77/frame 0xfffffe00df7cf600
teken_subr_regular_character() at 0xffffffff8086820a = teken_subr_regular_character+0x12a/frame 0xfffffe00df7cf640
teken_state_init() at 0xffffffff8086724c = teken_state_init+0x1c/frame 0xfffffe00df7cf650
teken_input_char() at 0xffffffff808676e8 = teken_input_char+0x48/frame 0xfffffe00df7cf670
teken_input_byte() at 0xffffffff80867368 = teken_input_byte+0x38/frame 0xfffffe00df7cf680
teken_input() at 0xffffffff8086731e = teken_input+0x2e/frame 0xfffffe00df7cf6b0
termcn_cnputc() at 0xffffffff806e136e = termcn_cnputc+0x6e/frame 0xfffffe00df7cf6e0
cnputc() at 0xffffffff8064350d = cnputc+0x7d/frame 0xfffffe00df7cf710
cnputs() at 0xffffffff80643628 = cnputs+0x68/frame 0xfffffe00df7cf730
prf_putbuf() at 0xffffffff806d485c = prf_putbuf+0x6c/frame 0xfffffe00df7cf750
putbuf() at 0xffffffff806d48e3 = putbuf+0x73/frame 0xfffffe00df7cf770
putchar() at 0xffffffff806d3b33 = putchar+0x63/frame 0xfffffe00df7cf7a0
kvprintf() at 0xffffffff806d3785 = kvprintf+0xfa5/frame 0xfffffe00df7cf8c0
_vprintf() at 0xffffffff806d3e99 = _vprintf+0x89/frame 0xfffffe00df7cf9a0
vprintf() at 0xffffffff806d409f = vprintf+0x1f/frame 0xfffffe00df7cf9b0
printf() at 0xffffffff806d4073 = printf+0x43/frame 0xfffffe00df7cfa10
lapic_calibrate_initcount() at 0xffffffff8094073e = lapic_calibrate_initcount+0x9e/frame 0xfffffe00df7cfa30
lapic_et_start() at 0xffffffff8094052c = lapic_et_start+0x4c/frame 0xfffffe00df7cfa80
et_start() at 0xffffffff80650769 = et_start+0xc9/frame 0xfffffe00df7cfab0
loadtimer() at 0xffffffff8091d287 = loadtimer+0x147/frame 0xfffffe00df7cfaf0
cpu_idleclock() at 0xffffffff8091d014 = cpu_idleclock+0x114/frame 0xfffffe00df7cfb30
cpu_idle() at 0xffffffff80938ec0 = cpu_idle+0x80/frame 0xfffffe00df7cfb60
sched_idletd() at 0xffffffff806ba7ce = sched_idletd+0x9e/frame 0xfffffe00df7cfba0
fork_exit() at 0xffffffff8065d680 = fork_exit+0xd0/frame 0xfffffe00df7cfbf0
fork_trampoline() at 0xffffffff808dc9ee = fork_trampoline+0xe/frame 0xfffffe00df7cfbf0

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.May 14 2018, 10:32 AM
jhb added inline comments.
sys/x86/x86/local_apic.c
794 ↗(On Diff #42511)

My only nit is to perhaps use IS_BSP() here instead of the direct curcpu check.

The motivation for postponing the calibration was to not spend whole second on boot in cases when LAPIC timer is not used. Though that should be much less frequent lately.

In D15422#325302, @mav wrote:

The motivation for postponing the calibration was to not spend whole second on boot in cases when LAPIC timer is not used. Though that should be much less frequent lately.

Oh, I didn't realize it uses a whole second... On the other hand, that second had to be spent, eventually. And sometimes on more than one CPU. And maybe it was even worse as that second was spent when trying to schedule something and while holding the et hw mutex.

Can that period be safely reduced to, say, 0.1 second?

P.S.
I see that TSC calibration uses another second.

This revision was automatically updated to reflect the committed changes.