[Git][ghc/ghc][wip/nonmoving-fixes] 9 commits: ThreadPaused: Don't zero slop until free vars are pushed
Ben Gamari
gitlab at gitlab.haskell.org
Thu Nov 26 21:29:36 UTC 2020
Ben Gamari pushed to branch wip/nonmoving-fixes at Glasgow Haskell Compiler / GHC
Commits:
feaadbf8 by Ben Gamari at 2020-11-26T16:28:50-05:00
ThreadPaused: Don't zero slop until free vars are pushed
When threadPaused blackholes a thunk it calls `OVERWRITING_CLOSURE` to
zero the slop for the benefit of the sanity checker. Previously this was
done *before* pushing the thunk's free variables to the update
remembered set. Consequently we would pull zero'd pointers to the update
remembered set.
- - - - -
fb124a30 by Ben Gamari at 2020-11-26T16:28:55-05:00
nonmoving: Fix regression from TSAN work
The TSAN rework (specifically aad1f803) introduced a subtle regression
in GC.c, swapping `g0` in place of `gen`. Whoops!
Fixes #18997.
- - - - -
076f8045 by Ben Gamari at 2020-11-26T16:28:59-05:00
rts/Messages: Add missing write barrier in THROWTO message update
After a THROWTO message has been handle the message closure is
overwritten by a NULL message. We must ensure that the original
closure's pointers continue to be visible to the nonmoving GC.
- - - - -
5196edf8 by Ben Gamari at 2020-11-26T16:29:02-05:00
nonmoving: Add missing write barrier in shrinkSmallByteArray
- - - - -
cd3c98b8 by Ben Gamari at 2020-11-26T16:29:05-05:00
Updates: Don't zero slop until closure has been pushed
Ensure that the the free variables have been pushed to the update
remembered set before we zero the slop.
- - - - -
32483eb7 by Ben Gamari at 2020-11-26T16:29:08-05:00
OSThreads: Fix error code checking
pthread_join returns its error code and apparently doesn't set errno.
- - - - -
becf7013 by Ben Gamari at 2020-11-26T16:29:12-05:00
nonmoving: Don't join to mark_thread on shutdown
The mark thread is not joinable as we detach from it on creation.
- - - - -
cc002412 by Ben Gamari at 2020-11-26T16:29:17-05:00
nonmoving: Ensure that evacuated large objects are marked
When evacuating a large objects we n
- - - - -
e151c961 by Ben Gamari at 2020-11-26T16:29:20-05:00
nonmoving: Add reference to Ueno 2016
- - - - -
11 changed files:
- includes/rts/storage/ClosureMacros.h
- rts/Messages.c
- rts/Messages.h
- rts/PrimOps.cmm
- rts/RaiseAsync.c
- rts/ThreadPaused.c
- rts/Updates.h
- rts/posix/OSThreads.c
- rts/sm/Evac.c
- rts/sm/GC.c
- rts/sm/NonMoving.c
Changes:
=====================================
includes/rts/storage/ClosureMacros.h
=====================================
@@ -520,11 +520,15 @@ INLINE_HEADER StgWord8 *mutArrPtrsCard (StgMutArrPtrs *a, W_ n)
#if defined(PROFILING) || defined(DEBUG)
#define OVERWRITING_CLOSURE(c) \
overwritingClosure(c)
+#define OVERWRITING_CLOSURE_SIZE(c, size) \
+ overwritingClosureSize(c, size)
#define OVERWRITING_CLOSURE_MUTABLE(c, off) \
overwritingMutableClosureOfs(c, off)
#else
#define OVERWRITING_CLOSURE(c) \
do { (void) sizeof(c); } while(0)
+#define OVERWRITING_CLOSURE_SIZE(c, size) \
+ do { (void) sizeof(c); (void) sizeof(size); } while(0)
#define OVERWRITING_CLOSURE_MUTABLE(c, off) \
do { (void) sizeof(c); (void) sizeof(off); } while(0)
#endif
=====================================
rts/Messages.c
=====================================
@@ -97,7 +97,7 @@ loop:
case THROWTO_SUCCESS: {
// this message is done
StgTSO *source = t->source;
- doneWithMsgThrowTo(t);
+ doneWithMsgThrowTo(cap, t);
tryWakeupThread(cap, source);
break;
}
=====================================
rts/Messages.h
=====================================
@@ -23,8 +23,15 @@ void sendMessage (Capability *from_cap, Capability *to_cap, Message *msg);
#include "SMPClosureOps.h"
INLINE_HEADER void
-doneWithMsgThrowTo (MessageThrowTo *m)
+doneWithMsgThrowTo (Capability *cap, MessageThrowTo *m)
{
+ ASSERT(m->header.info == &stg_MSG_THROWTO_info);
+ IF_NONMOVING_WRITE_BARRIER_ENABLED {
+ updateRemembSetPushClosure(cap, (StgClosure *) m->link);
+ updateRemembSetPushClosure(cap, (StgClosure *) m->source);
+ updateRemembSetPushClosure(cap, (StgClosure *) m->target);
+ updateRemembSetPushClosure(cap, (StgClosure *) m->exception);
+ }
OVERWRITING_CLOSURE((StgClosure*)m);
unlockClosure((StgClosure*)m, &stg_MSG_NULL_info);
LDV_RECORD_CREATE(m);
=====================================
rts/PrimOps.cmm
=====================================
@@ -227,6 +227,21 @@ stg_shrinkSmallMutableArrayzh ( gcptr mba, W_ new_size )
{
ASSERT(new_size <= StgSmallMutArrPtrs_ptrs(mba));
+ IF_NONMOVING_WRITE_BARRIER_ENABLED {
+ // Ensure that the elements we are about to shrink out of existence
+ // remain visible to the non-moving collector.
+ W_ p, end;
+ p = mba + SIZEOF_StgSmallMutArrPtrs + WDS(new_size);
+ end = mba + SIZEOF_StgSmallMutArrPtrs + WDS(StgSmallMutArrPtrs_ptrs(mba));
+again:
+ ccall updateRemembSetPushClosure_(BaseReg "ptr",
+ W_[p] "ptr");
+ if (p < end) {
+ p = p + SIZEOF_W;
+ goto again;
+ }
+ }
+
OVERWRITING_CLOSURE_MUTABLE(mba, (BYTES_TO_WDS(SIZEOF_StgSmallMutArrPtrs) +
new_size));
StgSmallMutArrPtrs_ptrs(mba) = new_size;
=====================================
rts/RaiseAsync.c
=====================================
@@ -336,7 +336,7 @@ check_target:
}
// nobody else can wake up this TSO after we claim the message
- doneWithMsgThrowTo(m);
+ doneWithMsgThrowTo(cap, m);
raiseAsync(cap, target, msg->exception, false, NULL);
return THROWTO_SUCCESS;
@@ -580,7 +580,7 @@ maybePerformBlockedException (Capability *cap, StgTSO *tso)
throwToSingleThreaded(cap, msg->target, msg->exception);
source = msg->source;
- doneWithMsgThrowTo(msg);
+ doneWithMsgThrowTo(cap, msg);
tryWakeupThread(cap, source);
return 1;
}
@@ -602,7 +602,7 @@ awakenBlockedExceptionQueue (Capability *cap, StgTSO *tso)
i = lockClosure((StgClosure *)msg);
if (i != &stg_MSG_NULL_info) {
source = msg->source;
- doneWithMsgThrowTo(msg);
+ doneWithMsgThrowTo(cap, msg);
tryWakeupThread(cap, source);
} else {
unlockClosure((StgClosure *)msg,i);
@@ -700,7 +700,7 @@ removeFromQueues(Capability *cap, StgTSO *tso)
// ASSERT(m->header.info == &stg_WHITEHOLE_info);
// unlock and revoke it at the same time
- doneWithMsgThrowTo(m);
+ doneWithMsgThrowTo(cap, m);
break;
}
=====================================
rts/ThreadPaused.c
=====================================
@@ -314,10 +314,6 @@ threadPaused(Capability *cap, StgTSO *tso)
continue;
}
- // zero out the slop so that the sanity checker can tell
- // where the next closure is.
- OVERWRITING_CLOSURE(bh);
-
// an EAGER_BLACKHOLE or CAF_BLACKHOLE gets turned into a
// BLACKHOLE here.
#if defined(THREADED_RTS)
@@ -345,11 +341,16 @@ threadPaused(Capability *cap, StgTSO *tso)
// overwrite to the update remembered set.
// N.B. We caught the WHITEHOLE case above.
updateRemembSetPushThunkEager(cap,
- THUNK_INFO_PTR_TO_STRUCT(bh_info),
- (StgThunk *) bh);
+ THUNK_INFO_PTR_TO_STRUCT(bh_info),
+ (StgThunk *) bh);
}
}
+ // zero out the slop so that the sanity checker can tell
+ // where the next closure is. N.B. We mustn't do this until we have
+ // pushed the free variables to the update remembered set above.
+ OVERWRITING_CLOSURE_SIZE(bh, closure_sizeW_(bh, INFO_PTR_TO_STRUCT(bh_info)));
+
// The payload of the BLACKHOLE points to the TSO
RELAXED_STORE(&((StgInd *)bh)->indirectee, (StgClosure *)tso);
SET_INFO_RELEASE(bh,&stg_BLACKHOLE_info);
=====================================
rts/Updates.h
=====================================
@@ -49,7 +49,6 @@
W_ bd; \
\
prim_write_barrier; \
- OVERWRITING_CLOSURE(p1); \
bd = Bdescr(p1); \
if (bdescr_gen_no(bd) != 0 :: bits16) { \
IF_NONMOVING_WRITE_BARRIER_ENABLED { \
@@ -60,6 +59,7 @@
} else { \
TICK_UPD_NEW_IND(); \
} \
+ OVERWRITING_CLOSURE(p1); \
StgInd_indirectee(p1) = p2; \
prim_write_barrier; \
SET_INFO(p1, stg_BLACKHOLE_info); \
=====================================
rts/posix/OSThreads.c
=====================================
@@ -401,8 +401,9 @@ interruptOSThread (OSThreadId id)
void
joinOSThread (OSThreadId id)
{
- if (pthread_join(id, NULL) != 0) {
- sysErrorBelch("joinOSThread: error %d", errno);
+ int ret = pthread_join(id, NULL);
+ if (ret != 0) {
+ sysErrorBelch("joinOSThread: error %d", ret);
}
}
=====================================
rts/sm/Evac.c
=====================================
@@ -505,6 +505,7 @@ evacuate_compact (StgPtr p)
bd->flags |= BF_EVACUATED;
if (RTS_UNLIKELY(RtsFlags.GcFlags.useNonmoving && new_gen == oldest_gen)) {
__atomic_fetch_or(&bd->flags, BF_NONMOVING, __ATOMIC_RELAXED);
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, (StgClosure *) str);
}
initBdescr(bd, new_gen, new_gen->to);
@@ -694,7 +695,9 @@ loop:
// We may have evacuated the block to the nonmoving generation. If so
// we need to make sure it is added to the mark queue since the only
// reference to it may be from the moving heap.
- if (major_gc && flags & BF_NONMOVING && !deadlock_detect_gc) {
+ //
+ // N.B. evaculate_large might have set BF_NONMOVING.
+ if (major_gc && bd->flags & BF_NONMOVING && !deadlock_detect_gc) {
markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, q);
}
return;
@@ -1014,6 +1017,10 @@ evacuate_BLACKHOLE(StgClosure **p)
// See #14497.
if (flags & BF_LARGE) {
evacuate_large((P_)q);
+ // N.B. evacuate_large might have evacuated to the non-moving
+ // generation.
+ if (bd->flags & BF_NONMOVING)
+ markQueuePushClosureGC(&gct->cap->upd_rem_set.queue, q);
return;
}
if (flags & BF_EVACUATED) {
=====================================
rts/sm/GC.c
=====================================
@@ -1701,13 +1701,8 @@ collect_gct_blocks (void)
static void
collect_pinned_object_blocks (void)
{
- generation *gen;
const bool use_nonmoving = RtsFlags.GcFlags.useNonmoving;
- if (use_nonmoving && major_gc) {
- gen = oldest_gen;
- } else {
- gen = g0;
- }
+ generation *const gen = (use_nonmoving && major_gc) ? oldest_gen : g0;
for (uint32_t n = 0; n < n_capabilities; n++) {
bdescr *last = NULL;
@@ -1732,7 +1727,7 @@ collect_pinned_object_blocks (void)
if (gen->large_objects != NULL) {
gen->large_objects->u.back = last;
}
- g0->large_objects = RELAXED_LOAD(&capabilities[n]->pinned_object_blocks);
+ gen->large_objects = RELAXED_LOAD(&capabilities[n]->pinned_object_blocks);
RELAXED_STORE(&capabilities[n]->pinned_object_blocks, NULL);
}
}
=====================================
rts/sm/NonMoving.c
=====================================
@@ -191,8 +191,8 @@ Mutex concurrent_coll_finished_lock;
* === Other references ===
*
* Apart from the design document in docs/storage/nonmoving-gc and the Ueno
- * 2016 paper (TODO citation) from which it drew inspiration, there are a
- * variety of other relevant Notes scattered throughout the tree:
+ * 2016 paper [ueno 2016] from which it drew inspiration, there are a variety
+ * of other relevant Notes scattered throughout the tree:
*
* - Note [Concurrent non-moving collection] (NonMoving.c) describes
* concurrency control of the nonmoving collector
@@ -232,6 +232,11 @@ Mutex concurrent_coll_finished_lock;
* how we use the DIRTY flags associated with MUT_VARs and TVARs to improve
* barrier efficiency.
*
+ * [ueno 2016]:
+ * Katsuhiro Ueno and Atsushi Ohori. 2016. A fully concurrent garbage
+ * collector for functional programs on multicore processors. SIGPLAN Not. 51,
+ * 9 (September 2016), 421–433. DOI:https://doi.org/10.1145/3022670.2951944
+ *
*
* Note [Concurrent non-moving collection]
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -726,7 +731,6 @@ void nonmovingStop(void)
"waiting for nonmoving collector thread to terminate");
ACQUIRE_LOCK(&concurrent_coll_finished_lock);
waitCondition(&concurrent_coll_finished, &concurrent_coll_finished_lock);
- joinOSThread(mark_thread);
}
#endif
}
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/4e41516e3a9d89ac3e7e40cbaa0d82cdcd4deaf3...e151c961266e3b4fdc966918f2cbde9aa657436a
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/4e41516e3a9d89ac3e7e40cbaa0d82cdcd4deaf3...e151c961266e3b4fdc966918f2cbde9aa657436a
You're receiving this email because of your account on gitlab.haskell.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20201126/ded173ae/attachment-0001.html>
More information about the ghc-commits
mailing list