Page MenuHomeFreeBSD

x86/mp: don't create empty cpu groups
ClosedPublic

Authored by corvink on May 20 2020, 2:39 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 24 2022, 10:47 AM
Unknown Object (File)
Dec 24 2022, 10:45 AM
Unknown Object (File)
Dec 24 2022, 10:45 AM
Unknown Object (File)
Dec 24 2022, 10:43 AM
Unknown Object (File)
Dec 24 2022, 10:42 AM
Unknown Object (File)
Dec 12 2022, 12:34 PM
Unknown Object (File)
Dec 12 2022, 2:09 AM
Unknown Object (File)
Dec 12 2022, 2:00 AM
Subscribers

Details

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

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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.

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.

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.
This revision is now accepted and ready to land.May 24 2022, 10:16 AM
kib added inline comments.
sys/x86/x86/mp_x86.c
877

It would be a more useful comment if you point out the specific function/place in code which does not handle empty groups.

sys/x86/x86/mp_x86.c
876

typo

937–939

nit: these parens are excessive, I think. Particularly around CPU_EMPTY, but also the comparison.

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

Thank you very much for your feedback. I've updated the comment and commit text.

This revision now requires review to proceed.May 25 2022, 8:25 AM
This revision is now accepted and ready to land.May 25 2022, 8:33 PM

Thanks.

sys/x86/x86/mp_x86.c
876

"emtpy" typo is still present, FYI

886

nit: suggest "avoid creating them," or "not create them."

This revision was automatically updated to reflect the committed changes.