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
