[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] Xen sched: Fix multiple runqueues in credit2



>>> On 06.02.14 at 09:58, Justin Weaver <jtweaver@xxxxxxxxxx> wrote:
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -85,8 +85,7 @@
>   * to a small value, and a fixed credit is added to everyone.
>   *
>   * The plan is for all cores that share an L2 will share the same
> - * runqueue.  At the moment, there is one global runqueue for all
> - * cores.
> + * runqueue. 

If this is the intention, then ...

> @@ -1962,12 +1963,28 @@ static void init_pcpu(const struct scheduler *ops, 
> int cpu)
>      /* Figure out which runqueue to put it in */
>      rqi = 0;
>  
> -    /* 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;
>      else
> -        rqi = cpu_to_socket(cpu);
> +    {
> +        cpu_socket = cpu_to_socket(cpu);
> +        cpu0_socket = cpu_to_socket(0);
> +
> +        /* If cpu is on the same socket as CPU 0, put it with CPU 0 on run 
> queue 0 */
> +        if ( cpu_socket == cpu0_socket )
> +            rqi = 0;
> +        else            
> +            /* If cpu is on socket 0, assign it to a run queue based on the
> +             * socket CPU 0 is actually on */
> +            if ( cpu_socket == 0 )
> +                rqi = cpu0_socket;
> +
> +            /* If cpu is NOT on socket 0, just assign it to a run queue 
> based on
> +             * its own socket */
> +            else
> +                rqi = cpu_socket;
> +    }

... this is too simplistic: Whether the L2 is shared by all cores on a
socket should be determined, not assumed.

Apart from that keeping the CPU0 special case at the top is pointless
with the cpu0_socket special casing.

As to coding style: Please fix your comments and get the indentation
of the if/else sequence above right (i.e. either use "else if" with no
added indentation, or enclose the inner if/else in figure braces (I'd
personally prefer the former).

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.