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

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

[Jan Beulich]

>>> 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?

> +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".

Jan


___
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

[Ian Campbell]

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.

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

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?

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


Ian.

___
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

[Ian Campbell]

On Thu, 2016-01-14 at 09:44 -0700, Jan Beulich wrote:
> > > > 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).

I don't think this one is useless clutter, it provides useful information
to someone looking at the .config rather than then having to go and check
the source to find out what the defaults were.

Ian.

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

[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
 _credit_def,
+#endif
+#ifdef CONFIG_SCHED_CREDIT2
 _credit2_def,
+#endif
+#ifdef CONFIG_SCHED_ARINC653
 _arinc653_def,
+#endif
+#ifdef CONFIG_SCHED_RTDS
 _rtds_def,
+#endif
 };
 
 static struct scheduler __read_mostly ops;
-- 
2.6.4

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

[Jan Beulich]

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

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

Indeed.

Jan


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