[Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

Creates a section to contain scheduler entry pointers that are gathered
together into an array. This will allow, in a follow-on patch, scheduler
entries to be automatically gathered together into the array for
automatic parsing.

CC: Ian Campbell 
CC: Stefano Stabellini 
CC: Keir Fraser 
CC: Jan Beulich 
CC: Andrew Cooper 
Signed-off-by: Jonathan Creekmore 
---
 xen/arch/arm/xen.lds.S | 4 
 xen/arch/x86/xen.lds.S | 4 
 2 files changed, 8 insertions(+)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 0488f37..39a4c86 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -57,6 +57,10 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
*(.data.page_aligned)
*(.data)
+   . = ALIGN(8);
+   __schedulers_start = .;
+   *(.data.schedulers)
+   __schedulers_end = .;
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e18e08f..70caf85 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -88,6 +88,10 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
*(.data.page_aligned)
*(.data)
+   . = ALIGN(8);
+   __schedulers_start = .;
+   *(.data.schedulers)
+   __schedulers_end = .;
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
-- 
2.6.4


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

[Xen-devel] [PATCH 1/4] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Allow the schedulers to be independently enabled or disabled at
compile-time instead of just allowing the scheduler to be selected on
the command line. To match existing behavior, all four schedulers are
compiled in by default, although the RTDS and ARINC653 are marked
EXPERIMENTAL to match their not currently supported status.

CC: George Dunlap 
CC: Dario Faggioli 
Signed-off-by: Jonathan Creekmore 
---
 xen/common/Kconfig| 64 +++
 xen/common/Makefile   |  8 +++
 xen/common/schedule.c | 12 --
 3 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 7d0e9a9..378a063 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -44,4 +44,68 @@ config KEXEC
 
  If unsure, say Y.
 
+# Enable schedulers
+menu "Schedulers"
+config SCHED_CREDIT
+   bool "Credit scheduler support"
+   default y
+   ---help---
+ The traditional credit scheduler is a general purpose scheduler.
+
+ If unsure, say Y.
+
+config SCHED_CREDIT2
+   bool "Credit2 scheduler support"
+   default y
+   ---help---
+ The credit2 scheduler is a general purpose scheduler that is
+ optimized for lower latency and higher VM density.
+
+ If unsure, say Y.
+
+config SCHED_RTDS
+   bool "RTDS scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The RTDS scheduler is a soft and firm real-time scheduler for
+ multicore, targeted for embedded, automotive, graphics and gaming
+ in the cloud, and general low-latency workloads.
+
+ If unsure, say N.
+
+config SCHED_ARINC653
+   bool "ARINC653 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The ARINC653 scheduler is a hard real-time scheduler for single
+ cores, targeted for avionics, drones, and medical devices.
+
+ If unsure, say N.
+
+choice
+   prompt "Default Scheduler?"
+   default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
+   default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
+   default SCHED_RTDS_DEFAULT if SCHED_RTDS
+   default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
+
+   config SCHED_CREDIT_DEFAULT
+   bool "Credit Scheduler" if SCHED_CREDIT
+   config SCHED_CREDIT2_DEFAULT
+   bool "Credit2 Scheduler" if SCHED_CREDIT2
+   config SCHED_RTDS_DEFAULT
+   bool "RT Scheduler" if SCHED_RTDS
+   config SCHED_ARINC653_DEFAULT
+   bool "ARINC653 Scheduler" if SCHED_ARINC653
+endchoice
+
+config SCHED_DEFAULT
+   string
+   default "credit" if SCHED_CREDIT_DEFAULT
+   default "credit2" if SCHED_CREDIT2_DEFAULT
+   default "rtds" if SCHED_RTDS_DEFAULT
+   default "arinc653" if SCHED_ARINC653_DEFAULT
+
+endmenu
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 8ab15ba..3a2026a 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -30,10 +30,10 @@ obj-y += rangeset.o
 obj-y += radix-tree.o
 obj-y += rbtree.o
 obj-y += rcupdate.o
-obj-y += sched_credit.o
-obj-y += sched_credit2.o
-obj-y += sched_arinc653.o
-obj-y += sched_rt.o
+obj-$(CONFIG_SCHED_CREDIT) += sched_credit.o
+obj-$(CONFIG_SCHED_CREDIT2) += sched_credit2.o
+obj-$(CONFIG_SCHED_ARINC653) += sched_arinc653.o
+obj-$(CONFIG_SCHED_RTDS) += sched_rt.o
 obj-y += schedule.o
 obj-y += shutdown.o
 obj-y += softirq.o
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index d121896..2f98a48 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -38,8 +38,8 @@
 #include 
 #include 
 
-/* opt_sched: scheduler - default to credit */
-static char __initdata opt_sched[10] = "credit";
+/* opt_sched: scheduler - default to configured value */
+static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
 string_param("sched", opt_sched);
 
 /* if sched_smt_power_savings is set,
@@ -65,10 +65,18 @@ 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
 };
 
 static struct scheduler __read_mostly ops;
-- 
2.6.4


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

[Xen-devel] [PATCH 3/4] sched: Register the schedulers into the list

[Jonathan Creekmore]

Adds a simple macro to place a pointer to a scheduler into an array
section at compile time. Also, goes ahead and generates the array
entries with each of the schedulers.

CC: George Dunlap 
CC: Dario Faggioli 
CC: Josh Whitehead 
CC: Robert VanVossen 
Signed-off-by: Jonathan Creekmore 
---
 xen/common/sched_arinc653.c | 2 ++
 xen/common/sched_credit.c   | 2 ++
 xen/common/sched_credit2.c  | 2 ++
 xen/common/sched_rt.c   | 2 ++
 xen/include/xen/sched-if.h  | 2 ++
 5 files changed, 10 insertions(+)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index dbe02ed..3b59514 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -767,6 +767,8 @@ const struct scheduler sched_arinc653_def = {
 .tick_resume= NULL,
 };
 
+REGISTER_SCHEDULER(sched_arinc653_def);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 0dce790..e586248 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -2027,3 +2027,5 @@ const struct scheduler sched_credit_def = {
 .tick_suspend   = csched_tick_suspend,
 .tick_resume= csched_tick_resume,
 };
+
+REGISTER_SCHEDULER(sched_credit_def);
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 3c49ffa..38b02d0 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2228,3 +2228,5 @@ const struct scheduler sched_credit2_def = {
 .alloc_domdata  = csched2_alloc_domdata,
 .free_domdata   = csched2_free_domdata,
 };
+
+REGISTER_SCHEDULER(sched_credit2_def);
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 3f1d047..7640cd0 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1199,3 +1199,5 @@ const struct scheduler sched_rtds_def = {
 .wake   = rt_vcpu_wake,
 .context_saved  = rt_context_saved,
 };
+
+REGISTER_SCHEDULER(sched_rtds_def);
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 493d43f..9c6e0f5 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -170,6 +170,8 @@ 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;
 
 struct cpupool
 {
-- 
2.6.4


___
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
-&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;
 
-- 
2.6.4


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

[Xen-devel] [PATCH 0/4] Allow schedulers to be selectable through Kconfig

[Jonathan Creekmore]

Add machinery to allow the schedulers to be individually selectable
through the Kconfig interface. The first patch in the series sets up
the Kconfig options for the schedulers and places the appropriate CONFIG_
options around the scheduler list. The second, third, and fourth patches
rework the scheduler list from being manually curated into a model
where just compiling any schedulers into the hypervisor causes the
scheduler list to be built up.

Jonathan Creekmore (4):
  build: Hook the schedulers into Kconfig
  build: Alloc space for sched list in the link file
  sched: Register the schedulers into the list
  sched: Use the auto-generated list of schedulers

 xen/arch/arm/xen.lds.S  |  4 +++
 xen/arch/x86/xen.lds.S  |  4 +++
 xen/common/Kconfig  | 64 +
 xen/common/Makefile |  8 +++---
 xen/common/sched_arinc653.c |  2 ++
 xen/common/sched_credit.c   |  2 ++
 xen/common/sched_credit2.c  |  2 ++
 xen/common/sched_rt.c   |  2 ++
 xen/common/schedule.c   | 20 +++---
 xen/include/xen/sched-if.h  |  7 ++---
 10 files changed, 95 insertions(+), 20 deletions(-)

-- 
2.6.4


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

Re: [Xen-devel] [PATCH 1/4] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

> On Dec 17, 2015, at 7:35 PM, Dario Faggioli  wrote:
> 
> On Thu, 2015-12-17 at 14:59 -0600, Jonathan Creekmore wrote:
>> Allow the schedulers to be independently enabled or disabled at
>> compile-time instead of just allowing the scheduler to be selected on
>> the command line. 
>> 
> Reading this quickly, that "instead" gave me a bit of an hard time. I'm
> not a native English speaker, and I'm sure it's me that am wrong, but
> for some reason the sentence made me think that the patch would somehow
> disallow specifying a scheduler during boot, in Xen's command line.
> 
> Fact is, I don't think the phrase "instead of just allowing the
> scheduler to be selected on the command line." adds much information,
> and I'd just remove it.

Sorry for the confusion — I did not want to give the impression that 
this patch would remove that functionality. I can definitely remove 
that language from the patch. 

> 
>> To match existing behavior, all four schedulers are
>> compiled in by default, although the RTDS and ARINC653 are marked
>> EXPERIMENTAL to match their not currently supported status.
>> 
> This may change shortly, but as of now, Credit2 is still experimental
> too. I think I'd still recommend enabling it in the help file (i.e.,
> I'm ok with "If unsure, say Y"), but we certainly should mark it with
> "(EPERIMENTAL)”.

OK, I can mark that EXPERIMENTAL as well. I was under the impression
that 4.7 was going to migrate Credit2 to being SUPPORTED, but I may
have jumped the gun in the Kconfig.

> I don't know much on how kconfig works, so all the changes to the
> makefile, etc, I'm not able to review them properly.
> 
> On the other hand, the code here below...
> 
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -38,8 +38,8 @@
>>  #include 
>>  #include 
>>  
>> -/* opt_sched: scheduler - default to credit */
>> -static char __initdata opt_sched[10] = "credit";
>> +/* opt_sched: scheduler - default to configured value */
>> +static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
>>  string_param("sched", opt_sched);
>>  
>>  /* if sched_smt_power_savings is set,
>> @@ -65,10 +65,18 @@ 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
>>  };
>>  
> ... can have, with the changelog changed as shown, my:
> 
>  Acked-by: Dario Faggioli 
> 
> Regards,
> Daio
> -- 
> <> (Raistlin Majere)
> -
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
> 


___
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 2/4] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

> On Dec 18, 2015, at 3:01 AM, Andrew Cooper  wrote:
> 
> On 17/12/2015 20:59, Jonathan Creekmore wrote:
>> Creates a section to contain scheduler entry pointers that are gathered
>> together into an array. This will allow, in a follow-on patch, scheduler
>> entries to be automatically gathered together into the array for
>> automatic parsing.
>> 
>> CC: Ian Campbell 
>> CC: Stefano Stabellini 
>> CC: Keir Fraser 
>> CC: Jan Beulich 
>> CC: Andrew Cooper 
>> Signed-off-by: Jonathan Creekmore 
>> ---
>> xen/arch/arm/xen.lds.S | 4 
>> xen/arch/x86/xen.lds.S | 4 
>> 2 files changed, 8 insertions(+)
>> 
>> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
>> index 0488f37..39a4c86 100644
>> --- a/xen/arch/arm/xen.lds.S
>> +++ b/xen/arch/arm/xen.lds.S
>> @@ -57,6 +57,10 @@ SECTIONS
>>. = ALIGN(PAGE_SIZE);
>>*(.data.page_aligned)
>>*(.data)
>> +   . = ALIGN(8);
>> +   __schedulers_start = .;
>> +   *(.data.schedulers)
>> +   __schedulers_end = .;
> 
> These arrays are only ever used in __init scheduler_init().  They should
> be in .init.data rather than .data, which allows their memory to be
> reclaimed after boot.
> 
> With that, Reviewed-by: Andrew Cooper 

So, they are used in scheduler_init() which is marked __init, but 
scheduler_alloc
also uses that array (and did before my patch) and it is not marked __init.


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

Re: [Xen-devel] [PATCH 1/4] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Andrew Cooper writes:

> On 17/12/2015 20:59, Jonathan Creekmore wrote:
>> Allow the schedulers to be independently enabled or disabled at
>> compile-time instead of just allowing the scheduler to be selected on
>> the command line. To match existing behavior, all four schedulers are
>> compiled in by default, although the RTDS and ARINC653 are marked
>> EXPERIMENTAL to match their not currently supported status.
>>
>> CC: George Dunlap 
>> CC: Dario Faggioli 
>> Signed-off-by: Jonathan Creekmore 
>
> With the suggestions Dario made, and one minor nit below,
>
> Reviewed-by: Andrew Cooper 
>

I plan on making those changes.

>> diff --git a/xen/common/Makefile b/xen/common/Makefile
>> index 8ab15ba..3a2026a 100644
>> --- a/xen/common/Makefile
>> +++ b/xen/common/Makefile
>> @@ -30,10 +30,10 @@ obj-y += rangeset.o
>>  obj-y += radix-tree.o
>>  obj-y += rbtree.o
>>  obj-y += rcupdate.o
>> -obj-y += sched_credit.o
>> -obj-y += sched_credit2.o
>> -obj-y += sched_arinc653.o
>> -obj-y += sched_rt.o
>> +obj-$(CONFIG_SCHED_CREDIT) += sched_credit.o
>> +obj-$(CONFIG_SCHED_CREDIT2) += sched_credit2.o
>> +obj-$(CONFIG_SCHED_ARINC653) += sched_arinc653.o
>> +obj-$(CONFIG_SCHED_RTDS) += sched_rt.o
>
> I know it was wrong before, but please reorder

Easy enough to do. I will take care of that.

___
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
>> -&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.

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

Re: [Xen-devel] [PATCH 2/4] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

> On Dec 18, 2015, at 11:07 AM, Jan Beulich  wrote:
> 
>>>> On 18.12.15 at 17:48,  wrote:
>> On 18/12/15 16:40, Jonathan Creekmore wrote:
>>>> On Dec 18, 2015, at 3:01 AM, Andrew Cooper  
>>>> wrote:
>>>> 
>>>> On 17/12/2015 20:59, Jonathan Creekmore wrote:
>>>>> Creates a section to contain scheduler entry pointers that are gathered
>>>>> together into an array. This will allow, in a follow-on patch, scheduler
>>>>> entries to be automatically gathered together into the array for
>>>>> automatic parsing.
>>>>> 
>>>>> CC: Ian Campbell 
>>>>> CC: Stefano Stabellini 
>>>>> CC: Keir Fraser 
>>>>> CC: Jan Beulich 
>>>>> CC: Andrew Cooper 
>>>>> Signed-off-by: Jonathan Creekmore 
>>>>> ---
>>>>> xen/arch/arm/xen.lds.S | 4 
>>>>> xen/arch/x86/xen.lds.S | 4 
>>>>> 2 files changed, 8 insertions(+)
>>>>> 
>>>>> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
>>>>> index 0488f37..39a4c86 100644
>>>>> --- a/xen/arch/arm/xen.lds.S
>>>>> +++ b/xen/arch/arm/xen.lds.S
>>>>> @@ -57,6 +57,10 @@ SECTIONS
>>>>>   . = ALIGN(PAGE_SIZE);
>>>>>   *(.data.page_aligned)
>>>>>   *(.data)
>>>>> +   . = ALIGN(8);
>>>>> +   __schedulers_start = .;
>>>>> +   *(.data.schedulers)
>>>>> +   __schedulers_end = .;
>>>> These arrays are only ever used in __init scheduler_init().  They should
>>>> be in .init.data rather than .data, which allows their memory to be
>>>> reclaimed after boot.
>>>> 
>>>> With that, Reviewed-by: Andrew Cooper 
>>> So, they are used in scheduler_init() which is marked __init, but 
>> scheduler_alloc
>>> also uses that array (and did before my patch) and it is not marked __init.
>> 
>> Ah yes - so they are.  Apologies for the noise.  This should be in .data
>> and my R-b stands.
> 
> In .rodata perhaps?

I initially put it in .rodata, but the current algorithm for choosing the 
scheduler NULLs out
the pointers in the array if the global_init() function fails. To emulate that 
behavior, I had to 
move the section to .data instead of .rodata.



___
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

[Xen-devel] [PATCH v2 2/4] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

Creates a section to contain scheduler entry pointers that are gathered
together into an array. This will allow, in a follow-on patch, scheduler
entries to be automatically gathered together into the array for
automatic parsing.

CC: Ian Campbell 
CC: Stefano Stabellini 
CC: Keir Fraser 
CC: Jan Beulich 
CC: Andrew Cooper 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Andrew Cooper 

---
Changed since v1:
  * rename the __start and __end symbols to better match
the rest of the file
---
 xen/arch/arm/xen.lds.S | 4 
 xen/arch/x86/xen.lds.S | 4 
 2 files changed, 8 insertions(+)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 0488f37..f501a2f 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -57,6 +57,10 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
*(.data.page_aligned)
*(.data)
+   . = ALIGN(8);
+   __start_schedulers_array = .;
+   *(.data.schedulers)
+   __end_schedulers_array = .;
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e18e08f..c1ce027 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -80,6 +80,10 @@ SECTIONS
__stop___pre_ex_table = .;
 
*(.data.read_mostly)
+   . = ALIGN(8);
+   __start_schedulers_array = .;
+   *(.data.schedulers)
+   __end_schedulers_array = .;
*(.data.rel.ro)
*(.data.rel.ro.*)
   } :text
-- 
2.6.4


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

[Xen-devel] [PATCH v2 3/4] sched: Register the schedulers into the list

[Jonathan Creekmore]

Adds a simple macro to place a pointer to a scheduler into an array
section at compile time. Also, goes ahead and generates the array
entries with each of the schedulers.

CC: George Dunlap 
CC: Dario Faggioli 
CC: Josh Whitehead 
CC: Robert VanVossen 
Signed-off-by: Jonathan Creekmore 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 
---
 xen/common/sched_arinc653.c | 2 ++
 xen/common/sched_credit.c   | 2 ++
 xen/common/sched_credit2.c  | 2 ++
 xen/common/sched_rt.c   | 2 ++
 xen/include/xen/sched-if.h  | 2 ++
 5 files changed, 10 insertions(+)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index dbe02ed..3b59514 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -767,6 +767,8 @@ const struct scheduler sched_arinc653_def = {
 .tick_resume= NULL,
 };
 
+REGISTER_SCHEDULER(sched_arinc653_def);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 0dce790..e586248 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -2027,3 +2027,5 @@ const struct scheduler sched_credit_def = {
 .tick_suspend   = csched_tick_suspend,
 .tick_resume= csched_tick_resume,
 };
+
+REGISTER_SCHEDULER(sched_credit_def);
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 3c49ffa..38b02d0 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2228,3 +2228,5 @@ const struct scheduler sched_credit2_def = {
 .alloc_domdata  = csched2_alloc_domdata,
 .free_domdata   = csched2_free_domdata,
 };
+
+REGISTER_SCHEDULER(sched_credit2_def);
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 3f1d047..7640cd0 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1199,3 +1199,5 @@ const struct scheduler sched_rtds_def = {
 .wake   = rt_vcpu_wake,
 .context_saved  = rt_context_saved,
 };
+
+REGISTER_SCHEDULER(sched_rtds_def);
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 493d43f..9c6e0f5 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -170,6 +170,8 @@ 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;
 
 struct cpupool
 {
-- 
2.6.4


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

[Xen-devel] [PATCH v2 1/4] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Allow the schedulers to be independently enabled or disabled at
compile-time. To match existing behavior, all four schedulers are
compiled in by default, although the Credit2, RTDS, and ARINC653 are
marked EXPERIMENTAL to match their not currently supported status.

CC: George Dunlap 
CC: Dario Faggioli 
Signed-off-by: Jonathan Creekmore 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 

---
Changed since v1:

  * Marked credit2 as EXPERIMENTAL
  * Removed confusing language from the commit message
  * Alphabetize the schedulers
---
 xen/common/Kconfig| 64 +++
 xen/common/Makefile   |  8 +++
 xen/common/schedule.c | 12 --
 3 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 7d0e9a9..7f02738 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -44,4 +44,68 @@ config KEXEC
 
  If unsure, say Y.
 
+# Enable schedulers
+menu "Schedulers"
+config SCHED_CREDIT
+   bool "Credit scheduler support"
+   default y
+   ---help---
+ The traditional credit scheduler is a general purpose scheduler.
+
+ If unsure, say Y.
+
+config SCHED_CREDIT2
+   bool "Credit2 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The credit2 scheduler is a general purpose scheduler that is
+ optimized for lower latency and higher VM density.
+
+ If unsure, say Y.
+
+config SCHED_RTDS
+   bool "RTDS scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The RTDS scheduler is a soft and firm real-time scheduler for
+ multicore, targeted for embedded, automotive, graphics and gaming
+ in the cloud, and general low-latency workloads.
+
+ If unsure, say N.
+
+config SCHED_ARINC653
+   bool "ARINC653 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The ARINC653 scheduler is a hard real-time scheduler for single
+ cores, targeted for avionics, drones, and medical devices.
+
+ If unsure, say N.
+
+choice
+   prompt "Default Scheduler?"
+   default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
+   default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
+   default SCHED_RTDS_DEFAULT if SCHED_RTDS
+   default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
+
+   config SCHED_CREDIT_DEFAULT
+   bool "Credit Scheduler" if SCHED_CREDIT
+   config SCHED_CREDIT2_DEFAULT
+   bool "Credit2 Scheduler" if SCHED_CREDIT2
+   config SCHED_RTDS_DEFAULT
+   bool "RT Scheduler" if SCHED_RTDS
+   config SCHED_ARINC653_DEFAULT
+   bool "ARINC653 Scheduler" if SCHED_ARINC653
+endchoice
+
+config SCHED_DEFAULT
+   string
+   default "credit" if SCHED_CREDIT_DEFAULT
+   default "credit2" if SCHED_CREDIT2_DEFAULT
+   default "rtds" if SCHED_RTDS_DEFAULT
+   default "arinc653" if SCHED_ARINC653_DEFAULT
+
+endmenu
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 8ab15ba..29a5916 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -30,10 +30,10 @@ obj-y += rangeset.o
 obj-y += radix-tree.o
 obj-y += rbtree.o
 obj-y += rcupdate.o
-obj-y += sched_credit.o
-obj-y += sched_credit2.o
-obj-y += sched_arinc653.o
-obj-y += sched_rt.o
+obj-$(CONFIG_SCHED_ARINC653) += sched_arinc653.o
+obj-$(CONFIG_SCHED_CREDIT) += sched_credit.o
+obj-$(CONFIG_SCHED_CREDIT2) += sched_credit2.o
+obj-$(CONFIG_SCHED_RTDS) += sched_rt.o
 obj-y += schedule.o
 obj-y += shutdown.o
 obj-y += softirq.o
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index d121896..2f98a48 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -38,8 +38,8 @@
 #include 
 #include 
 
-/* opt_sched: scheduler - default to credit */
-static char __initdata opt_sched[10] = "credit";
+/* opt_sched: scheduler - default to configured value */
+static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
 string_param("sched", opt_sched);
 
 /* if sched_smt_power_savings is set,
@@ -65,10 +65,18 @@ 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
 };
 
 static struct scheduler __read_mostly ops;
-- 
2.6.4


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

[Xen-devel] [PATCH v2 0/4] Allow schedulers to be selectable through Kconfig

[Jonathan Creekmore]

Add machinery to allow the schedulers to be individually selectable
through the Kconfig interface. The first patch in the series sets up
the Kconfig options for the schedulers and places the appropriate CONFIG_
options around the scheduler list. The second, third, and fourth patches
rework the scheduler list from being manually curated into a model
where just compiling any schedulers into the hypervisor causes the
scheduler list to be built up.

---
Changed since v1:
  * Marked credit2 as EXPERIMENTAL
  * Removed confusing language from the commit message
  * Alphabetize the schedulers
  * rename the __start and __end symbols to better match
the rest of the file
  * Simplify the calculation of the number of schedulers
  * Make the scheduler ops structures static to their files

Jonathan Creekmore (4):
  build: Hook the schedulers into Kconfig
  build: Alloc space for sched list in the link file
  sched: Register the schedulers into the list
  sched: Use the auto-generated list of schedulers

 xen/arch/arm/xen.lds.S  |  4 +++
 xen/arch/x86/xen.lds.S  |  4 +++
 xen/common/Kconfig  | 64 +
 xen/common/Makefile |  8 +++---
 xen/common/sched_arinc653.c |  4 ++-
 xen/common/sched_credit.c   |  4 ++-
 xen/common/sched_credit2.c  |  4 ++-
 xen/common/sched_rt.c   |  4 ++-
 xen/common/schedule.c   | 20 +++---
 xen/include/xen/sched-if.h  |  7 ++---
 10 files changed, 99 insertions(+), 24 deletions(-)

-- 
2.6.4


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

[Xen-devel] [PATCH v2 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 
Acked-by: Dario Faggioli 

---
Changed since v1:
  * Simplify the calculation of the number of schedulers
  * Make the scheduler ops structures static to their files
---
 xen/common/sched_arinc653.c |  2 +-
 xen/common/sched_credit.c   |  2 +-
 xen/common/sched_credit2.c  |  2 +-
 xen/common/sched_rt.c   |  2 +-
 xen/common/schedule.c   | 24 +++-
 xen/include/xen/sched-if.h  |  5 -
 6 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 3b59514..0606988 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -724,7 +724,7 @@ a653sched_adjust_global(const struct scheduler *ops,
  * callback functions.
  * The symbol must be visible to the rest of Xen at link time.
  */
-const struct scheduler sched_arinc653_def = {
+static const struct scheduler sched_arinc653_def = {
 .name   = "ARINC 653 Scheduler",
 .opt_name   = "arinc653",
 .sched_id   = XEN_SCHEDULER_ARINC653,
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index e586248..028e41b 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1991,7 +1991,7 @@ static void csched_tick_resume(const struct scheduler 
*ops, unsigned int cpu)
 
 static struct csched_private _csched_priv;
 
-const struct scheduler sched_credit_def = {
+static const struct scheduler 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 38b02d0..78220a7 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2194,7 +2194,7 @@ csched2_deinit(const struct scheduler *ops)
 
 static struct csched2_private _csched2_priv;
 
-const struct scheduler sched_credit2_def = {
+static const struct scheduler sched_credit2_def = {
 .name   = "SMP Credit Scheduler rev2",
 .opt_name   = "credit2",
 .sched_id   = XEN_SCHEDULER_CREDIT2,
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 7640cd0..2e5430f 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1170,7 +1170,7 @@ rt_dom_cntl(
 
 static struct rt_private _rt_priv;
 
-const struct scheduler sched_rtds_def = {
+static const struct scheduler sched_rtds_def = {
 .name   = "SMP RTDS Scheduler",
 .opt_name   = "rtds",
 .sched_id   = XEN_SCHEDULER_RTDS,
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 2f98a48..91e53c1 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 *__start_schedulers_array[], 
*__end_schedulers_array[];
+extern const size_t schedulers_array_size;
+#define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
+static const struct scheduler **schedulers = __start_schedulers_array;
 
 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/sche

[Xen-devel] [PATCH v3 0/5] Allow schedulers to be selectable through Kconfig

[Jonathan Creekmore]

Add machinery to allow the schedulers to be individually selectable
through the Kconfig interface. The first patch in the series sets up
the CONFIG_EXPERT Kconfig variable that is only enabled by passing an
environment variable to the build. The second patch in the series sets up
the Kconfig options for the schedulers and places the appropriate CONFIG_
options around the scheduler list. The third, fourth, and fifth patches
rework the scheduler list from being manually curated into a model
where just compiling any schedulers into the hypervisor causes the
scheduler list to be built up.

---
Changed since v2:
 * Added a predecessor patch that introduces a environment
   variable for the build to enable expert configuration options
   (1/5)
 * Hide the scheduler menu behind the expert option (2/5)
 * Provide static inlines for credit functions that are assumed to be
   present if it is compiled out (2/5)
 * Provide an absolute default of the credit scheduler if the 
   scheduler menu is not visible (2/5)

Changed since v1:
 * Marked credit2 as EXPERIMENTAL
 * Removed confusing language from the commit message
 * Alphabetize the schedulers
 * rename the __start and __end symbols to better match
   the rest of the file
 * Simplify the calculation of the number of schedulers
 * Make the scheduler ops structures static to their files

Jonathan Creekmore (5):
  build: Env var to enable expert config options
  build: Hook the schedulers into Kconfig
  build: Alloc space for sched list in the link file
  sched: Register the schedulers into the list
  sched: Use the auto-generated list of schedulers

 xen/Kconfig |  4 +++
 xen/Makefile|  1 +
 xen/arch/arm/xen.lds.S  |  4 +++
 xen/arch/x86/xen.lds.S  |  4 +++
 xen/common/Kconfig  | 67 +
 xen/common/Makefile |  8 +++---
 xen/common/sched_arinc653.c |  4 ++-
 xen/common/sched_credit.c   |  4 ++-
 xen/common/sched_credit2.c  |  4 ++-
 xen/common/sched_rt.c   |  4 ++-
 xen/common/schedule.c   | 20 ++
 xen/include/xen/sched-if.h  |  7 ++---
 xen/include/xen/sched.h |  5 
 13 files changed, 112 insertions(+), 24 deletions(-)

-- 
2.6.4


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

[Xen-devel] [PATCH v3 1/5] build: Env var to enable expert config options

[Jonathan Creekmore]

Add an additional environment variable, defaulting to disabled,
that enables the CONFIG_EXPERT configuration option. The purpose
of the CONFIG_EXPERT configuration option is to make non-standard
Kconfig options visible during the configuration process. The
CONFIG_EXPERT option is not, itself, visible during the Kconfig
configuration process, so typical users will never see it nor
any of the non-standard configuration options.

CC: Ian Campbell 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 

---
 xen/Kconfig  | 4 
 xen/Makefile | 1 +
 2 files changed, 5 insertions(+)

diff --git a/xen/Kconfig b/xen/Kconfig
index ffe3f45..fa8b27c 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -22,3 +22,7 @@ config DEFCONFIG_LIST
string
option defconfig_list
default "$ARCH_DEFCONFIG"
+
+config EXPERT
+   string
+   option env="XEN_CONFIG_EXPERT"
diff --git a/xen/Makefile b/xen/Makefile
index 9023863..4950afb 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -11,6 +11,7 @@ export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] 
&& /bin/dnsdomainname) |
 export XEN_BUILD_DATE  ?= $(shell LC_ALL=C date)
 export XEN_BUILD_TIME  ?= $(shell LC_ALL=C date +%T)
 export XEN_BUILD_HOST  ?= $(shell hostname)
+export XEN_CONFIG_EXPERT ?= n
 
 export BASEDIR := $(CURDIR)
 export XEN_ROOT := $(BASEDIR)/..
-- 
2.6.4


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

[Xen-devel] [PATCH v3 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Allow the schedulers to be independently enabled or disabled at
compile-time. To match existing behavior, all four schedulers are
compiled in by default, although the Credit2, RTDS, and ARINC653 are
marked EXPERIMENTAL to match their not currently supported status.

CC: George Dunlap 
CC: Dario Faggioli 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 

---
Changed since v2:

  * Hid the scheduler menu behind the EXPERT option
  * Provide static inlines for credit functions that are assumed to be
always available
  * Provide an absolute default of the credit scheduler if the
scheduler menu is not visible

Changed since v1:

  * Marked credit2 as EXPERIMENTAL
  * Removed confusing language from the commit message
  * Alphabetize the schedulers
---
 xen/common/Kconfig  | 67 +
 xen/common/Makefile |  8 +++---
 xen/common/schedule.c   | 12 +++--
 xen/include/xen/sched.h |  5 
 4 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 046e257..db7411b 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -51,4 +51,71 @@ config KEXEC
 
  If unsure, say Y.
 
+# Enable schedulers
+menu "Schedulers"
+   visible if EXPERT = "y"
+
+config SCHED_CREDIT
+   bool "Credit scheduler support"
+   default y
+   ---help---
+ The traditional credit scheduler is a general purpose scheduler.
+
+ If unsure, say Y.
+
+config SCHED_CREDIT2
+   bool "Credit2 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The credit2 scheduler is a general purpose scheduler that is
+ optimized for lower latency and higher VM density.
+
+ If unsure, say Y.
+
+config SCHED_RTDS
+   bool "RTDS scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The RTDS scheduler is a soft and firm real-time scheduler for
+ multicore, targeted for embedded, automotive, graphics and gaming
+ in the cloud, and general low-latency workloads.
+
+ If unsure, say N.
+
+config SCHED_ARINC653
+   bool "ARINC653 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The ARINC653 scheduler is a hard real-time scheduler for single
+ cores, targeted for avionics, drones, and medical devices.
+
+ If unsure, say N.
+
+choice
+   prompt "Default Scheduler?"
+   default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
+   default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
+   default SCHED_RTDS_DEFAULT if SCHED_RTDS
+   default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
+
+   config SCHED_CREDIT_DEFAULT
+   bool "Credit Scheduler" if SCHED_CREDIT
+   config SCHED_CREDIT2_DEFAULT
+   bool "Credit2 Scheduler" if SCHED_CREDIT2
+   config SCHED_RTDS_DEFAULT
+   bool "RT Scheduler" if SCHED_RTDS
+   config SCHED_ARINC653_DEFAULT
+   bool "ARINC653 Scheduler" if SCHED_ARINC653
+endchoice
+
+config SCHED_DEFAULT
+   string
+   default "credit" if SCHED_CREDIT_DEFAULT
+   default "credit2" if SCHED_CREDIT2_DEFAULT
+   default "rtds" if SCHED_RTDS_DEFAULT
+   default "arinc653" if SCHED_ARINC653_DEFAULT
+   default "credit"
+
+endmenu
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 8ab15ba..29a5916 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -30,10 +30,10 @@ obj-y += rangeset.o
 obj-y += radix-tree.o
 obj-y += rbtree.o
 obj-y += rcupdate.o
-obj-y += sched_credit.o
-obj-y += sched_credit2.o
-obj-y += sched_arinc653.o
-obj-y += sched_rt.o
+obj-$(CONFIG_SCHED_ARINC653) += sched_arinc653.o
+obj-$(CONFIG_SCHED_CREDIT) += sched_credit.o
+obj-$(CONFIG_SCHED_CREDIT2) += sched_credit2.o
+obj-$(CONFIG_SCHED_RTDS) += sched_rt.o
 obj-y += schedule.o
 obj-y += shutdown.o
 obj-y += softirq.o
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index d121896..2f98a48 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -38,8 +38,8 @@
 #include 
 #include 
 
-/* opt_sched: scheduler - default to credit */
-static char __initdata opt_sched[10] = "credit";
+/* opt_sched: scheduler - default to configured value */
+static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
 string_param("sched", opt_sched);
 
 /* if sched_smt_power_savings is set,
@@ -65,10 +65,18 @@ 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
 &

[Xen-devel] [PATCH v3 4/5] sched: Register the schedulers into the list

[Jonathan Creekmore]

Adds a simple macro to place a pointer to a scheduler into an array
section at compile time. Also, goes ahead and generates the array
entries with each of the schedulers.

CC: George Dunlap 
CC: Dario Faggioli 
CC: Josh Whitehead 
CC: Robert VanVossen 
Signed-off-by: Jonathan Creekmore 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 
---
 xen/common/sched_arinc653.c | 2 ++
 xen/common/sched_credit.c   | 2 ++
 xen/common/sched_credit2.c  | 2 ++
 xen/common/sched_rt.c   | 2 ++
 xen/include/xen/sched-if.h  | 2 ++
 5 files changed, 10 insertions(+)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index dbe02ed..3b59514 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -767,6 +767,8 @@ const struct scheduler sched_arinc653_def = {
 .tick_resume= NULL,
 };
 
+REGISTER_SCHEDULER(sched_arinc653_def);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 0dce790..e586248 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -2027,3 +2027,5 @@ const struct scheduler sched_credit_def = {
 .tick_suspend   = csched_tick_suspend,
 .tick_resume= csched_tick_resume,
 };
+
+REGISTER_SCHEDULER(sched_credit_def);
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 3c49ffa..38b02d0 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2228,3 +2228,5 @@ const struct scheduler sched_credit2_def = {
 .alloc_domdata  = csched2_alloc_domdata,
 .free_domdata   = csched2_free_domdata,
 };
+
+REGISTER_SCHEDULER(sched_credit2_def);
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 3f1d047..7640cd0 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1199,3 +1199,5 @@ const struct scheduler sched_rtds_def = {
 .wake   = rt_vcpu_wake,
 .context_saved  = rt_context_saved,
 };
+
+REGISTER_SCHEDULER(sched_rtds_def);
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 493d43f..9c6e0f5 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -170,6 +170,8 @@ 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;
 
 struct cpupool
 {
-- 
2.6.4


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

Re: [Xen-devel] [PATCH v3 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Andrew Cooper writes:

> On 08/01/16 15:47, Konrad Rzeszutek Wilk wrote:
>> On Thu, Jan 07, 2016 at 11:29:18AM -0600, Jonathan Creekmore wrote:
>>> Allow the schedulers to be independently enabled or disabled at
>>> compile-time. To match existing behavior, all four schedulers are
>>> compiled in by default, although the Credit2, RTDS, and ARINC653 are
>>> marked EXPERIMENTAL to match their not currently supported status.
>> By unmarking all of them in sequence I was able to get this:
>>
>> #
>> # Schedulers
>> #
>> # CONFIG_SCHED_CREDIT is not set
>> # CONFIG_SCHED_CREDIT2 is not set
>> # CONFIG_SCHED_RTDS is not set
>> # CONFIG_SCHED_ARINC653 is not set
>> # CONFIG_SCHED_CREDIT_DEFAULT is not set
>> # CONFIG_SCHED_CREDIT2_DEFAULT is not set
>> # CONFIG_SCHED_RTDS_DEFAULT is not set
>> # CONFIG_SCHED_ARINC653_DEFAULT is not set
>> CONFIG_SCHED_DEFAULT="credit"
>>
>>
>> And the hypervisor did build with:
>>
>> [konrad@char xen]$ nm --defined xen-syms |grep schedulers
>> 82d080290d58 D __end_schedulers_array
>> 82d080290d58 D __start_schedulers_array
>>
>> :-)
>>
>> Not exactly sure if there is some way to make us _not_ shoot
>> ourselves in the foot by mistake.
>>
>> Perhaps the build should complain if the size of the
>> __schedulers_array is zero?
>
> Hmm yes - an ASSERT() at the bottom of the linker file would be a very
> good defensive measure.
>
> A hypervisor without any schedulers compiled in will be rather sad.

I can definitely add that in.

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

Re: [Xen-devel] [PATCH v3 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Juergen Gross writes:

> On 08/01/16 17:02, Jonathan Creekmore wrote:
>>
>> Andrew Cooper writes:
>>
>>> On 08/01/16 15:47, Konrad Rzeszutek Wilk wrote:
>>>> On Thu, Jan 07, 2016 at 11:29:18AM -0600, Jonathan Creekmore wrote:
>>>>> Allow the schedulers to be independently enabled or disabled at
>>>>> compile-time. To match existing behavior, all four schedulers are
>>>>> compiled in by default, although the Credit2, RTDS, and ARINC653 are
>>>>> marked EXPERIMENTAL to match their not currently supported status.
>>>> By unmarking all of them in sequence I was able to get this:
>>>>
>>>> #
>>>> # Schedulers
>>>> #
>>>> # CONFIG_SCHED_CREDIT is not set
>>>> # CONFIG_SCHED_CREDIT2 is not set
>>>> # CONFIG_SCHED_RTDS is not set
>>>> # CONFIG_SCHED_ARINC653 is not set
>>>> # CONFIG_SCHED_CREDIT_DEFAULT is not set
>>>> # CONFIG_SCHED_CREDIT2_DEFAULT is not set
>>>> # CONFIG_SCHED_RTDS_DEFAULT is not set
>>>> # CONFIG_SCHED_ARINC653_DEFAULT is not set
>>>> CONFIG_SCHED_DEFAULT="credit"
>>>>
>>>>
>>>> And the hypervisor did build with:
>>>>
>>>> [konrad@char xen]$ nm --defined xen-syms |grep schedulers
>>>> 82d080290d58 D __end_schedulers_array
>>>> 82d080290d58 D __start_schedulers_array
>>>>
>>>> :-)
>>>>
>>>> Not exactly sure if there is some way to make us _not_ shoot
>>>> ourselves in the foot by mistake.
>>>>
>>>> Perhaps the build should complain if the size of the
>>>> __schedulers_array is zero?
>>>
>>> Hmm yes - an ASSERT() at the bottom of the linker file would be a very
>>> good defensive measure.
>>>
>>> A hypervisor without any schedulers compiled in will be rather sad.
>>
>> I can definitely add that in.
>
> I think there should be an ASSERT (or some other measure) to
> ensure the default scheduler is available.

So, in a normal configuration case, that should not be able to
happen. The way the Kconfig is set up, the default will only give you an
option to choose schedulers that are enabled in the build.

1) "normal" (non-expert) configuration --- the user never sees that
schedulers are available for configuration, so all of them are compiled
in and the credit scheduler is chosen by the default.

2) "expert" configuration --- the user can select which schedulers are
available in the build and can choose which one is the default. If a
scheduler is not selected to be part of the build, it is not available
in the default selection dialog.

3) "pathological" configuration --- the user disabled all of the
schedulers, but due to a quirk with hiding the scheduler menu for the
non-expert case, it fell back to the "credit" scheduler for the
default.

So, based on the Kconfig setup and the linker ASSERT, there should be no
way to have a default scheduler that is not in the build. I wish Kconfig
allowed you to state that you must have at least one option selected or
up to all of the options selected (so I could require 1-4 schedulers
compiled in, but not 0). Unfortunately, the only way that seems allowed
is if you compile code in as modules, which Xen does not (with a choice
block, you can select multiple items as M, but only 1 as Y, but at least
1 must be present --- since we only support Y, the choice block wouldn't
work for the schedulers).

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

Re: [Xen-devel] [PATCH v3 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Doug Goldstein writes:

> On 1/8/16 10:49 AM, Jan Beulich wrote:
> On 08.01.16 at 17:30,  wrote:
>>> So, based on the Kconfig setup and the linker ASSERT, there should be no
>>> way to have a default scheduler that is not in the build. I wish Kconfig
>>> allowed you to state that you must have at least one option selected or
>>> up to all of the options selected (so I could require 1-4 schedulers
>>> compiled in, but not 0). Unfortunately, the only way that seems allowed
>>> is if you compile code in as modules, which Xen does not (with a choice
>>> block, you can select multiple items as M, but only 1 as Y, but at least
>>> 1 must be present --- since we only support Y, the choice block wouldn't
>>> work for the schedulers).
>>
>> Perhaps credit should just not be configurable then?
>>
>> Jan
>>
>
> How much effort are we willing to put in saving someone? We've already
> got these options completely hidden away. We've already given the user a
> warning that what they're doing isn't supported and they shouldn't be
> touching this. Then they have to disable the scheduler that they pick as
> the default. How many people are we expecting to really do this? I'm all
> for idiot proofing things but at some point we've got to say there's
> enough barriers to prevent this from happening. Otherwise we need to put
> a Xen developer in every Xen users home or office.

At this point, with the (not submitted yet) ASSERT in the linker file,
it is a compile error to not have any schedulers linked in and Kconfig
enforces that only schedulers compiled in can be a default.

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

[Xen-devel] [PATCH v4 1/5] build: Env var to enable expert config options

[Jonathan Creekmore]

Add an additional environment variable, defaulting to disabled,
that enables the CONFIG_EXPERT configuration option. The purpose
of the CONFIG_EXPERT configuration option is to make non-standard
Kconfig options visible during the configuration process. The
CONFIG_EXPERT option is not, itself, visible during the Kconfig
configuration process, so typical users will never see it nor
any of the non-standard configuration options.

CC: Ian Campbell 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 xen/Kconfig  | 4 
 xen/Makefile | 1 +
 2 files changed, 5 insertions(+)

diff --git a/xen/Kconfig b/xen/Kconfig
index ffe3f45..fa8b27c 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -22,3 +22,7 @@ config DEFCONFIG_LIST
string
option defconfig_list
default "$ARCH_DEFCONFIG"
+
+config EXPERT
+   string
+   option env="XEN_CONFIG_EXPERT"
diff --git a/xen/Makefile b/xen/Makefile
index 9023863..4950afb 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -11,6 +11,7 @@ export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] 
&& /bin/dnsdomainname) |
 export XEN_BUILD_DATE  ?= $(shell LC_ALL=C date)
 export XEN_BUILD_TIME  ?= $(shell LC_ALL=C date +%T)
 export XEN_BUILD_HOST  ?= $(shell hostname)
+export XEN_CONFIG_EXPERT ?= n
 
 export BASEDIR := $(CURDIR)
 export XEN_ROOT := $(BASEDIR)/..
-- 
2.6.4


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

[Xen-devel] [PATCH v4 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Allow the schedulers to be independently enabled or disabled at
compile-time. To match existing behavior, all four schedulers are
compiled in by default, although the Credit2, RTDS, and ARINC653 are
marked EXPERIMENTAL to match their not currently supported status.

CC: George Dunlap 
CC: Dario Faggioli 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 

---
Changed since v2:

  * Hid the scheduler menu behind the EXPERT option
  * Provide static inlines for credit functions that are assumed to be
always available
  * Provide an absolute default of the credit scheduler if the
scheduler menu is not visible

Changed since v1:

  * Marked credit2 as EXPERIMENTAL
  * Removed confusing language from the commit message
  * Alphabetize the schedulers
---
 xen/common/Kconfig  | 67 +
 xen/common/Makefile |  8 +++---
 xen/common/schedule.c   | 12 +++--
 xen/include/xen/sched.h |  5 
 4 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 046e257..db7411b 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -51,4 +51,71 @@ config KEXEC
 
  If unsure, say Y.
 
+# Enable schedulers
+menu "Schedulers"
+   visible if EXPERT = "y"
+
+config SCHED_CREDIT
+   bool "Credit scheduler support"
+   default y
+   ---help---
+ The traditional credit scheduler is a general purpose scheduler.
+
+ If unsure, say Y.
+
+config SCHED_CREDIT2
+   bool "Credit2 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The credit2 scheduler is a general purpose scheduler that is
+ optimized for lower latency and higher VM density.
+
+ If unsure, say Y.
+
+config SCHED_RTDS
+   bool "RTDS scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The RTDS scheduler is a soft and firm real-time scheduler for
+ multicore, targeted for embedded, automotive, graphics and gaming
+ in the cloud, and general low-latency workloads.
+
+ If unsure, say N.
+
+config SCHED_ARINC653
+   bool "ARINC653 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The ARINC653 scheduler is a hard real-time scheduler for single
+ cores, targeted for avionics, drones, and medical devices.
+
+ If unsure, say N.
+
+choice
+   prompt "Default Scheduler?"
+   default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
+   default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
+   default SCHED_RTDS_DEFAULT if SCHED_RTDS
+   default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
+
+   config SCHED_CREDIT_DEFAULT
+   bool "Credit Scheduler" if SCHED_CREDIT
+   config SCHED_CREDIT2_DEFAULT
+   bool "Credit2 Scheduler" if SCHED_CREDIT2
+   config SCHED_RTDS_DEFAULT
+   bool "RT Scheduler" if SCHED_RTDS
+   config SCHED_ARINC653_DEFAULT
+   bool "ARINC653 Scheduler" if SCHED_ARINC653
+endchoice
+
+config SCHED_DEFAULT
+   string
+   default "credit" if SCHED_CREDIT_DEFAULT
+   default "credit2" if SCHED_CREDIT2_DEFAULT
+   default "rtds" if SCHED_RTDS_DEFAULT
+   default "arinc653" if SCHED_ARINC653_DEFAULT
+   default "credit"
+
+endmenu
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 8ab15ba..29a5916 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -30,10 +30,10 @@ obj-y += rangeset.o
 obj-y += radix-tree.o
 obj-y += rbtree.o
 obj-y += rcupdate.o
-obj-y += sched_credit.o
-obj-y += sched_credit2.o
-obj-y += sched_arinc653.o
-obj-y += sched_rt.o
+obj-$(CONFIG_SCHED_ARINC653) += sched_arinc653.o
+obj-$(CONFIG_SCHED_CREDIT) += sched_credit.o
+obj-$(CONFIG_SCHED_CREDIT2) += sched_credit2.o
+obj-$(CONFIG_SCHED_RTDS) += sched_rt.o
 obj-y += schedule.o
 obj-y += shutdown.o
 obj-y += softirq.o
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index d121896..2f98a48 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -38,8 +38,8 @@
 #include 
 #include 
 
-/* opt_sched: scheduler - default to credit */
-static char __initdata opt_sched[10] = "credit";
+/* opt_sched: scheduler - default to configured value */
+static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
 string_param("sched", opt_sched);
 
 /* if sched_smt_power_savings is set,
@@ -65,10 +65,18 @@ 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
 &

[Xen-devel] [PATCH v4 4/5] sched: Register the schedulers into the list

[Jonathan Creekmore]

Adds a simple macro to place a pointer to a scheduler into an array
section at compile time. Also, goes ahead and generates the array
entries with each of the schedulers.

CC: George Dunlap 
CC: Dario Faggioli 
CC: Josh Whitehead 
CC: Robert VanVossen 
Signed-off-by: Jonathan Creekmore 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 xen/common/sched_arinc653.c | 2 ++
 xen/common/sched_credit.c   | 2 ++
 xen/common/sched_credit2.c  | 2 ++
 xen/common/sched_rt.c   | 2 ++
 xen/include/xen/sched-if.h  | 2 ++
 5 files changed, 10 insertions(+)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index dbe02ed..3b59514 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -767,6 +767,8 @@ const struct scheduler sched_arinc653_def = {
 .tick_resume= NULL,
 };
 
+REGISTER_SCHEDULER(sched_arinc653_def);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 0dce790..e586248 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -2027,3 +2027,5 @@ const struct scheduler sched_credit_def = {
 .tick_suspend   = csched_tick_suspend,
 .tick_resume= csched_tick_resume,
 };
+
+REGISTER_SCHEDULER(sched_credit_def);
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 3c49ffa..38b02d0 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2228,3 +2228,5 @@ const struct scheduler sched_credit2_def = {
 .alloc_domdata  = csched2_alloc_domdata,
 .free_domdata   = csched2_free_domdata,
 };
+
+REGISTER_SCHEDULER(sched_credit2_def);
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 3f1d047..7640cd0 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1199,3 +1199,5 @@ const struct scheduler sched_rtds_def = {
 .wake   = rt_vcpu_wake,
 .context_saved  = rt_context_saved,
 };
+
+REGISTER_SCHEDULER(sched_rtds_def);
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 493d43f..9c6e0f5 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -170,6 +170,8 @@ 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;
 
 struct cpupool
 {
-- 
2.6.4


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

[Xen-devel] [PATCH v4 0/5] Allow schedulers to be selectable through Kconfig

[Jonathan Creekmore]

Add machinery to allow the schedulers to be individually selectable
through the Kconfig interface. The first patch in the series sets up
the CONFIG_EXPERT Kconfig variable that is only enabled by passing an
environment variable to the build. The second patch in the series sets up
the Kconfig options for the schedulers and places the appropriate CONFIG_
options around the scheduler list. The third, fourth, and fifth patches
rework the scheduler list from being manually curated into a model
where just compiling any schedulers into the hypervisor causes the
scheduler list to be built up.

---
Changed since v3:
 * Add defensive check for schedulers in the linker

Changed since v2:
 * Added a predecessor patch that introduces a environment
   variable for the build to enable expert configuration options
   (1/5)
 * Hide the scheduler menu behind the expert option (2/5)
 * Provide static inlines for credit functions that are assumed to be
   present if it is compiled out (2/5)
 * Provide an absolute default of the credit scheduler if the 
   scheduler menu is not visible (2/5)

Changed since v1:
 * Marked credit2 as EXPERIMENTAL
 * Removed confusing language from the commit message
 * Alphabetize the schedulers
 * rename the __start and __end symbols to better match
   the rest of the file
 * Simplify the calculation of the number of schedulers
 * Make the scheduler ops structures static to their files

Jonathan Creekmore (5):
  build: Env var to enable expert config options
  build: Hook the schedulers into Kconfig
  build: Alloc space for sched list in the link file
  sched: Register the schedulers into the list
  sched: Use the auto-generated list of schedulers

 xen/Kconfig |  4 +++
 xen/Makefile|  1 +
 xen/arch/arm/xen.lds.S  |  5 
 xen/arch/x86/xen.lds.S  |  5 
 xen/common/Kconfig  | 67 +
 xen/common/Makefile |  8 +++---
 xen/common/sched_arinc653.c |  4 ++-
 xen/common/sched_credit.c   |  4 ++-
 xen/common/sched_credit2.c  |  4 ++-
 xen/common/sched_rt.c   |  4 ++-
 xen/common/schedule.c   | 20 ++
 xen/include/xen/sched-if.h  |  7 ++---
 xen/include/xen/sched.h |  5 
 13 files changed, 114 insertions(+), 24 deletions(-)

-- 
2.6.4


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

[Xen-devel] [PATCH v4 5/5] 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 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 

---
Changed since v1:
  * Simplify the calculation of the number of schedulers
  * Make the scheduler ops structures static to their files
---
 xen/common/sched_arinc653.c |  2 +-
 xen/common/sched_credit.c   |  2 +-
 xen/common/sched_credit2.c  |  2 +-
 xen/common/sched_rt.c   |  2 +-
 xen/common/schedule.c   | 24 +++-
 xen/include/xen/sched-if.h  |  5 -
 6 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 3b59514..0606988 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -724,7 +724,7 @@ a653sched_adjust_global(const struct scheduler *ops,
  * callback functions.
  * The symbol must be visible to the rest of Xen at link time.
  */
-const struct scheduler sched_arinc653_def = {
+static const struct scheduler sched_arinc653_def = {
 .name   = "ARINC 653 Scheduler",
 .opt_name   = "arinc653",
 .sched_id   = XEN_SCHEDULER_ARINC653,
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index e586248..028e41b 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1991,7 +1991,7 @@ static void csched_tick_resume(const struct scheduler 
*ops, unsigned int cpu)
 
 static struct csched_private _csched_priv;
 
-const struct scheduler sched_credit_def = {
+static const struct scheduler 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 38b02d0..78220a7 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2194,7 +2194,7 @@ csched2_deinit(const struct scheduler *ops)
 
 static struct csched2_private _csched2_priv;
 
-const struct scheduler sched_credit2_def = {
+static const struct scheduler sched_credit2_def = {
 .name   = "SMP Credit Scheduler rev2",
 .opt_name   = "credit2",
 .sched_id   = XEN_SCHEDULER_CREDIT2,
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 7640cd0..2e5430f 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1170,7 +1170,7 @@ rt_dom_cntl(
 
 static struct rt_private _rt_priv;
 
-const struct scheduler sched_rtds_def = {
+static const struct scheduler sched_rtds_def = {
 .name   = "SMP RTDS Scheduler",
 .opt_name   = "rtds",
 .sched_id   = XEN_SCHEDULER_RTDS,
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 2f98a48..91e53c1 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 *__start_schedulers_array[], 
*__end_schedulers_array[];
+extern const size_t schedulers_array_size;
+#define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
+static const struct scheduler **schedulers = __start_schedulers_array;
 
 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/sche

[Xen-devel] [PATCH v4 3/5] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

Creates a section to contain scheduler entry pointers that are gathered
together into an array. This will allow, in a follow-on patch, scheduler
entries to be automatically gathered together into the array for
automatic parsing.

CC: Ian Campbell 
CC: Stefano Stabellini 
CC: Keir Fraser 
CC: Jan Beulich 
CC: Andrew Cooper 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 

---
Changed since v3:
  * Add defensive check for schedulers in the linker

Changed since v1:
  * rename the __start and __end symbols to better match
the rest of the file
---
 xen/arch/arm/xen.lds.S | 5 +
 xen/arch/x86/xen.lds.S | 5 +
 2 files changed, 10 insertions(+)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 0488f37..2492def 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -57,6 +57,10 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
*(.data.page_aligned)
*(.data)
+   . = ALIGN(8);
+   __start_schedulers_array = .;
+   *(.data.schedulers)
+   __end_schedulers_array = .;
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
@@ -193,3 +197,4 @@ SECTIONS
  * code running on the boot time identity map cannot cross a section boundary.
  */
 ASSERT( _end_boot - start <= PAGE_SIZE, "Boot code is larger than 4K")
+ASSERT((__end_schedulers_array - __start_schedulers_array) > 0, "no schedulers 
compiled in")
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e18e08f..63f89af 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -80,6 +80,10 @@ SECTIONS
__stop___pre_ex_table = .;
 
*(.data.read_mostly)
+   . = ALIGN(8);
+   __start_schedulers_array = .;
+   *(.data.schedulers)
+   __end_schedulers_array = .;
*(.data.rel.ro)
*(.data.rel.ro.*)
   } :text
@@ -226,3 +230,4 @@ ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, 
"kexec_reloc is too large")
 #endif
 
 ASSERT((cpu0_stack & (STACK_SIZE - 1)) == 0, "cpu0_stack misaligned")
+ASSERT((__end_schedulers_array - __start_schedulers_array) > 0, "no schedulers 
compiled in")
-- 
2.6.4


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

Re: [Xen-devel] [PATCH v4 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Andrew Cooper writes:

> On 08/01/16 21:22, Jonathan Creekmore wrote:
>> +# Enable schedulers
>> +menu "Schedulers"
>> +visible if EXPERT = "y"
>> +
>> +config SCHED_CREDIT
>> +bool "Credit scheduler support"
>> +default y
>> +---help---
>> +  The traditional credit scheduler is a general purpose scheduler.
>> +
>> +  If unsure, say Y.
>> +
>> +config SCHED_CREDIT2
>> +bool "Credit2 scheduler support (EXPERIMENTAL)"
>> +default y
>> +---help---
>> +  The credit2 scheduler is a general purpose scheduler that is
>> +  optimized for lower latency and higher VM density.
>> +
>> +  If unsure, say Y.
>> +
>> +config SCHED_RTDS
>> +bool "RTDS scheduler support (EXPERIMENTAL)"
>> +default y
>> +---help---
>> +  The RTDS scheduler is a soft and firm real-time scheduler for
>> +  multicore, targeted for embedded, automotive, graphics and gaming
>> +  in the cloud, and general low-latency workloads.
>> +
>> +  If unsure, say N.
>> +
>> +config SCHED_ARINC653
>> +bool "ARINC653 scheduler support (EXPERIMENTAL)"
>> +default y
>> +---help---
>> +  The ARINC653 scheduler is a hard real-time scheduler for single
>> +  cores, targeted for avionics, drones, and medical devices.
>> +
>> +  If unsure, say N.
>
> Sorry for not noticing this before.  The "If unsure, say $X" should
> really match the default value.
>
> On the other hand, given that we are hiding all these options behind
> CONFIG_EXPERT, I am not sure that we need "If unsure" clauses.  Anyone
> who isn't sure shouldn't have turned on CONFIG_EXPERT to start with.

I was trying to mimic language that the Linux kernel would use for
EXPERIMENTAL marked items. Given the documentation on the wiki, I think
marking those three schedulers EXPERIMENTAL is correct. Given that, I
still think that the language saying "If unsure, say N" is correct (the
thought being, the only people who should be messing with the
EXPERIMENTAL schedulers are people developing or specifically testing
them). The *only* reason I marked them default of Y is to keep backwards
compatibility with the current build.

However, if you would prefer me to remove the "If unsure" language
completely, I can do that. The text came in before the whole
CONFIG_EXPERT flag did.

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

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

[Jonathan Creekmore]

Andrew Cooper writes:

> On 08/01/16 21:22, Jonathan Creekmore wrote:
>> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
>> index 2f98a48..91e53c1 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 *__start_schedulers_array[], 
>> *__end_schedulers_array[];
>> +extern const size_t schedulers_array_size;
>
> Is schedulers_array_size declared or used anywhere?  I can't see any use.

You are right. That is a holdover from a previous version. I will drop that.

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

Re: [Xen-devel] [PATCH v4 3/5] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

Andrew Cooper writes:

> On 08/01/16 21:22, Jonathan Creekmore wrote:
>> Creates a section to contain scheduler entry pointers that are gathered
>> together into an array. This will allow, in a follow-on patch, scheduler
>> entries to be automatically gathered together into the array for
>> automatic parsing.
>>
>> CC: Ian Campbell 
>> CC: Stefano Stabellini 
>> CC: Keir Fraser 
>> CC: Jan Beulich 
>> CC: Andrew Cooper 
>> Signed-off-by: Jonathan Creekmore 
>> Reviewed-by: Doug Goldstein 
>>
>> ---
>> Changed since v3:
>>   * Add defensive check for schedulers in the linker
>>
>> Changed since v1:
>>   * rename the __start and __end symbols to better match
>> the rest of the file
>> ---
>>  xen/arch/arm/xen.lds.S | 5 +
>>  xen/arch/x86/xen.lds.S | 5 +
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
>> index 0488f37..2492def 100644
>> --- a/xen/arch/arm/xen.lds.S
>> +++ b/xen/arch/arm/xen.lds.S
>> @@ -57,6 +57,10 @@ SECTIONS
>> . = ALIGN(PAGE_SIZE);
>> *(.data.page_aligned)
>> *(.data)
>> +   . = ALIGN(8);
>> +   __start_schedulers_array = .;
>> +   *(.data.schedulers)
>> +   __end_schedulers_array = .;
>> *(.data.rel)
>> *(.data.rel.*)
>> CONSTRUCTORS
>> @@ -193,3 +197,4 @@ SECTIONS
>>   * code running on the boot time identity map cannot cross a section 
>> boundary.
>>   */
>>  ASSERT( _end_boot - start <= PAGE_SIZE, "Boot code is larger than 4K")
>> +ASSERT((__end_schedulers_array - __start_schedulers_array) > 0, "no 
>> schedulers compiled in")
>> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
>> index e18e08f..63f89af 100644
>> --- a/xen/arch/x86/xen.lds.S
>> +++ b/xen/arch/x86/xen.lds.S
>> @@ -80,6 +80,10 @@ SECTIONS
>> __stop___pre_ex_table = .;
>>
>> *(.data.read_mostly)
>> +   . = ALIGN(8);
>> +   __start_schedulers_array = .;
>> +   *(.data.schedulers)
>> +   __end_schedulers_array = .;
>> *(.data.rel.ro)
>> *(.data.rel.ro.*)
>>} :text
>> @@ -226,3 +230,4 @@ ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, 
>> "kexec_reloc is too large")
>>  #endif
>>
>>  ASSERT((cpu0_stack & (STACK_SIZE - 1)) == 0, "cpu0_stack misaligned")
>> +ASSERT((__end_schedulers_array - __start_schedulers_array) > 0, "no 
>> schedulers compiled in")
>
> This patch won't build on its own, as the ASSERT() will fire.
> Therefore, it breaks bisectability.
>
> In this patch (or the previous one), you need to move the schedulers
> list into __section(".data.schedulers"), and undo the movement in patch
> 4 when populating .data.schedulers properly.
>
> Alternatively, you could just merge patches 3 and 4.  I don't think that
> would reduce the clarity of what you were doing.

Quite right. I think merging patches 3 and 4 is probably the most
straightforward way. That way, one patch introduces the new section and
actually populates that section.

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

Re: [Xen-devel] [PATCH v4 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Andrew Cooper writes:

> On 09/01/16 17:50, Jonathan Creekmore wrote:
>> Andrew Cooper writes:
>>
>>> On 08/01/16 21:22, Jonathan Creekmore wrote:
>>>> +# Enable schedulers
>>>> +menu "Schedulers"
>>>> +  visible if EXPERT = "y"
>>>> +
>>>> +config SCHED_CREDIT
>>>> +  bool "Credit scheduler support"
>>>> +  default y
>>>> +  ---help---
>>>> +The traditional credit scheduler is a general purpose scheduler.
>>>> +
>>>> +If unsure, say Y.
>>>> +
>>>> +config SCHED_CREDIT2
>>>> +  bool "Credit2 scheduler support (EXPERIMENTAL)"
>>>> +  default y
>>>> +  ---help---
>>>> +The credit2 scheduler is a general purpose scheduler that is
>>>> +optimized for lower latency and higher VM density.
>>>> +
>>>> +If unsure, say Y.
>>>> +
>>>> +config SCHED_RTDS
>>>> +  bool "RTDS scheduler support (EXPERIMENTAL)"
>>>> +  default y
>>>> +  ---help---
>>>> +The RTDS scheduler is a soft and firm real-time scheduler for
>>>> +multicore, targeted for embedded, automotive, graphics and gaming
>>>> +in the cloud, and general low-latency workloads.
>>>> +
>>>> +If unsure, say N.
>>>> +
>>>> +config SCHED_ARINC653
>>>> +  bool "ARINC653 scheduler support (EXPERIMENTAL)"
>>>> +  default y
>>>> +  ---help---
>>>> +The ARINC653 scheduler is a hard real-time scheduler for single
>>>> +cores, targeted for avionics, drones, and medical devices.
>>>> +
>>>> +If unsure, say N.
>>> Sorry for not noticing this before.  The "If unsure, say $X" should
>>> really match the default value.
>>>
>>> On the other hand, given that we are hiding all these options behind
>>> CONFIG_EXPERT, I am not sure that we need "If unsure" clauses.  Anyone
>>> who isn't sure shouldn't have turned on CONFIG_EXPERT to start with.
>> I was trying to mimic language that the Linux kernel would use for
>> EXPERIMENTAL marked items. Given the documentation on the wiki, I think
>> marking those three schedulers EXPERIMENTAL is correct.
>
> I concur about their status.
>
>>  Given that, I
>> still think that the language saying "If unsure, say N" is correct (the
>> thought being, the only people who should be messing with the
>> EXPERIMENTAL schedulers are people developing or specifically testing
>> them). The *only* reason I marked them default of Y is to keep backwards
>> compatibility with the current build.
>
> Also very important.
>
>>
>> However, if you would prefer me to remove the "If unsure" language
>> completely, I can do that. The text came in before the whole
>> CONFIG_EXPERT flag did.
>
> I would suggest dropping it (although you probably want to wait for
> opinions from others).  We have already diverged from Linux with regards
> to the EXPERT flag; people who are unsure cannot accidentally get here.

OK, if I haven't heard anything from any others by the time I send out
my v5 on Monday with the other items you caught, then I will go ahead
and remove the language.

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

Re: [Xen-devel] [PATCH v4 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Jan Beulich writes:

 On 08.01.16 at 22:22,  wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -51,4 +51,71 @@ config KEXEC
>>
>>If unsure, say Y.
>>
>> +# Enable schedulers
>> +menu "Schedulers"
>> +visible if EXPERT = "y"
>
> Does "visible if EXPERT" not suffice here?

No, because EXPERT is a string and not a boolean. It has to be a string
since it is pulling in a string from the environment to set its value.

>
>> +config SCHED_CREDIT
>> +bool "Credit scheduler support"
>> +default y
>
> I continue to think that not making the primary scheduler configurable
> would be the better solution to the problems resulting from possibly
> all of them getting turned off.

Except that is completely contrary to my goal with this patchset (being
able to compile in just the scheduler that I want to use). Yes, at the
moment, credit is the only non-experimental scheduler and will likely be
the one we choose. However, in the future, when credit2 and possibly
others are non-experimental, we may choose one of the other schedulers
and do not want to carry along credit in our build just because it is
the primary scheduler.

>
>> +config SCHED_CREDIT2
>> +bool "Credit2 scheduler support (EXPERIMENTAL)"
>> +default y
>> +---help---
>> +  The credit2 scheduler is a general purpose scheduler that is
>> +  optimized for lower latency and higher VM density.
>> +
>> +  If unsure, say Y.
>> +
>> +config SCHED_RTDS
>> +bool "RTDS scheduler support (EXPERIMENTAL)"
>> +default y
>> +---help---
>> +  The RTDS scheduler is a soft and firm real-time scheduler for
>> +  multicore, targeted for embedded, automotive, graphics and gaming
>> +  in the cloud, and general low-latency workloads.
>> +
>> +  If unsure, say N.
>> +
>> +config SCHED_ARINC653
>> +bool "ARINC653 scheduler support (EXPERIMENTAL)"
>> +default y
>> +---help---
>> +  The ARINC653 scheduler is a hard real-time scheduler for single
>> +  cores, targeted for avionics, drones, and medical devices.
>> +
>> +  If unsure, say N.
>> +
>> +choice
>> +prompt "Default Scheduler?"
>> +default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
>> +default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
>> +default SCHED_RTDS_DEFAULT if SCHED_RTDS
>> +default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
>> +
>> +config SCHED_CREDIT_DEFAULT
>> +bool "Credit Scheduler" if SCHED_CREDIT
>> +config SCHED_CREDIT2_DEFAULT
>> +bool "Credit2 Scheduler" if SCHED_CREDIT2
>> +config SCHED_RTDS_DEFAULT
>> +bool "RT Scheduler" if SCHED_RTDS
>> +config SCHED_ARINC653_DEFAULT
>> +bool "ARINC653 Scheduler" if SCHED_ARINC653
>> +endchoice
>> +
>> +config SCHED_DEFAULT
>> +string
>> +default "credit" if SCHED_CREDIT_DEFAULT
>> +default "credit2" if SCHED_CREDIT2_DEFAULT
>> +default "rtds" if SCHED_RTDS_DEFAULT
>> +default "arinc653" if SCHED_ARINC653_DEFAULT
>> +default "credit"
>
> What use is this last line?
>

When the scheduler menu is not visible, the choice selection does not
cause a scheduler to be chosen. The last line forces credit to be the
default if none of the _DEFAULT values are selected. It is purely an
artifact of introducing the visibility option on the menu. It could be
moved into the defaults section for the choice like this:

+   default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
+   default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
+   default SCHED_RTDS_DEFAULT if SCHED_RTDS
+   default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
+   default SCHED_CREDIT_DEFAULT


>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -850,8 +850,13 @@ static inline bool_t is_vcpu_online(const struct vcpu 
>> *v)
>>  return !test_bit(_VPF_down, &v->pause_flags);
>>  }
>>
>> +#ifdef CONFIG_SCHED_CREDIT
>>  void set_vcpu_migration_delay(unsigned int delay);
>>  unsigned int get_vcpu_migration_delay(void);
>> +#else
>> +static inline void set_vcpu_migration_delay(unsigned int delay) { }
>> +static inline unsigned int get_vcpu_migration_delay(void) { return 0; }
>> +#endif
>
> I don't think these are appropriate: The respective sysctl sub-ops
> would probably better indicate failure to the caller.

I can make that change if you want me to. As it stands now, the existing
sysctl sub-ops are probably not doing the right thing since they are
setting and getting this migration delay in the credit scheduler
regardless of which scheduler is actually in use.

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

[Xen-devel] [PATCH v5 1/5] build: Env var to enable expert config options

[Jonathan Creekmore]

Add an additional environment variable, defaulting to disabled,
that enables the CONFIG_EXPERT configuration option. The purpose
of the CONFIG_EXPERT configuration option is to make non-standard
Kconfig options visible during the configuration process. The
CONFIG_EXPERT option is not, itself, visible during the Kconfig
configuration process, so typical users will never see it nor
any of the non-standard configuration options.

CC: Ian Campbell 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 xen/Kconfig  | 4 
 xen/Makefile | 1 +
 2 files changed, 5 insertions(+)

diff --git a/xen/Kconfig b/xen/Kconfig
index ffe3f45..fa8b27c 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -22,3 +22,7 @@ config DEFCONFIG_LIST
string
option defconfig_list
default "$ARCH_DEFCONFIG"
+
+config EXPERT
+   string
+   option env="XEN_CONFIG_EXPERT"
diff --git a/xen/Makefile b/xen/Makefile
index 9023863..4950afb 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -11,6 +11,7 @@ export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] 
&& /bin/dnsdomainname) |
 export XEN_BUILD_DATE  ?= $(shell LC_ALL=C date)
 export XEN_BUILD_TIME  ?= $(shell LC_ALL=C date +%T)
 export XEN_BUILD_HOST  ?= $(shell hostname)
+export XEN_CONFIG_EXPERT ?= n
 
 export BASEDIR := $(CURDIR)
 export XEN_ROOT := $(BASEDIR)/..
-- 
2.6.4


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

[Xen-devel] [PATCH v5 3/5] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

Creates a section to contain scheduler entry pointers that are gathered
together into an array. This will allow, in a follow-on patch, scheduler
entries to be automatically gathered together into the array for
automatic parsing.

CC: Ian Campbell 
CC: Stefano Stabellini 
CC: Keir Fraser 
CC: Jan Beulich 
CC: Andrew Cooper 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 
Reviewed-by: Konrad Rzeszutek Wilk 

---
Changed since v4:
  * Remove defensive check for schedulers since the credit scheduler
must always be present

Changed since v3:
  * Add defensive check for schedulers in the linker

Changed since v1:
  * rename the __start and __end symbols to better match
the rest of the file
---
 xen/arch/arm/xen.lds.S | 4 
 xen/arch/x86/xen.lds.S | 4 
 2 files changed, 8 insertions(+)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 0488f37..f501a2f 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -57,6 +57,10 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
*(.data.page_aligned)
*(.data)
+   . = ALIGN(8);
+   __start_schedulers_array = .;
+   *(.data.schedulers)
+   __end_schedulers_array = .;
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e18e08f..c1ce027 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -80,6 +80,10 @@ SECTIONS
__stop___pre_ex_table = .;
 
*(.data.read_mostly)
+   . = ALIGN(8);
+   __start_schedulers_array = .;
+   *(.data.schedulers)
+   __end_schedulers_array = .;
*(.data.rel.ro)
*(.data.rel.ro.*)
   } :text
-- 
2.6.4


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

[Xen-devel] [PATCH v5 5/5] 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 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 

---
Changed since v4:
  * Remove extra size field that was accidentally left in

Changed since v1:
  * Simplify the calculation of the number of schedulers
  * Make the scheduler ops structures static to their files
---
 xen/common/sched_arinc653.c |  2 +-
 xen/common/sched_credit.c   |  2 +-
 xen/common/sched_credit2.c  |  2 +-
 xen/common/sched_rt.c   |  2 +-
 xen/common/schedule.c   | 23 ++-
 xen/include/xen/sched-if.h  |  5 -
 6 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 3b59514..0606988 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -724,7 +724,7 @@ a653sched_adjust_global(const struct scheduler *ops,
  * callback functions.
  * The symbol must be visible to the rest of Xen at link time.
  */
-const struct scheduler sched_arinc653_def = {
+static const struct scheduler sched_arinc653_def = {
 .name   = "ARINC 653 Scheduler",
 .opt_name   = "arinc653",
 .sched_id   = XEN_SCHEDULER_ARINC653,
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index e586248..028e41b 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1991,7 +1991,7 @@ static void csched_tick_resume(const struct scheduler 
*ops, unsigned int cpu)
 
 static struct csched_private _csched_priv;
 
-const struct scheduler sched_credit_def = {
+static const struct scheduler 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 38b02d0..78220a7 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2194,7 +2194,7 @@ csched2_deinit(const struct scheduler *ops)
 
 static struct csched2_private _csched2_priv;
 
-const struct scheduler sched_credit2_def = {
+static const struct scheduler sched_credit2_def = {
 .name   = "SMP Credit Scheduler rev2",
 .opt_name   = "credit2",
 .sched_id   = XEN_SCHEDULER_CREDIT2,
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 7640cd0..2e5430f 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1170,7 +1170,7 @@ rt_dom_cntl(
 
 static struct rt_private _rt_priv;
 
-const struct scheduler sched_rtds_def = {
+static const struct scheduler sched_rtds_def = {
 .name   = "SMP RTDS Scheduler",
 .opt_name   = "rtds",
 .sched_id   = XEN_SCHEDULER_RTDS,
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 2f98a48..0232179 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -64,20 +64,9 @@ 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 *__start_schedulers_array[], 
*__end_schedulers_array[];
+#define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
+static const struct scheduler **schedulers = __start_schedulers_array;
 
 static struct scheduler __read_mostly ops;
 
@@ -1468,7 +1457,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 +1468,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 +1588,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 

[Xen-devel] [PATCH v5 0/5] Allow schedulers to be selectable through Kconfig

[Jonathan Creekmore]

Add machinery to allow the schedulers to be individually selectable
through the Kconfig interface. The first patch in the series sets up
the CONFIG_EXPERT Kconfig variable that is only enabled by passing an
environment variable to the build. The second patch in the series sets up
the Kconfig options for the schedulers and places the appropriate CONFIG_
options around the scheduler list. The third, fourth, and fifth patches
rework the scheduler list from being manually curated into a model
where just compiling any schedulers into the hypervisor causes the
scheduler list to be built up.

---
Changed since v4:
 * Removed the ability to configure the credit scheduler. It is always
   compiled in, so there is no need for the defensive check for schedulers
   in the linker (added in v4) and no reason to provide the static inlines
   (added in v3) since credit is always present. 
 * Remove extra size field that was accidentally left in.

Changed since v3:
 * Add defensive check for schedulers in the linker

Changed since v2:
 * Added a predecessor patch that introduces a environment
   variable for the build to enable expert configuration options
   (1/5)
 * Hide the scheduler menu behind the expert option (2/5)
 * Provide static inlines for credit functions that are assumed to be
   present if it is compiled out (2/5)
 * Provide an absolute default of the credit scheduler if the 
   scheduler menu is not visible (2/5)

Changed since v1:
 * Marked credit2 as EXPERIMENTAL
 * Removed confusing language from the commit message
 * Alphabetize the schedulers
 * rename the __start and __end symbols to better match
   the rest of the file
 * Simplify the calculation of the number of schedulers
 * Make the scheduler ops structures static to their files


Jonathan Creekmore (5):
  build: Env var to enable expert config options
  build: Hook the schedulers into Kconfig
  build: Alloc space for sched list in the link file
  sched: Register the schedulers into the list
  sched: Use the auto-generated list of schedulers

 xen/Kconfig |  4 +++
 xen/Makefile|  1 +
 xen/arch/arm/xen.lds.S  |  4 +++
 xen/arch/x86/xen.lds.S  |  4 +++
 xen/common/Kconfig  | 59 +
 xen/common/Makefile |  8 +++---
 xen/common/sched_arinc653.c |  4 ++-
 xen/common/sched_credit.c   |  4 ++-
 xen/common/sched_credit2.c  |  4 ++-
 xen/common/sched_rt.c   |  4 ++-
 xen/common/schedule.c   | 19 ++-
 xen/include/xen/sched-if.h  |  7 ++
 12 files changed, 98 insertions(+), 24 deletions(-)

-- 
2.6.4


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

[Xen-devel] [PATCH v5 4/5] sched: Register the schedulers into the list

[Jonathan Creekmore]

Adds a simple macro to place a pointer to a scheduler into an array
section at compile time. Also, goes ahead and generates the array
entries with each of the schedulers.

CC: George Dunlap 
CC: Dario Faggioli 
CC: Josh Whitehead 
CC: Robert VanVossen 
Signed-off-by: Jonathan Creekmore 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 xen/common/sched_arinc653.c | 2 ++
 xen/common/sched_credit.c   | 2 ++
 xen/common/sched_credit2.c  | 2 ++
 xen/common/sched_rt.c   | 2 ++
 xen/include/xen/sched-if.h  | 2 ++
 5 files changed, 10 insertions(+)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index dbe02ed..3b59514 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -767,6 +767,8 @@ const struct scheduler sched_arinc653_def = {
 .tick_resume= NULL,
 };
 
+REGISTER_SCHEDULER(sched_arinc653_def);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 0dce790..e586248 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -2027,3 +2027,5 @@ const struct scheduler sched_credit_def = {
 .tick_suspend   = csched_tick_suspend,
 .tick_resume= csched_tick_resume,
 };
+
+REGISTER_SCHEDULER(sched_credit_def);
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 3c49ffa..38b02d0 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2228,3 +2228,5 @@ const struct scheduler sched_credit2_def = {
 .alloc_domdata  = csched2_alloc_domdata,
 .free_domdata   = csched2_free_domdata,
 };
+
+REGISTER_SCHEDULER(sched_credit2_def);
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 3f1d047..7640cd0 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1199,3 +1199,5 @@ const struct scheduler sched_rtds_def = {
 .wake   = rt_vcpu_wake,
 .context_saved  = rt_context_saved,
 };
+
+REGISTER_SCHEDULER(sched_rtds_def);
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 493d43f..9c6e0f5 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -170,6 +170,8 @@ 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;
 
 struct cpupool
 {
-- 
2.6.4


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

[Xen-devel] [PATCH v5 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Allow the schedulers to be independently enabled or disabled (except credit) 
at compile-time. To match existing behavior, all four schedulers are
compiled in by default, although the Credit2, RTDS, and ARINC653 are
marked EXPERIMENTAL to match their not currently supported status.

CC: George Dunlap 
CC: Dario Faggioli 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 

---
Changed since v4:

  * Removed the "if unsure" language
  * Removed ability to disable the "credit" scheduler
  * Remove stub vcpu_migration_delay functions since credit cannot be disabled

Changed since v2:

  * Hid the scheduler menu behind the EXPERT option
  * Provide static inlines for credit functions that are assumed to be
always available
  * Provide an absolute default of the credit scheduler if the
scheduler menu is not visible

Changed since v1:

  * Marked credit2 as EXPERIMENTAL
  * Removed confusing language from the commit message
  * Alphabetize the schedulers
---
 xen/common/Kconfig| 59 +++
 xen/common/Makefile   |  8 +++
 xen/common/schedule.c | 12 +--
 3 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 046e257..286792a 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -51,4 +51,63 @@ config KEXEC
 
  If unsure, say Y.
 
+# Enable schedulers
+menu "Schedulers"
+   visible if EXPERT = "y"
+
+config SCHED_CREDIT
+   bool
+   default y
+   ---help---
+ The traditional credit scheduler is a general purpose scheduler.
+
+config SCHED_CREDIT2
+   bool "Credit2 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The credit2 scheduler is a general purpose scheduler that is
+ optimized for lower latency and higher VM density.
+
+config SCHED_RTDS
+   bool "RTDS scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The RTDS scheduler is a soft and firm real-time scheduler for
+ multicore, targeted for embedded, automotive, graphics and gaming
+ in the cloud, and general low-latency workloads.
+
+config SCHED_ARINC653
+   bool "ARINC653 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The ARINC653 scheduler is a hard real-time scheduler for single
+ cores, targeted for avionics, drones, and medical devices.
+
+choice
+   prompt "Default Scheduler?"
+   default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
+   default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
+   default SCHED_RTDS_DEFAULT if SCHED_RTDS
+   default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
+
+   config SCHED_CREDIT_DEFAULT
+   bool "Credit Scheduler" if SCHED_CREDIT
+   config SCHED_CREDIT2_DEFAULT
+   bool "Credit2 Scheduler" if SCHED_CREDIT2
+   config SCHED_RTDS_DEFAULT
+   bool "RT Scheduler" if SCHED_RTDS
+   config SCHED_ARINC653_DEFAULT
+   bool "ARINC653 Scheduler" if SCHED_ARINC653
+endchoice
+
+config SCHED_DEFAULT
+   string
+   default "credit" if SCHED_CREDIT_DEFAULT
+   default "credit2" if SCHED_CREDIT2_DEFAULT
+   default "rtds" if SCHED_RTDS_DEFAULT
+   default "arinc653" if SCHED_ARINC653_DEFAULT
+   default "credit"
+
+endmenu
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 8ab15ba..29a5916 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -30,10 +30,10 @@ obj-y += rangeset.o
 obj-y += radix-tree.o
 obj-y += rbtree.o
 obj-y += rcupdate.o
-obj-y += sched_credit.o
-obj-y += sched_credit2.o
-obj-y += sched_arinc653.o
-obj-y += sched_rt.o
+obj-$(CONFIG_SCHED_ARINC653) += sched_arinc653.o
+obj-$(CONFIG_SCHED_CREDIT) += sched_credit.o
+obj-$(CONFIG_SCHED_CREDIT2) += sched_credit2.o
+obj-$(CONFIG_SCHED_RTDS) += sched_rt.o
 obj-y += schedule.o
 obj-y += shutdown.o
 obj-y += softirq.o
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index d121896..2f98a48 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -38,8 +38,8 @@
 #include 
 #include 
 
-/* opt_sched: scheduler - default to credit */
-static char __initdata opt_sched[10] = "credit";
+/* opt_sched: scheduler - default to configured value */
+static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
 string_param("sched", opt_sched);
 
 /* if sched_smt_power_savings is set,
@@ -65,10 +65,18 @@ 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,
+#en

Re: [Xen-devel] [PATCH v5 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Jan Beulich writes:

 On 14.01.16 at 15:49,  wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -51,4 +51,63 @@ config KEXEC
>>
>>If unsure, say Y.
>>
>> +# Enable schedulers
>> +menu "Schedulers"
>> +visible if EXPERT = "y"
>> +
>> +config SCHED_CREDIT
>> +bool
>> +default y
>> +---help---
>> +  The traditional credit scheduler is a general purpose scheduler.
>
> So is this option now useful for anything?

It keeps the code between all of the schedulers consistent (all of them
have a #define if they are compiled it) and helps keep my downstream
patch smaller.

>
>> +choice
>> +prompt "Default Scheduler?"
>> +default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
>> +default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
>> +default SCHED_RTDS_DEFAULT if SCHED_RTDS
>> +default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
>
> And certainly all these defaults are now pointless, considering
> that the condition of the first one is "if y".

Yes, I could rip all of those out now since credit is always the
default. I left it in there for the ideal case that credit didn't have
to be special cased but, at this point, I will rip it out if you want.

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

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

[Jonathan Creekmore]

Jan Beulich writes:

 On 14.01.16 at 15:49,  wrote:
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -64,20 +64,9 @@ 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 *__start_schedulers_array[],
>> *__end_schedulers_array[];
>> +#define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
>> +static const struct scheduler **schedulers = __start_schedulers_array;
>
> #define or properly constified (to help the compiler recognize it
> won't ever change).

I will make it a define in the next version.

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

Re: [Xen-devel] [PATCH v5 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Jan Beulich writes:

>>>> On 14.01.16 at 17:34,  wrote:
>> On Thu, 2016-01-14 at 10:23 -0600, Jonathan Creekmore wrote:
>>> Jan Beulich writes:
>>>
>>> > > > > On 14.01.16 at 15:49,  wrote:
>>> > > --- a/xen/common/Kconfig
>>> > > +++ b/xen/common/Kconfig
>>> > > @@ -51,4 +51,63 @@ config KEXEC
>>> > >
>>> > >   If unsure, say Y.
>>> > >
>>> > > +# Enable schedulers
>>> > > +menu "Schedulers"
>>> > > +   visible if EXPERT = "y"
>>> > > +
>>> > > +config SCHED_CREDIT
>>> > > +   bool
>>> > > +   default y
>>> > > +   ---help---
>>> > > + The traditional credit scheduler is a general purpose
>>> > > scheduler.
>>> >
>>> > So is this option now useful for anything?
>>>
>>> It keeps the code between all of the schedulers consistent (all of them
>>> have a #define if they are compiled it)
>>
>> FWIW I think this (consistency) is a reasonable argument for having this
>> option even if it doesn't actually do anything.
>
> While I can see your point, I dislike useless clutter in .config (also
> on Linux, where I every once in a while send some cleanup
> patches).
>
>>> > > +choice
>>> > > +   prompt "Default Scheduler?"
>>> > > +   default SCHED_CREDIT_DEFAULT if SCHED_CREDIT
>>> > > +   default SCHED_CREDIT2_DEFAULT if SCHED_CREDIT2
>>> > > +   default SCHED_RTDS_DEFAULT if SCHED_RTDS
>>> > > +   default SCHED_ARINC653_DEFAULT if SCHED_ARINC653
>>> >
>>> > And certainly all these defaults are now pointless, considering
>>> > that the condition of the first one is "if y".
>>>
>>> Yes, I could rip all of those out now since credit is always the
>>> default. I left it in there for the ideal case that credit didn't have
>>> to be special cased but, at this point, I will rip it out if you want.
>>
>> What is the behaviour of the above set of "default"s if more than one of
>> the SCHED_* is enabled? Does it pick the first, last, one at random?
>
> The first for which the condition is true.

Absolutely correct. The first true condition is the one that is chosen.

>> If credit is now always the default I think that would be better expressed
>> with a single "default SCHED_CREDIT_DEFAULT".
>
> Indeed.

And that is what I have already done for a v6 that I am sitting on to
see if I get more review comments.

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

[Xen-devel] [PATCH v6 2/5] build: Hook the schedulers into Kconfig

[Jonathan Creekmore]

Allow the schedulers to be independently enabled or disabled at
compile-time. To match existing behavior, all four schedulers are
compiled in by default, although the Credit2, RTDS, and ARINC653 are
marked EXPERIMENTAL to match their not currently supported status.

CC: George Dunlap 
CC: Dario Faggioli 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 

---
Changed since v5:

  * Remove extra defaults for schedulers since credit is always there

Changed since v4:

  * Removed the "if unsure" language
  * Removed ability to disable the "credit" scheduler
  * Remove stub vcpu_migration_delay functions since credit cannot be disabled

Changed since v2:

  * Hid the scheduler menu behind the EXPERT option
  * Provide static inlines for credit functions that are assumed to be
always available
  * Provide an absolute default of the credit scheduler if the
scheduler menu is not visible

Changed since v1:

  * Marked credit2 as EXPERIMENTAL
  * Removed confusing language from the commit message
  * Alphabetize the schedulers
---
 xen/common/Kconfig| 56 +++
 xen/common/Makefile   |  8 
 xen/common/schedule.c | 12 +--
 3 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index eadfc3b..7cc99c7 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -97,4 +97,60 @@ config XSM
 
  If unsure, say N.
 
+# Enable schedulers
+menu "Schedulers"
+   visible if EXPERT = "y"
+
+config SCHED_CREDIT
+   bool
+   default y
+   ---help---
+ The traditional credit scheduler is a general purpose scheduler.
+
+config SCHED_CREDIT2
+   bool "Credit2 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The credit2 scheduler is a general purpose scheduler that is
+ optimized for lower latency and higher VM density.
+
+config SCHED_RTDS
+   bool "RTDS scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The RTDS scheduler is a soft and firm real-time scheduler for
+ multicore, targeted for embedded, automotive, graphics and gaming
+ in the cloud, and general low-latency workloads.
+
+config SCHED_ARINC653
+   bool "ARINC653 scheduler support (EXPERIMENTAL)"
+   default y
+   ---help---
+ The ARINC653 scheduler is a hard real-time scheduler for single
+ cores, targeted for avionics, drones, and medical devices.
+
+choice
+   prompt "Default Scheduler?"
+   default SCHED_CREDIT_DEFAULT 
+
+   config SCHED_CREDIT_DEFAULT
+   bool "Credit Scheduler" if SCHED_CREDIT
+   config SCHED_CREDIT2_DEFAULT
+   bool "Credit2 Scheduler" if SCHED_CREDIT2
+   config SCHED_RTDS_DEFAULT
+   bool "RT Scheduler" if SCHED_RTDS
+   config SCHED_ARINC653_DEFAULT
+   bool "ARINC653 Scheduler" if SCHED_ARINC653
+endchoice
+
+config SCHED_DEFAULT
+   string
+   default "credit" if SCHED_CREDIT_DEFAULT
+   default "credit2" if SCHED_CREDIT2_DEFAULT
+   default "rtds" if SCHED_RTDS_DEFAULT
+   default "arinc653" if SCHED_ARINC653_DEFAULT
+   default "credit"
+
+endmenu
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 9f8b214..4df71ee 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -30,10 +30,10 @@ obj-y += rangeset.o
 obj-y += radix-tree.o
 obj-y += rbtree.o
 obj-y += rcupdate.o
-obj-y += sched_credit.o
-obj-y += sched_credit2.o
-obj-y += sched_arinc653.o
-obj-y += sched_rt.o
+obj-$(CONFIG_SCHED_ARINC653) += sched_arinc653.o
+obj-$(CONFIG_SCHED_CREDIT) += sched_credit.o
+obj-$(CONFIG_SCHED_CREDIT2) += sched_credit2.o
+obj-$(CONFIG_SCHED_RTDS) += sched_rt.o
 obj-y += schedule.o
 obj-y += shutdown.o
 obj-y += softirq.o
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index d121896..2f98a48 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -38,8 +38,8 @@
 #include 
 #include 
 
-/* opt_sched: scheduler - default to credit */
-static char __initdata opt_sched[10] = "credit";
+/* opt_sched: scheduler - default to configured value */
+static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
 string_param("sched", opt_sched);
 
 /* if sched_smt_power_savings is set,
@@ -65,10 +65,18 @@ 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
 };
 
 static struct scheduler __read_mostly ops;
-- 
2.6.4


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

[Xen-devel] [PATCH v6 3/5] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

Creates a section to contain scheduler entry pointers that are gathered
together into an array. This will allow, in a follow-on patch, scheduler
entries to be automatically gathered together into the array for
automatic parsing.

CC: Ian Campbell 
CC: Stefano Stabellini 
CC: Keir Fraser 
CC: Jan Beulich 
CC: Andrew Cooper 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 
Reviewed-by: Konrad Rzeszutek Wilk 

---
Changed since v4:
  * Remove defensive check for schedulers since the credit scheduler
must always be present

Changed since v3:
  * Add defensive check for schedulers in the linker

Changed since v1:
  * rename the __start and __end symbols to better match
the rest of the file
---
 xen/arch/arm/xen.lds.S | 4 
 xen/arch/x86/xen.lds.S | 4 
 2 files changed, 8 insertions(+)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 0488f37..f501a2f 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -57,6 +57,10 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
*(.data.page_aligned)
*(.data)
+   . = ALIGN(8);
+   __start_schedulers_array = .;
+   *(.data.schedulers)
+   __end_schedulers_array = .;
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e18e08f..c1ce027 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -80,6 +80,10 @@ SECTIONS
__stop___pre_ex_table = .;
 
*(.data.read_mostly)
+   . = ALIGN(8);
+   __start_schedulers_array = .;
+   *(.data.schedulers)
+   __end_schedulers_array = .;
*(.data.rel.ro)
*(.data.rel.ro.*)
   } :text
-- 
2.6.4


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

[Xen-devel] [PATCH v6 5/5] 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 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 

---
Changed since v6:
  * Make schedulers a #define instead of a static const **

Changed since v5:
  * Remove extra size field that was accidentally left in

Changed since v1:
  * Simplify the calculation of the number of schedulers
  * Make the scheduler ops structures static to their files
---
 xen/common/sched_arinc653.c |  2 +-
 xen/common/sched_credit.c   |  2 +-
 xen/common/sched_credit2.c  |  2 +-
 xen/common/sched_rt.c   |  2 +-
 xen/common/schedule.c   | 23 ++-
 xen/include/xen/sched-if.h  |  5 -
 6 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 3b59514..0606988 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -724,7 +724,7 @@ a653sched_adjust_global(const struct scheduler *ops,
  * callback functions.
  * The symbol must be visible to the rest of Xen at link time.
  */
-const struct scheduler sched_arinc653_def = {
+static const struct scheduler sched_arinc653_def = {
 .name   = "ARINC 653 Scheduler",
 .opt_name   = "arinc653",
 .sched_id   = XEN_SCHEDULER_ARINC653,
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 1645f9c..03fb2c2 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1991,7 +1991,7 @@ static void csched_tick_resume(const struct scheduler 
*ops, unsigned int cpu)
 
 static struct csched_private _csched_priv;
 
-const struct scheduler sched_credit_def = {
+static const struct scheduler 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 38b02d0..78220a7 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2194,7 +2194,7 @@ csched2_deinit(const struct scheduler *ops)
 
 static struct csched2_private _csched2_priv;
 
-const struct scheduler sched_credit2_def = {
+static const struct scheduler sched_credit2_def = {
 .name   = "SMP Credit Scheduler rev2",
 .opt_name   = "credit2",
 .sched_id   = XEN_SCHEDULER_CREDIT2,
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 7640cd0..2e5430f 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1170,7 +1170,7 @@ rt_dom_cntl(
 
 static struct rt_private _rt_priv;
 
-const struct scheduler sched_rtds_def = {
+static const struct scheduler sched_rtds_def = {
 .name   = "SMP RTDS Scheduler",
 .opt_name   = "rtds",
 .sched_id   = XEN_SCHEDULER_RTDS,
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 2f98a48..7306d71 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -64,20 +64,9 @@ 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 *__start_schedulers_array[], 
*__end_schedulers_array[];
+#define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
+#define schedulers __start_schedulers_array
 
 static struct scheduler __read_mostly ops;
 
@@ -1468,7 +1457,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 +1468,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 +1588,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 

[Xen-devel] [PATCH v6 4/5] sched: Register the schedulers into the list

[Jonathan Creekmore]

Adds a simple macro to place a pointer to a scheduler into an array
section at compile time. Also, goes ahead and generates the array
entries with each of the schedulers.

CC: George Dunlap 
CC: Dario Faggioli 
CC: Josh Whitehead 
CC: Robert VanVossen 
Signed-off-by: Jonathan Creekmore 
Acked-by: Dario Faggioli 
Reviewed-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 xen/common/sched_arinc653.c | 2 ++
 xen/common/sched_credit.c   | 2 ++
 xen/common/sched_credit2.c  | 2 ++
 xen/common/sched_rt.c   | 2 ++
 xen/include/xen/sched-if.h  | 2 ++
 5 files changed, 10 insertions(+)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index dbe02ed..3b59514 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -767,6 +767,8 @@ const struct scheduler sched_arinc653_def = {
 .tick_resume= NULL,
 };
 
+REGISTER_SCHEDULER(sched_arinc653_def);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 02afddf..1645f9c 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -2027,3 +2027,5 @@ const struct scheduler sched_credit_def = {
 .tick_suspend   = csched_tick_suspend,
 .tick_resume= csched_tick_resume,
 };
+
+REGISTER_SCHEDULER(sched_credit_def);
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 3c49ffa..38b02d0 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -2228,3 +2228,5 @@ const struct scheduler sched_credit2_def = {
 .alloc_domdata  = csched2_alloc_domdata,
 .free_domdata   = csched2_free_domdata,
 };
+
+REGISTER_SCHEDULER(sched_credit2_def);
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 3f1d047..7640cd0 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1199,3 +1199,5 @@ const struct scheduler sched_rtds_def = {
 .wake   = rt_vcpu_wake,
 .context_saved  = rt_context_saved,
 };
+
+REGISTER_SCHEDULER(sched_rtds_def);
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 493d43f..9c6e0f5 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -170,6 +170,8 @@ 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;
 
 struct cpupool
 {
-- 
2.6.4


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

[Xen-devel] [PATCH v6 0/5] Allow schedulers to be selectable through Kconfig

[Jonathan Creekmore]

Add machinery to allow the schedulers to be individually selectable
through the Kconfig interface. The first patch in the series sets up
the CONFIG_EXPERT Kconfig variable that is only enabled by passing an
environment variable to the build. The second patch in the series sets up
the Kconfig options for the schedulers and places the appropriate CONFIG_
options around the scheduler list. The third, fourth, and fifth patches
rework the scheduler list from being manually curated into a model
where just compiling any schedulers into the hypervisor causes the
scheduler list to be built up.

---
Changed since v5:
 * Removed extra default options since credit is always compiled in
 * Changed to a #define for the schedulers array instead of a 
   static const **

Changed since v4:
 * Removed the ability to configure the credit scheduler. It is always
   compiled in, so there is no need for the defensive check for schedulers
   in the linker (added in v4) and no reason to provide the static inlines
   (added in v3) since credit is always present. 
 * Remove extra size field that was accidentally left in.

Changed since v3:
 * Add defensive check for schedulers in the linker

Changed since v2:
 * Added a predecessor patch that introduces a environment
   variable for the build to enable expert configuration options
   (1/5)
 * Hide the scheduler menu behind the expert option (2/5)
 * Provide static inlines for credit functions that are assumed to be
   present if it is compiled out (2/5)
 * Provide an absolute default of the credit scheduler if the 
   scheduler menu is not visible (2/5)

Changed since v1:
 * Marked credit2 as EXPERIMENTAL
 * Removed confusing language from the commit message
 * Alphabetize the schedulers
 * rename the __start and __end symbols to better match
   the rest of the file
 * Simplify the calculation of the number of schedulers
 * Make the scheduler ops structures static to their files


Jonathan Creekmore (5):
  build: Env var to enable expert config options
  build: Hook the schedulers into Kconfig
  build: Alloc space for sched list in the link file
  sched: Register the schedulers into the list
  sched: Use the auto-generated list of schedulers

 xen/Kconfig |  4 
 xen/Makefile|  1 +
 xen/arch/arm/xen.lds.S  |  4 
 xen/arch/x86/xen.lds.S  |  4 
 xen/common/Kconfig  | 56 +
 xen/common/Makefile |  8 +++
 xen/common/sched_arinc653.c |  4 +++-
 xen/common/sched_credit.c   |  4 +++-
 xen/common/sched_credit2.c  |  4 +++-
 xen/common/sched_rt.c   |  4 +++-
 xen/common/schedule.c   | 19 +++
 xen/include/xen/sched-if.h  |  7 ++
 12 files changed, 95 insertions(+), 24 deletions(-)

-- 
2.6.4


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

[Xen-devel] [PATCH v6 1/5] build: Env var to enable expert config options

[Jonathan Creekmore]

Add an additional environment variable, defaulting to disabled,
that enables the CONFIG_EXPERT configuration option. The purpose
of the CONFIG_EXPERT configuration option is to make non-standard
Kconfig options visible during the configuration process. The
CONFIG_EXPERT option is not, itself, visible during the Kconfig
configuration process, so typical users will never see it nor
any of the non-standard configuration options.

CC: Ian Campbell 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
Signed-off-by: Jonathan Creekmore 
Reviewed-by: Doug Goldstein 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 xen/Kconfig  | 4 
 xen/Makefile | 1 +
 2 files changed, 5 insertions(+)

diff --git a/xen/Kconfig b/xen/Kconfig
index ffe3f45..fa8b27c 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -22,3 +22,7 @@ config DEFCONFIG_LIST
string
option defconfig_list
default "$ARCH_DEFCONFIG"
+
+config EXPERT
+   string
+   option env="XEN_CONFIG_EXPERT"
diff --git a/xen/Makefile b/xen/Makefile
index 3699b20..e03e79b 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -11,6 +11,7 @@ export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] 
&& /bin/dnsdomainname) |
 export XEN_BUILD_DATE  ?= $(shell LC_ALL=C date)
 export XEN_BUILD_TIME  ?= $(shell LC_ALL=C date +%T)
 export XEN_BUILD_HOST  ?= $(shell hostname)
+export XEN_CONFIG_EXPERT ?= n
 
 export BASEDIR := $(CURDIR)
 export XEN_ROOT := $(BASEDIR)/..
-- 
2.6.4


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

Re: [Xen-devel] [PATCH v6 1/5] build: Env var to enable expert config options

[Jonathan Creekmore]

> On Jan 15, 2016, at 11:20 AM, Jan Beulich  wrote:
> 
 On 15.01.16 at 18:01,  wrote:
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -11,6 +11,7 @@ export XEN_DOMAIN  ?= $(shell ([ -x /bin/dnsdomainname ] 
>> && /bin/dnsdomainname) |
>> export XEN_BUILD_DATE?= $(shell LC_ALL=C date)
>> export XEN_BUILD_TIME?= $(shell LC_ALL=C date +%T)
>> export XEN_BUILD_HOST?= $(shell hostname)
>> +export XEN_CONFIG_EXPERT ?= n
> 
> This, I'm afraid, invalidates what I've said in another reply on
> the earlier thread a few minutes ago. What Makefile.kconfig
> gets to see must be consistent for FORCE to not be added to
> include/config/auto.conf's dependencies by auto.conf.cmd.
> 
> Or in other words - did you check (in conjunction with that other
> patch fixing incremental rebuilds) whether incremental rebuilds
> aren't again becoming full rebuilds because of this when there's
> no XEN_CONFIG_EXPERT in the environment?

I have applied your patch on top of my branch and I do not see incremental 
rebuilds becoming full rebuilds whether XEN_CONFIG_EXPERT is not in the
environment.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v6 3/5] build: Alloc space for sched list in the link file

[Jonathan Creekmore]

> On Jan 15, 2016, at 11:01 AM, Jonathan Creekmore 
>  wrote:
> 
> Creates a section to contain scheduler entry pointers that are gathered
> together into an array. This will allow, in a follow-on patch, scheduler
> entries to be automatically gathered together into the array for
> automatic parsing.
> 
> CC: Ian Campbell 
> CC: Stefano Stabellini 
> CC: Keir Fraser 
> CC: Jan Beulich 
> CC: Andrew Cooper 
> Signed-off-by: Jonathan Creekmore 
> Reviewed-by: Andrew Cooper 
> Reviewed-by: Doug Goldstein 
> Reviewed-by: Konrad Rzeszutek Wilk 
> 
> ---
> Changed since v4:
>  * Remove defensive check for schedulers since the credit scheduler
>must always be present
> 
> Changed since v3:
>  * Add defensive check for schedulers in the linker
> 
> Changed since v1:
>  * rename the __start and __end symbols to better match
>the rest of the file
> ---
> xen/arch/arm/xen.lds.S | 4 
> xen/arch/x86/xen.lds.S | 4 
> 2 files changed, 8 insertions(+)
> 
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 0488f37..f501a2f 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -57,6 +57,10 @@ SECTIONS
>. = ALIGN(PAGE_SIZE);
>*(.data.page_aligned)
>*(.data)
> +   . = ALIGN(8);
> +   __start_schedulers_array = .;
> +   *(.data.schedulers)
> +   __end_schedulers_array = .;
>*(.data.rel)
>*(.data.rel.*)
>CONSTRUCTORS
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index e18e08f..c1ce027 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -80,6 +80,10 @@ SECTIONS
>__stop___pre_ex_table = .;
> 
>*(.data.read_mostly)
> +   . = ALIGN(8);
> +   __start_schedulers_array = .;
> +   *(.data.schedulers)
> +   __end_schedulers_array = .;
>*(.data.rel.ro)
>*(.data.rel.ro.*)
>   } :text
> -- 
> 2.6.4


I am pretty sure, with Dario’s latest ACK on (2/5), that this patch is the only 
one in the series that has not been 
ACKed yet. Is there anything else that I need to do to get this series in, 
especially since the (1/5) CONFIG_EXPERT
patch has already landed?


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

Re: [Xen-devel] [PATCH v5 1/8] Kconfig: import kconfig.h from Linux 4.3

[Jonathan Creekmore]

Shannon Zhao writes:

> From: Shannon Zhao 
>
> To support using CONFIG_ options in C/CPP expressions, import kconfig.h
> from the Linux v4.3 tag (commit id
> 6a13feb9c82803e2b815eca72fa7a9f5561d7861).
>
> CC: Doug Goldstein 
> Signed-off-by: Shannon Zhao 
> ---
>  xen/include/xen/config.h  |  2 +-
>  xen/include/xen/kconfig.h | 54 
> +++
>  2 files changed, 55 insertions(+), 1 deletion(-)
>  create mode 100644 xen/include/xen/kconfig.h
>
> diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
> index 7595599..eeb49db 100644
> --- a/xen/include/xen/config.h
> +++ b/xen/include/xen/config.h
> @@ -7,7 +7,7 @@
>  #ifndef __XEN_CONFIG_H__
>  #define __XEN_CONFIG_H__
>
> -#include 
> +#include 
>
>  #ifndef __ASSEMBLY__
>  #include 
> diff --git a/xen/include/xen/kconfig.h b/xen/include/xen/kconfig.h
> new file mode 100644
> index 000..d68a7ed
> --- /dev/null
> +++ b/xen/include/xen/kconfig.h
> @@ -0,0 +1,54 @@
> +#ifndef __XEN_KCONFIG_H
> +#define __XEN_KCONFIG_H
> +
> +#include 
> +
> +/*
> + * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
> + * these only work with boolean and tristate options.
> + */
> +
> +/*
> + * Getting something that works in C and CPP for an arg that may or may
> + * not be defined is tricky.  Here, if we have "#define CONFIG_BOOGER 1"
> + * we match on the placeholder define, insert the "0," for arg1 and generate
> + * the triplet (0, 1, 0).  Then the last step cherry picks the 2nd arg (a 
> one).
> + * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
> + * the last step cherry picks the 2nd arg, we get a zero.
> + */
> +#define __ARG_PLACEHOLDER_1 0,
> +#define config_enabled(cfg) _config_enabled(cfg)
> +#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
> +#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
> +#define ___config_enabled(__ignored, val, ...) val
> +
> +/*
> + * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
> + * otherwise. For boolean options, this is equivalent to
> + * IS_ENABLED(CONFIG_FOO).
> + */
> +#define IS_BUILTIN(option) config_enabled(option)
> +
> +/*
> + * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
> + * otherwise.
> + */
> +#define IS_MODULE(option) config_enabled(option##_MODULE)
> +
> +/*
> + * IS_REACHABLE(CONFIG_FOO) evaluates to 1 if the currently compiled
> + * code can call a function defined in code compiled based on CONFIG_FOO.
> + * This is similar to IS_ENABLED(), but returns false when invoked from
> + * built-in code when CONFIG_FOO is set to 'm'.
> + */
> +#define IS_REACHABLE(option) (config_enabled(option) || \
> +  (config_enabled(option##_MODULE) && config_enabled(MODULE)))
> +
> +/*
> + * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
> + * 0 otherwise.
> + */
> +#define IS_ENABLED(option) \
> + (IS_BUILTIN(option) || IS_MODULE(option))
> +
> +#endif /* __XEN_KCONFIG_H */

I am not sure that the complexity of this file is necessary since Xen
does not support loadable modules. Essentially, IS_ENABLED(CONFIG_FOO)
is as simple as #ifdef CONFIG_FOO.

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

Re: [Xen-devel] [PATCH v5 2/8] ACPI: add config for BIOS table scan

[Jonathan Creekmore]

Shannon Zhao writes:

> From: Graeme Gregory 
>
> With the addition of ARM64 that does not have a traditional BIOS to
> scan, add a config option which is selected on x86 (ia64 doesn't need
> it either, it is EFI/UEFI based system) to do the traditional BIOS
> scanning for tables.
>
> Signed-off-by: Graeme Gregory 
> Signed-off-by: Hanjun Guo 
> Signed-off-by: Rafael J. Wysocki 
> [Linux commit 8a1664be0b922dd6afd60eca96a992ef5ec22c40]
> Signed-off-by: Shannon Zhao 
> ---
> Cc: Jan Beulich 
> ---
>  xen/arch/x86/Kconfig | 1 +
>  xen/drivers/acpi/Kconfig | 3 +++
>  xen/drivers/acpi/osl.c   | 4 +++-
>  3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 7d2ed96..3a25288 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -5,6 +5,7 @@ config X86
>   def_bool y
>   select COMPAT
>   select HAS_ACPI
> + select ACPI_LEGACY_TABLES_LOOKUP if HAS_ACPI

Since HAS_ACPI is selected right above this, it seems pointless to do
the if HAS_ACPI here. Just select ACPI_LEGACY_TABLES_LOOKUP. Or, see below.

>   select HAS_CPUFREQ
>   select HAS_EHCI
>   select HAS_GDBSX
> diff --git a/xen/drivers/acpi/Kconfig b/xen/drivers/acpi/Kconfig
> index 11ab5e4..82d73ca 100644
> --- a/xen/drivers/acpi/Kconfig
> +++ b/xen/drivers/acpi/Kconfig
> @@ -2,3 +2,6 @@
>  # Select HAS_ACPI if ACPI is supported
>  config HAS_ACPI
>   bool
> +
> +config ACPI_LEGACY_TABLES_LOOKUP
> + bool

Or, better, default the value of ACPI_LEGACY_TABLES_LOOKUP based on
HAS_ACPI. That way, you only select HAS_ACPI to default this to on and,
if another platform besides X86 ever enabled HAS_ACPI, it would turn on
this option without you having to select it as well.

> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
> index ce15470..a2fc8c4 100644
> --- a/xen/drivers/acpi/osl.c
> +++ b/xen/drivers/acpi/osl.c
> @@ -75,12 +75,14 @@ acpi_physical_address __init 
> acpi_os_get_root_pointer(void)
>  "System description tables not found\n");
>   return 0;
>   }
> - } else {
> + } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {

I would use an #ifdef CONFIG_ACPI_LEGACY_TABLES_LOOKUP instead of using
the kconfig.h IS_ENABLED macro to keep from pulling that file in.

>   acpi_physical_address pa = 0;
>
>   acpi_find_root_pointer(&pa);
>   return pa;
>   }
> +
> + return 0;
>  }
>
>  void __iomem *

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

[Xen-devel] [PATCH] libxenstore: Use poll() with a non-blocking read()

[Jonathan Creekmore]

With the addition of FMODE_ATOMIC_POS in the Linux 3.14 kernel,
concurrent blocking file accesses to a single open file descriptor can
cause a deadlock trying to grab the file position lock. If a watch has
been set up, causing a read_thread to blocking read on the file
descriptor, then future writes that would cause the background read to
complete will block waiting on the file position lock before they can
execute. This race condition only occurs when libxenstore is accessing
the xenstore daemon through the /proc/xen/xenbus file and not through
the unix domain socket, which is the case when the xenstore daemon is
running as a stub domain or when oxenstored is passed --disable-socket.

Arguably, since the /proc/xen/xenbus file is declared nonseekable, then
the file position lock need not be grabbed, since the file cannot be
seeked. However, that is not how the kernel API works. On the other
hand, using the poll() API to implement the blocking for the read_all()
function prevents the file descriptor from being switched back and forth
between blocking and non-blocking modes between two threads.

Signed-off-by: Jonathan Creekmore 
---
 tools/xenstore/xs.c | 52 
 1 file changed, 16 insertions(+), 36 deletions(-)

diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index d1e01ba..9b75493 100644
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "xenstore.h"
 #include "list.h"
 #include "utils.h"
@@ -145,22 +146,6 @@ struct xs_handle {
 
 static int read_message(struct xs_handle *h, int nonblocking);
 
-static bool setnonblock(int fd, int nonblock) {
-   int flags = fcntl(fd, F_GETFL);
-   if (flags == -1)
-   return false;
-
-   if (nonblock)
-   flags |= O_NONBLOCK;
-   else
-   flags &= ~O_NONBLOCK;
-
-   if (fcntl(fd, F_SETFL, flags) == -1)
-   return false;
-
-   return true;
-}
-
 int xs_fileno(struct xs_handle *h)
 {
char c = 0;
@@ -216,7 +201,7 @@ error:
 static int get_dev(const char *connect_to)
 {
/* We cannot open read-only because requests are writes */
-   return open(connect_to, O_RDWR);
+   return open(connect_to, O_RDWR | O_NONBLOCK);
 }
 
 static struct xs_handle *get_handle(const char *connect_to)
@@ -365,42 +350,37 @@ static bool read_all(int fd, void *data, unsigned int 
len, int nonblocking)
/* With nonblocking, either reads either everything requested,
 * or nothing. */
 {
-   if (!len)
-   return true;
-
-   if (nonblocking && !setnonblock(fd, 1))
-   return false;
+   int done;
+   struct pollfd fds[] = {
+   {
+   .fd = fd,
+   .events = POLLIN
+   }
+   };
 
while (len) {
-   int done;
+   if (!nonblocking) {
+   if (poll(fds, 1, -1) < 1) {
+   return false;
+   }
+   }
 
done = read(fd, data, len);
if (done < 0) {
if (errno == EINTR)
continue;
-   goto out_false;
+   return false;
}
if (done == 0) {
/* It closed fd on us?  EBADF is appropriate. */
errno = EBADF;
-   goto out_false;
+   return false;
}
data += done;
len -= done;
-
-   if (nonblocking) {
-   nonblocking = 0;
-   if (!setnonblock(fd, 0))
-   goto out_false;
-   }
}
 
return true;
-
-out_false:
-   if (nonblocking)
-   setnonblock(fd, 0);
-   return false;
 }
 
 #ifdef XSTEST
-- 
2.1.4


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

Re: [Xen-devel] [PATCH] libxenstore: Use poll() with a non-blocking read()

[Jonathan Creekmore]

> On Aug 16, 2015, at 1:59 AM, Ian Campbell  wrote:
> 
> On Thu, 2015-08-13 at 16:44 -0500, Jonathan Creekmore wrote:
>> With the addition of FMODE_ATOMIC_POS in the Linux 3.14 kernel,
>> concurrent blocking file accesses to a single open file descriptor can
>> cause a deadlock trying to grab the file position lock. If a watch has
>> been set up, causing a read_thread to blocking read on the file
>> descriptor, then future writes that would cause the background read to
>> complete will block waiting on the file position lock before they can
>> execute.
> 
> This sounds like you are describing a kernel bug. Shouldn't this be
> fixed in the kernel?
> 
> In fact it even sounds a bit familiar, I wonder if it is fixed in some
> version of Linux >> 3.14? (CCing a few relevant maintainers)
> 

So, the latest I saw on the LKML, the problem still existed as of 3.17 and, 
looking forward through 4.2, the problem still exists there as well. I saw a 
few posts back in March 2015 talking about the deadlock, but it appeared to 
have gotten no traction. It isn’t a deadlock in the kernel, but rather a 
deadlock between the two blocking reads or a blocking read or write. The slight 
rewrite I applied in my patch effectively works around the problem and prevents 
the library from flip-flopping the nonblocking flag on the file descriptor 
between two threads. 


>> This race condition only occurs when libxenstore is accessing
>> the xenstore daemon through the /proc/xen/xenbus file and not through
>> the unix domain socket, which is the case when the xenstore daemon is
>> running as a stub domain or when oxenstored is passed --disable-socket.
>> 
>> Arguably, since the /proc/xen/xenbus file is declared nonseekable, then
>> the file position lock need not be grabbed, since the file cannot be
>> seeked. However, that is not how the kernel API works. On the other
>> hand, using the poll() API to implement the blocking for the read_all()
>> function prevents the file descriptor from being switched back and forth
>> between blocking and non-blocking modes between two threads.
>> 
>> Signed-off-by: Jonathan Creekmore 
>> ---
>> tools/xenstore/xs.c | 52 ---
>> -
>> 1 file changed, 16 insertions(+), 36 deletions(-)
>> 
>> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
>> index d1e01ba..9b75493 100644
>> --- a/tools/xenstore/xs.c
>> +++ b/tools/xenstore/xs.c
>> @@ -31,6 +31,7 @@
>> #include 
>> #include 
>> #include 
>> +#include 
>> #include "xenstore.h"
>> #include "list.h"
>> #include "utils.h"
>> @@ -145,22 +146,6 @@ struct xs_handle {
>> 
>> static int read_message(struct xs_handle *h, int nonblocking);
>> 
>> -static bool setnonblock(int fd, int nonblock) {
>> -int flags = fcntl(fd, F_GETFL);
>> -if (flags == -1)
>> -return false;
>> -
>> -if (nonblock)
>> -flags |= O_NONBLOCK;
>> -else
>> -flags &= ~O_NONBLOCK;
>> -
>> -if (fcntl(fd, F_SETFL, flags) == -1)
>> -return false;
>> -
>> -return true;
>> -}
>> -
>> int xs_fileno(struct xs_handle *h)
>> {
>>  char c = 0;
>> @@ -216,7 +201,7 @@ error:
>> static int get_dev(const char *connect_to)
>> {
>>  /* We cannot open read-only because requests are writes */
>> -return open(connect_to, O_RDWR);
>> +return open(connect_to, O_RDWR | O_NONBLOCK);
>> }
>> 
>> static struct xs_handle *get_handle(const char *connect_to)
>> @@ -365,42 +350,37 @@ static bool read_all(int fd, void *data, 
>> unsigned int len, int nonblocking)
>>  /* With nonblocking, either reads either everything 
>> requested,
>>   * or nothing. */
>> {
>> -if (!len)
>> -return true;
>> -
>> -if (nonblocking && !setnonblock(fd, 1))
>> -return false;
>> +int done;
>> +struct pollfd fds[] = {
>> +{
>> +.fd = fd,
>> +.events = POLLIN
>> +}
>> +};
>> 
>>  while (len) {
>> -int done;
>> +if (!nonblocking) {
>> +if (poll(fds, 1, -1) < 1) {
>> +return false;
>> +}
>> +}
>> 
>>  done = read(fd, data, len);
>>  if (done < 0) {
>>  if (errn

Re: [Xen-devel] [PATCH] libxenstore: Use poll() with a non-blocking read()

[Jonathan Creekmore]

David Vrabel  writes:

> On 16/08/15 09:59, Ian Campbell wrote:
>> On Thu, 2015-08-13 at 16:44 -0500, Jonathan Creekmore wrote:
>>> With the addition of FMODE_ATOMIC_POS in the Linux 3.14 kernel,
>>> concurrent blocking file accesses to a single open file descriptor can
>>> cause a deadlock trying to grab the file position lock. If a watch has
>>> been set up, causing a read_thread to blocking read on the file
>>> descriptor, then future writes that would cause the background read to
>>> complete will block waiting on the file position lock before they can
>>> execute.
>
> I think you should make libxenstore open /dev/xen/xenbus instead.  Since
> this is a character device it should work correctly.
>

So, I did a quick test and verified that 1) /dev/xen/xenbus did exist on
my system and 2) that opening /dev/xen/xenbus instead of
/proc/xen/xenbus fixed the problem I was seeing as well. I did think
that it was a little weird that libxenstore was treating a proc file as
a character device. It makes more sense to me to open the actual
character device instead. 

> It may be necessary to try /dev/xen/xenbus first and fallback to
> /proc/xen/xenbus.
>

When did /dev/xen/xenbus get created? It is a fairly straightforward
change to just switch to /dev/xen/xenbus and is a bit larger refactor to
probe for its existence first before falling back to
/proc/xen/xenbus.

Either way, I could work up a patch to do this if people are ok with
that user-space change. 


That said, I still think that the original patch I submitted is
worthwhile to prevent a subtle, but probably benign, race condition with
changing the blocking flag on a file descriptor that is shared between
two threads. 

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

[Xen-devel] [PATCH v2] libxenstore: prefer using the character device

[Jonathan Creekmore]

With the addition of FMODE_ATOMIC_POS in the Linux 3.14 kernel,
concurrent blocking file accesses to a single open file descriptor can
cause a deadlock trying to grab the file position lock. If a watch has
been set up, causing a read_thread to blocking read on the file
descriptor, then future writes that would cause the background read to
complete will block waiting on the file position lock before they can
execute. This race condition only occurs when libxenstore is accessing
the xenstore daemon through the /proc/xen/xenbus file and not through
the unix domain socket, which is the case when the xenstore daemon is
running as a stub domain or when oxenstored is passed
--disable-socket. Accessing the daemon from the true character device
also does not exhibit this problem.

On Linux, prefer using the character device file over the proc file if
the character device exists.

Signed-off-by: Jonathan Creekmore 
---
 tools/xenstore/xs_lib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/xenstore/xs_lib.c b/tools/xenstore/xs_lib.c
index af4f75a..0c7744e 100644
--- a/tools/xenstore/xs_lib.c
+++ b/tools/xenstore/xs_lib.c
@@ -81,6 +81,8 @@ const char *xs_domain_dev(void)
 #if defined(__RUMPUSER_XEN__) || defined(__RUMPRUN__)
return "/dev/xen/xenbus";
 #elif defined(__linux__)
+   if (access("/dev/xen/xenbus", F_OK) == 0)
+   return "/dev/xen/xenbus";
return "/proc/xen/xenbus";
 #elif defined(__NetBSD__)
return "/kern/xen/xenbus";
-- 
2.1.4


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

[Xen-devel] [PATCH] x86: wrap kexec feature with CONFIG_KEXEC

[Jonathan Creekmore]

Add the appropriate #if checks around the kexec code in the x86 codebase
so that the feature can actually be turned off by the flag instead of
always required to be enabled on x86.

Signed-off-by: Jonathan Creekmore 
---
 xen/arch/x86/Makefile  |  4 ++--
 xen/arch/x86/apic.c|  3 ++-
 xen/arch/x86/crash.c   |  2 ++
 xen/arch/x86/setup.c   | 14 ++
 xen/arch/x86/x86_64/compat/entry.S |  9 +
 xen/arch/x86/x86_64/entry.S|  8 
 xen/drivers/passthrough/vtd/dmar.h | 17 +
 xen/include/asm-x86/config.h   |  2 +-
 8 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5f24951..5725a8d 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -56,8 +56,8 @@ obj-y += trace.o
 obj-y += traps.o
 obj-y += usercopy.o
 obj-y += x86_emulate.o
-obj-y += machine_kexec.o
-obj-y += crash.o
+obj-$(HAS_KEXEC) += machine_kexec.o
+obj-$(HAS_KEXEC) += crash.o
 obj-y += tboot.o
 obj-y += hpet.o
 obj-y += vm_event.o
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 2c9ae4e..729adf5 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -307,6 +307,7 @@ void disable_local_APIC(void)
~(MSR_IA32_APICBASE_ENABLE|MSR_IA32_APICBASE_EXTD));
 }
 
+#ifdef CONFIG_KEXEC
 if ( kexecing && (current_local_apic_mode() != apic_boot_mode) )
 {
 uint64_t msr_content;
@@ -334,7 +335,7 @@ void disable_local_APIC(void)
 break;
 }
 }
-
+#endif
 }
 
 /*
diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c
index 888a214..55f803a 100644
--- a/xen/arch/x86/crash.c
+++ b/xen/arch/x86/crash.c
@@ -64,7 +64,9 @@ static void noreturn do_nmi_crash(const struct cpu_user_regs 
*regs)
  */
 set_ist(&idt_tables[cpu][TRAP_machine_check], IST_NONE);
 
+#ifdef CONFIG_KEXEC
 kexec_crash_save_cpu();
+#endif
 __stop_this_cpu();
 
 this_cpu(crash_save_done) = 1;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ff34670..14ff15a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -479,6 +479,8 @@ static void __init parse_video_info(void)
 }
 }
 
+#ifdef CONFIG_KEXEC
+
 static void __init kexec_reserve_area(struct e820map *e820)
 {
 unsigned long kdump_start = kexec_crash_area.start;
@@ -505,6 +507,8 @@ static void __init kexec_reserve_area(struct e820map *e820)
 }
 }
 
+#endif
+
 static void noinline init_done(void)
 {
 system_state = SYS_STATE_active;
@@ -617,9 +621,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 }
 cmdline_parse(cmdline);
 
+#ifdef CONFIG_KEXEC
 /* Must be after command line argument parsing and before
  * allocing any xenheap structures wanted in lower memory. */
 kexec_early_calculations();
+#endif
 
 parse_video_info();
 
@@ -782,6 +788,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 /* Create a temporary copy of the E820 map. */
 memcpy(&boot_e820, &e820, sizeof(e820));
 
+#ifdef CONFIG_KEXEC
 /* Early kexec reservation (explicit static start address). */
 nr_pages = 0;
 for ( i = 0; i < e820.nr_map; i++ )
@@ -789,6 +796,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 nr_pages += e820.map[i].size >> PAGE_SHIFT;
 set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
 kexec_reserve_area(&boot_e820);
+#endif
 
 initial_images = mod;
 nr_initial_images = mbi->mods_count;
@@ -973,6 +981,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 }
 }
 
+#ifdef CONFIG_KEXEC
 /* Don't overlap with modules. */
 e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size),
  mod, mbi->mods_count, -1);
@@ -981,6 +990,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 e = (e - kexec_crash_area.size) & PAGE_MASK;
 kexec_crash_area.start = e;
 }
+#endif
 }
 
 if ( modules_headroom && !mod->reserved )
@@ -997,8 +1007,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 reserve_e820_ram(&boot_e820, efi_enabled ? mbi->mem_upper : __pa(&_start),
  __pa(&_end));
 
+#ifdef CONFIG_KEXEC
 /* Late kexec reservation (dynamic start address). */
 kexec_reserve_area(&boot_e820);
+#endif
 
 setup_max_pdx(raw_max_page);
 if ( highmem_start )
@@ -1125,6 +1137,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
 }
 
+#ifdef CONFIG_KEXEC
 if ( kexec_crash_area.size )
 {
 unsigned long s = PFN_DOWN(kexec_crash_area.start);
@@ -1135,6 +1148,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 map_pages_to_xen((unsigned long)__va(kexec_crash_area.start),
  s, e - s

Re: [Xen-devel] [PATCH] x86: wrap kexec feature with CONFIG_KEXEC

[Jonathan Creekmore]

> On Aug 27, 2015, at 10:27 AM, David Vrabel  wrote:
> 
> On 27/08/15 15:47, Jonathan Creekmore wrote:
>> Add the appropriate #if checks around the kexec code in the x86 codebase
>> so that the feature can actually be turned off by the flag instead of
>> always required to be enabled on x86.
> 
> What's your use case for this?
> 

The use case is for a slimmed down version of the hypervisor that can be used 
as a security hypervisor, exposing as little extra functionality as possible. 
When looking for features to trim out to reduce the attack surface, I saw the 
flag for KEXEC and wanted to disable that, then ran into compile problems.

> I think you should consider providing empty stub functions for !KEXEC
> instead of all the #ifdefs.
> 

Good points, all, who mentioned the empty stub functions. I am revising it now 
to use stubs instead of all of the #ifdefs.

> 
>> --- a/xen/include/asm-x86/config.h
>> +++ b/xen/include/asm-x86/config.h
>> @@ -1,6 +1,6 @@
>> /**
>>  * config.h
>> - * 
>> + *
>>  * A Linux-style configuration list.
>>  */
> 
> Stray whitespace change.

I guess I really need to just turn off that feature (stripping trailing 
whitespace) in my editor. So useful when writing new code, not as helpful when 
trying to modify existing code.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] libxenstore: prefer using the character device

[Jonathan Creekmore]

Ian Jackson  writes:

Wei Liu writes ("Re: [Xen-devel] [PATCH v2] libxenstore: prefer 
using the character device"): 
On Thu, Aug 27, 2015 at 09:04:38AM -0500, Jonathan Creekmore 
wrote: 
> With the addition of FMODE_ATOMIC_POS in the Linux 3.14 
> kernel, concurrent blocking file accesses to a single open 
> file descriptor can cause a deadlock trying to grab the file 
> position lock. If a watch has been set up, causing a 
> read_thread to blocking read on the file descriptor, then 
> future writes that would cause the background read to 
> complete will block waiting on the file position lock before 
> they can execute. This race condition only occurs when 
> libxenstore is accessing the xenstore daemon through the 
> /proc/xen/xenbus file and not through the unix domain socket, 
> which is the case when the xenstore daemon is running as a 
> stub domain or when oxenstored is passed 
> --disable-socket. Accessing the daemon from the true 
> character device also does not exhibit this problem.   On 
> Linux, prefer using the character device file over the proc 
> file if the character device exists. 


I confess I still see this as working around a kernel bug.  Only 
this time we are switching from a buggy to non-buggy kernel 
interface. 


Why don't we have the kernel provide only non-buggy interfaces ?


I was just trying to implement what David suggested; CC'ing him on 
this as well.




> diff --git a/tools/xenstore/xs_lib.c 
> b/tools/xenstore/xs_lib.c index af4f75a..0c7744e 100644 --- 
> a/tools/xenstore/xs_lib.c +++ b/tools/xenstore/xs_lib.c @@ 
> -81,6 +81,8 @@ const char *xs_domain_dev(void) 
>  #if defined(__RUMPUSER_XEN__) || defined(__RUMPRUN__) return 
>  "/dev/xen/xenbus"; #elif defined(__linux__) 
> +	if (access("/dev/xen/xenbus", F_OK) == 0) + 
> return "/dev/xen/xenbus"; 


Also, previously xs_domain_dev was a function which simply 
returned a static value.  I feel vaguely uneasy at putting this 
kind of autodetection logic here. 


Not entirely; the existing code queried an environment variable first
and, only if that was not set, did it return a static value. I added the
autodetection logic to fall back in the case where, for some reason,
/dev/xen/xenbus did not exist. Handling that kind of a fallback at a
higher layer would be a larger refactor to probe for the existence of
the character device first.

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

[Xen-devel] [PATCH v2] x86: wrap kexec feature with CONFIG_KEXEC

[Jonathan Creekmore]

Add the appropriate #if checks around the kexec code in the x86 codebase
so that the feature can actually be turned off by the flag instead of
always required to be enabled on x86.

Signed-off-by: Jonathan Creekmore 

---
Changed since v1:

  * Reorder kexec files to be alphabetical in the makefile
  * Create macros for the kexec functions that are called when disabled
  * #define the kexec hypercalls to do_ni_hypercall when disabled
  * Remove unnecessary macro duplication
  * Remove unrelated whitespace changes
---
 xen/arch/x86/Makefile  |  4 ++--
 xen/arch/x86/setup.c   | 25 +
 xen/arch/x86/x86_64/compat/entry.S |  4 
 xen/arch/x86/x86_64/entry.S|  4 
 xen/include/xen/kexec.h| 11 +++
 5 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5f24951..2fc7f97 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -14,6 +14,7 @@ obj-bin-y += bzimage.init.o
 obj-bin-y += clear_page.o
 obj-bin-y += copy_page.o
 obj-y += compat.o
+obj-$(HAS_KEXEC) += crash.o
 obj-y += debug.o
 obj-y += delay.o
 obj-bin-y += dmi_scan.init.o
@@ -31,6 +32,7 @@ obj-y += io_apic.o
 obj-y += msi.o
 obj-y += ioport_emulate.o
 obj-y += irq.o
+obj-$(HAS_KEXEC) += machine_kexec.o
 obj-y += microcode_amd.o
 obj-y += microcode_intel.o
 # This must come after the vendor specific files.
@@ -56,8 +58,6 @@ obj-y += trace.o
 obj-y += traps.o
 obj-y += usercopy.o
 obj-y += x86_emulate.o
-obj-y += machine_kexec.o
-obj-y += crash.o
 obj-y += tboot.o
 obj-y += hpet.o
 obj-y += vm_event.o
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ff34670..333ba82 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -481,8 +481,8 @@ static void __init parse_video_info(void)
 
 static void __init kexec_reserve_area(struct e820map *e820)
 {
-unsigned long kdump_start = kexec_crash_area.start;
-unsigned long kdump_size  = kexec_crash_area.size;
+unsigned long kdump_start = get_kexec_crash_area_start();
+unsigned long kdump_size  = get_kexec_crash_area_size();
 static bool_t __initdata is_reserved = 0;
 
 kdump_size = (kdump_size + PAGE_SIZE - 1) & PAGE_MASK;
@@ -496,7 +496,8 @@ static void __init kexec_reserve_area(struct e820map *e820)
 {
 printk("Kdump: DISABLED (failed to reserve %luMB (%lukB) at %#lx)"
"\n", kdump_size >> 20, kdump_size >> 10, kdump_start);
-kexec_crash_area.start = kexec_crash_area.size = 0;
+set_kexec_crash_area_start(0);
+set_kexec_crash_area_size(0);
 }
 else
 {
@@ -974,12 +975,12 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 }
 
 /* Don't overlap with modules. */
-e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size),
+e = consider_modules(s, e, PAGE_ALIGN(get_kexec_crash_area_size()),
  mod, mbi->mods_count, -1);
-if ( !kexec_crash_area.start && (s < e) )
+if ( !get_kexec_crash_area_start() && (s < e) )
 {
-e = (e - kexec_crash_area.size) & PAGE_MASK;
-kexec_crash_area.start = e;
+e = (e - get_kexec_crash_area_size()) & PAGE_MASK;
+set_kexec_crash_area_start(e);
 }
 }
 
@@ -1125,14 +1126,14 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
 }
 
-if ( kexec_crash_area.size )
+if ( get_kexec_crash_area_size() )
 {
-unsigned long s = PFN_DOWN(kexec_crash_area.start);
-unsigned long e = min(s + PFN_UP(kexec_crash_area.size),
+unsigned long s = PFN_DOWN(get_kexec_crash_area_start());
+unsigned long e = min(s + PFN_UP(get_kexec_crash_area_size()),
   PFN_UP(__pa(HYPERVISOR_VIRT_END - 1)));
 
-if ( e > s ) 
-map_pages_to_xen((unsigned long)__va(kexec_crash_area.start),
+if ( e > s )
+map_pages_to_xen((unsigned long)__va(get_kexec_crash_area_start()),
  s, e - s, PAGE_HYPERVISOR);
 }
 
diff --git a/xen/arch/x86/x86_64/compat/entry.S 
b/xen/arch/x86/x86_64/compat/entry.S
index 1521779..fbe4885 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -12,6 +12,10 @@
 #include 
 #include 
 
+#ifndef CONFIG_KEXEC
+#define compat_kexec_op do_ni_hypercall
+#endif
+
 ENTRY(compat_hypercall)
 ASM_CLAC
 pushq $0
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 74677a2..a8815a7 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -13,6 +13,10 @@
 #include 
 #include 
 
+#ifndef CONFIG_KEXEC
+#define do_kexec_op do_ni_hypercall
+#endif
+
 /* %rbx: struct vcpu */
 ENTRY(switch_to_kernel)
 leaq  VCPU_trap_bounce(%rbx),%

[Xen-devel] [PATCH v3] x86: wrap kexec feature with CONFIG_KEXEC

[Jonathan Creekmore]

Add the appropriate #if checks around the kexec code in the x86 codebase
so that the feature can actually be turned off by the flag instead of
always required to be enabled on x86.

Signed-off-by: Jonathan Creekmore 

---
Changed since v2:
  * Switch macros over to static inline functions
  * #ifdef code segments that would cause larger code changes without it
  * Move do_ni_hypercall defines closer to the hypercall tables
  * Add kexec flag to the toplevel makefile

Changed since v1:
  * Reorder kexec files to be alphabetical in the makefile
  * Create macros for the kexec functions that are called when disabled
  * #define the kexec hypercalls to do_ni_hypercall when disabled
  * Remove unnecessary macro duplication
  * Remove unrelated whitespace changes
---
 xen/Rules.mk   |  1 +
 xen/arch/x86/Makefile  |  4 ++--
 xen/arch/x86/Rules.mk  |  3 ++-
 xen/arch/x86/setup.c   |  6 ++
 xen/arch/x86/x86_64/compat/entry.S |  4 
 xen/arch/x86/x86_64/entry.S|  4 
 xen/include/asm-x86/config.h   |  1 -
 xen/include/xen/kexec.h| 21 +
 8 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index feb08d6..a4b29ac 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -10,6 +10,7 @@ lock_profile  ?= n
 crash_debug   ?= n
 frame_pointer ?= n
 lto   ?= n
+kexec ?= y
 
 include $(XEN_ROOT)/Config.mk
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5f24951..2fc7f97 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -14,6 +14,7 @@ obj-bin-y += bzimage.init.o
 obj-bin-y += clear_page.o
 obj-bin-y += copy_page.o
 obj-y += compat.o
+obj-$(HAS_KEXEC) += crash.o
 obj-y += debug.o
 obj-y += delay.o
 obj-bin-y += dmi_scan.init.o
@@ -31,6 +32,7 @@ obj-y += io_apic.o
 obj-y += msi.o
 obj-y += ioport_emulate.o
 obj-y += irq.o
+obj-$(HAS_KEXEC) += machine_kexec.o
 obj-y += microcode_amd.o
 obj-y += microcode_intel.o
 # This must come after the vendor specific files.
@@ -56,8 +58,6 @@ obj-y += trace.o
 obj-y += traps.o
 obj-y += usercopy.o
 obj-y += x86_emulate.o
-obj-y += machine_kexec.o
-obj-y += crash.o
 obj-y += tboot.o
 obj-y += hpet.o
 obj-y += vm_event.o
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 4a04a8a..584e4b7 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -10,7 +10,7 @@ HAS_PCI := y
 HAS_PASSTHROUGH := y
 HAS_NS16550 := y
 HAS_EHCI := y
-HAS_KEXEC := y
+HAS_KEXEC := $(kexec)
 HAS_GDBSX := y
 HAS_PDX := y
 xenoprof := y
@@ -44,3 +44,4 @@ endif
 
 CFLAGS-$(shadow-paging) += -DCONFIG_SHADOW_PAGING
 CFLAGS-$(bigmem)+= -DCONFIG_BIGMEM
+CFLAGS-$(kexec) += -DCONFIG_KEXEC
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ff34670..319d175 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -481,6 +481,7 @@ static void __init parse_video_info(void)
 
 static void __init kexec_reserve_area(struct e820map *e820)
 {
+#ifdef CONFIG_KEXEC
 unsigned long kdump_start = kexec_crash_area.start;
 unsigned long kdump_size  = kexec_crash_area.size;
 static bool_t __initdata is_reserved = 0;
@@ -503,6 +504,7 @@ static void __init kexec_reserve_area(struct e820map *e820)
 printk("Kdump: %luMB (%lukB) at %#lx\n",
kdump_size >> 20, kdump_size >> 10, kdump_start);
 }
+#endif
 }
 
 static void noinline init_done(void)
@@ -973,6 +975,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 }
 }
 
+#ifdef CONFIG_KEXEC
 /* Don't overlap with modules. */
 e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size),
  mod, mbi->mods_count, -1);
@@ -981,6 +984,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 e = (e - kexec_crash_area.size) & PAGE_MASK;
 kexec_crash_area.start = e;
 }
+#endif
 }
 
 if ( modules_headroom && !mod->reserved )
@@ -1125,6 +1129,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
 }
 
+#ifdef CONFIG_KEXEC
 if ( kexec_crash_area.size )
 {
 unsigned long s = PFN_DOWN(kexec_crash_area.start);
@@ -1135,6 +1140,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 map_pages_to_xen((unsigned long)__va(kexec_crash_area.start),
  s, e - s, PAGE_HYPERVISOR);
 }
+#endif
 
 xen_virt_end = ((unsigned long)_end + (1UL << L2_PAGETABLE_SHIFT) - 1) &
~((1UL << L2_PAGETABLE_SHIFT) - 1);
diff --git a/xen/arch/x86/x86_64/compat/entry.S 
b/xen/arch/x86/x86_64/compat/entry.S
index 1521779..3088aa7 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -390,6 +390,10 @@ compat_crash_page_fault:
 
 .section .rodata, "a", @progbits
 
+#ifndef

Re: [Xen-devel] [PATCH v2] libxenstore: prefer using the character device

[Jonathan Creekmore]

Just wanted to follow-up and see if there was any more debate on 
this, since I hadn't seen any other commentary since last week.


David Vrabel  writes:


On 27/08/15 19:03, Ian Jackson wrote:

Wei Liu writes ("Re: [Xen-devel] [PATCH v2] libxenstore: prefer using the character 
device"):

On Thu, Aug 27, 2015 at 09:04:38AM -0500, Jonathan Creekmore wrote:

With the addition of FMODE_ATOMIC_POS in the Linux 3.14 kernel,
concurrent blocking file accesses to a single open file descriptor can
cause a deadlock trying to grab the file position lock. If a watch has
been set up, causing a read_thread to blocking read on the file
descriptor, then future writes that would cause the background read to
complete will block waiting on the file position lock before they can
execute. This race condition only occurs when libxenstore is accessing
the xenstore daemon through the /proc/xen/xenbus file and not through
the unix domain socket, which is the case when the xenstore daemon is
running as a stub domain or when oxenstored is passed
--disable-socket. Accessing the daemon from the true character device
also does not exhibit this problem.

On Linux, prefer using the character device file over the proc file if
the character device exists.


I confess I still see this as working around a kernel bug.  Only this
time we are switching from a buggy to non-buggy kernel interface.


/proc/xen/xenbus is deprecated.  The tools should use the non-deprecated
interface.


Why don't we have the kernel provide only non-buggy interfaces ?


Fixing /proc/xen/xenbus is non-trival and since there's a fully working
non-deprecated interface (/dev/xen/xenbus), it's unlikely that anyone is
going to be inspired to fix it.


diff --git a/tools/xenstore/xs_lib.c b/tools/xenstore/xs_lib.c
index af4f75a..0c7744e 100644
--- a/tools/xenstore/xs_lib.c
+++ b/tools/xenstore/xs_lib.c
@@ -81,6 +81,8 @@ const char *xs_domain_dev(void)
 #if defined(__RUMPUSER_XEN__) || defined(__RUMPRUN__)
return "/dev/xen/xenbus";
 #elif defined(__linux__)
+   if (access("/dev/xen/xenbus", F_OK) == 0)
+   return "/dev/xen/xenbus";


Also, previously xs_domain_dev was a function which simply returned a
static value.  I feel vaguely uneasy at putting this kind of
autodetection logic here.


"Vaguely uneasy"?  Are we engineers or witchdoctors?

xs_domain_dev() already does a system call to query the environment so
it did not just "return a static value":

const char *xs_domain_dev(void)
{
char *s = getenv("XENSTORED_PATH");
if (s)
return s;
...

David


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

[Xen-devel] [PATCH v4] x86: wrap kexec feature with CONFIG_KEXEC

[Jonathan Creekmore]

Add the appropriate #if checks around the kexec code in the x86 codebase
so that the feature can actually be turned off by the flag instead of
always required to be enabled on x86.

Signed-off-by: Jonathan Creekmore 

---
Changed since v3:
  * Correct makefile to meet the standards for feature flags
  * Remove unnecessary nooping of the unused parameter

Changed since v2:
  * Switch macros over to static inline functions
  * #ifdef code segments that would cause larger code changes without it
  * Move do_ni_hypercall defines closer to the hypercall tables
  * Add kexec flag to the toplevel makefile

Changed since v1:
  * Reorder kexec files to be alphabetical in the makefile
  * Create macros for the kexec functions that are called when disabled
  * #define the kexec hypercalls to do_ni_hypercall when disabled
  * Remove unnecessary macro duplication
  * Remove unrelated whitespace changes
---
 xen/Rules.mk   |  5 +
 xen/arch/x86/Makefile  |  4 ++--
 xen/arch/x86/setup.c   |  6 ++
 xen/arch/x86/x86_64/compat/entry.S |  4 
 xen/arch/x86/x86_64/entry.S|  4 
 xen/common/Makefile|  4 ++--
 xen/include/asm-x86/config.h   |  1 -
 xen/include/xen/kexec.h| 21 +
 8 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index feb08d6..0abb4cf 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -10,6 +10,7 @@ lock_profile  ?= n
 crash_debug   ?= n
 frame_pointer ?= n
 lto   ?= n
+kexec ?= y
 
 include $(XEN_ROOT)/Config.mk
 
@@ -71,6 +72,10 @@ endif
 ifneq ($(max_phys_irqs),)
 CFLAGS-y+= -DMAX_PHYS_IRQS=$(max_phys_irqs)
 endif
+ifeq ($(HAS_KEXEC)$(kexec),yy)
+CONFIG_KEXEC := y
+endif
+CFLAGS-$(CONFIG_KEXEC)  += -DCONFIG_KEXEC
 
 AFLAGS-y+= -D__ASSEMBLY__ -include 
$(BASEDIR)/include/xen/config.h
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5f24951..39a8059 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -14,6 +14,7 @@ obj-bin-y += bzimage.init.o
 obj-bin-y += clear_page.o
 obj-bin-y += copy_page.o
 obj-y += compat.o
+obj-$(CONFIG_KEXEC) += crash.o
 obj-y += debug.o
 obj-y += delay.o
 obj-bin-y += dmi_scan.init.o
@@ -31,6 +32,7 @@ obj-y += io_apic.o
 obj-y += msi.o
 obj-y += ioport_emulate.o
 obj-y += irq.o
+obj-$(CONFIG_KEXEC) += machine_kexec.o
 obj-y += microcode_amd.o
 obj-y += microcode_intel.o
 # This must come after the vendor specific files.
@@ -56,8 +58,6 @@ obj-y += trace.o
 obj-y += traps.o
 obj-y += usercopy.o
 obj-y += x86_emulate.o
-obj-y += machine_kexec.o
-obj-y += crash.o
 obj-y += tboot.o
 obj-y += hpet.o
 obj-y += vm_event.o
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ff34670..319d175 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -481,6 +481,7 @@ static void __init parse_video_info(void)
 
 static void __init kexec_reserve_area(struct e820map *e820)
 {
+#ifdef CONFIG_KEXEC
 unsigned long kdump_start = kexec_crash_area.start;
 unsigned long kdump_size  = kexec_crash_area.size;
 static bool_t __initdata is_reserved = 0;
@@ -503,6 +504,7 @@ static void __init kexec_reserve_area(struct e820map *e820)
 printk("Kdump: %luMB (%lukB) at %#lx\n",
kdump_size >> 20, kdump_size >> 10, kdump_start);
 }
+#endif
 }
 
 static void noinline init_done(void)
@@ -973,6 +975,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 }
 }
 
+#ifdef CONFIG_KEXEC
 /* Don't overlap with modules. */
 e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size),
  mod, mbi->mods_count, -1);
@@ -981,6 +984,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 e = (e - kexec_crash_area.size) & PAGE_MASK;
 kexec_crash_area.start = e;
 }
+#endif
 }
 
 if ( modules_headroom && !mod->reserved )
@@ -1125,6 +1129,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
 }
 
+#ifdef CONFIG_KEXEC
 if ( kexec_crash_area.size )
 {
 unsigned long s = PFN_DOWN(kexec_crash_area.start);
@@ -1135,6 +1140,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 map_pages_to_xen((unsigned long)__va(kexec_crash_area.start),
  s, e - s, PAGE_HYPERVISOR);
 }
+#endif
 
 xen_virt_end = ((unsigned long)_end + (1UL << L2_PAGETABLE_SHIFT) - 1) &
~((1UL << L2_PAGETABLE_SHIFT) - 1);
diff --git a/xen/arch/x86/x86_64/compat/entry.S 
b/xen/arch/x86/x86_64/compat/entry.S
index 1521779..3088aa7 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -390,6 +390,10 @@ compat_crash_page_fault:
 
 .section .rodata, "a", @progbits
 
+#ifndef CONFIG_KEXEC
+#define compat_kexec_op 

Re: [Xen-devel] [PATCH] x86: wrap kexec feature with CONFIG_KEXEC

[Jonathan Creekmore]

Andrew Cooper  writes:

On 01/09/15 11:54, Jan Beulich wrote: 
On 01.09.15 at 12:44,  wrote: 
On 01/09/15 11:36, Ian Campbell wrote: 
In general (i.e. not 100% consistently, I think) we have 
tended to avoid making things user-facing compile time 
options. Many of the existing CONFIG_* and HAVE_* are really 
about things which are arch dependent, or require specific 
porting to each arch etc. I think the KEXEC flag is one of 
those. 

This keeps the test matrix more reasonable (unlike 
e.g. Linux's Kconfig) and also helps us by ensuring that 
users are mostly running one of a small number of possible 
configs. 

I slightly fear that after Kexec you are going to want to 
strip out more and more stuff... 
I for one welcome a Kconfig style approach.  We will never be 
in the same order of magnitude of options as Linux, and it 
will help to properly modularise the code. 
Indeed the idea was brought up a few times already, and I would 
also welcome such a step (accepting the downside of the larger 
test matrix). Not the least considering the "no shadow mode" 
and "big memory" build options that got introduced not so long 
ago. 


From a large attack surface point of view, support for 32bit ABI 
on 64bit Xen is a welcome candidate for more controlled 
environments. 

From a code volume point of view, having things like PV or 
support configurable would be interesting. 


I am not interested in unnecessarily stripping out more and more 
code. However, I do want to reduce the number of features and 
backwards-compatibility code-paths that are compiled into my 
build. Areas like the 32-bit ABI on 64-bit Xen like Andrew 
mentioned or the legacy Xenlinux ABI are paths that I am 
interested in pruning. One area I have a preliminary patch for is 
selectively compiling individual scheduling algorithms in, giving 
me the option to compile out the experimental algorithms from my 
build.


Obviously, I can carry my own patchqueues for these types of changes, but
I would prefer to upstream them where possible.

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

[Xen-devel] [PATCH v5] x86: wrap kexec feature with CONFIG_KEXEC

[Jonathan Creekmore]

Add the appropriate #if checks around the kexec code in the x86 codebase
so that the feature can actually be turned off by the flag instead of
always required to be enabled on x86.

Signed-off-by: Jonathan Creekmore 
Reviewed-by: Andrew Cooper 
Acked-by: David Vrabel 

---
Changed since v4:
  * Made static inline stubs into single-line functions
  * Remove makefile conditional in favor of list mechanism

Changed since v3:
  * Correct makefile to meet the standards for feature flags
  * Remove unnecessary nooping of the unused parameter

Changed since v2:
  * Switch macros over to static inline functions
  * #ifdef code segments that would cause larger code changes without it
  * Move do_ni_hypercall defines closer to the hypercall tables
  * Add kexec flag to the toplevel makefile

Changed since v1:
  * Reorder kexec files to be alphabetical in the makefile
  * Create macros for the kexec functions that are called when disabled
  * #define the kexec hypercalls to do_ni_hypercall when disabled
  * Remove unnecessary macro duplication
  * Remove unrelated whitespace changes
---
 xen/Rules.mk   | 6 ++
 xen/arch/x86/Makefile  | 4 ++--
 xen/arch/x86/setup.c   | 6 ++
 xen/arch/x86/x86_64/compat/entry.S | 4 
 xen/arch/x86/x86_64/entry.S| 4 
 xen/common/Makefile| 4 ++--
 xen/include/asm-x86/config.h   | 1 -
 xen/include/xen/kexec.h| 6 ++
 8 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index feb08d6..bf19d20 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -10,6 +10,7 @@ lock_profile  ?= n
 crash_debug   ?= n
 frame_pointer ?= n
 lto   ?= n
+kexec ?= y
 
 include $(XEN_ROOT)/Config.mk
 
@@ -72,6 +73,11 @@ ifneq ($(max_phys_irqs),)
 CFLAGS-y+= -DMAX_PHYS_IRQS=$(max_phys_irqs)
 endif
 
+CONFIG_KEXEC-$(HAS_KEXEC) := $(kexec)
+CONFIG_KEXEC  := $(CONFIG_KEXEC-y)
+
+CFLAGS-$(CONFIG_KEXEC)  += -DCONFIG_KEXEC
+
 AFLAGS-y+= -D__ASSEMBLY__ -include 
$(BASEDIR)/include/xen/config.h
 
 # Clang's built-in assembler can't handle .code16/.code32/.code64 yet
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5f24951..39a8059 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -14,6 +14,7 @@ obj-bin-y += bzimage.init.o
 obj-bin-y += clear_page.o
 obj-bin-y += copy_page.o
 obj-y += compat.o
+obj-$(CONFIG_KEXEC) += crash.o
 obj-y += debug.o
 obj-y += delay.o
 obj-bin-y += dmi_scan.init.o
@@ -31,6 +32,7 @@ obj-y += io_apic.o
 obj-y += msi.o
 obj-y += ioport_emulate.o
 obj-y += irq.o
+obj-$(CONFIG_KEXEC) += machine_kexec.o
 obj-y += microcode_amd.o
 obj-y += microcode_intel.o
 # This must come after the vendor specific files.
@@ -56,8 +58,6 @@ obj-y += trace.o
 obj-y += traps.o
 obj-y += usercopy.o
 obj-y += x86_emulate.o
-obj-y += machine_kexec.o
-obj-y += crash.o
 obj-y += tboot.o
 obj-y += hpet.o
 obj-y += vm_event.o
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ff34670..319d175 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -481,6 +481,7 @@ static void __init parse_video_info(void)
 
 static void __init kexec_reserve_area(struct e820map *e820)
 {
+#ifdef CONFIG_KEXEC
 unsigned long kdump_start = kexec_crash_area.start;
 unsigned long kdump_size  = kexec_crash_area.size;
 static bool_t __initdata is_reserved = 0;
@@ -503,6 +504,7 @@ static void __init kexec_reserve_area(struct e820map *e820)
 printk("Kdump: %luMB (%lukB) at %#lx\n",
kdump_size >> 20, kdump_size >> 10, kdump_start);
 }
+#endif
 }
 
 static void noinline init_done(void)
@@ -973,6 +975,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 }
 }
 
+#ifdef CONFIG_KEXEC
 /* Don't overlap with modules. */
 e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size),
  mod, mbi->mods_count, -1);
@@ -981,6 +984,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 e = (e - kexec_crash_area.size) & PAGE_MASK;
 kexec_crash_area.start = e;
 }
+#endif
 }
 
 if ( modules_headroom && !mod->reserved )
@@ -1125,6 +1129,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
 }
 
+#ifdef CONFIG_KEXEC
 if ( kexec_crash_area.size )
 {
 unsigned long s = PFN_DOWN(kexec_crash_area.start);
@@ -1135,6 +1140,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 map_pages_to_xen((unsigned long)__va(kexec_crash_area.start),
  s, e - s, PAGE_HYPERVISOR);
 }
+#endif
 
 xen_virt_end = ((unsigned long)_end + (1UL << L2_PAGETABLE_SHIFT) - 1) &
~((1UL << L2_PAGETABLE_SHIFT) - 1);
diff --git a/xen/arch/x86/x86_64/compat/entry.S 
b/xen/arch/x86

Re: [Xen-devel] [PATCH] x86: wrap kexec feature with CONFIG_KEXEC

[Jonathan Creekmore]

"Jan Beulich"  writes:

On 01.09.15 at 16:29,  wrote: 


Which is both appreciated and understandable. I suppose you 
agree though that if you were to follow the model used for the 
kexec part, things would quickly become unwieldy. Hence I would 
strongly suggest considering to introduce Linux'es or a 
Linux-like configure mechanism before continuing to make code 
conditionally compilable. 


Oh, I definitely agree.

What exactly are you looking for? Are you looking for just a KConfig
type of mechanism, or a full KBuild implementation. In a previous life,
I ported both to an embedded RTOS build. I think it might be simpler to
just start with a KConfig mechanism, though. If you want, I can take a
swing at starting down that path.

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

[Xen-devel] [PATCH] oxenstored: link in the systemd system library

[Jonathan Creekmore]

If systemd is configured for use AND you are building oxenstored, the C
systemd library must be linked in to the oxenstored binary instead of
just into the static ocaml stub.

Signed-off-by: Jonathan Creekmore 
---
 tools/ocaml/xenstored/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/ocaml/xenstored/Makefile b/tools/ocaml/xenstored/Makefile
index d861f11..38ca347 100644
--- a/tools/ocaml/xenstored/Makefile
+++ b/tools/ocaml/xenstored/Makefile
@@ -64,9 +64,11 @@ XENSTOREDLIBS = \
-ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/xb 
$(OCAML_TOPLEVEL)/libs/xb/xenbus.cmxa \
-ccopt -L -ccopt $(XEN_ROOT)/tools/libxc
 
+XENSTORED_LDFLAGS-$(CONFIG_SYSTEMD) += -cclib $(SYSTEMD_LIBS)
+
 PROGRAMS = oxenstored
 
-oxenstored_LIBS = $(XENSTOREDLIBS)
+oxenstored_LIBS = $(XENSTOREDLIBS) $(XENSTORED_LDFLAGS-y)
 oxenstored_OBJS = $(OBJS)
 
 OCAML_PROGRAM = oxenstored
-- 
2.1.4


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

[Xen-devel] [PATCH v2] oxenstored: link in the systemd system library

[Jonathan Creekmore]

If systemd is configured for use AND you are building oxenstored, the C
systemd library must be linked in to the systemd.cxma library.

Signed-off-by: Jonathan Creekmore 

---
Changed since v1:
  * Link the systemd library in to the systemd.cxma instead of the final
  binary.
---
 tools/ocaml/xenstored/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/ocaml/xenstored/Makefile b/tools/ocaml/xenstored/Makefile
index d861f11..59875f7 100644
--- a/tools/ocaml/xenstored/Makefile
+++ b/tools/ocaml/xenstored/Makefile
@@ -30,6 +30,8 @@ systemd_OBJS = systemd
 systemd_C_OBJS = systemd_stubs
 OCAML_LIBRARY += systemd
 
+LIBS_systemd += $(LDFLAGS-y)
+
 OBJS = define \
stdext \
trie \
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v2] oxenstored: link in the systemd system library

[Jonathan Creekmore]

> On Jul 23, 2015, at 9:00 AM, Ian Campbell  wrote:
> 
> On Thu, 2015-07-23 at 08:40 -0500, Jonathan Creekmore wrote:
>> If systemd is configured for use AND you are building oxenstored, the 
>> C
>> systemd library must be linked in to the systemd.cxma library.
>> 
>> Signed-off-by: Jonathan Creekmore 
> 
> Acked-by: Ian Campbell 
> 
> OOI what was the failure mode for you without this? Runtime rather than
> build time I suspect?

No, it was build-time. It failed to link the oxenstored binary due to missing 
symbols. 


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

[Xen-devel] [PATCH] build: remove .d files from xen/ on a clean

[Jonathan Creekmore]

Dependency files were getting left behind in the xen
directory (since 8b6ef9c152edceabecc7f90c811cd538a7b7a110),
so append the $(DEPS) to the clean rule that runs in the
hypervisor directory.

CC: Ian Campbell 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
Signed-off-by: Jonathan Creekmore 
---
 xen/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index c556198..fa9cf0a 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -88,7 +88,7 @@ _clean: delete-unfresh-files
$(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
$(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
-   rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
$(TARGET)-syms *~ core
+   rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
$(TARGET)-syms *~ core $(DEPS)
rm -f include/asm-*/asm-offsets.h
rm -f .banner
 
-- 
2.4.9 (Apple Git-60)


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

Re: [Xen-devel] [PATCH] build: remove .d files from xen/ on a clean

[Jonathan Creekmore]

> On Nov 24, 2015, at 11:07 AM, Jan Beulich  wrote:
> 
 On 24.11.15 at 17:56,  wrote:
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -88,7 +88,7 @@ _clean: delete-unfresh-files
>>  $(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
>>  $(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
>>  $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
>> -rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
>> $(TARGET)-syms *~ core
>> +rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
>> $(TARGET)-syms *~ core $(DEPS)
> 
> Is this really a problem only in xen/ ? The referenced commit clearly
> introduces "stray" *.d files elsewhere. Also there aren't any source
> files in xen/, so I'd expect $(DEPS) to be empty. Please clarify.

So, the files in xen/ were the dependencies files for xen.efi and 
xen-syms that were getting left behind. $(DEPS) appears to always
have ‘.*.d’ in it, based on me putting an echo into the clean rule to 
print it out. However, looking at this, I am also seeing ‘.d’ files left
behind in xen/common/compat that I did not notice before.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] build: remove .d files from xen/ on a clean

[Jonathan Creekmore]

> On Nov 24, 2015, at 11:16 AM, Jonathan Creekmore 
>  wrote:
> 
>> 
>> On Nov 24, 2015, at 11:07 AM, Jan Beulich  wrote:
>> 
>>>>> On 24.11.15 at 17:56,  wrote:
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -88,7 +88,7 @@ _clean: delete-unfresh-files
>>> $(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
>>> $(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
>>> $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
>>> -   rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
>>> $(TARGET)-syms *~ core
>>> +   rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
>>> $(TARGET)-syms *~ core $(DEPS)
>> 
>> Is this really a problem only in xen/ ? The referenced commit clearly
>> introduces "stray" *.d files elsewhere. Also there aren't any source
>> files in xen/, so I'd expect $(DEPS) to be empty. Please clarify.
> 
> So, the files in xen/ were the dependencies files for xen.efi and 
> xen-syms that were getting left behind. $(DEPS) appears to always
> have ‘.*.d’ in it, based on me putting an echo into the clean rule to 
> print it out. However, looking at this, I am also seeing ‘.d’ files left
> behind in xen/common/compat that I did not notice before.

Actually, looking closer at it, xen/common/compat does not appear to be
cleaning at all, so I think that is a separate, unrelated issue.

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

Re: [Xen-devel] [PATCH] build: remove .d files from xen/ on a clean

[Jonathan Creekmore]

> On Nov 24, 2015, at 11:30 AM, Jan Beulich  wrote:
> 
>>>> On 24.11.15 at 18:22,  wrote:
> 
>>> On Nov 24, 2015, at 11:16 AM, Jonathan Creekmore 
>>  wrote:
>>>> On Nov 24, 2015, at 11:07 AM, Jan Beulich  wrote:
>>>>>>> On 24.11.15 at 17:56,  wrote:
>>>>> --- a/xen/Makefile
>>>>> +++ b/xen/Makefile
>>>>> @@ -88,7 +88,7 @@ _clean: delete-unfresh-files
>>>>>   $(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
>>>>>   $(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
>>>>>   $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
>>>>> - rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
>>>>> $(TARGET)-syms 
>> *~ core
>>>>> + rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
>>>>> $(TARGET)-syms 
>> *~ core $(DEPS)
>>>> 
>>>> Is this really a problem only in xen/ ? The referenced commit clearly
>>>> introduces "stray" *.d files elsewhere. Also there aren't any source
>>>> files in xen/, so I'd expect $(DEPS) to be empty. Please clarify.
>>> 
>>> So, the files in xen/ were the dependencies files for xen.efi and 
>>> xen-syms that were getting left behind. $(DEPS) appears to always
>>> have ‘.*.d’ in it, based on me putting an echo into the clean rule to 
>>> print it out. However, looking at this, I am also seeing ‘.d’ files left
>>> behind in xen/common/compat that I did not notice before.
>> 
>> Actually, looking closer at it, xen/common/compat does not appear to be
>> cleaning at all, so I think that is a separate, unrelated issue.
> 
> That would be quite related, as it would be a result of the same
> commit.

Yeah, I now see where that change got introduced. I don’t see a clear way of 
cleaning
those objects files since the build system no longer goes into the 
common/compat directory at
all. The existing clean rules walk all of the subdirectories, cleaning object 
files and dependency
files as it goes. 


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

Re: [Xen-devel] [PATCH] build: remove .d files from xen/ on a clean

[Jonathan Creekmore]

> On Nov 25, 2015, at 2:58 AM, Jan Beulich  wrote:
> 
>>>> On 24.11.15 at 19:19,  wrote:
> 
>>> On Nov 24, 2015, at 11:30 AM, Jan Beulich  wrote:
>>> 
>>>>>> On 24.11.15 at 18:22,  wrote:
>>> 
>>>>> On Nov 24, 2015, at 11:16 AM, Jonathan Creekmore 
>>>>  wrote:
>>>>> 
>>>>> So, the files in xen/ were the dependencies files for xen.efi and 
>>>>> xen-syms that were getting left behind. $(DEPS) appears to always
>>>>> have ‘.*.d’ in it, based on me putting an echo into the clean rule to 
>>>>> print it out. However, looking at this, I am also seeing ‘.d’ files left
>>>>> behind in xen/common/compat that I did not notice before.
>>>> 
>>>> Actually, looking closer at it, xen/common/compat does not appear to be
>>>> cleaning at all, so I think that is a separate, unrelated issue.
>>> 
>>> That would be quite related, as it would be a result of the same
>>> commit.
>> 
>> Yeah, I now see where that change got introduced. I don’t see a clear way of 
>> cleaning
>> those objects files since the build system no longer goes into the 
>> common/compat directory at
>> all. The existing clean rules walk all of the subdirectories, cleaning 
>> object files and dependency
>> files as it goes. 
> 
> But wouldn't the way DEPS gets populated in xen/Rules.mk cover for
> this? If so, the alternative to your original patch might be to simply
> rm those ..xen*.o.d files right in the $(TARGET)-syms and
> $(TARGET).efi rules (along with their corresponding
> $(@D)/.$(@F).[0-9]* getting removed, due to which those .o.d
> ones are of no use anyway). Or maybe it should really do both,
> considering that *.o get removed by _clean too.
> 

So, I think we are talking a bit at cross purposes here. There are two
problems as I see it:

1. Dependency files get left in the xen/ directory for xen and xen-syms.
Those dependency files just started appearing in the xen/ directory when
the dependency generation was redone and the clean rule for the 
top-level directory did not handle cleaning dependency files in the 
top-level, because it has no source files. That is what my patch was
specifically aiming at fixing. The way DEPS gets populated in xen/Rules.mk
does cover it, but since DEPS was never in that top-level directory, it 
wasn’t clearing the dep files that were left in that directory.

However, you could make the argument that the real problem is that the 
dependency files are being dropped in that directory in the first place. 

2. The xen/common/compat directory is not being cleaned at all, although
there are .o and .o.d files left in that directory. My patch does not handle 
that
and was never meant to handle that. Given the way the clean rule works, I
don’t see how to clean out the files in that directory now that it is no longer
in the subdir-y list without just special casing it, which is kind of gross.




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

Re: [Xen-devel] [PATCH] build: fix dependencies for files compiled from their parent directory

[Jonathan Creekmore]

On Wed, Nov 25, 2015 at 10:16 AM, Jan Beulich  wrote:
> The use of $(basename ...) here was wrong (yet I'm sure I tested it).
>
> Signed-off-by: Jan Beulich 
>
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -105,7 +105,7 @@ include Makefile
>  DEPS = .*.d
>  define gendep
>  ifneq ($(1),$(subst /,:,$(1)))
> -DEPS += $(dir $(1)).$(basename $(notdir $(1))).d
> +DEPS += $(dir $(1)).$(notdir $(1)).d
>  endif
>  endef
>  $(foreach o,$(filter-out %/,$(obj-y)),$(eval $(call gendep,$(o

I finally got a chance to take a look at this patch today and it does fix the
dependency tracking for files in the xen/common/compat directory. However,
it still doesn't do anything to fix the failure to clean the object
files that are
left behind in that directory by 'make clean'. After running it, I still have:

$ find . -name "*.o*"
./common/compat/kernel.o
./common/compat/domain.o
./common/compat/tmem_xen.o
./common/compat/memory.o
./common/compat/multicall.o
./common/compat/xlat.o

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

[Xen-devel] [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jonathan Creekmore]

In commit 8b6ef9c152edceabecc7f90c811cd538a7b7a110,
several files in xen/common/compat were changed to be built
using the Makefile in xen/common, by appending the compat
prefix to the object files. Additionally, the
xen/common/compat directory was removed from the subdirs-y
variable, so it is no longer visited by the clean rule. This
resulted in some object files being built by inclusion into
obj-y, but not cleaned because they lived in a directory that
was unvisited by the clean rules. Appending obj-y to the clean
rule causes object files of this type to be cleaned up along
with files in the visited directory.

CC: Ian Campbell 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
Signed-off-by: Jonathan Creekmore 
---
 xen/Rules.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 02db110..539722f 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -173,7 +173,7 @@ FORCE:
 
 .PHONY: clean
 clean:: $(addprefix _clean_, $(subdir-all))
-   rm -f *.o *~ core $(DEPS)
+   rm -f *.o *~ core $(DEPS) $(obj-y)
 _clean_%/: FORCE
$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
 
-- 
2.6.2


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

Re: [Xen-devel] [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jonathan Creekmore]

> On Dec 1, 2015, at 6:41 AM, Jan Beulich  wrote:
> 
 On 30.11.15 at 18:29,  wrote:
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -173,7 +173,7 @@ FORCE:
>> 
>> .PHONY: clean
>> clean:: $(addprefix _clean_, $(subdir-all))
>> -rm -f *.o *~ core $(DEPS)
>> +rm -f *.o *~ core $(DEPS) $(obj-y)
> 
> While for the moment it would do, using a minimalistic approach
> like this will make us touch this again the moment we gain files in
> subdirectories that can be built just optionally. At the very least
> I'd therefore suggest also adding $(obj-n) and $(obj-) here. It
> might even be reasonable to grab subdirectories from $(obj-...)
> and add $(foreach d,$(filtered-subdirs), $(d)/*.o). Or, completely
> differently, have xen/common/Makefile just have an add-on
> clean:: (and require this also for future other cases like this).

I am not sure why this would require us to touch it again if we gain
files in subdirectories that can be built optionally. If you are running
a clean using the same configuration as you built, then the “optional”
subdirectories would be in the subdir-all list, so that subdirectory
would be processed just like it is now. Adding in obj-n and obj- seems
to imply that, for the current options selected, you are concerned about
object files showing up that you explicitly stated are not to be built 
(since that is how they show up in the obj-n list). My understanding of
the build system is that the only object files built are in obj-y for each
subdirectory. 

Initially, I tried eliminating the *.o completely and just pulling in obj-y and
obj-bin-y to the clean rule to be more like the Linux kernel clean rules, 
but I did not immediately see how, with $FOO.init.o in the obj-bin-y list,
I was seeing $FOO.o objects left behind instead. Thus, I left the *.o to
clean those files up.

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

Re: [Xen-devel] [PATCH] build: fix clean rule to cover objects in unvisited subdirs

[Jonathan Creekmore]

> On Dec 1, 2015, at 10:07 AM, Jan Beulich  wrote:
> 
> For one build run, yes. But then you can (a) build individual object
> files and (b) as mentioned above change configuration (implying
> that you know what you're doing). Also you could, using the
> example above, do a kexec=y build, then a kexec=n one, then
> notice you needed to clean in between, so you then clean using
> kexec=n and build again with that option, but cleaning again
> would still leave the kexec files around.
> 
> And btw., we have a similar issue already when you switch
> between arches (no cleaning happens cross-arch).

OK, so you are working on a different assumption than I was. I was
treating the clean rule as needing to be run when you are wanting to
explicitly rebuild all object files needed for the current build configuration 
(i.e., only cleaning files that would be linked into the current hypervisor 
build).
It sounds like you are expecting the clean rule to clean out all object
files no matter whether they are part of the current build configuration
or not. 

Working on that assumption, it seems like running a:
find . -name “*.o” -type f -delete 
from the xen/ directory would accomplish that and would be less
fragile than trying to grab various different variables and munge
them to try to grab all possible .o files specified by the system. Plus,
the find command would likely execute quicker. 

Does something like that seem acceptable?
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] build: fix clean to remove all .o and .d files

[Jonathan Creekmore]

In commit 8b6ef9c152edceabecc7f90c811cd538a7b7a110, several files in
xen/common/compat were changed to be built using the Makefile in
xen/common, by appending the compat prefix to the object
files. Additionally, the xen/common/compat directory was removed from
the subdirs-y variable, so it is no longer visited by the clean
rule. This resulted in some object files and dependency files being
generated by inclusion into obj-y, but not cleaned because they lived in a
directory that was unvisited by the clean rules.

Since there is a desire for all of the object files and dependency files
to be cleaned, just search for all objects and dependency files and
delete them on clean. The previous method of only tracking with the
$(DEPS) and *.o in the clean rules had the disadvantage that, if the
configuration changed between a build and a clean, some of the
dependencies or objects could get left behind. This method does not have
the same disadvantage.

CC: Ian Campbell 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Keir Fraser 
CC: Tim Deegan 
Signed-off-by: Jonathan Creekmore 
---
 xen/Makefile | 3 ++-
 xen/Rules.mk | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 3a1de99..365c477 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -91,7 +91,8 @@ _clean: delete-unfresh-files
$(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
$(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
-   rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
$(TARGET)-syms *~ core $(DEPS)
+   find . \( -name "*.o" -o -name "*.d" \) -exec rm -f {} \;
+   rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET)-syms 
*~ core
rm -f include/asm-*/asm-offsets.h
rm -f .banner
 
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 02db110..123a4a7 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -173,7 +173,7 @@ FORCE:
 
 .PHONY: clean
 clean:: $(addprefix _clean_, $(subdir-all))
-   rm -f *.o *~ core $(DEPS)
+   rm -f *~ core
 _clean_%/: FORCE
$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
 
-- 
2.6.2


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

Re: [Xen-devel] [PATCH v2] build: fix clean to remove all .o and .d files

[Jonathan Creekmore]

> On Dec 2, 2015, at 9:36 AM, Jan Beulich  wrote:
> 
 On 02.12.15 at 16:29,  wrote:
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -91,7 +91,8 @@ _clean: delete-unfresh-files
>>  $(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
>>  $(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
>>  $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
>> -rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
>> $(TARGET)-syms *~ core $(DEPS)
>> +find . \( -name "*.o" -o -name "*.d" \) -exec rm -f {} \;
> 
> If you really meant *.d, then *.[od] would have done. But in fact I
> think we want to limit this to ".*.d". Which I could fix up while
> committing, but then I'm not sure …

Easy enough to change it to “.*.d”.

> 
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -173,7 +173,7 @@ FORCE:
>> 
>> .PHONY: clean
>> clean:: $(addprefix _clean_, $(subdir-all))
>> -rm -f *.o *~ core $(DEPS)
>> +rm -f *~ core
> 
> ... this is a good idea, as it's not clear to me whether "clean" actually
> works when invoked in sub-trees of xen/.

I can definitely take that bit out; I just didn’t see the point in replicating 
the
removal of the files. Note that, whether I take it out or leave it in, “clean” 
still
will not totally work when invoked in sub-trees of xen/ because *.o is still not
enough to catch all of the object files (which was the whole point of this patch
in the first place).
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] build: fix clean to remove all .o and .d files

[Jonathan Creekmore]

> On Dec 2, 2015, at 9:46 AM, Jan Beulich  wrote:
> 
 On 02.12.15 at 16:41,  wrote:
> 
>>> On Dec 2, 2015, at 9:36 AM, Jan Beulich  wrote:
>>> 
>> On 02.12.15 at 16:29,  wrote:
 --- a/xen/Makefile
 +++ b/xen/Makefile
 @@ -91,7 +91,8 @@ _clean: delete-unfresh-files
$(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
$(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
 -  rm -f include/asm *.o $(TARGET) $(TARGET).gz $(TARGET).efi 
 $(TARGET)-syms 
>> *~ core $(DEPS)
 +  find . \( -name "*.o" -o -name "*.d" \) -exec rm -f {} \;
>>> 
>>> If you really meant *.d, then *.[od] would have done. But in fact I
>>> think we want to limit this to ".*.d". Which I could fix up while
>>> committing, but then I'm not sure …
>> 
>> Easy enough to change it to “.*.d”.
>> 
>>> 
 --- a/xen/Rules.mk
 +++ b/xen/Rules.mk
 @@ -173,7 +173,7 @@ FORCE:
 
 .PHONY: clean
 clean:: $(addprefix _clean_, $(subdir-all))
 -  rm -f *.o *~ core $(DEPS)
 +  rm -f *~ core
>>> 
>>> ... this is a good idea, as it's not clear to me whether "clean" actually
>>> works when invoked in sub-trees of xen/.
>> 
>> I can definitely take that bit out; I just didn’t see the point in 
>> replicating the
>> removal of the files. Note that, whether I take it out or leave it in, 
>> “clean” still
>> will not totally work when invoked in sub-trees of xen/ because *.o is still 
>> not
>> enough to catch all of the object files (which was the whole point of this 
>> patch
>> in the first place).
> 
> True, but let's not make matters worse. I'd be fine leaving this
> second change in only if we knew "clean" doesn't work at all when
> invoked against sub-trees of xen/.
> 
> And again - no reason to re-submit, I can easily tweak the patch
> upon commit, as long as you're fine with this being done with your
> S-o-b in place.
> 

Yeah, that is fine.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Problem Reading from XenStore in DomU

[Jonathan Creekmore]

Dagaen Golomb writes:

> It does, being the custom kernel on version 4.1.0. But Dom0 uses this same
> exact kernel and reads/writes just fine! The only solution if this is
> indeed the problem appears to be changing the kernel source we build on or
> some hacky method such as symlinking /proc/.. to /dev/.., there has to be
> an elegant real solution to this...

When I was chasing down a similar problem back in August, I discovered
that Dom0 accesses XenStore through a unix domain socket and not through
the kernel interface, so hitting it through xenstore-ls always seemed to
work. That is why Dom0 reads/writes to XenStore ok with the same kernel
that fails for your DomU. When I moved XenStore to a stubdomain, I /did/
see the problem in Dom0 with new enough kernels, which is why the
XenStore access library was changed to prefer the dev device instead of
the proc file.

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

Re: [Xen-devel] [PATCH] xen: sched: use default scheduler upon an invalid "sched="

[Jonathan Creekmore]

> On Jun 17, 2016, at 5:31 AM, Dario Faggioli  wrote:
> 
> instead of just the first scheduler we find in the array.
> 
> In fact, right now, if someone makes a typo when passing
> the "sched=" command line option to Xen, we (with all
> schedulers configured in) pick ARINC653, which is most
> likely not what one would expect.
> 
> Go for the default scheduler instead.
> 
> Signed-off-by: Dario Faggioli 
> ---
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Doug Goldstein 
> Cc: Jonathan Creekmore 

Reviewed-By: Jonathan Creekmore 


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