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

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2045 bytes --]

On Mon, 2015-03-16 at 12:56 +0000, Jan Beulich wrote:
> >>> On 16.03.15 at 13:51, <george.dunlap@eu.citrix.com> wrote:
> > On 03/16/2015 12:48 PM, Jan Beulich wrote:

> >> 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.
> 
Would something like this make sense?

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index cfca5a7..2f2aa73 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1936,12 +1936,8 @@ 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;
+    if ( system_state == SYS_STATE_boot )
+        rqi = boot_cpu_to_socket(cpu);
     else
         rqi = cpu_to_socket(cpu);
 
@@ -1986,9 +1982,13 @@ static void init_pcpu(const struct scheduler *ops, int cpu)
 static void *
 csched2_alloc_pdata(const struct scheduler *ops, int cpu)
 {
-    /* Check to see if the cpu is online yet */
-    /* Note: cpu 0 doesn't get a STARTING callback */
-    if ( cpu == 0 || cpu_to_socket(cpu) >= 0 )
+    /*
+     * Actual initialization is deferred to when the pCPU will be
+     * online, via a STARTING callback. The only exception is
+     * the boot cpu, which does not get such a notification, and
+     * hence needs to be taken care of here.
+     */
+    if ( system_state == SYS_STATE_boot )
         init_pcpu(ops, cpu);
     else
         printk("%s: cpu %d not online yet, deferring initializatgion\n",


[-- Attachment #1.1.2: credit2-system-state.patch --]
[-- Type: text/x-patch, Size: 1358 bytes --]

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index cfca5a7..2f2aa73 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1936,12 +1936,8 @@ 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;
+    if ( system_state == SYS_STATE_boot )
+        rqi = boot_cpu_to_socket(cpu);
     else
         rqi = cpu_to_socket(cpu);
 
@@ -1986,9 +1982,13 @@ static void init_pcpu(const struct scheduler *ops, int cpu)
 static void *
 csched2_alloc_pdata(const struct scheduler *ops, int cpu)
 {
-    /* Check to see if the cpu is online yet */
-    /* Note: cpu 0 doesn't get a STARTING callback */
-    if ( cpu == 0 || cpu_to_socket(cpu) >= 0 )
+    /*
+     * Actual initialization is deferred to when the pCPU will be
+     * online, via a STARTING callback. The only exception is
+     * the boot cpu, which does not get such a notification, and
+     * hence needs to be taken care of here.
+     */
+    if ( system_state == SYS_STATE_boot )
         init_pcpu(ops, cpu);
     else
         printk("%s: cpu %d not online yet, deferring initializatgion\n",

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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