avoid interferences among places for memory accounting

All places uses the same accounting bit for objects
that are in the shared space. Each place also flips
the bit value it wants on each accounting, so if two
places are accounting at the same time with opposite
bit values and can reach the same objects, they can
interefere. It's even possible for them to race
through cycles and cause each other to loop forever.

Add a lock to ensure that there's only one bit value
in play for the shared space at any given time. A
place must stall if other places are busy with memory
accounting and an opposite bit value.

pkgs/racket-test/tests/racket/place-chmsg-gc-acct.rkt

@@ -6,7 +6,6 @@
    pch
 
    ;; Trigger memory accounting in every place:
-   #;
    (custodian-limit-memory
     (current-custodian)
     (* 1024 1024 1024))

racket/src/racket/gc2/mem_account.c

@@ -268,15 +268,85 @@ inline static uintptr_t custodian_usage(NewGC*gc, void *custodian)
   return gcWORDS_TO_BYTES(retval);
 }
 
+#ifdef MZ_USE_PLACES
+
+static mzrt_mutex *master_btc_lock;
+static mzrt_sema *master_btc_sema;
+static int master_btc_lock_count = 0;
+static int master_btc_lock_waiters = 0;
+
+void init_master_btc_locks()
+{
+  mzrt_mutex_create(&master_btc_lock);
+  mzrt_sema_create(&master_btc_sema, 0);
+}
+
+static void check_master_btc_mark(NewGC *gc, mpage *page)
+{
+  if (!gc->master_page_btc_mark_checked) {
+    int pause = 1;
+    RELEASE_PAGE_LOCK(1, page);
+    while (pause) {
+      mzrt_mutex_lock(master_btc_lock);
+      if (master_btc_lock_count
+          && (gc->new_btc_mark != MASTERGC->new_btc_mark)) {
+        pause = 1;
+        master_btc_lock_waiters++;
+      } else {
+        pause = 0;
+        MASTERGC->new_btc_mark = gc->new_btc_mark;
+        master_btc_lock_count++;
+      }
+      mzrt_mutex_unlock(master_btc_lock);
+
+      if (pause)
+        mzrt_sema_wait(master_btc_sema);
+    }
+    TAKE_PAGE_LOCK(1, page);
+    gc->master_page_btc_mark_checked = 1;
+  }
+}
+
+static void release_master_btc_mark(NewGC *gc)
+{
+  if (gc->master_page_btc_mark_checked) {
+    /* release the lock on the master's new_btc_mark value */
+    mzrt_mutex_lock(master_btc_lock);
+    --master_btc_lock_count;
+    if (!master_btc_lock_count && master_btc_lock_waiters) {
+      --master_btc_lock_waiters;
+      mzrt_sema_post(master_btc_sema);
+    }
+    mzrt_mutex_unlock(master_btc_lock);
+  }
+}
+
+#else
+
+static void check_master_btc_mark(NewGC *gc) { }
+static void release_master_btc_mark(NewGC *gc) { }
+
+#endif
+
 inline static void BTC_memory_account_mark(NewGC *gc, mpage *page, void *ptr, int is_a_master_page)
 {
   GCDEBUG((DEBUGOUTF, "BTC_memory_account_mark: %p/%p\n", page, ptr));
 
   /* In the case of is_a_master_page, whether this place is charged is
-     a little random: there's no guarantee that the btc_mark values are
-     in sync, and there are races among places. Approximations are ok for
-     accounting, though, as long as the probably for completely wrong
-     accounting is very low. */
+     a little random: there's no guarantee that the btc_mark values
+     are in sync, and there are races among places. Approximations are
+     ok for accounting, though, as long as the probably for completely
+     wrong accounting is very low.
+
+     At the same time, we need to synchronize enough so that two
+     places with different new_btc_mark values don't send each other
+     into infinite loops (with the btc_mark value bouncing back and
+     forth) or overcounting. We synchronize enough by having a single
+     new_btc_mark value for master pages, and we stall if the value
+     isn't what this place wants. */
+
+  if (is_a_master_page)
+    check_master_btc_mark(gc, page);
 
   if(page->size_class) {
     if(page->size_class > 1) {
@@ -427,6 +497,7 @@ static void BTC_do_accounting(NewGC *gc)
     gc->doing_memory_accounting = 1;
     gc->in_unsafe_allocation_mode = 1;
     gc->unsafe_allocation_abort = btc_overmem_abort;
+    gc->master_page_btc_mark_checked = 0;
 
     /* clear the memory use numbers out */
     for(i = 1; i < table_size; i++)
@@ -468,6 +539,8 @@ static void BTC_do_accounting(NewGC *gc)
       gc->phantom_count = save_count;
     }
 
+    release_master_btc_mark(gc);
+
     /* walk backward folding totals int parent */
     cur = last;
     while (cur) {

racket/src/racket/gc2/newgc.c

@@ -178,6 +178,20 @@ struct Log_Master_Info {
 # define PLACES_AND(v) 0
 #endif
 
+#ifdef MZ_USE_PLACES
+static void adjust_page_lock(int is_a_master_page, mpage *page, intptr_t prev, intptr_t next)
+{
+  if (is_a_master_page) {
+    while (!mzrt_cas(&page->page_lock, prev, next)) { /* spin! */ }
+  }
+}
+# define TAKE_PAGE_LOCK(is_a_master_page, page) adjust_page_lock(is_a_master_page, page, 0, 1);
+# define RELEASE_PAGE_LOCK(is_a_master_page, page) adjust_page_lock(is_a_master_page, page, 1, 0);
+#else
+# define TAKE_PAGE_LOCK(is_a_master_page, page) /* empty */
+# define RELEASE_PAGE_LOCK(is_a_master_page, page) /* empty */
+#endif
+
 inline static size_t real_page_size(mpage* page);
 inline static int page_mmu_type(mpage *page);
 inline static int page_mmu_protectable(mpage *page);
@@ -3137,6 +3151,9 @@ void GC_init_type_tags(int count, int pair, int mutable_pair, int weakbox, int e
     initialized = 1;
     init_type_tags_worker(NULL, NULL, count, pair, mutable_pair, weakbox, ephemeron, weakarray, 
                           custbox, phantom);
+#if defined(MZ_USE_PLACES) && defined(NEWGC_BTC_ACCOUNT)
+    init_master_btc_locks();
+#endif
   } else {
     GCPRINT(GCOUTF, "GC_init_type_tags should only be called once!\n");
     abort();
@@ -3406,20 +3423,6 @@ static void mark_recur_or_push_ptr(struct NewGC *gc, void *p, int is_a_master_pa
   push_ptr(gc, p);
 }
 
-#ifdef MZ_USE_PLACES
-static void adjust_page_lock(int is_a_master_page, mpage *page, intptr_t prev, intptr_t next)
-{
-  if (is_a_master_page) {
-    while (!mzrt_cas(&page->page_lock, prev, next)) { /* spin! */ }
-  }
-}
-# define TAKE_PAGE_LOCK(is_a_master_page, page) adjust_page_lock(is_a_master_page, page, 0, 1);
-# define RELEASE_PAGE_LOCK(is_a_master_page, page) adjust_page_lock(is_a_master_page, page, 1, 0);
-#else
-# define TAKE_PAGE_LOCK(is_a_master_page, page) /* empty */
-# define RELEASE_PAGE_LOCK(is_a_master_page, page) /* empty */
-#endif
-
 /* This is the first mark routine. It's a bit complicated. */
 void GC_mark2(const void *const_p, struct NewGC *gc)
 {

racket/src/racket/gc2/newgc.h

@@ -184,6 +184,7 @@ typedef struct NewGC {
   OTEntry **owner_table;
   unsigned int owner_table_size;
   AccountHook *hooks;
+  int master_page_btc_mark_checked;
 
   uintptr_t number_of_gc_runs;
   unsigned int since_last_full;