* [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in
@ 2022-03-03 0:40 Andrew Cooper
2022-03-03 8:24 ` Jan Beulich
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andrew Cooper @ 2022-03-03 0:40 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné,
Wei Liu, Juergen Gross, Dario Faggioli
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.
3) I can't think of a way of build time check to enforce that new schedulers
get added to the preprocessor magic.
And the blocker:
4) This isn't compatible with how sched_idle_ops get used for granularity > 1.
Suggestions very welcome.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
CC: Dario Faggioli <dfaggioli@suse.com>
---
xen/common/sched/arinc653.c | 2 +-
xen/common/sched/core.c | 4 +-
xen/common/sched/credit.c | 2 +-
xen/common/sched/credit2.c | 2 +-
xen/common/sched/null.c | 2 +-
xen/common/sched/private.h | 91 ++++++++++++++++++++++++++++++++-------------
xen/common/sched/rt.c | 2 +-
7 files changed, 72 insertions(+), 33 deletions(-)
diff --git a/xen/common/sched/arinc653.c b/xen/common/sched/arinc653.c
index a82c0d7314a1..73738b007e7d 100644
--- a/xen/common/sched/arinc653.c
+++ b/xen/common/sched/arinc653.c
@@ -694,7 +694,7 @@ a653sched_adjust_global(const struct scheduler *ops,
* callback functions.
* The symbol must be visible to the rest of Xen at link time.
*/
-static const struct scheduler sched_arinc653_def = {
+const struct scheduler MAYBE_SCHED(sched_arinc653_def) = {
.name = "ARINC 653 Scheduler",
.opt_name = "arinc653",
.sched_id = XEN_SCHEDULER_ARINC653,
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 19ab67818106..020a5741ca31 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -2263,7 +2263,7 @@ static struct sched_unit *do_schedule(struct sched_unit *prev, s_time_t now,
struct sched_unit *next;
/* get policy-specific decision on scheduling... */
- sched->do_schedule(sched, prev, now, sched_tasklet_check(cpu));
+ sched_vcall(sched, do_schedule, sched, prev, now, sched_tasklet_check(cpu));
next = prev->next_task;
@@ -2975,7 +2975,7 @@ void __init scheduler_init(void)
#undef sched_test_func
- if ( schedulers[i]->global_init && schedulers[i]->global_init() < 0 )
+ if ( sched_global_init(schedulers[i]) < 0 )
{
printk("scheduler %s failed initialization, dropped\n",
schedulers[i]->opt_name);
diff --git a/xen/common/sched/credit.c b/xen/common/sched/credit.c
index 4d3bd8cba6fc..8b85e9617fc0 100644
--- a/xen/common/sched/credit.c
+++ b/xen/common/sched/credit.c
@@ -2230,7 +2230,7 @@ csched_deinit(struct scheduler *ops)
}
}
-static const struct scheduler sched_credit_def = {
+const struct scheduler MAYBE_SCHED(sched_credit_def) = {
.name = "SMP Credit Scheduler",
.opt_name = "credit",
.sched_id = XEN_SCHEDULER_CREDIT,
diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c
index 0e3f89e5378e..fda3812d7ac1 100644
--- a/xen/common/sched/credit2.c
+++ b/xen/common/sched/credit2.c
@@ -4199,7 +4199,7 @@ csched2_deinit(struct scheduler *ops)
xfree(prv);
}
-static const struct scheduler sched_credit2_def = {
+const struct scheduler MAYBE_SCHED(sched_credit2_def) = {
.name = "SMP Credit Scheduler rev2",
.opt_name = "credit2",
.sched_id = XEN_SCHEDULER_CREDIT2,
diff --git a/xen/common/sched/null.c b/xen/common/sched/null.c
index 65a0a6c5312d..907a8ae1ca50 100644
--- a/xen/common/sched/null.c
+++ b/xen/common/sched/null.c
@@ -1025,7 +1025,7 @@ static void cf_check null_dump(const struct scheduler *ops)
spin_unlock_irqrestore(&prv->lock, flags);
}
-static const struct scheduler sched_null_def = {
+const struct scheduler MAYBE_SCHED(sched_null_def) = {
.name = "null Scheduler",
.opt_name = "null",
.sched_id = XEN_SCHEDULER_NULL,
diff --git a/xen/common/sched/private.h b/xen/common/sched/private.h
index a870320146ef..f3ba0101ecc7 100644
--- 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
+#define REGISTER_SCHEDULER(x) static const struct scheduler *x##_entry \
+ __used_section(".data.schedulers") = &sched_ops;
+
+#define sched_call(s, fn, ...) \
+ alternative_call(sched_ops.fn, ##__VA_ARGS__)
+
+#define sched_vcall(s, fn, ...) \
+ alternative_vcall(sched_ops.fn, ##__VA_ARGS__)
+
+#else
+
+#define MAYBE_SCHED(x) static x
+#define REGISTER_SCHEDULER(x) static const struct scheduler *x##_entry \
+ __used_section(".data.schedulers") = &x;
+
+#define sched_call(s, fn, ...) (s)->fn(__VA_ARGS__)
+#define sched_vcall(s, fn, ...) (s)->fn(__VA_ARGS__)
+
+#endif
+
struct scheduler {
const char *name; /* full name for this scheduler */
const char *opt_name; /* option name for this scheduler */
@@ -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;
+}
+
static inline int sched_init(struct scheduler *s)
{
- return s->init(s);
+ return sched_call(s, init, s);
}
static inline void sched_deinit(struct scheduler *s)
{
- s->deinit(s);
+ sched_vcall(s, deinit, s);
}
static inline spinlock_t *sched_switch_sched(struct scheduler *s,
unsigned int cpu,
void *pdata, void *vdata)
{
- return s->switch_sched(s, cpu, pdata, vdata);
+ return sched_call(s, switch_sched, s, cpu, pdata, vdata);
}
static inline void sched_dump_settings(const struct scheduler *s)
{
if ( s->dump_settings )
- s->dump_settings(s);
+ sched_vcall(s, dump_settings, s);
}
static inline void sched_dump_cpu_state(const struct scheduler *s, int cpu)
{
if ( s->dump_cpu_state )
- s->dump_cpu_state(s, cpu);
+ sched_vcall(s, dump_cpu_state, s, cpu);
}
static inline void *sched_alloc_domdata(const struct scheduler *s,
struct domain *d)
{
- return s->alloc_domdata ? s->alloc_domdata(s, d) : NULL;
+ if ( s->alloc_domdata )
+ return sched_call(s, alloc_domdata, s, d);
+ return NULL;
}
static inline void sched_free_domdata(const struct scheduler *s,
@@ -373,12 +409,14 @@ static inline void sched_free_domdata(const struct scheduler *s,
{
ASSERT(s->free_domdata || !data);
if ( s->free_domdata )
- s->free_domdata(s, data);
+ sched_vcall(s, free_domdata, s, data);
}
static inline void *sched_alloc_pdata(const struct scheduler *s, int cpu)
{
- return s->alloc_pdata ? s->alloc_pdata(s, cpu) : NULL;
+ if ( s->alloc_pdata )
+ return sched_call(s, alloc_pdata, s, cpu);
+ return NULL;
}
static inline void sched_free_pdata(const struct scheduler *s, void *data,
@@ -386,74 +424,74 @@ static inline void sched_free_pdata(const struct scheduler *s, void *data,
{
ASSERT(s->free_pdata || !data);
if ( s->free_pdata )
- s->free_pdata(s, data, cpu);
+ sched_vcall(s, free_pdata, s, data, cpu);
}
static inline void sched_deinit_pdata(const struct scheduler *s, void *data,
int cpu)
{
if ( s->deinit_pdata )
- s->deinit_pdata(s, data, cpu);
+ sched_vcall(s, deinit_pdata, s, data, cpu);
}
static inline void *sched_alloc_udata(const struct scheduler *s,
struct sched_unit *unit, void *dom_data)
{
- return s->alloc_udata(s, unit, dom_data);
+ return sched_call(s, alloc_udata, s, unit, dom_data);
}
static inline void sched_free_udata(const struct scheduler *s, void *data)
{
- s->free_udata(s, data);
+ sched_vcall(s, free_udata, s, data);
}
static inline void sched_insert_unit(const struct scheduler *s,
struct sched_unit *unit)
{
if ( s->insert_unit )
- s->insert_unit(s, unit);
+ sched_vcall(s, insert_unit, s, unit);
}
static inline void sched_remove_unit(const struct scheduler *s,
struct sched_unit *unit)
{
if ( s->remove_unit )
- s->remove_unit(s, unit);
+ sched_vcall(s, remove_unit, s, unit);
}
static inline void sched_sleep(const struct scheduler *s,
struct sched_unit *unit)
{
if ( s->sleep )
- s->sleep(s, unit);
+ sched_vcall(s, sleep, s, unit);
}
static inline void sched_wake(const struct scheduler *s,
struct sched_unit *unit)
{
if ( s->wake )
- s->wake(s, unit);
+ sched_vcall(s, wake, s, unit);
}
static inline void sched_yield(const struct scheduler *s,
struct sched_unit *unit)
{
if ( s->yield )
- s->yield(s, unit);
+ sched_vcall(s, yield, s, unit);
}
static inline void sched_context_saved(const struct scheduler *s,
struct sched_unit *unit)
{
if ( s->context_saved )
- s->context_saved(s, unit);
+ sched_vcall(s, context_saved, s, unit);
}
static inline void sched_migrate(const struct scheduler *s,
struct sched_unit *unit, unsigned int cpu)
{
if ( s->migrate )
- s->migrate(s, unit, cpu);
+ sched_vcall(s, migrate, s, unit, cpu);
else
sched_set_res(unit, get_sched_res(cpu));
}
@@ -461,7 +499,7 @@ static inline void sched_migrate(const struct scheduler *s,
static inline struct sched_resource *sched_pick_resource(
const struct scheduler *s, const struct sched_unit *unit)
{
- return s->pick_resource(s, unit);
+ return sched_call(s, pick_resource, s, unit);
}
static inline void sched_adjust_affinity(const struct scheduler *s,
@@ -470,19 +508,23 @@ static inline void sched_adjust_affinity(const struct scheduler *s,
const cpumask_t *soft)
{
if ( s->adjust_affinity )
- s->adjust_affinity(s, unit, hard, soft);
+ sched_vcall(s, adjust_affinity, s, unit, hard, soft);
}
static inline int sched_adjust_dom(const struct scheduler *s, struct domain *d,
struct xen_domctl_scheduler_op *op)
{
- return s->adjust ? s->adjust(s, d, op) : 0;
+ if ( s->adjust )
+ return sched_call(s, adjust, s, d, op);
+ return 0;
}
static inline int sched_adjust_cpupool(const struct scheduler *s,
struct xen_sysctl_scheduler_op *op)
{
- return s->adjust_global ? s->adjust_global(s, op) : 0;
+ if ( s->adjust_global )
+ return sched_call(s, adjust_global, s, op);
+ return 0;
}
static inline void sched_unit_pause_nosync(const struct sched_unit *unit)
@@ -501,9 +543,6 @@ static inline void sched_unit_unpause(const struct sched_unit *unit)
vcpu_unpause(v);
}
-#define REGISTER_SCHEDULER(x) static const struct scheduler *x##_entry \
- __used_section(".data.schedulers") = &x;
-
struct cpupool
{
unsigned int cpupool_id;
diff --git a/xen/common/sched/rt.c b/xen/common/sched/rt.c
index d6de25531b3c..9b42852b2de5 100644
--- a/xen/common/sched/rt.c
+++ b/xen/common/sched/rt.c
@@ -1529,7 +1529,7 @@ static void cf_check repl_timer_handler(void *data)
spin_unlock_irq(&prv->lock);
}
-static const struct scheduler sched_rtds_def = {
+const struct scheduler MAYBE_SCHED(sched_rtds_def) = {
.name = "SMP RTDS Scheduler",
.opt_name = "rtds",
.sched_id = XEN_SCHEDULER_RTDS,
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in
2022-03-03 0:40 [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in Andrew Cooper
@ 2022-03-03 8:24 ` Jan Beulich
2022-03-03 8:33 ` Juergen Gross
2022-03-04 5:21 ` Juergen Gross
2 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2022-03-03 8:24 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné, Wei Liu, Juergen Gross, Dario Faggioli, Xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in
2022-03-03 0:40 [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in Andrew Cooper
2022-03-03 8:24 ` Jan Beulich
@ 2022-03-03 8:33 ` Juergen Gross
2022-03-03 10:06 ` Juergen Gross
2022-03-03 10:14 ` Juergen Gross
2022-03-04 5:21 ` Juergen Gross
2 siblings, 2 replies; 6+ messages in thread
From: Juergen Gross @ 2022-03-03 8:33 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Jan Beulich, Roger Pau Monné, Wei Liu, Dario Faggioli
[-- Attachment #1.1.1: Type: text/plain, Size: 1378 bytes --]
On 03.03.22 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.
>
> 3) I can't think of a way of build time check to enforce that new schedulers
> get added to the preprocessor magic.
>
> And the blocker:
> 4) This isn't compatible with how sched_idle_ops get used for granularity > 1.
>
> Suggestions very welcome.
Did you consider to generate the needed code dynamically instead?
I guess this could even be extended to avoid function pointers
completely using the same technique as in my hypercall series.
In order to avoid the need for a central table the per-scheduler
hooks could use standard names (as most of them do already).
I think I could come up with a patch in a few hours if you like
that approach.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in
2022-03-03 8:33 ` Juergen Gross
@ 2022-03-03 10:06 ` Juergen Gross
2022-03-03 10:14 ` Juergen Gross
1 sibling, 0 replies; 6+ messages in thread
From: Juergen Gross @ 2022-03-03 10:06 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Jan Beulich, Roger Pau Monné, Wei Liu, Dario Faggioli
[-- Attachment #1.1.1: Type: text/plain, Size: 1634 bytes --]
On 03.03.22 09:33, Juergen Gross wrote:
> On 03.03.22 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.
>>
>> 3) I can't think of a way of build time check to enforce that new
>> schedulers
>> get added to the preprocessor magic.
>>
>> And the blocker:
>> 4) This isn't compatible with how sched_idle_ops get used for
>> granularity > 1.
>>
>> Suggestions very welcome.
>
> Did you consider to generate the needed code dynamically instead?
>
> I guess this could even be extended to avoid function pointers
> completely using the same technique as in my hypercall series.
>
> In order to avoid the need for a central table the per-scheduler
> hooks could use standard names (as most of them do already).
>
> I think I could come up with a patch in a few hours if you like
> that approach.
BTW, in theory this approach could be generalized for other function
vectors in the hypervisor, too (maybe even all?).
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in
2022-03-03 8:33 ` Juergen Gross
2022-03-03 10:06 ` Juergen Gross
@ 2022-03-03 10:14 ` Juergen Gross
1 sibling, 0 replies; 6+ messages in thread
From: Juergen Gross @ 2022-03-03 10:14 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Jan Beulich, Roger Pau Monné, Wei Liu, Dario Faggioli
[-- Attachment #1.1.1: Type: text/plain, Size: 1634 bytes --]
On 03.03.22 09:33, Juergen Gross wrote:
> On 03.03.22 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.
>>
>> 3) I can't think of a way of build time check to enforce that new
>> schedulers
>> get added to the preprocessor magic.
>>
>> And the blocker:
>> 4) This isn't compatible with how sched_idle_ops get used for
>> granularity > 1.
>>
>> Suggestions very welcome.
>
> Did you consider to generate the needed code dynamically instead?
>
> I guess this could even be extended to avoid function pointers
> completely using the same technique as in my hypercall series.
>
> In order to avoid the need for a central table the per-scheduler
> hooks could use standard names (as most of them do already).
>
> I think I could come up with a patch in a few hours if you like
> that approach.
BTW, in theory this approach could be generalized for other function
vectors in the hypervisor, too (maybe even all?).
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3151 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in
2022-03-03 0:40 [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in Andrew Cooper
2022-03-03 8:24 ` Jan Beulich
2022-03-03 8:33 ` Juergen Gross
@ 2022-03-04 5:21 ` Juergen Gross
2 siblings, 0 replies; 6+ messages in thread
From: Juergen Gross @ 2022-03-04 5:21 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Jan Beulich, Roger Pau Monné, Wei Liu, Dario Faggioli
[-- Attachment #1.1.1: Type: text/plain, Size: 1488 bytes --]
On 03.03.22 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.
>
> 3) I can't think of a way of build time check to enforce that new schedulers
> get added to the preprocessor magic.
>
> And the blocker:
> 4) This isn't compatible with how sched_idle_ops get used for granularity > 1.
Correct. Either you have core scheduling or you can optimize the
indirect calls to direct ones.
> Suggestions very welcome.
Dynamic code generation for replacing the function vector with scheduler
id based if/switch statements.
Something like the attached patch for generating the code (won't build
right now as the related scheduler code adaptions are missing, but the
header is created).
The code generation script could share quite some code with my hypercall
series, and expansion for other function vectors should be rather easy.
Juergen
[-- Attachment #1.1.2: 0001-xen-sched-generate-code-for-calling-scheduler-callba.patch --]
[-- Type: text/x-patch, Size: 10263 bytes --]
From 4bdee23501739236bee34d9c5dae90fef2bde057 Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Thu, 3 Mar 2022 10:32:22 +0100
Subject: [PATCH] xen/sched: generate code for calling scheduler callbacks
In order to avoid calls via function pointers don't use per scheduler
call vectors, but generate the respective call stubs dynamically based
on the defined schedulers.
Use a definition file for defining the templates of the callbacks and
the configured schedulers. The stubs will be using if () and switch ()
statements to call into the correct scheduler using the scheduler id.
The default scheduler is likely to be used in most cases, so it is
called using an if ( likely() ) construct.
In case only a single scheduler is configured the stub will be just a
wrapper for that scheduler.
The callbacks inside the single schedulers need to have standard names
in the form "sched_callback" with "sched" being the scheduler name
(e.g. "credit2"), and "callback" being the specific function (e.g.
"init"), resulting in a function name like "credit2_init".
Signed-off-by: Juergen Gross <jgross@suse.com>
---
.gitignore | 1 +
xen/common/sched/Makefile | 12 +++
xen/common/sched/gen-sched-defs.awk | 152 ++++++++++++++++++++++++++++
xen/common/sched/private.h | 2 +
xen/common/sched/sched-defs.c | 62 ++++++++++++
5 files changed, 229 insertions(+)
create mode 100644 xen/common/sched/gen-sched-defs.awk
create mode 100644 xen/common/sched/sched-defs.c
diff --git a/.gitignore b/.gitignore
index d425be4bd9..d976cbf794 100644
--- a/.gitignore
+++ b/.gitignore
@@ -316,6 +316,7 @@ xen/arch/*/efi/runtime.c
xen/arch/*/include/asm/asm-offsets.h
xen/common/config_data.S
xen/common/config.gz
+xen/common/sched/sched-defs.h
xen/include/headers*.chk
xen/include/compat/*
xen/include/config/
diff --git a/xen/common/sched/Makefile b/xen/common/sched/Makefile
index 3537f2a68d..f7ba8b184c 100644
--- a/xen/common/sched/Makefile
+++ b/xen/common/sched/Makefile
@@ -5,3 +5,15 @@ obj-$(CONFIG_SCHED_CREDIT2) += credit2.o
obj-$(CONFIG_SCHED_RTDS) += rt.o
obj-$(CONFIG_SCHED_NULL) += null.o
obj-y += core.o
+
+quiet_cmd_gendefs = GEN $@
+cmd_gendefs = awk -f $(src)/gen-sched-defs.awk <$< >$@
+
+$(addprefix $(obj)/,$(obj-y)): $(obj)/sched-defs.h
+
+$(obj)/sched-defs.h: $(obj)/sched-defs.i $(src)/gen-sched-defs.awk FORCE
+ $(call if_changed,gendefs)
+
+targets += sched-defs.h
+
+clean-files := sched-defs.h sched-defs.i
diff --git a/xen/common/sched/gen-sched-defs.awk b/xen/common/sched/gen-sched-defs.awk
new file mode 100644
index 0000000000..b445271db0
--- /dev/null
+++ b/xen/common/sched/gen-sched-defs.awk
@@ -0,0 +1,152 @@
+# awk script to generate scheduler calling stubs without using function vectors
+
+BEGIN {
+ printf("/* Generated file, do not edit! */\n\n");
+ s = 0;
+ n = 0;
+ ret[0] = 0;
+}
+
+# Issue error to stderr
+function do_err(msg) {
+ print "Error: "msg": "$0 >"/dev/stderr";
+ exit 1;
+}
+
+function parse_def(id, ret) {
+ sub("^ *", "", id); # Remove leading white space
+ sub(" +", " ", id); # Replace multiple spaces with single ones
+ sub(" *$", "", id); # Remove trailing white space
+ ret[1] = index(id, "*"); # Is it a pointer type?
+ sub("[*]", "", id); # Remove "*"
+ if (index(id, " ") == 0)
+ do_err("Identifier with no type or no name");
+ ret[0] = id;
+ sub(" [^ ]+$", "", ret[0]); # Remove identifier name
+ ret[2] = id;
+ sub("^([^ ]+ )+", "", ret[2]); # Remove type
+}
+
+function print_id(t, p, n, x) {
+ printf("%s ", t);
+ if (p > 0)
+ printf("*");
+ if (x != "")
+ printf("%s_", x);
+ printf("%s", n);
+}
+
+function print_call(s, f, p) {
+ printf(" return %s_%s(", s, fn_name[f]);
+ if (n_args[f] > 0) {
+ for (p = 1; p <= n_args[f]; p++) {
+ if (p > 1)
+ printf(", ");
+ printf("%s", arg[f, p]);
+ }
+ }
+ printf(");\n");
+}
+
+# Skip preprocessing artefacts
+$1 == "extern" {
+ next;
+}
+/^#/ {
+ next;
+}
+
+# Drop empty lines
+NF == 0 {
+ next;
+}
+
+# Handle "scheduler:" line
+$1 == "scheduler:" {
+ if (NF < 2)
+ do_err("\"scheduler:\" requires one parameter");
+ scheds[$2] = 1;
+ s++;
+ next;
+}
+
+# Handle "default:" line
+$1 == "default:" {
+ if (NF < 2)
+ do_err("\"default:\" requires one parameter");
+ if (substr($2, 1, 1) != "\"" || substr($2, length($2)) != "\"")
+ do_err("\"default:\" parameter must be enclosed in \"");
+ sched_def = substr($2, 2, length($2) - 2);
+ next;
+}
+
+# Handle prototype line
+{
+ bro = index($0, "(");
+ brc = index($0, ")");
+ if (bro < 2 || brc < bro)
+ do_err("No valid prototype line");
+ n++;
+ fun = substr($0, 1, bro - 1);
+ parse_def(fun, ret);
+ fn_type[n] = ret[0]; fn_ptr[n] = ret[1]; fn_name[n] = ret[2];
+ args = substr($0, bro + 1, brc - bro - 1);
+ n_args[n] = split(args, a, ",");
+ if (n_args[n] == 1 && a[1] == "void") {
+ n_args[n] = 0;
+ next;
+ }
+ for (i = 1; i <= n_args[n]; i++) {
+ parse_def(a[i], ret);
+ typ[n, i] = ret[0]; ptr[n, i] = ret[1]; arg[n, i] = ret[2];
+ }
+}
+
+# Generate the output
+END {
+ for (i in scheds) {
+ for (f = 1; f <= n; f++) {
+ print_id(fn_type[f], fn_ptr[f], fn_name[f], i);
+ printf("(");
+ if (n_args[f] == 0) {
+ printf("void);\n");
+ continue;
+ }
+ for (p = 1; p <= n_args[f]; p++) {
+ if (p > 1)
+ printf(", ");
+ print_id(typ[f, p], ptr[f, p], arg[f, p], "");
+ }
+ printf(");\n");
+ }
+ printf("\n");
+ }
+
+ for (f = 1; f <= n; f++) {
+ printf("static inline ");
+ print_id(fn_type[f], fn_ptr[f], fn_name[f], "sched");
+ printf("(unsigned int id");
+ if (n_args[f] > 0) {
+ for (p = 1; p <= n_args[f]; p++) {
+ printf(", ");
+ print_id(typ[f, p], ptr[f, p], arg[f, p], "");
+ }
+ }
+ printf(")\n");
+ printf("{\n");
+ printf(" if ( likely(id == XEN_SCHEDULER_%s) )\n", toupper(sched_def));
+ print_call(sched_def, f);
+ printf(" switch ( id )\n");
+ printf(" {\n");
+ for (i in scheds) {
+ if (i == sched_def)
+ continue;
+ printf(" case XEN_SCHEDULER_%s:\n", toupper(i));
+ print_call(i, f);
+ }
+ printf(" default:\n");
+ printf(" BUG();\n");
+ printf(" }\n");
+ printf("}\n\n");
+ }
+}
diff --git a/xen/common/sched/private.h b/xen/common/sched/private.h
index a870320146..1669e82f6b 100644
--- a/xen/common/sched/private.h
+++ b/xen/common/sched/private.h
@@ -608,4 +608,6 @@ void cpupool_put(struct cpupool *pool);
int cpupool_add_domain(struct domain *d, unsigned int poolid);
void cpupool_rm_domain(struct domain *d);
+#include "sched-defs.h"
+
#endif /* __XEN_SCHED_IF_H__ */
diff --git a/xen/common/sched/sched-defs.c b/xen/common/sched/sched-defs.c
new file mode 100644
index 0000000000..82096a65d4
--- /dev/null
+++ b/xen/common/sched/sched-defs.c
@@ -0,0 +1,62 @@
+/*
+ * Scheduler callbacks
+ *
+ * Syntax for definitions:
+ * scheduler: <name>
+ * Adds the scheduler "name" to the possible schedulers
+ * default: "<name>"
+ * Specifies the default scheduler
+ * All other non-empty lines being no preprocessor commands or comments are
+ * treated as callback prototypes. The function name is used to create the
+ * stub call-function "sched_<name>()" with the scheduler id to be passed as
+ * first parameter, followed by the other prototype's parameters. The called
+ * functions are named "<scheduler>_<name>()", with the prototypes of those
+ * being in the generated file, too.
+ */
+
+#ifdef CONFIG_SCHED_CREDIT
+scheduler: credit
+#endif
+#ifdef CONFIG_SCHED_CREDIT2
+scheduler: credit2
+#endif
+#ifdef CONFIG_SCHED_RTDS
+scheduler: rtds
+#endif
+#ifdef CONFIG_SCHED_ARINC653
+scheduler: arinc653
+#endif
+#ifdef CONFIG_SCHED_NULL
+scheduler: null
+#endif
+scheduler: idle
+default: CONFIG_SCHED_DEFAULT
+
+int global_init(void);
+int init(struct scheduler *s);
+void *deinit(struct scheduler *s);
+void *free_udata(const struct scheduler *s, void *data);
+void *alloc_udata(const struct scheduler *s, struct sched_unit *unit, void *dom_data);
+void free_pdata(const struct scheduler *s, void *data, int cpu);
+void *alloc_pdata(const struct scheduler *s, int cpu);
+void deinit_pdata(const struct scheduler *s, void *data, int cpu);
+/* Returns ERR_PTR(-err) for error, NULL for 'nothing needed'. */
+void *alloc_domdata(const struct scheduler *s, struct domain *d);
+/* Idempotent. */
+void free_domdata(const struct scheduler *s, void *data);
+spinlock_t *switch_sched(struct scheduler *s, unsigned int cpu, void *pdata, void *vdata);
+/* Activate / deactivate units in a cpu pool */
+void insert_unit(const struct scheduler *s, struct sched_unit *unit);
+void remove_unit(const struct scheduler *s, struct sched_unit *unit);
+void sleep(const struct scheduler *s, struct sched_unit *unit);
+void wake(const struct scheduler *s, struct sched_unit *unit);
+void yield(const struct scheduler *s, struct sched_unit *unit);
+void context_saved(const struct scheduler *s, struct sched_unit *unit);
+void do_schedule(const struct scheduler *s, struct sched_unit *prev, s_time_t now, bool tasklet_work_scheduled);
+struct sched_resource *pick_resource(const struct scheduler *s, const struct sched_unit *unit);
+void migrate(const struct scheduler *s, struct sched_unit *unit, unsigned int cpu);
+int adjust(const struct scheduler *s, struct domain *d, struct xen_domctl_scheduler_op *op);
+void adjust_affinity(const struct scheduler *s, struct sched_unit *unit, const struct cpumask *hard, const struct cpumask *soft);
+int adjust_global(const struct scheduler *s, struct xen_sysctl_scheduler_op *op);
+void dump_settings(const struct scheduler *s);
+void dump_cpu_state(const struct scheduler *s, int cpu);
--
2.34.1
[-- Attachment #1.1.3: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-04 5:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-03 0:40 [PATCH RFC] xen/sched: Optimise when only one scheduler is compiled in Andrew Cooper
2022-03-03 8:24 ` Jan Beulich
2022-03-03 8:33 ` Juergen Gross
2022-03-03 10:06 ` Juergen Gross
2022-03-03 10:14 ` Juergen Gross
2022-03-04 5:21 ` Juergen Gross
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.