| Message ID | 1450385974-12732-5-git-send-email-jonathan.creekmore@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2015-12-17 at 14:59 -0600, Jonathan Creekmore wrote: > Instead of having a manually-curated list of schedulers, use the > array > that was auto-generated simply by compiling in the scheduler files as > the sole source of truth of the available schedulers. > > CC: George Dunlap <george.dunlap@eu.citrix.com> > CC: Dario Faggioli <dario.faggioli@citrix.com> > Signed-off-by: Jonathan Creekmore <jonathan.creekmore@gmail.com> > Acked-by: Dario Faggioli <dario.faggioli@citrix.com> Regards, Dario
On 17/12/2015 20:59, Jonathan Creekmore wrote: > Instead of having a manually-curated list of schedulers, use the array > that was auto-generated simply by compiling in the scheduler files as > the sole source of truth of the available schedulers. > > CC: George Dunlap <george.dunlap@eu.citrix.com> > CC: Dario Faggioli <dario.faggioli@citrix.com> > Signed-off-by: Jonathan Creekmore <jonathan.creekmore@gmail.com> > --- > xen/common/schedule.c | 24 +++++++----------------- > xen/include/xen/sched-if.h | 5 ----- > 2 files changed, 7 insertions(+), 22 deletions(-) > > diff --git a/xen/common/schedule.c b/xen/common/schedule.c > index 2f98a48..efbd67d 100644 > --- a/xen/common/schedule.c > +++ b/xen/common/schedule.c > @@ -64,20 +64,10 @@ static void poll_timer_fn(void *data); > DEFINE_PER_CPU(struct schedule_data, schedule_data); > DEFINE_PER_CPU(struct scheduler *, scheduler); > > -static const struct scheduler *schedulers[] = { > -#ifdef CONFIG_SCHED_CREDIT > - &sched_credit_def, > -#endif > -#ifdef CONFIG_SCHED_CREDIT2 > - &sched_credit2_def, > -#endif > -#ifdef CONFIG_SCHED_ARINC653 > - &sched_arinc653_def, > -#endif > -#ifdef CONFIG_SCHED_RTDS > - &sched_rtds_def, > -#endif > -}; > +extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; > +#define NUM_SCHEDULERS (((uintptr_t)__schedulers_end-(uintptr_t)__schedulers_start) \ > + / sizeof(struct scheduler *)) > +static const struct scheduler **schedulers = __schedulers_start; You should be able to play some tricks with getting the linker to set a size of a variable it creates, which would hopefully avoid some of this complexity. > > static struct scheduler __read_mostly ops; > > @@ -1468,7 +1458,7 @@ void __init scheduler_init(void) > > open_softirq(SCHEDULE_SOFTIRQ, schedule); > > - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) > + for ( i = 0; i < NUM_SCHEDULERS; i++) > { > if ( schedulers[i]->global_init && schedulers[i]->global_init() < 0 ) > schedulers[i] = NULL; > @@ -1479,7 +1469,7 @@ void __init scheduler_init(void) > if ( !ops.name ) > { > printk("Could not find scheduler: %s\n", opt_sched); > - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) > + for ( i = 0; i < NUM_SCHEDULERS; i++ ) > if ( schedulers[i] ) > { > ops = *schedulers[i]; > @@ -1599,7 +1589,7 @@ struct scheduler *scheduler_alloc(unsigned int sched_id, int *perr) > int i; > struct scheduler *sched; > > - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) > + for ( i = 0; i < NUM_SCHEDULERS; i++ ) > if ( schedulers[i] && schedulers[i]->sched_id == sched_id ) > goto found; > *perr = -ENOENT; > diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h > index 9c6e0f5..66dc9c8 100644 > --- a/xen/include/xen/sched-if.h > +++ b/xen/include/xen/sched-if.h > @@ -165,11 +165,6 @@ struct scheduler { > void (*tick_resume) (const struct scheduler *, unsigned int); > }; > > -extern const struct scheduler sched_credit_def; > -extern const struct scheduler sched_credit2_def; > -extern const struct scheduler sched_arinc653_def; > -extern const struct scheduler sched_rtds_def; > - With these changes, you can make the structures themselves static, which would be a nice tidyup. ~Andrew > #define REGISTER_SCHEDULER(x) static const struct scheduler *x##_entry \ > __used_section(".data.schedulers") = &x; >
>>> On 17.12.15 at 21:59, <jonathan.creekmore@gmail.com> wrote: > +extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; > +#define NUM_SCHEDULERS (((uintptr_t)__schedulers_end-(uintptr_t)__schedulers_start) \ > + / sizeof(struct scheduler *)) > +static const struct scheduler **schedulers = __schedulers_start; I really wonder whether we should continue follow this route of __start_ / __stop_ symbols, instead of leveraging gas+ld's .startof. and .sizeof. operators. Jan
Jan Beulich writes: >>>> On 17.12.15 at 21:59, <jonathan.creekmore@gmail.com> wrote: >> +extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; >> +#define NUM_SCHEDULERS (((uintptr_t)__schedulers_end-(uintptr_t)__schedulers_start) \ >> + / sizeof(struct scheduler *)) >> +static const struct scheduler **schedulers = __schedulers_start; > > I really wonder whether we should continue follow this route of > __start_ / __stop_ symbols, instead of leveraging gas+ld's > .startof. and .sizeof. operators. So, I would love to explore using those operators if you can give me some link to documentation for using them. I have yet to be able to find a construct for LD and GCC that works correctly for generating equivalent symbols.
>>> On 18.12.15 at 17:00, <jonathan.creekmore@gmail.com> wrote: > Jan Beulich writes: >>>>> On 17.12.15 at 21:59, <jonathan.creekmore@gmail.com> wrote: >>> +extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; >>> +#define NUM_SCHEDULERS > (((uintptr_t)__schedulers_end-(uintptr_t)__schedulers_start) \ >>> + / sizeof(struct scheduler *)) >>> +static const struct scheduler **schedulers = __schedulers_start; >> >> I really wonder whether we should continue follow this route of >> __start_ / __stop_ symbols, instead of leveraging gas+ld's >> .startof. and .sizeof. operators. > > So, I would love to explore using those operators if you can give me > some link to documentation for using them. I have yet to be able to find > a construct for LD and GCC that works correctly for generating > equivalent symbols. With binutils docs missing any notion of these, I can only refer you to binutils sources, I'm afraid. Jan
Andrew Cooper writes: > On 17/12/2015 20:59, Jonathan Creekmore wrote: >> Instead of having a manually-curated list of schedulers, use the array >> that was auto-generated simply by compiling in the scheduler files as >> the sole source of truth of the available schedulers. >> >> CC: George Dunlap <george.dunlap@eu.citrix.com> >> CC: Dario Faggioli <dario.faggioli@citrix.com> >> Signed-off-by: Jonathan Creekmore <jonathan.creekmore@gmail.com> >> --- >> xen/common/schedule.c | 24 +++++++----------------- >> xen/include/xen/sched-if.h | 5 ----- >> 2 files changed, 7 insertions(+), 22 deletions(-) >> >> diff --git a/xen/common/schedule.c b/xen/common/schedule.c >> index 2f98a48..efbd67d 100644 >> --- a/xen/common/schedule.c >> +++ b/xen/common/schedule.c >> @@ -64,20 +64,10 @@ static void poll_timer_fn(void *data); >> DEFINE_PER_CPU(struct schedule_data, schedule_data); >> DEFINE_PER_CPU(struct scheduler *, scheduler); >> >> -static const struct scheduler *schedulers[] = { >> -#ifdef CONFIG_SCHED_CREDIT >> - &sched_credit_def, >> -#endif >> -#ifdef CONFIG_SCHED_CREDIT2 >> - &sched_credit2_def, >> -#endif >> -#ifdef CONFIG_SCHED_ARINC653 >> - &sched_arinc653_def, >> -#endif >> -#ifdef CONFIG_SCHED_RTDS >> - &sched_rtds_def, >> -#endif >> -}; >> +extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; >> +#define NUM_SCHEDULERS (((uintptr_t)__schedulers_end-(uintptr_t)__schedulers_start) \ >> + / sizeof(struct scheduler *)) >> +static const struct scheduler **schedulers = __schedulers_start; > > You should be able to play some tricks with getting the linker to set a > size of a variable it creates, which would hopefully avoid some of this > complexity. Yeah, that looks fairly straightforward. I can do that in a v2. >> >> static struct scheduler __read_mostly ops; >> >> @@ -1468,7 +1458,7 @@ void __init scheduler_init(void) >> >> open_softirq(SCHEDULE_SOFTIRQ, schedule); >> >> - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) >> + for ( i = 0; i < NUM_SCHEDULERS; i++) >> { >> if ( schedulers[i]->global_init && schedulers[i]->global_init() < 0 ) >> schedulers[i] = NULL; >> @@ -1479,7 +1469,7 @@ void __init scheduler_init(void) >> if ( !ops.name ) >> { >> printk("Could not find scheduler: %s\n", opt_sched); >> - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) >> + for ( i = 0; i < NUM_SCHEDULERS; i++ ) >> if ( schedulers[i] ) >> { >> ops = *schedulers[i]; >> @@ -1599,7 +1589,7 @@ struct scheduler *scheduler_alloc(unsigned int sched_id, int *perr) >> int i; >> struct scheduler *sched; >> >> - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) >> + for ( i = 0; i < NUM_SCHEDULERS; i++ ) >> if ( schedulers[i] && schedulers[i]->sched_id == sched_id ) >> goto found; >> *perr = -ENOENT; >> diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h >> index 9c6e0f5..66dc9c8 100644 >> --- a/xen/include/xen/sched-if.h >> +++ b/xen/include/xen/sched-if.h >> @@ -165,11 +165,6 @@ struct scheduler { >> void (*tick_resume) (const struct scheduler *, unsigned int); >> }; >> >> -extern const struct scheduler sched_credit_def; >> -extern const struct scheduler sched_credit2_def; >> -extern const struct scheduler sched_arinc653_def; >> -extern const struct scheduler sched_rtds_def; >> - > > With these changes, you can make the structures themselves static, which > would be a nice tidyup. > I like that as well and will handle that in the v2.
> On Dec 18, 2015, at 10:43 AM, Jan Beulich <jbeulich@suse.com> wrote: > >>>> On 18.12.15 at 17:00, <jonathan.creekmore@gmail.com> wrote: >> Jan Beulich writes: >>>>>> On 17.12.15 at 21:59, <jonathan.creekmore@gmail.com> wrote: >>>> +extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; >>>> +#define NUM_SCHEDULERS >> (((uintptr_t)__schedulers_end-(uintptr_t)__schedulers_start) \ >>>> + / sizeof(struct scheduler *)) >>>> +static const struct scheduler **schedulers = __schedulers_start; >>> >>> I really wonder whether we should continue follow this route of >>> __start_ / __stop_ symbols, instead of leveraging gas+ld's >>> .startof. and .sizeof. operators. >> >> So, I would love to explore using those operators if you can give me >> some link to documentation for using them. I have yet to be able to find >> a construct for LD and GCC that works correctly for generating >> equivalent symbols. > > With binutils docs missing any notion of these, I can only refer > you to binutils sources, I'm afraid. > Well, I would prefer not to delve into undocumented behavior in what should be a fairly straightforward patch set, so I plan on keeping the use of the __start and __stop symbols.
On 18/12/15 17:24, Jonathan Creekmore wrote: >> On Dec 18, 2015, at 10:43 AM, Jan Beulich <jbeulich@suse.com> wrote: >> >>>>> On 18.12.15 at 17:00, <jonathan.creekmore@gmail.com> wrote: >>> Jan Beulich writes: >>>>>>> On 17.12.15 at 21:59, <jonathan.creekmore@gmail.com> wrote: >>>>> +extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; >>>>> +#define NUM_SCHEDULERS >>> (((uintptr_t)__schedulers_end-(uintptr_t)__schedulers_start) \ >>>>> + / sizeof(struct scheduler *)) >>>>> +static const struct scheduler **schedulers = __schedulers_start; >>>> I really wonder whether we should continue follow this route of >>>> __start_ / __stop_ symbols, instead of leveraging gas+ld's >>>> .startof. and .sizeof. operators. >>> So, I would love to explore using those operators if you can give me >>> some link to documentation for using them. I have yet to be able to find >>> a construct for LD and GCC that works correctly for generating >>> equivalent symbols. >> With binutils docs missing any notion of these, I can only refer >> you to binutils sources, I'm afraid. >> > Well, I would prefer not to delve into undocumented behavior in what > should be a fairly straightforward patch set, so I plan on keeping the use > of the __start and __stop symbols. One hint to pick up from the Linux side of things is that you can do: extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; #define NUM_SCHEDULERS (__schedulers_end - __schedulers_start) and rely on the semantics of pointer arithmetic. ~Andrew
diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 2f98a48..efbd67d 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -64,20 +64,10 @@ static void poll_timer_fn(void *data); DEFINE_PER_CPU(struct schedule_data, schedule_data); DEFINE_PER_CPU(struct scheduler *, scheduler); -static const struct scheduler *schedulers[] = { -#ifdef CONFIG_SCHED_CREDIT - &sched_credit_def, -#endif -#ifdef CONFIG_SCHED_CREDIT2 - &sched_credit2_def, -#endif -#ifdef CONFIG_SCHED_ARINC653 - &sched_arinc653_def, -#endif -#ifdef CONFIG_SCHED_RTDS - &sched_rtds_def, -#endif -}; +extern const struct scheduler *__schedulers_start[], *__schedulers_end[]; +#define NUM_SCHEDULERS (((uintptr_t)__schedulers_end-(uintptr_t)__schedulers_start) \ + / sizeof(struct scheduler *)) +static const struct scheduler **schedulers = __schedulers_start; static struct scheduler __read_mostly ops; @@ -1468,7 +1458,7 @@ void __init scheduler_init(void) open_softirq(SCHEDULE_SOFTIRQ, schedule); - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) + for ( i = 0; i < NUM_SCHEDULERS; i++) { if ( schedulers[i]->global_init && schedulers[i]->global_init() < 0 ) schedulers[i] = NULL; @@ -1479,7 +1469,7 @@ void __init scheduler_init(void) if ( !ops.name ) { printk("Could not find scheduler: %s\n", opt_sched); - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) + for ( i = 0; i < NUM_SCHEDULERS; i++ ) if ( schedulers[i] ) { ops = *schedulers[i]; @@ -1599,7 +1589,7 @@ struct scheduler *scheduler_alloc(unsigned int sched_id, int *perr) int i; struct scheduler *sched; - for ( i = 0; i < ARRAY_SIZE(schedulers); i++ ) + for ( i = 0; i < NUM_SCHEDULERS; i++ ) if ( schedulers[i] && schedulers[i]->sched_id == sched_id ) goto found; *perr = -ENOENT; diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h index 9c6e0f5..66dc9c8 100644 --- a/xen/include/xen/sched-if.h +++ b/xen/include/xen/sched-if.h @@ -165,11 +165,6 @@ struct scheduler { void (*tick_resume) (const struct scheduler *, unsigned int); }; -extern const struct scheduler sched_credit_def; -extern const struct scheduler sched_credit2_def; -extern const struct scheduler sched_arinc653_def; -extern const struct scheduler sched_rtds_def; - #define REGISTER_SCHEDULER(x) static const struct scheduler *x##_entry \ __used_section(".data.schedulers") = &x;
Instead of having a manually-curated list of schedulers, use the array that was auto-generated simply by compiling in the scheduler files as the sole source of truth of the available schedulers. CC: George Dunlap <george.dunlap@eu.citrix.com> CC: Dario Faggioli <dario.faggioli@citrix.com> Signed-off-by: Jonathan Creekmore <jonathan.creekmore@gmail.com> --- xen/common/schedule.c | 24 +++++++----------------- xen/include/xen/sched-if.h | 5 ----- 2 files changed, 7 insertions(+), 22 deletions(-)