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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.03.15 at 15:57, <uma.sharma523@xxxxxxxxx> wrote:
> @@ -161,10 +161,16 @@
>   */
>  #define __CSFLAG_runq_migrate_request 3
>  #define CSFLAG_runq_migrate_request (1<<__CSFLAG_runq_migrate_request)
> -
> +/* CREDIT2_OPT_RUNQUEUE: Used to define the runqueue used
> + */
> +#define CREDIT2_OPT_RUNQUEUE_CORE 1
> +#define CREDIT2_OPT_RUNQUEUE_SOCKET 2
>  
>  int opt_migrate_resist=500;
>  integer_param("sched_credit2_migrate_resist", opt_migrate_resist);
> +static char __initdata opt_credit2_runqueue_string[10] = "core";
> +string_param("credit2_runqueue", opt_credit2_runqueue_string);
> +int opt_credit2_runqueue=CREDIT2_OPT_RUNQUEUE_CORE;

static? Spaces around =.

> @@ -1940,10 +1946,14 @@ 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();
> +    }
>      else
> -        rqi = cpu_to_socket(cpu);
> +    {
> +        rqi = (cpu) ? cpu_to_core(cpu) : boot_cpu_to_core();
> +    }

Rather than extending the bad assumption of CPU 0 being the boot
CPU (What if it gets offlined and this or another one onlined back
as CPU 0?), can't you find a way to avoid depending on the numeric
value of "cpu"?

Also - pointless braces and parentheses.

> @@ -2109,6 +2119,21 @@ csched2_init(struct scheduler *ops)
>          opt_load_window_shift = LOADAVG_WINDOW_SHIFT_MIN;
>      }
>  
> +    /* Defines the runqueue used. */
> +    if ( !strcmp(opt_credit2_runqueue_string, "socket") )
> +    {
> +        opt_credit2_runqueue=CREDIT2_OPT_RUNQUEUE_SOCKET;
> +        printk("Runqueue : runqueue_per_socket\n");
> +    }
> +    else if ( !strcmp(opt_credit2_runqueue_string, "core") )
> +    {
> +        opt_credit2_runqueue=CREDIT2_OPT_RUNQUEUE_CORE;
> +        printk("Runqueue : runqueue_per_core\n");
> +    }
> +    else {
> +        printk("Runqueue: credit2_runqueue entered incorrect Continuing with 
> core\n");
> +    }

Pointless braces again (or if you absolutely want to keep them,
then please on a separate line like you do everywhere else). And
spaces around = again.

Perhaps the printk() format strings could be made a little more
meaningful too, or - if meant to be purely for debugging - be
converted to dprintk().

Jan


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