Page MenuHomeFreeBSD

cpu_auxmsr: assert caller is preventing CPU migration
ClosedPublic

Authored by adam_fenn.io on Aug 23 2020, 9:27 PM.

Details

Test Plan

Peformed the following sanity-checks:

  • i386
    • Confirmed assertion is not triggered during boot of a 4-CPU system (a bhyve VM).
  • amd64
    • Confirmed assertion is not triggered during boot of a 4-CPU system (a VMWare VM).
    • Confirmed assertion is not triggered in the VM entry/exit code path (used an invocation of dtrace -f fbt:kernel:cpu_auxmsr to confirm this routine was, as expected, being called while a bhyve VM was running).

The resume codepaths were not sanity-checked, but, by inspection, these appear equally clear as contexts with interrupts disabled as the cases that were sanity-checked.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 33129
Build 30491: arc lint + arc unit

Event Timeline

sys/amd64/amd64/initcpu.c
223

I suggest to trim this sentence to just

Caller should prevent CPU migration.
233

I am not sure the comment is useful. If somebody is using this in context that only prevents scheduling, e.g. in critical section, he should be qualified enough to change assert to accommodate new situation. Comment above the function gives enough clue, if needed.

235

Indent of contig lines should be +4 spaces, no alignment to opening braces. Also I believe the message could be made much shorter without loosing the information.

adam_fenn.io marked an inline comment as done.

Incorporate @kib's review feedback.

sys/amd64/amd64/initcpu.c
228

I suggest to write the message Context switch possible instead of interrupts enabled which just repeats the assert code. It is more to the point IMO.

229

Remove blank line.

This revision is now accepted and ready to land.Aug 23 2020, 11:44 PM

Thanks kib@, I can commit this.