Re: [Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Andrew Cooper]

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 
> CC: Dario Faggioli 
> Signed-off-by: Jonathan Creekmore 
> ---
>  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
> -_credit_def,
> -#endif
> -#ifdef CONFIG_SCHED_CREDIT2
> -_credit2_def,
> -#endif
> -#ifdef CONFIG_SCHED_ARINC653
> -_arinc653_def,
> -#endif
> -#ifdef CONFIG_SCHED_RTDS
> -_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") = 
>  


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Jan Beulich]

>>> On 17.12.15 at 21:59,  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


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Jonathan Creekmore]

Jan Beulich writes:

 On 17.12.15 at 21:59,  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.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Jan Beulich]

>>> On 18.12.15 at 17:00,  wrote:
> Jan Beulich writes:
> On 17.12.15 at 21:59,  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


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Jonathan Creekmore]

> On Dec 18, 2015, at 10:43 AM, Jan Beulich  wrote:
> 
 On 18.12.15 at 17:00,  wrote:
>> Jan Beulich writes:
>> On 17.12.15 at 21:59,  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.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Jonathan Creekmore]

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 
>> CC: Dario Faggioli 
>> Signed-off-by: Jonathan Creekmore 
>> ---
>>  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
>> -_credit_def,
>> -#endif
>> -#ifdef CONFIG_SCHED_CREDIT2
>> -_credit2_def,
>> -#endif
>> -#ifdef CONFIG_SCHED_ARINC653
>> -_arinc653_def,
>> -#endif
>> -#ifdef CONFIG_SCHED_RTDS
>> -_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.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Andrew Cooper]

On 18/12/15 17:24, Jonathan Creekmore wrote:
>> On Dec 18, 2015, at 10:43 AM, Jan Beulich  wrote:
>>
> On 18.12.15 at 17:00,  wrote:
>>> Jan Beulich writes:
>>> On 17.12.15 at 21:59,  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

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Jonathan Creekmore]

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 
CC: Dario Faggioli 
Signed-off-by: Jonathan Creekmore 
---
 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
-_credit_def,
-#endif
-#ifdef CONFIG_SCHED_CREDIT2
-_credit2_def,
-#endif
-#ifdef CONFIG_SCHED_ARINC653
-_arinc653_def,
-#endif
-#ifdef CONFIG_SCHED_RTDS
-_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") = 
 
-- 
2.6.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/4] sched: Use the auto-generated list of schedulers

[Dario Faggioli]

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 
> CC: Dario Faggioli 
> Signed-off-by: Jonathan Creekmore 
>
Acked-by: Dario Faggioli 

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel