Move the context_switch flag into the Capability

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

diff --git a/includes/Cmm.h b/includes/Cmm.h
index 7a68a51..9915830 100644 (file)
--- a/includes/Cmm.h
+++ b/includes/Cmm.h
@@ -25,7 +25,7 @@
  *       - Hp += n  ==> Hp_adj(n)
  *       - R1.i   ==>  R1   (similarly for R1.w, R1.cl etc.)
  *       - You need to explicitly dereference variables; eg. 
- *             context_switch   ==>  CInt[context_switch]
+ *             alloc_blocks   ==>  CInt[alloc_blocks]
  *       - convert all word offsets into byte offsets:
  *             - e ==> WDS(e)
  *       - sizeofW(StgFoo)  ==>  SIZEOF_StgFoo

diff --git a/includes/StgMiscClosures.h b/includes/StgMiscClosures.h
index 61518e8..16cd476 100644 (file)
--- a/includes/StgMiscClosures.h
+++ b/includes/StgMiscClosures.h
@@ -619,7 +619,6 @@ extern StgWord rts_stop_on_exception[];
 extern StgWord rts_breakpoint_io_action[];
 
 // Schedule.c
-extern int RTS_VAR(context_switch);
 extern StgWord RTS_VAR(blocked_queue_hd), RTS_VAR(blocked_queue_tl);
 extern StgWord RTS_VAR(sleeping_queue);
 extern StgWord RTS_VAR(blackhole_queue);

diff --git a/includes/mkDerivedConstants.c b/includes/mkDerivedConstants.c
index 51e52f0..aaa4531 100644 (file)
--- a/includes/mkDerivedConstants.c
+++ b/includes/mkDerivedConstants.c
@@ -232,6 +232,7 @@ main(int argc, char *argv[])
     field_offset(Capability, r);
     field_offset(Capability, lock);
     struct_field(Capability, mut_lists);
+    struct_field(Capability, context_switch);
 
     struct_field(bdescr, start);
     struct_field(bdescr, free);

diff --git a/rts/Capability.c b/rts/Capability.c
index 1f1a1ae..4d5748c 100644 (file)
--- 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 94306eb..70d9ee9 100644 (file)
--- 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 333d0c0..3980ca2 100644 (file)
--- 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 d541dfc..4324f7f 100644 (file)
--- 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 f2ce415..e754eb0 100644 (file)
--- 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 3f42814..8c254cc 100644 (file)
--- 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 6ed7598..d76f42b 100644 (file)
--- 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 a9d4a72..65eaf8d 100644 (file)
--- 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 9822239..96ea5e2 100644 (file)
--- 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 e34190c..09dacbe 100644 (file)
--- 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;
 }
 
 /* -----------------------------------------------------------------------------