Commit 8f52645b authored by Simon Marlow's avatar Simon Marlow

Move the context_switch flag into the Capability

Fixes a long-standing bug that could in some cases cause sub-optimal
scheduling behaviour.
parent 09bb1eb4
......@@ -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
......
......@@ -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);
......
......@@ -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);
......
......@@ -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;
}
/* ---------------------------------------------------------------------------
......@@ -217,6 +218,19 @@ initCapabilities( void )
last_free_capability = &capabilities[0];
}
/* ----------------------------------------------------------------------------
* 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_).
......
......@@ -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);
......
......@@ -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 { \
......
......@@ -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);
}
......
......@@ -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);
}
......
......@@ -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();
}
......
......@@ -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.
......
......@@ -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,
......
......@@ -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 */
}
}
......
......@@ -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;
}
/* -----------------------------------------------------------------------------
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment