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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

Jan


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