Details

Reviewers

kib
mjg
avg
cem
jeff
p.bruenn_beckhoff.com
manu

Commits

rGd31a21b939f7: x86/mp: don't create empty cpu groups
rG7468332f5518: x86/mp: don't create empty cpu groups

Summary

When some APICs are disabled by tunables, nodes can end up with an empty
cpuset. Nodes with an emtpy cpuset will be translated into cpu groups
with emtpy cpusets. smp_topo_fill will then set cg_first and cg_last to
-1. This isn't correctly handled in all functions. E.g. when
cpu_search_lowest and cpu_search_highest loop through all cpus, thay
call CPU_ISSET on cpu -1 which ends up in a general protection fault.

We could fix the scheduler to handle empty cpu groups correctly.
Nevertheless, empty cpu groups are causing overhead for no value. So, it
makes more sense to just don't create them.

Test Plan

It doesn't occur on all multi core systems and depends on the system topology. I'm using a Intel Core i7-9700E (8 cores not Hyperthreading).

Add following lines to /boot/devices.hints:

hint.lapic.0.disabled=0
hint.lapic.1.disabled=0
hint.lapic.2.disabled=0
hint.lapic.3.disabled=0
hint.lapic.4.disabled=1
hint.lapic.5.disabled=1
hint.lapic.6.disabled=1
hint.lapic.7.disabled=1

When rebooting the system, it will end up in a boot loop because FreeBSD panics at boot. This patch fixes the behaviour.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 31196
Build 28852: arc lint + arc unit

Event Timeline

p.bruenn_beckhoff.com requested review of this revision.May 20 2020, 2:39 PM

p.bruenn_beckhoff.com created this revision.

Harbormaster completed remote builds in B31195: Diff 72026.May 20 2020, 2:39 PM

Fix typos

Harbormaster completed remote builds in B31196: Diff 72027.May 20 2020, 2:51 PM

This is a bugfix we use for TwinCAT/BSD. In our environment it's pretty common to isolate single CPU cores from the operating system and let them run application specific code.
Again, I found both of you in the history of the changed file and want to ask for your feedback.
We would like to have this issue solved in upstream FreeBSD to keep our vendor diff small and are willing to improve our implementation, if it doesn't meet your standards.

kib added reviewers: avg, cem.May 23 2020, 7:20 PM

cem added a reviewer: jeff.May 24 2020, 3:31 AM

FWIW, there is the vaguely related D23659, which I did not have time to rework correctly. Some of Jeff's feedback there may be useful here.

(For evaluation purposes, we had a 2P configuration and were attempting to disable all CPUs in the first socket except for the BSP — just because boot relies on the BSP. Mostly we wanted to qualify the second socket CPU device latency.)

This looks fine to me, mostly. Lets wait for Jeff.

is this getting anywhere?

corvink commandeered this revision.May 24 2022, 8:31 AM

corvink updated this revision to Diff 106312.

corvink added a reviewer: p.bruenn_beckhoff.com.

corvink retitled this revision from Fix panic in sched_setup_smp when some apics are disabled to x86/mp: don't create empty cpu groups.

corvink edited the summary of this revision. (Show Details)

corvink added a reviewer: manu.

corvink set the repository for this revision to rG FreeBSD src repository.

Herald added a subscriber: imp. · View Herald TranscriptMay 24 2022, 8:31 AM

corvink edited the test plan for this revision. (Show Details)May 24 2022, 8:35 AM

avg accepted this revision.May 24 2022, 10:16 AM