Move the context_switch flag into the Capability

Fixes a long-standing bug that could in some cases cause sub-optimal
scheduling behaviour.

10 files changed, 37 insertions, 26 deletions

diff --git a/rts/Capability.c b/rts/Capability.c
index 1f1a1aea2a..4d5748cec2 100644
--- a/rts/Capability.c
+++ b/rts/Capability.c
@@ -161,6 +161,7 @@ initCapability( Capability *cap, nat i )
     cap->free_trec_chunks = END_STM_CHUNK_LIST;
     cap->free_trec_headers = NO_TREC;
     cap->transaction_tokens = 0;
+    cap->context_switch = 0;
 }
 
 /* ---------------------------------------------------------------------------
@@ -218,6 +219,19 @@ initCapabilities( void )
 }
 
 /* ----------------------------------------------------------------------------
+ * setContextSwitches: cause all capabilities to context switch as
+ * soon as possible.
+ * ------------------------------------------------------------------------- */
+
+void setContextSwitches(void)
+{
+  nat i;
+  for (i=0; i < n_capabilities; i++) {
+    capabilities[i].context_switch = 1;
+  }
+}
+
+/* ----------------------------------------------------------------------------
  * Give a Capability to a Task.  The task must currently be sleeping
  * on its condition variable.
  *
@@ -568,6 +582,7 @@ wakeupThreadOnCapability (Capability *my_cap,
 	releaseCapability_(other_cap);
     } else {
 	appendToWakeupQueue(my_cap,other_cap,tso);
+        other_cap->context_switch = 1;
 	// someone is running on this Capability, so it cannot be
 	// freed without first checking the wakeup queue (see
 	// releaseCapability_).
diff --git a/rts/Capability.h b/rts/Capability.h
index 94306eb6c0..70d9ee9e8d 100644
--- a/rts/Capability.h
+++ b/rts/Capability.h
@@ -66,6 +66,10 @@ struct Capability_ {
     // each GC.
     bdescr **mut_lists;
 
+    // Context switch flag. We used to have one global flag, now one 
+    // per capability. Locks required  : none (conflicts are harmless)
+    int context_switch;
+
 #if defined(THREADED_RTS)
     // Worker Tasks waiting in the wings.  Singly-linked.
     Task *spare_workers;
@@ -232,6 +236,9 @@ extern void grabCapability (Capability **pCap);
 
 #endif /* !THREADED_RTS */
 
+// cause all capabilities to context switch as soon as possible.
+void setContextSwitches(void);
+
 // Free a capability on exit
 void freeCapability (Capability *cap);
 
diff --git a/rts/HeapStackCheck.cmm b/rts/HeapStackCheck.cmm
index 333d0c09e0..3980ca2dd1 100644
--- a/rts/HeapStackCheck.cmm
+++ b/rts/HeapStackCheck.cmm
@@ -66,7 +66,7 @@ import LeaveCriticalSection;
             CLOSE_NURSERY();					\
             CurrentNursery = bdescr_link(CurrentNursery);	\
             OPEN_NURSERY();					\
-            if (CInt[context_switch] != 0 :: CInt) {		\
+            if (Capability_context_switch(MyCapability()) != 0 :: CInt) { \
                 R1 = ThreadYielding;				\
                 goto sched;					\
             } else {						\
diff --git a/rts/Interpreter.c b/rts/Interpreter.c
index d541dfc409..4324f7f44a 100644
--- a/rts/Interpreter.c
+++ b/rts/Interpreter.c
@@ -1281,7 +1281,7 @@ run_BCO:
 	    // context switching: sometimes the scheduler can invoke
 	    // the interpreter with context_switch == 1, particularly
 	    // if the -C0 flag has been given on the cmd line.
-	    if (context_switch) {
+	    if (cap->context_switch) {
 		Sp--; Sp[0] = (W_)&stg_enter_info;
 		RETURN_TO_SCHEDULER(ThreadInterpret, ThreadYielding);
 	    }
diff --git a/rts/PrimOps.cmm b/rts/PrimOps.cmm
index f2ce41551a..e754eb05bd 100644
--- a/rts/PrimOps.cmm
+++ b/rts/PrimOps.cmm
@@ -970,7 +970,7 @@ forkzh_fast
   foreign "C" scheduleThread(MyCapability() "ptr", threadid "ptr") [];
 
   // switch at the earliest opportunity
-  CInt[context_switch] = 1 :: CInt;
+  Capability_context_switch(MyCapability()) = 1 :: CInt;
   
   RET_P(threadid);
 }
@@ -999,7 +999,7 @@ forkOnzh_fast
   foreign "C" scheduleThreadOn(MyCapability() "ptr", cpu, threadid "ptr") [];
 
   // switch at the earliest opportunity
-  CInt[context_switch] = 1 :: CInt;
+  Capability_context_switch(MyCapability()) = 1 :: CInt;
   
   RET_P(threadid);
 }
diff --git a/rts/Schedule.c b/rts/Schedule.c
index 3f428141df..8c254cc56a 100644
--- a/rts/Schedule.c
+++ b/rts/Schedule.c
@@ -89,11 +89,6 @@ StgTSO *blackhole_queue = NULL;
  */
 rtsBool blackholes_need_checking = rtsFalse;
 
-/* flag set by signal handler to precipitate a context switch
- * LOCK: none (just an advisory flag)
- */
-int context_switch = 0;
-
 /* flag that tracks whether we have done any execution in this time slice.
  * LOCK: currently none, perhaps we should lock (but needs to be
  * updated in the fast path of the scheduler).
@@ -504,7 +499,7 @@ schedule (Capability *initialCapability, Task *task)
      */
     if (RtsFlags.ConcFlags.ctxtSwitchTicks == 0
 	&& !emptyThreadQueues(cap)) {
-	context_switch = 1;
+	cap->context_switch = 1;
     }
 	 
 run_thread:
@@ -1179,12 +1174,12 @@ scheduleHandleHeapOverflow( Capability *cap, StgTSO *t )
 	       "--<< thread %ld (%s) stopped: HeapOverflow",
 	       (long)t->id, whatNext_strs[t->what_next]);
 
