Page MenuHomeFreeBSD

sched: Automatically namespace schedulers' implementation functions
AcceptedPublic

Authored by olce on Mon, Oct 12, 12:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 14, 6:37 AM
Unknown Object (File)
Tue, Oct 13, 2:33 PM
Unknown Object (File)
Tue, Oct 13, 2:27 PM
Unknown Object (File)
Tue, Oct 13, 8:14 AM
Unknown Object (File)
Tue, Oct 13, 8:12 AM
Unknown Object (File)
Tue, Oct 13, 4:38 AM
Unknown Object (File)
Tue, Oct 13, 3:29 AM
Unknown Object (File)
Tue, Oct 13, 2:02 AM
Subscribers

Details

Reviewers
emaste
mchoo
Group Reviewers
scheduler
Summary

This renames back scheduler implementation functions to their old names,
i.e., the same name as the interface functions, in the source code,
while providing the infrastructure to still produce from them C symbols
namespaced by the scheduler implementation, in order to avoid symbol
collisions with interface dispatching functions and other scheduler
implementations when multiple ones are compiled in. In a nutshell, the
implementation consists in re-routing interface names to implementation
names via the C preprocessor.

The main reasons for this change are that different implementation names:

  1. Prevented inlining opportunities in internal scheduler code. The only way to regain them without this change would have been to further clutter the existing code (changing calls to use the scheduler-specific names), and would have imposed again the discipline of specifying implementation names in new calls instead of the well-known and shorter interface names.
  2. Broke navigating code with ctags-like tools like Global.
  3. Introduced undesirable churn and cognitive burden, in particular in the form of having to ask oneself whether internal scheduler functions should be namespaced or not, and which are their real names when using them.

However, C symbol collisions create difficulties with debugging and ELF
manipulation tools, including when printing backtraces (which scheduler
implementation is in use?). For example, as of this writing,
'disassemble' in GDB will choose one of the existing functions and
disassemble it without mentioning the existing others; our ddb(4) will
set a breakpoint on one of these functions only (which should be
improved); etc. So keeping namespaced symbol names was desirable, which
is why symbol re-routing via the C preprocessor was chosen.

The implementation of the machinery to substitute interface names by
implementation names has two parts.

First, a new generated per-implementation-specific header has to be
included in each implementation ("sched_<NAME>_ns.h"). It contains
simple C preprocessor substitutions, such as, taking the example of ULE:

#define sched_clock sched_ule_clock
#define sched_switch sched_ule_switch

allowing to use the interface names in place of the implementation names
in the implementation itself. It also checks that an implementation
actually defined SCHED_IMPLEMENTATION and to the same value that was
used to produce it.

These headers are generated from the pattern header
'sys/kern/sched_ns.h' through directives in 'sys/conf/files', via the
C preprocessor using the already existing X macros and some light
post-processing by the new 'sys/tools/sched_impl_ns.awk' script.

Second, <sys/sched.h> declares all implementation functions when
included from an implementation, so that out-of-order references within
the implementation do not cause problems, as it was before multiple
schedulers could coexist.

Implementations must define SCHED_IMPLEMENTATION to their name used as
the namespacing symbol part (e.g., ule or 4bsd), which used to be the
name passed as the first argument to DECLARE_SCHEDULER(). This argument
is now redundant and has been removed. That same macro with same value
has to be defined in the directives from 'sys/conf/files' for each
implementation's namespacing header to be produced properly.

For added safety, <sys/sched.h> does not provide interface functions'
declarations when SCHED_IMPLEMENTATION is defined anymore, so that
implementations can never inadvertently call these functions.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 78063
Build 74946: arc lint + arc unit

Event Timeline

olce requested review of this revision.Mon, Oct 12, 12:45 PM

Add before-depend directive in sys/conf/files. (Spotted while updating sources in another tree and observing that sched_ule_ns.h and sched_4bsd_ns.h would not be generated.)

If we use preprocessors instead of handwriting the scheduler-specific namespacing (e.g. sched_foo), how will it affect the following:

  • Increased complexity of symbol handling like ELF tools and debuggers because the names don't match from what you see in the code
  • Coding helpers like LSPs and ctags

The only advantage of doing this I see is that scheduler devs can do less typing.

Usually I tend to avoid macros like this in C. So before we commit this, I want to make sure that the tradeoff is obvious towards the new implementation.

  • Increased complexity of symbol handling like ELF tools and debuggers because the names don't match from what you see in the code

You will of course see, e.g., sched_ule_clock() in stack traces instead of sched_clock() in the code, which is the desired effect. ELF tools and debuggers do not care about the source code itself. Proper file and line information is encoded in debug info as usual, so if you want to, e.g., list the source for sched_ule_clock() appearing in the backtrace, you'll be taken to the right point in the source (sched_clock()). But you would have to use the real symbol names in ELF tools yes, e.g., when using objdump --disassemble-symbols=sched_ule_clock.

  • Coding helpers like LSPs and ctags

One of the reasons for doing this is that it actually *helps* ctags tools. Tested with base's ctags, Universal Ctags and Global. They all list sched_clock() et alter's definitions in the implementations. None of these can expand macros defined in other files, and probably will never do since that would raise the parsing time significantly.

I don't know about LSPs, because am not using them. Are you?

The only advantage of doing this I see is that scheduler devs can do less typing.

I already listed some other advantages. But yes, prefixing everything by hand in the code, including internal calls, is the only alternative, and can be made to enjoy most of the benefits, except for readily working ctags tools.

Usually I tend to avoid macros like this in C. So before we commit this, I want to make sure that the tradeoff is obvious towards the new implementation.

Macros should always be used only for specific reasons.

Initially, we had only a single scheduler implementation per kernel, so implementation functions naturally all had the same name. This is an effort to go back to that situation in the source code. Is it worth it? I tend to think it is because I expect that we will (and that is certainly the case for me already) by far spend more time working on the code, including for adding temporary traces, than examining the disassembly of some implementation function (debugging will show the full function name, and gdb can match that with the precise source code location regardless). We can always go back to a full explicit scheme. But yes, this is a convenience change, not a hard requirement.

A drawback with this is that common functions, which are naturally not prefixed, will not be distinguishable in the source code from implementation-specific ones. Honestly, that's the main reason that makes me hesitate at the moment.

If people think this change is too much, I'll at least keep the part of not declaring interface functions in implementations, to avoid breaking inlining inadvertently in implementations.

I don't use LSP for kernel development, but LSP users can try this out after merge and create bug reports if there are any issues.

I'm okay with this change.

This revision is now accepted and ready to land.Thu, Oct 15, 5:08 PM