summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBen Gamari <ben@smart-cactus.org>2020-10-31 01:12:46 -0400
committerBen Gamari <ben@smart-cactus.org>2020-11-01 13:02:11 -0500
commitef25aaa107ae099a2a9bd80d3130664334c69482 (patch)
tree05c9f64ca1d205a3619f15def80231534bc572d1
parentf7b45cde43f47f94b77411477aabdb56f8f63d66 (diff)
downloadhaskell-ef25aaa107ae099a2a9bd80d3130664334c69482.tar.gz
rts: Annotate hopefully "benign" races in freeGroup
-rw-r--r--rts/sm/BlockAlloc.c25
1 files changed, 25 insertions, 0 deletions
diff --git a/rts/sm/BlockAlloc.c b/rts/sm/BlockAlloc.c
index 9ef5e3c2d2..451c182ac3 100644
--- a/rts/sm/BlockAlloc.c
+++ b/rts/sm/BlockAlloc.c
@@ -787,6 +787,26 @@ free_mega_group (bdescr *mg)
}
+/* Note [Data races in freeGroup]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * freeGroup commits a rather serious concurrency sin in its block coalescence
+ * logic: When freeing a block it looks at bd->free of the previous/next block
+ * to see whether it is allocated. However, the free'ing thread likely does not
+ * own the previous/next block, nor do we make any attempt to synchronize with
+ * the thread that *does* own it; this makes this access a data race.
+ *
+ * The original design argued that this was correct because `bd->free` will
+ * only take a value of -1 when the block is free and thereby owned by the
+ * storage manager. However, this is nevertheless unsafe under the C11 data
+ * model, which guarantees no particular semantics for data races.
+ *
+ * We currently assume (and hope) we won't see torn values and consequently
+ * we will never see `bd->free == -1` for an allocated block which we do not
+ * own. However, this is all extremely dodgy.
+ *
+ * This is tracked as #18913.
+ */
+
void
freeGroup(bdescr *p)
{
@@ -834,6 +854,9 @@ freeGroup(bdescr *p)
{
bdescr *next;
next = p + p->blocks;
+
+ // See Note [Data races in freeGroup].
+ TSAN_ANNOTATE_BENIGN_RACE(&next->free, "freeGroup");
if (next <= LAST_BDESCR(MBLOCK_ROUND_DOWN(p))
&& RELAXED_LOAD(&next->free) == (P_)-1)
{
@@ -856,6 +879,8 @@ freeGroup(bdescr *p)
prev = p - 1;
if (prev->blocks == 0) prev = prev->link; // find the head
+ // See Note [Data races in freeGroup].
+ TSAN_ANNOTATE_BENIGN_RACE(&prev->free, "freeGroup");
if (RELAXED_LOAD(&prev->free) == (P_)-1)
{
ln = log_2(prev->blocks);