Re: [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.03.2022 01:40, Andrew Cooper wrote:
> When only one scheduler is compiled in, function pointers can be optimised to
> direct calls, and the hooks hardened against controlflow hijacking.
> 
> RFC for several reasons.
> 
> 1) There's an almost beautiful way of not introducing MAYBE_SCHED() and hiding
>    the magic in REGISTER_SCHEDULER(), except it falls over
>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91765 which has no comment or
>    resolution at all.
> 
> 2) A different alternative which almost works is to remove the indirection in
>    .data.schedulers, but the singleton scheduler object can't be both there
>    and in .init.rodata.cf_clobber.

Couldn't we name the section differently when there's just one member,
placing that section inside __initdata_cf_clobber_{start,end} in the
linker script?

> 3) I can't think of a way of build time check to enforce that new schedulers
>    get added to the preprocessor magic.

An assertion in the linker script, checking that .data.schedulers has a
single entry when the sched_ops symbol exists? This may involve a
PROVIDE(sched_ops = 0) as there doesn't look to be a way to probe for
symbol defined-ness in expressions.

> And the blocker:
> 4) This isn't compatible with how sched_idle_ops get used for granularity > 1.

Special case it just like we special case plt_tsc in x86/time.c?

> --- a/xen/common/sched/private.h
> +++ b/xen/common/sched/private.h
> @@ -271,6 +271,33 @@ static inline spinlock_t *pcpu_schedule_trylock(unsigned 
> int cpu)
>      return NULL;
>  }
>  
> +#if 1 ==                                                                \
> +    defined(CONFIG_SCHED_CREDIT) + defined(CONFIG_SCHED_CREDIT2) +      \
> +    defined(CONFIG_SCHED_RTDS) + defined(CONFIG_SCHED_ARINC653) +       \
> +    defined(CONFIG_SCHED_NULL)
> +
> +extern const struct scheduler sched_ops;
> +#define MAYBE_SCHED(x) __initdata_cf_clobber sched_ops

__initconst_cf_clobber, seeing that all use sites also use const?

> @@ -333,39 +360,48 @@ struct scheduler {
>      void         (*dump_cpu_state) (const struct scheduler *, int);
>  };
>  
> +static inline int sched_global_init(const struct scheduler *s)
> +{
> +    if ( s->global_init )
> +        return sched_call(s, global_init);
> +    return 0;
> +}

Is it really a good idea to expose this here when it's supposed to be
used from core.c only, and even there in just a single place?

Jan