Page MenuHomeFreeBSD

Add a DEFINE_IFUNC man page.
ClosedPublic

Authored by markj on May 18 2019, 6:45 PM.

Details

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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
101 ↗(On Diff #57540)

__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.

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 ↗(On Diff #57546)

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 ↗(On Diff #57546)

?

136 ↗(On Diff #57546)

Should we .Xr arch 7 and add a table showing if ifuncs are supported for that architecture?

share/man/man9/DEFINE_IFUNC.9
88 ↗(On Diff #57545)

'gcc-style function attribute'

95 ↗(On Diff #57545)

Another possibility is IRELOC.

141 ↗(On Diff #57545)

...boot or module relocation.

markj marked an inline comment as done.

Address feedback.

share/man/man9/DEFINE_IFUNC.9
95 ↗(On Diff #57545)

I'm not sure what you're referring to. The IPLT relocations?

40 ↗(On Diff #57546)

I tried rewording to make this more clear.

106 ↗(On Diff #57546)

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 ↗(On Diff #57546)

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.

kib added inline comments.
share/man/man9/DEFINE_IFUNC.9
95 ↗(On Diff #57545)

R_X86_64_IRELATIV and R_I386_IRELATIV relocs. They are legal, and we do handle them in kernel linker.

This revision is now accepted and ready to land.May 18 2019, 9:23 PM

Try to make the wording around relocations more generic.

This revision now requires review to proceed.May 18 2019, 9:44 PM
share/man/man9/DEFINE_IFUNC.9
95 ↗(On Diff #57552)

replace dot with comma ?

Remove unintended line break.

This revision was not accepted when it landed; it landed in state Needs Review.May 20 2019, 7:12 PM
This revision was automatically updated to reflect the committed changes.