Re: scheduler: abstract away spc_whichqs

2016-03-11 Thread Michael McConville
Michal Mazurek wrote:
> spc_whichqs is an implementation specific variable of the bsd scheduler.
> Abstract it away, easing potential future rewrite.
> 
> This gets rid of curcpu_is_idle() that's used only once, and replaces it
> with a more general cpu_is_idle(curcpu()).
> 
> As far as I can tell no binary change.

I don't usually like helper macros like that, but those conditions are
sufficiently verbose and unreadable that I think it's warranted.

ok mmcc@, pending an ok from someone who works with this code.

> Index: sys/arch/amd64/amd64/cpu.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 cpu.c
> --- sys/arch/amd64/amd64/cpu.c3 Feb 2016 03:25:07 -   1.95
> +++ sys/arch/amd64/amd64/cpu.c11 Mar 2016 15:19:41 -
> @@ -253,7 +253,7 @@ cpu_idle_mwait_cycle(void)
>   panic("idle with interrupts blocked!");
>  
>   /* something already queued? */
> - if (ci->ci_schedstate.spc_whichqs != 0)
> + if (!cpu_is_idle(ci))
>   return;
>  
>   /*
> @@ -267,7 +267,7 @@ cpu_idle_mwait_cycle(void)
>* the check in sched_idle() and here.
>*/
>   atomic_setbits_int(&ci->ci_mwait, MWAIT_IDLING | MWAIT_ONLY);
> - if (ci->ci_schedstate.spc_whichqs == 0) {
> + if (cpu_is_idle(ci)) {
>   monitor(&ci->ci_mwait, 0, 0);
>   if ((ci->ci_mwait & MWAIT_IDLING) == MWAIT_IDLING)
>   mwait(0, 0);
> Index: sys/arch/i386/i386/cpu.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/cpu.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 cpu.c
> --- sys/arch/i386/i386/cpu.c  7 Mar 2016 05:32:46 -   1.72
> +++ sys/arch/i386/i386/cpu.c  11 Mar 2016 15:19:41 -
> @@ -755,7 +755,7 @@ cpu_idle_mwait_cycle(void)
>   panic("idle with interrupts blocked!");
>  
>   /* something already queued? */
> - if (ci->ci_schedstate.spc_whichqs != 0)
> + if (!cpu_is_idle(ci))
>   return;
>  
>   /*
> @@ -769,7 +769,7 @@ cpu_idle_mwait_cycle(void)
>* the check in sched_idle() and here.
>*/
>   atomic_setbits_int(&ci->ci_mwait, MWAIT_IDLING | MWAIT_ONLY);
> - if (ci->ci_schedstate.spc_whichqs == 0) {
> + if (cpu_is_idle(ci)) {
>   monitor(&ci->ci_mwait, 0, 0);
>   if ((ci->ci_mwait & MWAIT_IDLING) == MWAIT_IDLING)
>   mwait(0, 0);
> Index: sys/dev/acpi/acpicpu.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 acpicpu.c
> --- sys/dev/acpi/acpicpu.c29 Dec 2015 04:46:28 -  1.72
> +++ sys/dev/acpi/acpicpu.c11 Mar 2016 15:19:42 -
> @@ -1188,7 +1188,7 @@ acpicpu_idle(void)
>  #endif
>  
>   /* something already queued? */
> - if (ci->ci_schedstate.spc_whichqs != 0)
> + if (!cpu_is_idle(ci))
>   return;
>  
>   /*
> @@ -1204,7 +1204,7 @@ acpicpu_idle(void)
>   hints = (unsigned)best->address;
>   microuptime(&start);
>   atomic_setbits_int(&ci->ci_mwait, MWAIT_IDLING);
> - if (ci->ci_schedstate.spc_whichqs == 0) {
> + if (cpu_is_idle(ci)) {
>   /* intel errata AAI65: cflush before monitor */
>   if (ci->ci_cflushsz != 0) {
>   membar_sync();
> Index: sys/kern/kern_sched.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sched.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 kern_sched.c
> --- sys/kern/kern_sched.c 23 Dec 2015 14:51:17 -  1.41
> +++ sys/kern/kern_sched.c 11 Mar 2016 15:19:42 -
> @@ -148,7 +148,7 @@ sched_idle(void *v)
>   KASSERT(curproc == spc->spc_idleproc);
>  
>   while (1) {
> - while (!curcpu_is_idle()) {
> + while (!cpu_is_idle(curcpu())) {
>   struct proc *dead;
>  
>   SCHED_LOCK(s);
> Index: sys/sys/sched.h
> ===
> RCS file: /cvs/src/sys/sys/sched.h,v
> retrieving revision 1.40
> diff -u -p -r1.40 sched.h
> --- sys/sys/sched.h   9 Mar 2016 13:38:50 -   1.40
> +++ sys/sys/sched.h   11 Mar 2016 15:19:42 -
> @@ -163,7 +163,7 @@ void sched_start_secondary_cpus(void);
>  void sched_stop_secondary_cpus(void);
>  #endif
>  
> -#define curcpu_is_idle() (curcpu()->ci_schedstate.spc_whichqs == 0)
> +#define cpu_is_idle(ci)  ((ci)->ci_schedstate.spc_whichqs == 0)
>  
>  void sched_init_runqueues(void);
>  void setrunqueue(struct proc *);
> 
> -- 
> Michal Mazurek
> 



scheduler: abstract away spc_whichqs

2016-03-11 Thread Michal Mazurek
spc_whichqs is an implementation specific variable of the bsd scheduler.
Abstract it away, easing potential future rewrite.

This gets rid of curcpu_is_idle() that's used only once, and replaces it
with a more general cpu_is_idle(curcpu()).

As far as I can tell no binary change.

Index: sys/arch/amd64/amd64/cpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
retrieving revision 1.95
diff -u -p -r1.95 cpu.c
--- sys/arch/amd64/amd64/cpu.c  3 Feb 2016 03:25:07 -   1.95
+++ sys/arch/amd64/amd64/cpu.c  11 Mar 2016 15:19:41 -
@@ -253,7 +253,7 @@ cpu_idle_mwait_cycle(void)
panic("idle with interrupts blocked!");
 
/* something already queued? */
-   if (ci->ci_schedstate.spc_whichqs != 0)
+   if (!cpu_is_idle(ci))
return;
 
/*
@@ -267,7 +267,7 @@ cpu_idle_mwait_cycle(void)
 * the check in sched_idle() and here.
 */
atomic_setbits_int(&ci->ci_mwait, MWAIT_IDLING | MWAIT_ONLY);
-   if (ci->ci_schedstate.spc_whichqs == 0) {
+   if (cpu_is_idle(ci)) {
monitor(&ci->ci_mwait, 0, 0);
if ((ci->ci_mwait & MWAIT_IDLING) == MWAIT_IDLING)
mwait(0, 0);
Index: sys/arch/i386/i386/cpu.c
===
RCS file: /cvs/src/sys/arch/i386/i386/cpu.c,v
retrieving revision 1.72
diff -u -p -r1.72 cpu.c
--- sys/arch/i386/i386/cpu.c7 Mar 2016 05:32:46 -   1.72
+++ sys/arch/i386/i386/cpu.c11 Mar 2016 15:19:41 -
@@ -755,7 +755,7 @@ cpu_idle_mwait_cycle(void)
panic("idle with interrupts blocked!");
 
/* something already queued? */
-   if (ci->ci_schedstate.spc_whichqs != 0)
+   if (!cpu_is_idle(ci))
return;
 
/*
@@ -769,7 +769,7 @@ cpu_idle_mwait_cycle(void)
 * the check in sched_idle() and here.
 */
atomic_setbits_int(&ci->ci_mwait, MWAIT_IDLING | MWAIT_ONLY);
-   if (ci->ci_schedstate.spc_whichqs == 0) {
+   if (cpu_is_idle(ci)) {
monitor(&ci->ci_mwait, 0, 0);
if ((ci->ci_mwait & MWAIT_IDLING) == MWAIT_IDLING)
mwait(0, 0);
Index: sys/dev/acpi/acpicpu.c
===
RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
retrieving revision 1.72
diff -u -p -r1.72 acpicpu.c
--- sys/dev/acpi/acpicpu.c  29 Dec 2015 04:46:28 -  1.72
+++ sys/dev/acpi/acpicpu.c  11 Mar 2016 15:19:42 -
@@ -1188,7 +1188,7 @@ acpicpu_idle(void)
 #endif
 
/* something already queued? */
-   if (ci->ci_schedstate.spc_whichqs != 0)
+   if (!cpu_is_idle(ci))
return;
 
/*
@@ -1204,7 +1204,7 @@ acpicpu_idle(void)
hints = (unsigned)best->address;
microuptime(&start);
atomic_setbits_int(&ci->ci_mwait, MWAIT_IDLING);
-   if (ci->ci_schedstate.spc_whichqs == 0) {
+   if (cpu_is_idle(ci)) {
/* intel errata AAI65: cflush before monitor */
if (ci->ci_cflushsz != 0) {
membar_sync();
Index: sys/kern/kern_sched.c
===
RCS file: /cvs/src/sys/kern/kern_sched.c,v
retrieving revision 1.41
diff -u -p -r1.41 kern_sched.c
--- sys/kern/kern_sched.c   23 Dec 2015 14:51:17 -  1.41
+++ sys/kern/kern_sched.c   11 Mar 2016 15:19:42 -
@@ -148,7 +148,7 @@ sched_idle(void *v)
KASSERT(curproc == spc->spc_idleproc);
 
while (1) {
-   while (!curcpu_is_idle()) {
+   while (!cpu_is_idle(curcpu())) {
struct proc *dead;
 
SCHED_LOCK(s);
Index: sys/sys/sched.h
===
RCS file: /cvs/src/sys/sys/sched.h,v
retrieving revision 1.40
diff -u -p -r1.40 sched.h
--- sys/sys/sched.h 9 Mar 2016 13:38:50 -   1.40
+++ sys/sys/sched.h 11 Mar 2016 15:19:42 -
@@ -163,7 +163,7 @@ void sched_start_secondary_cpus(void);
 void sched_stop_secondary_cpus(void);
 #endif
 
-#define curcpu_is_idle()   (curcpu()->ci_schedstate.spc_whichqs == 0)
+#define cpu_is_idle(ci)((ci)->ci_schedstate.spc_whichqs == 0)
 
 void sched_init_runqueues(void);
 void setrunqueue(struct proc *);

-- 
Michal Mazurek