From 8f52645bd99ee3e636a34826c0cbfc5939920da1 Mon Sep 17 00:00:00 2001 From: Simon Marlow Date: Fri, 19 Sep 2008 10:26:01 +0000 Subject: [PATCH] Move the context_switch flag into the Capability Fixes a long-standing bug that could in some cases cause sub-optimal scheduling behaviour. --- includes/Cmm.h | 2 +- includes/StgMiscClosures.h | 1 - includes/mkDerivedConstants.c | 1 + rts/Capability.c | 15 +++++++++++++++ rts/Capability.h | 7 +++++++ rts/HeapStackCheck.cmm | 2 +- rts/Interpreter.c | 2 +- rts/PrimOps.cmm | 4 ++-- rts/Schedule.c | 18 ++++++------------ rts/Schedule.h | 5 ----- rts/Threads.c | 4 ++-- rts/Timer.c | 2 +- rts/posix/Signals.c | 4 ++-- 13 files changed, 39 insertions(+), 28 deletions(-) diff --git a/includes/Cmm.h b/includes/Cmm.h index 7a68a51..9915830 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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; } /* ----------------------------------------------------------------------------- -- 1.7.10.4