-    if (context_switch) {
+    if (cap->context_switch) {
         // Sometimes we miss a context switch, e.g. when calling
         // primitives in a tight loop, MAYBE_GC() doesn't check the
         // context switch flag, and we end up waiting for a GC.
         // See #1984, and concurrent/should_run/1984
-        context_switch = 0;
+        cap->context_switch = 0;
         addToRunQueue(cap,t);
     } else {
         pushOnRunQueue(cap,t);
@@ -1234,7 +1229,7 @@ scheduleHandleYield( Capability *cap, StgTSO *t, nat prev_what_next )
     // the CPU because the tick always arrives during GC).  This way
     // penalises threads that do a lot of allocation, but that seems
     // better than the alternative.
-    context_switch = 0;
+    cap->context_switch = 0;
     
     /* put the thread back on the run queue.  Then, if we're ready to
      * GC, check whether this is the last task to stop.  If so, wake
@@ -1435,6 +1430,7 @@ scheduleDoGC (Capability *cap, Task *task USED_IF_THREADS, rtsBool force_major)
 	return cap;  // NOTE: task->cap might have changed here
     }
 
+    setContextSwitches();
     for (i=0; i < n_capabilities; i++) {
 	debugTrace(DEBUG_sched, "ready_to_gc, grabbing all the capabilies (%d/%d)", i, n_capabilities);
 	if (cap != &capabilities[i]) {
@@ -1445,7 +1441,6 @@ scheduleDoGC (Capability *cap, Task *task USED_IF_THREADS, rtsBool force_major)
 	    // all the Capabilities, but even so it's a slightly
 	    // unsavoury invariant.
 	    task->cap = pcap;
-	    context_switch = 1;
 	    waitForReturnCapability(&pcap, task);
 	    if (pcap != &capabilities[i]) {
 		barf("scheduleDoGC: got the wrong capability");
@@ -1954,7 +1949,6 @@ initScheduler(void)
 
   blackhole_queue   = END_TSO_QUEUE;
 
-  context_switch = 0;
   sched_state    = SCHED_RUNNING;
   recent_activity = ACTIVITY_YES;
 
@@ -2247,7 +2241,7 @@ void
 interruptStgRts(void)
 {
     sched_state = SCHED_INTERRUPTING;
-    context_switch = 1;
+    setContextSwitches();
     wakeUpRts();
 }
 
diff --git a/rts/Schedule.h b/rts/Schedule.h
index 6ed7598212..d76f42b3af 100644
--- a/rts/Schedule.h
+++ b/rts/Schedule.h
@@ -89,11 +89,6 @@ void    awaken_blocked_queue(StgTSO *q);
 void    initThread(StgTSO *tso, nat stack_size);
 #endif
 
-/* Context switch flag.
- * Locks required  : none (conflicts are harmless)
- */
-extern int RTS_VAR(context_switch);
-
 /* The state of the scheduler.  This is used to control the sequence
  * of events during shutdown, and when the runtime is interrupted
  * using ^C.
diff --git a/rts/Threads.c b/rts/Threads.c
index a9d4a721d3..65eaf8dc04 100644
--- a/rts/Threads.c
+++ b/rts/Threads.c
@@ -507,14 +507,14 @@ unblockOne_ (Capability *cap, StgTSO *tso,
       appendToRunQueue(cap,tso);
       // we're holding a newly woken thread, make sure we context switch
       // quickly so we can migrate it if necessary.
-      context_switch = 1;
+      cap->context_switch = 1;
   } else {
       // we'll try to wake it up on the Capability it was last on.
       wakeupThreadOnCapability(cap, tso->cap, tso);
   }
 #else
   appendToRunQueue(cap,tso);
-  context_switch = 1;
+  cap->context_switch = 1;
 #endif
 
   debugTrace(DEBUG_sched,
diff --git a/rts/Timer.c b/rts/Timer.c
index 9822239b33..96ea5e2211 100644
--- a/rts/Timer.c
+++ b/rts/Timer.c
@@ -47,7 +47,7 @@ handle_tick(int unused STG_UNUSED)
       ticks_to_ctxt_switch--;
       if (ticks_to_ctxt_switch <= 0) {
 	  ticks_to_ctxt_switch = RtsFlags.ConcFlags.ctxtSwitchTicks;
-	  context_switch = 1;	/* schedule a context switch */
+	  setContextSwitches();	/* schedule a context switch */
       }
   }
 
diff --git a/rts/posix/Signals.c b/rts/posix/Signals.c
index e34190c439..09dacbeb11 100644
--- a/rts/posix/Signals.c
+++ b/rts/posix/Signals.c
@@ -226,14 +226,14 @@ generic_handler(int sig)
 	stg_exit(EXIT_FAILURE);
     }
     
+    MainCapability.context_switch = 1;
+
 #endif /* THREADED_RTS */
 
     // re-establish the signal handler, and carry on
     sigemptyset(&signals);
     sigaddset(&signals, sig);
     sigprocmask(SIG_UNBLOCK, &signals, NULL);
-
-    context_switch = 1;
 }
 
 /* -----------------------------------------------------------------------------