From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH 1/4] build: Hook the schedulers into Kconfig Date: Fri, 18 Dec 2015 10:52:50 +0000 Message-ID: <5673E582.6030000@citrix.com> References: <1450385974-12732-1-git-send-email-jonathan.creekmore@gmail.com> <1450385974-12732-2-git-send-email-jonathan.creekmore@gmail.com> <1450402503.19320.97.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1a9sef-00061Q-2f for xen-devel@lists.xenproject.org; Fri, 18 Dec 2015 10:52:57 +0000 In-Reply-To: <1450402503.19320.97.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli , Jonathan Creekmore , xen-devel@lists.xenproject.org Cc: George Dunlap List-Id: xen-devel@lists.xenproject.org On 18/12/15 01:35, 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. I think the "just" in the sentence is an attempt to disambiguate (i.e., instead of *only* allowing the scheduler to be chosen on the command-line, *also* allow it to be chosen at compile-time). But I agree that the whole clause isn't really necessary and it would be clearer without it. -George > > 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. > >> 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)". > > 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 >