Details

Reviewers

emaste
kib

Group Reviewers

manpages

Commits

rS348003: Add a man page for DEFINE_IFUNC.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 24313
Build 23136: arc lint + arc unit

Event Timeline

markj created this revision.May 18 2019, 6:45 PM

Harbormaster completed remote builds in B24309: Diff 57540.May 18 2019, 6:45 PM

markj added reviewers: emaste, kib, manpages.May 18 2019, 6:45 PM

Why do we need this page ?

Perhaps add a note about attribute((resolver)). And that for an architecture to support kernel ifuncs, in-kernel linker and toolchain must both support them.

share/man/man9/DEFINE_IFUNC.9
102	__unused is excessive

In D20310#437745, @kib wrote:

Why do we need this page ?

I have a few motivations:

I have explained ifuncs to several people so far and would like to be able to refer them to a man page.
The resolver signature for userland ifuncs is FreeBSD- and architecture-dependent. IMO it is worth documenting. I know this page is for the kernel variant of the macro; I would add another page which describes the resolver signature and refers to the kernel page for the general explanation.
My general sense is that new developers appreciate having useful man9 pages, and the definition of DEFINE_IFUNC is difficult to read. Other aspects of the implementation are not clear from the macro definition; for example, when do resolvers get called and what are they (not) permitted to do?

I understand that man9 pages impose a maintenance burden and have no wish to provide comprehensive documentation for kernel interfaces. To me, DEFINE_IFUNC is obscure enough to deserve some reference.

Perhaps add a note about attribute((resolver)). And that for an architecture to support kernel ifuncs, in-kernel linker and toolchain must both support them.

Ok.

Address feedback.

Harbormaster completed remote builds in B24312: Diff 57545.May 18 2019, 8:15 PM

Typo.

Harbormaster completed remote builds in B24313: Diff 57546.May 18 2019, 8:16 PM

I agree with Mark's motivation, having documentation for FreeBSD-specific kernel interfaces/defines/macros benefits new developers and downstream consumers.

share/man/man9/DEFINE_IFUNC.9
40	Is dynamically really correct, or should we use a term like run-time instead? We want to be clear that it's distinct from compile-time, but with e.g. VM migration we don't want to imply that it will be reevaluated. Maybe it doesn't matter because the specific resolution process is described shortly after, just something to consider.
106	?
136	Should we `.Xr arch 7` and add a table showing if ifuncs are supported for that architecture?

kib added inline comments.May 18 2019, 8:39 PM

share/man/man9/DEFINE_IFUNC.9
89	'gcc-style function attribute'
96	Another possibility is IRELOC.
142	...boot or module relocation.

Address feedback.

Harbormaster completed remote builds in B24314: Diff 57550.May 18 2019, 9:07 PM

markj added inline comments.May 18 2019, 9:08 PM

share/man/man9/DEFINE_IFUNC.9
40	I tried rewording to make this more clear.
96	I'm not sure what you're referring to. The IPLT relocations?
106	I'm trying to be funny. I couldn't come up with a short example that is actually useful. I'll try to think of something better.
136	ifuncs are not really useful for the vast majority of developers, so I suspect it would just be noise. I also don't think we want to create the impression that ifuncs should be used whenever possible; they should be limited to low-level system interfaces.