Page MenuHomeFreeBSD

Centralize __pcpu definitions.
ClosedPublic

Authored by kib on Aug 26 2019, 4:32 PM.
Tags
None
Referenced Files
F80136387: D21418.diff
Thu, Mar 28, 10:13 AM
Unknown Object (File)
Jan 16 2024, 4:09 PM
Unknown Object (File)
Dec 20 2023, 5:58 PM
Unknown Object (File)
Dec 5 2023, 3:05 PM
Unknown Object (File)
Dec 3 2023, 12:55 AM
Unknown Object (File)
Nov 26 2023, 5:11 AM
Unknown Object (File)
Nov 22 2023, 11:05 PM
Unknown Object (File)
Nov 22 2023, 1:19 PM

Details

Summary

Many extern struct pcpu <something>__pcpu declarations were copied/pasted in sources. The issue is the definition is MD, but it cannot be provided by machine/pcpu.h due to actual struct pcpu defined in sys/pcpu.h later than the inclusion of machine/pcpu.h. There is no way around it, due to machine/pcpu.h supplying part of struct pcpu fields.

To work around the problem, add a new machine/pcpu_aux.h header, which should fill any needed MD definitions after struct pcpu definition is completed. This allows to remove copies of __pcpu spread around the source.

While there, fix bhyve.

[I only handled x86 and riscv, the later was not compiled. I will finish the patch after the discussion]

Diff Detail

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

Event Timeline

Seems a bit wasteful to have multiple files with the same bits, but since some are arrays and others are pointers, I'm unsure of what to suggest instead.

This revision is now accepted and ready to land.Aug 26 2019, 4:41 PM
sys/sys/pcpu.h
175 ↗(On Diff #61292)

Maybe it would be simpler if we inverted these macros and had sys/pcpu.h define a PCPU_MI_FIELDS macro before #including <machine/pcpu.h> and then the MD header would define the actual 'struct pcpu' using 'PCPU_MI_FIELDS' as the first line? We could add a static assert that offsetof(struct pcpu, pc_curthread) == 0 in subr_pcpu.c. Would this let us keep a single header?

sys/sys/pcpu.h
175 ↗(On Diff #61292)

I suspect it would not, because due some definitions in sys/pcpu.h now would require placement after machine/pcpu.h defined complete struct pcpu. cpuhead is the first example.

Also IMO it would cause much larger churn in source.

pcpu_aux.h also allows to heal some wounds like OFFSETOF_CURTHREAD and remove clang-specific workarounds, since offsetof() is not literal 0 AKA NULL.

This revision now requires review to proceed.Aug 26 2019, 5:59 PM

It seems reasonable to me.

sys/riscv/include/pcpu_aux.h
46 ↗(On Diff #61305)

"... zones, the page size should be a multiple of the size of struct pcpu."

49 ↗(On Diff #61305)

This could be written more simply as PAGE_SIZE % sizeof(struct pcpu) == 0.

kib marked 2 inline comments as done.

risc-v pcpu_aux.h editing.

Complete pass over rest of the arches. This version passes tinderbox.

sys/sparc64/include/pcpu_aux.h~
1 ↗(On Diff #61426)

this file looks accidentally added.

There is both sparc64/pcpu_aux.h and sparc64/pcpu_aux.h~ in the diff.

This revision is now accepted and ready to land.Aug 28 2019, 7:52 PM
This revision now requires review to proceed.Aug 28 2019, 8:05 PM
This revision was not accepted when it landed; it landed in state Needs Review.Aug 29 2019, 7:25 AM
Closed by commit rS351594: Centralize __pcpu definitions. (authored by kib). · Explain Why
This revision was automatically updated to reflect the committed changes.