Page MenuHomeFreeBSD

Don't enable interrupts in init_secondary().
ClosedPublic

Authored by markj on Thu, Jan 3, 9:37 PM.

Details

Summary

sched_throw() expects the caller to hold a spinlock section; see the
td_md initialization in cpu_fork(). This bug can cause a deadlock when
APs are started: statclock() fires before the thread lock acquired in
sched_throw() is released, and recurses on the thread lock.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj created this revision.Thu, Jan 3, 9:37 PM
jhb accepted this revision.Thu, Jan 3, 10:38 PM
jhb added inline comments.
sys/riscv/riscv/mp_machdep.c
231 ↗(On Diff #52531)

Are interrupts enabled here at all? We really depend on them not being enabled on APs until after the first context switch.

This revision is now accepted and ready to land.Thu, Jan 3, 10:38 PM
markj marked an inline comment as done.Thu, Jan 3, 10:52 PM
markj added inline comments.
sys/riscv/riscv/mp_machdep.c
231 ↗(On Diff #52531)

Hmm, it was added here: https://svnweb.freebsd.org/changeset/base/335005

We boot in QEMU without the intr_disable(), though. I added code to read and print sstatus here, and SIE is disabled. So it seems safe to remove the intr_disable().

markj updated this revision to Diff 52533.Thu, Jan 3, 10:52 PM
markj marked an inline comment as done.
  • Remove unneeded intr_disable() call.
This revision now requires review to proceed.Thu, Jan 3, 10:52 PM
This revision was not accepted when it landed; it landed in state Needs Review.Fri, Jan 4, 5:15 PM
Closed by commit rS342766: Don't enable interrupts in init_secondary(). (authored by markj, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.