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

[Jonathan Creekmore <jonathan.creekmore@gmail.com>]

> On Dec 17, 2015, at 7:35 PM, Dario Faggioli <dario.faggioli@citrix.com> 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 <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
>>  };
>>  
> ... can have, with the changelog changed as shown, my:
> 
>  Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> Regards,
> Daio
> -- 
> <<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)
> 


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