xen/sched_credit2.c : Runqueue per core
PATCH 2/2 xen/sched_credit2.c : Runqueue per core
This patch inserts runqueue_per_core code.
And also makes generic selection for runqueue by using boot paarmeter.
---
xen/common/sched_credit2.c +31 -7
@@ -165,6 +165,8 @@int opt_migrate_resist=500 ;integer_param ("sched_credit2_migrate_resist" , opt_migrate_resist); +static char __initdata opt_credit2_runqueue[10 ] ="socket" ; +string_param ("credit2_runqueue" , opt_credit2_runqueue);/* * Useful macros
@@ -1921,6 +1923,7 @@ static void deactivate_runqueue(struct csched2_private *prv, int rqi)static void init_pcpu (const struct scheduler *ops,int cpu) {int rqi; +char rq;unsigned long flags;struct csched2_private *prv =CSCHED2_PRIV (ops);struct csched2_runqueue_data *rqd;
@@ -1935,15 +1938,36 @@ static void init_pcpu(const struct scheduler *ops, int cpu)return ; } +/*Figure out which type of runqueue are to be created */ +if (!strcmp (opt_credit2_runquque,"socket" )) { + rq ='s' ; + }else if (!strcmp (opt_credit2_runquque,"core" )) { + rq ='c' ; + }else { + rq ='s' ; + } +/* 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 0 doesn't get a STARTING callback, so use boot CPU data for it */ +if ( cpu ==0 ) { +switch (rq) { +case 's' : rqi =boot_cpu_to_socket (); +break ; +case 'c' : rqi =boot_cpu_to_core (); +break ; +default : rqi =boot_cpu_to_socket (); + } + }else { +switch (rq) { +case 's' : rqi =cpu_to_socket (cpu); +break ; +case 'c' : rqi =cpu_to_core (cpu); +break ; +default : rqi =cpu_to_socket (cpu); + } + }if ( rqi <0 ) {
@@ -1988,7 +2012,7 @@ 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 ) +if ( cpu ==0 ||cpu_to_socket (cpu) >=0 ||cpu_to_core (cpu))init_pcpu (ops, cpu);else printk("%s: cpu %d not online yet, deferring initializatgion\n",
xen/common/sched_credit2.c line 169
Quoted 9 lines
Quoted 1 line
Quoted 1 line · On 09.03.15 at 09:55, <[email protected]> wrote:
xen/common/sched_credit2.c +2
-0@@ -165,6 +165,8 @@int opt_migrate_resist=500 ;integer_param ("sched_credit2_migrate_resist" , opt_migrate_resist); +static char __initdata opt_credit2_runqueue[10 ] ="socket" ; +string_param ("credit2_runqueue" , opt_credit2_runqueue);
This then needs to be added to docs/misc/xen-command-line.markdown, which will then hopefully make clear what meaning it has.
Quoted 44 lines
xen/common/sched_credit2.c +27
-6@@ -1935,15 +1938,36 @@ static void init_pcpu(const struct scheduler *ops, int cpu)return ; } +/*Figure out which type of runqueue are to be created */ +if (!strcmp (opt_credit2_runquque,"socket" )) { + rq ='s' ; + }else if (!strcmp (opt_credit2_runquque,"core" )) { + rq ='c' ; + }else { + rq ='s' ; + } +/* 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 0 doesn't get a STARTING callback, so use boot CPU data for it */ +if ( cpu ==0 ) { +switch (rq) { +case 's' : rqi =boot_cpu_to_socket (); +break ; +case 'c' : rqi =boot_cpu_to_core (); +break ; +default : rqi =boot_cpu_to_socket (); + } + }else { +switch (rq) { +case 's' : rqi =cpu_to_socket (cpu); +break ; +case 'c' : rqi =cpu_to_core (cpu); +break ; +default : rqi =cpu_to_socket (cpu); + } + }if ( rqi <0 ) {
xen/common/sched_credit2.c csched2_alloc_pdata(const struct scheduler *ops, int cpu) · line 2018
Quoted 8 lines
xen/common/sched_credit2.c +1
-1@@ -1988,7 +2012,7 @@ 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 ) +if ( cpu ==0 ||cpu_to_socket (cpu) >=0 ||cpu_to_core (cpu))init_pcpu (ops, cpu);else printk("%s: cpu %d not online yet, deferring initializatgion\n",
In all of the above, please obey to ./CODING_STYLE.
Jan
Quoted 4 lines · Uma Sharma
This patch inserts
runqueue_per_corecode.
And also makes generic selection for runqueue by using boot paarmeter.Signed-off-by Uma Sharma <[email protected]>
Thanks Uma! Exciting to see this.
A couple of comments below.
Quoted 25 lines
---
xen/common/sched_credit2.c +3
-0@@ -165,6 +165,8 @@int opt_migrate_resist=500 ;integer_param ("sched_credit2_migrate_resist" , opt_migrate_resist); +static char __initdata opt_credit2_runqueue[10 ] ="socket" ; +string_param ("credit2_runqueue" , opt_credit2_runqueue);/* * Useful macros @@ -1921,6 +1923,7 @@ static void deactivate_runqueue(struct csched2_private *prv, int rqi)static void init_pcpu (const struct scheduler *ops,int cpu) {int rqi; +char rq;unsigned long flags;struct csched2_private *prv =CSCHED2_PRIV (ops);struct csched2_runqueue_data *rqd;@@ -1935,15 +1938,36 @@ static void init_pcpu(const struct scheduler *ops, int cpu)return ;
xen/common/sched_credit2.c static void init_pcpu(const struct scheduler *ops, int cpu) · line 1948
Quoted 10 lines
xen/common/sched_credit2.c +8
-0} +/*Figure out which type of runqueue are to be created */ +if (!strcmp (opt_credit2_runquque,"socket" )) { + rq ='s' ; + }else if (!strcmp (opt_credit2_runquque,"core" )) { + rq ='c' ; + }else { + rq ='s' ; + }
This should be parsed once, in csched2_init(), like the other options. We need to tell the user which queue we're using, and if we end up using socket as a default because we didn't recognize the string, we need to warn the user (in case they accidentally typed "credit2_runqueue=cote" and are trying to figure out why they're not getting the performance they expect).
I think probably what that means is having the string_param be "opt_credit2_runqueue_string", and having a separate "opt_credit2_runqueue" which we set after parsing opt_credit2_runqueue_string.
It would be more typical, rather than have this be a char resolving to 's' and 'c', to have it be an int, and have the values be #defines; for example, "CREDIT2_OPT_RUNQUEUE_CORE" and "CREDIT2_OPT_RUNQUEUE_SOCKET".
Also, given that your experiments show 'core' to work quite a bit better than 'socket', I'd suggest making it default to core rather than socket. :-)
Quoted 18 lines
xen/common/sched_credit2.c +9
-6+/* 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 0 doesn't get a STARTING callback, so use boot CPU data for it */ +if ( cpu ==0 ) { +switch (rq) { +case 's' : rqi =boot_cpu_to_socket (); +break ; +case 'c' : rqi =boot_cpu_to_core (); +break ; +default : rqi =boot_cpu_to_socket ();
xen/common/sched_credit2.c static void init_pcpu(const struct scheduler *ops, int cpu) · line 1970
Quoted 10 lines
xen/common/sched_credit2.c +10
-0+ } + }else { +switch (rq) { +case 's' : rqi =cpu_to_socket (cpu); +break ; +case 'c' : rqi =cpu_to_core (cpu); +break ; +default : rqi =cpu_to_socket (cpu); + } + }
I think here we can probably ASSERT( runqueue==CORE || runqueue==SOCKET), then say:
if(runqueue==SOCKET) {
rqi = (cpu) ? cpu_to_socket(cpu) : boot_cpu_to_socket();
} else {
rqi = (cpu) ? cpu_to_core(cpu) : boot_cpu_to_core();
}
(With the full-length variables, and keeping the comment to explain why the cpu matters, of course.)
Thanks!
-George
Quoted 9 lines · George Dunlap
Quoted 7 lines · Uma Sharma
xen/common/sched_credit2.c +0
-0@@ -1935,15 +1938,36 @@ static void init_pcpu(const struct scheduler *ops, int cpu)return ; }
Quoted 15 lines
xen/common/sched_credit2.c static void init_pcpu(const struct scheduler *ops, int cpu) · line 1948
Quoted 8 lines
xen/common/sched_credit2.c +8
-0+/*Figure out which type of runqueue are to be created */ +if (!strcmp (opt_credit2_runquque,"socket" )) { + rq ='s' ; + }else if (!strcmp (opt_credit2_runquque,"core" )) { + rq ='c' ; + }else { + rq ='s' ; + }It would be more typical, rather than have this be a char resolving to 's' and 'c', to have it be an int, and have the values be #defines; for example, "
CREDIT2_OPT_RUNQUEUE_CORE" and "CREDIT2_OPT_RUNQUEUE_SOCKET".
I was about to suggest the same.
Quoted 4 lines
Also, given that your experiments show 'core' to work quite a bit better than 'socket', I'd suggest making it default to core rather than socket. :-)
+1.
Of course, as I said already, you should explain and provide the numbers about this performance improvement in the cover letter of the series and, IMO, reference that in the changelog of this patch too (not putting the full results, but a quick summary of them would be good).
Regards,
Dario
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Thank you :-)
I will work on the things you mentioned and resend the patch. It's great to work on patches. I was trying to figure out how to change the code so it looks neat and now I have the answer. Thank you. :-)
I will summarize the performance in cover patch.
Regards,
Uma Sharma
Quoted 40 lines · Dario Faggioli
Quoted 9 lines · George Dunlap
Quoted 7 lines · Uma Sharma
xen/common/sched_credit2.c +0
-0@@ -1935,15 +1938,36 @@ static void init_pcpu(const struct scheduler *ops, int cpu)return ; }Quoted 15 lines
xen/common/sched_credit2.c static void init_pcpu(const struct scheduler *ops, int cpu) · line 1948
Quoted 8 lines
xen/common/sched_credit2.c +8
-0+/*Figure out which type of runqueue are to be created */ +if (!strcmp (opt_credit2_runquque,"socket" )) { + rq ='s' ; + }else if (!strcmp (opt_credit2_runquque,"core" )) { + rq ='c' ; + }else { + rq ='s' ; + }It would be more typical, rather than have this be a char resolving to 's' and 'c', to have it be an int, and have the values be #defines; for example, "
CREDIT2_OPT_RUNQUEUE_CORE" and "CREDIT2_OPT_RUNQUEUE_SOCKET".I was about to suggest the same.
Quoted 4 lines
Also, given that your experiments show 'core' to work quite a bit better than 'socket', I'd suggest making it default to core rather than socket. :-)
+1.
Of course, as I said already, you should explain and provide the numbers about this performance improvement in the cover letter of the series and, IMO, reference that in the changelog of this patch too (not putting the full results, but a quick summary of them would be good).
Regards,
Dario
Signature · 3 lines
Regards, Uma Sharma http://about.me/umasharma
Hi,
I have been working on the patches and made them.
But when I am trying to install xen again to test them changes are not getting included.
What should I do ?
I was on the working branch then did make debball and installed using dpkg. I even wrote printk statements to check but not getting included. :'(
Quoted 61 lines · Uma Sharma
Thank you :-)
I will work on the things you mentioned and resend the patch. It's great to work on patches. I was trying to figure out how to change the code so it looks neat and now I have the answer. Thank you. :-)
I will summarize the performance in cover patch.
Regards,
Uma SharmaQuoted 33 lines · Dario Faggioli
Quoted 9 lines · George Dunlap
Quoted 7 lines · Uma Sharma
xen/common/sched_credit2.c +0
-0@@ -1935,15 +1938,36 @@ static void init_pcpu(const struct scheduler *ops, int cpu)return ; }Quoted 15 lines
xen/common/sched_credit2.c static void init_pcpu(const struct scheduler *ops, int cpu) · line 1948
Quoted 8 lines
xen/common/sched_credit2.c +8
-0+/*Figure out which type of runqueue are to be created */ +if (!strcmp (opt_credit2_runquque,"socket" )) { + rq ='s' ; + }else if (!strcmp (opt_credit2_runquque,"core" )) { + rq ='c' ; + }else { + rq ='s' ; + }It would be more typical, rather than have this be a char resolving to 's' and 'c', to have it be an int, and have the values be #defines; for example, "
CREDIT2_OPT_RUNQUEUE_CORE" and "CREDIT2_OPT_RUNQUEUE_SOCKET".I was about to suggest the same.
Quoted 4 lines
Also, given that your experiments show 'core' to work quite a bit better than 'socket', I'd suggest making it default to core rather than socket. :-)
+1.
Quoted 7 lines
Of course, as I said already, you should explain and provide the numbers about this performance improvement in the cover letter of the series and, IMO, reference that in the changelog of this patch too (not putting the full results, but a quick summary of them would be good).
Regards,
Dario--
Regards,
Uma Sharma
http://about.me/umasharma
Signature · 3 lines
Regards, Uma Sharma http://about.me/umasharma