From 88e63d7d036e23e30f1a195cb5b4bd207aecf1a9 Mon Sep 17 00:00:00 2001
From: Douglas Katzman <dougk@google.com>
Date: Tue, 21 Nov 2017 08:34:33 -0500
Subject: [PATCH] Don't use a mutex in page fault handler

Technically pthread_mutex_lock isn't allowed, but it worked for us
since we don't have a lock ordering problem with free_pages_lock.
Better to avoid it nonetheless.
---
 src/runtime/gc-private.h      |  5 +++--
 src/runtime/gencgc-internal.h |  7 +++++++
 src/runtime/gencgc.c          | 21 ++++++++++++++-------
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/runtime/gc-private.h b/src/runtime/gc-private.h
index 88b59e35e..4908f8137 100644
--- a/src/runtime/gc-private.h
+++ b/src/runtime/gc-private.h
@@ -232,8 +232,9 @@ static inline boolean layout_bitmap_logbitp(int index, lispobj bitmap)
 static inline void unprotect_page_index(page_index_t page_index)
 {
     os_protect(page_address(page_index), GENCGC_CARD_BYTES, OS_VM_PROT_ALL);
-    page_table[page_index].write_protected_cleared = 1;
-    page_table[page_index].write_protected = 0;
+    unsigned char *pflagbits = (unsigned char*)&page_table[page_index].gen - 1;
+    __sync_fetch_and_or(pflagbits, WP_CLEARED_BIT);
+    __sync_fetch_and_and(pflagbits, ~WRITE_PROTECTED_BIT);
 }
 
 static inline void protect_page(void* page_addr, page_index_t page_index)
diff --git a/src/runtime/gencgc-internal.h b/src/runtime/gencgc-internal.h
index 46f1144eb..13fea7062 100644
--- a/src/runtime/gencgc-internal.h
+++ b/src/runtime/gencgc-internal.h
@@ -139,6 +139,13 @@ struct page {
     generation_index_t gen;
 };
 extern struct page *page_table;
+#ifdef LISP_FEATURE_BIG_ENDIAN
+#define WRITE_PROTECTED_BIT (1<<4)
+#define WP_CLEARED_BIT (1<<3)
+#else
+#define WRITE_PROTECTED_BIT (1<<3)
+#define WP_CLEARED_BIT (1<<4)
+#endif
 
 struct __attribute__((packed)) corefile_pte {
   uword_t sso; // scan start offset
diff --git a/src/runtime/gencgc.c b/src/runtime/gencgc.c
index 6ab0a8327..e6cef4ea6 100644
--- a/src/runtime/gencgc.c
+++ b/src/runtime/gencgc.c
@@ -3943,6 +3943,16 @@ gc_init(void)
 #if defined(LISP_FEATURE_SB_SAFEPOINT)
     alloc_gc_page();
 #endif
+    // Verify that foo_BIT constants agree with the C compiler's bit packing
+    // and that we can compute the correct adddress of the bitfields.
+    // These tests can be optimized out of the emitted code by a good compiler.
+    struct page test;
+    unsigned char *pflagbits = (unsigned char*)&test.gen - 1;
+    memset(&test, 0, sizeof test);
+    *pflagbits = WRITE_PROTECTED_BIT;
+    gc_assert(test.write_protected);
+    *pflagbits = WP_CLEARED_BIT;
+    gc_assert(test.write_protected_cleared);
 }
 
 void gc_allocate_ptes()
@@ -4307,10 +4317,9 @@ gencgc_handle_wp_violation(void* fault_addr)
         return 0;
 
     } else {
-        int ret;
-        ret = thread_mutex_lock(&free_pages_lock);
-        gc_assert(ret == 0);
-        if (page_table[page_index].write_protected) {
+        unsigned char *pflagbits = (unsigned char*)&page_table[page_index].gen - 1;
+        unsigned char flagbits = __sync_fetch_and_add(pflagbits, 0);
+        if (flagbits & WRITE_PROTECTED_BIT) {
             unprotect_page_index(page_index);
         } else if (!ignore_memoryfaults_on_unprotected_pages) {
             /* The only acceptable reason for this signal on a heap
@@ -4319,7 +4328,7 @@ gencgc_handle_wp_violation(void* fault_addr)
              * we had better not have the second one lose here if it
              * does this test after the first one has already set wp=0
              */
-            if(page_table[page_index].write_protected_cleared != 1) {
+            if (!(flagbits & WP_CLEARED_BIT)) {
                 void lisp_backtrace(int frames);
                 lisp_backtrace(10);
                 fprintf(stderr,
@@ -4346,8 +4355,6 @@ gencgc_handle_wp_violation(void* fault_addr)
                     lose("Feh.\n");
             }
         }
-        ret = thread_mutex_unlock(&free_pages_lock);
-        gc_assert(ret == 0);
         /* Don't worry, we can handle it. */
         return 1;
     }
-- 
2.11.4.GIT