| Message ID | 1452187761-38328-3-git-send-email-jonathan.creekmore@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 ffff82d080290d58 D __end_schedulers_array ffff82d080290d58 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?
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 > ffff82d080290d58 D __end_schedulers_array > ffff82d080290d58 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. ~Andrew
On Fri, 2016-01-08 at 15:54 +0000, Andrew Cooper wrote: > On 08/01/16 15:47, Konrad Rzeszutek Wilk wrote: > > > > And the hypervisor did build with: > > > > [konrad@char xen]$ nm --defined xen-syms |grep schedulers > > ffff82d080290d58 D __end_schedulers_array > > ffff82d080290d58 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. > +1 > A hypervisor without any schedulers compiled in will be rather sad. > Well, it depends on how many pCPUs the host you want to run the hypervisor on has: if it has 0 of them, no scheduler at all is the perfect solution... not to mention that the combination of these things (0 CPUs and no scheduling), would make a lot of issues just instantly disappear! :-D Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
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 >> ffff82d080290d58 D __end_schedulers_array >> ffff82d080290d58 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.
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 >>> ffff82d080290d58 D __end_schedulers_array >>> ffff82d080290d58 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. Juergen
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 >>>> ffff82d080290d58 D __end_schedulers_array >>>> ffff82d080290d58 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).
>>> On 08.01.16 at 17:13, <JGross@suse.com> wrote: > I think there should be an ASSERT (or some other measure) to > ensure the default scheduler is available. Indeed I was about to say exactly this. Jan
>>> On 08.01.16 at 17:30, <jonathan.creekmore@gmail.com> 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
On 1/8/16 10:49 AM, Jan Beulich wrote: >>>> On 08.01.16 at 17:30, <jonathan.creekmore@gmail.com> 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.
Doug Goldstein writes: > On 1/8/16 10:49 AM, Jan Beulich wrote: >>>>> On 08.01.16 at 17:30, <jonathan.creekmore@gmail.com> 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.
On 08/01/16 17:04, Jonathan Creekmore wrote: > Doug Goldstein writes: > >> On 1/8/16 10:49 AM, Jan Beulich wrote: >>>>>> On 08.01.16 at 17:30, <jonathan.creekmore@gmail.com> 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. I think that should be sufficient. A user has to go a long way out of their way to get into this situation in the first place. If there is a nice way for checking for an empty string in the preprocessor, then something like BUILD_BUG_ON(ARRAY_SIZE(CONFIG_SCHED_DEFAULT) == 1); would also be a good sanity check, but I doubt this specific example compiles. ~Andrew
On Fri, Jan 08, 2016 at 11:04:56AM -0600, Jonathan Creekmore wrote: > > Doug Goldstein writes: > > > On 1/8/16 10:49 AM, Jan Beulich wrote: > >>>>> On 08.01.16 at 17:30, <jonathan.creekmore@gmail.com> 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. That would be good!
>>> On 08.01.16 at 17:58, <cardoe@cardoe.com> wrote: > On 1/8/16 10:49 AM, Jan Beulich wrote: >>>>> On 08.01.16 at 17:30, <jonathan.creekmore@gmail.com> 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? > > 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. I personally think that we shouldn't allow .config-s to be produced that will yield a build failure. No matter whether supported, we'd expect (as discussed) people to take care of problems themselves, but we'd also hope for them to contribute back the adjustments they had to make. I.e. there's need to be a way for someone to fix the build issue here, and since we're already aware of the issue I can't see why we should allow in the brokenness. The precautions are only against issues we didn't happen to notice. Jan
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 <public/sched.h> #include <xsm/xsm.h> -/* 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; diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index fc61fc3..246338e 100644 --- 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 extern bool_t sched_smt_power_savings;