Re: [Xen-devel] [PATCH v2 2/2] sched_credit2.c: runqueue_per_core code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.03.15 at 13:51, <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 03/16/2015 12:48 PM, Jan Beulich wrote:
>>>>> On 13.03.15 at 20:13, <george.dunlap@xxxxxxxxxxxxx> wrote:
>>> On 03/13/2015 06:29 PM, Andrew Cooper wrote:
>>>>> @@ -1940,10 +1946,10 @@ static void init_pcpu(const struct scheduler 
>>>>> *ops, 
> int cpu)
>>>>>  
>>>>>      /* Figure out which runqueue to put it in */
>>>>>      /* NB: cpu 0 doesn't get a STARTING callback, so we hard-code it to 
> runqueue 0. */
>>>>> -    if ( cpu == 0 )
>>>>> -        rqi = 0;
>>>>> +    if ( opt_credit2_runqueue == CREDIT2_OPT_RUNQUEUE_SOCKET )
>>>>> +        rqi = cpu ? cpu_to_socket(cpu) : boot_cpu_to_socket();
>>>>
>>>> This conditional is bogus.  If cpu0 is offlined and re-onlined, it must
>>>> use cpu_to_core()
>>>>
>>>> This entire hunk should probably be
>>>>
>>>> rqi = (opt_credit2_runqueue == CREDIT2_OPT_RUNQUEUE_SOCKET) ?
>>>> cpu_to_socket(cpu) : cpu_to_core(cpu);
>>>>
>>>> (with suitable alignment)
>>>
>>> You're ignoring the fact that she's following suit from existing code;
>>> and that that code is there for a reason: When this is first called for
>>> cpu 0, cpu_to_socket() (and cpu_to_core()) return garbage since they
>>> haven't been initialized yet.
>>>
>>> That is something that needs to be fixed, but it's not Uma's job to fix it.
>> 
>> Them returning garbage isn't what needs fixing. Instead the code
>> here should use a different condition to check whether this is the
>> boot CPU (e.g. looking at system_state). And that can very well be
>> done directly in this patch.
> 
> What do you suggest, then?

My preferred solution would be, as said, to leverage system_state.
Provided the state to look for is consistent between x86 and ARM.

Jan


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