On Thu, 20 Jun 2024 at 15:15, Tejun Heo <tj@xxxxxxxxxx> wrote:
>
> The changes are straightforward and the code looks better afterwards.
> However, when !CONFIG_SCHED_CLASS_EXT, this just ends up adding an unused
> hook which is unlikely to be useful to other sched_classes. We can #ifdef
> the op with CONFIG_SCHED_CLASS_EXT but then I'm not sure the code
> necessarily looks better afterwards.
So honestly, if people _really_ care about performance here, then I
think that in the long run the right thing to do is
- expose all the DEFINE_SCHED_CLASS() definitions in a header file
- rename for_each_class() to FOR_EACH_CLASS() and make it unroll the
whole damn loop statically
which would turn the indirect branches into actual direct branches,
and would statically just remove any "if (!class->zyz)" conditionals.
Pretty? No. But it probably wouldn't be hugely ugly either, and
honestly, looking at the existing for_each_class() uses (and the one
single "for_class_range()" one), they are so small and the number of
classes is so small that unrolling the loop entirely doesn't sound
bad.
It wouldn't help deal with *this* case (since it's a "call variable
class"), but considering that the current __pick_next_task()
(a) special-cases one class as-is
(b) does a "for_each_class()" and calls an indirect call for each
when that doesn't trigger
I would claim that people don't care enough about this that one test
for a NULL 'switch_class' function would be worth worrying about.
Btw, indirect calls are now expensive enough that when you have only a
handful of choices, instead of a variable
class->some_callback(some_arguments);
you might literally be better off with a macro that does
#define call_sched_fn(class, name, arg...) switch (class) { \
case &fair_name_class: fair_name_class.name(arg); break; \
... unroll them all here..
which then just generates a (very small) tree of if-statements.
Again, this is entirely too ugly to do unless people *really* care.
But for situations where you have a small handful of cases known at
compile-time, it's not out of the question, and it probably does
generate better code.
NOTE NOTE NOTE! This is a comp[letely independent aside, and has
nothing to do with sched_ext except for the very obvious indirect fact
that sched_ext would be one of the classes in this kind of code.
And yes, I suspect it is too ugly to actually do this.
Linus
