Author: Remi Meier <remi.me...@inf.ethz.ch>
Branch: c8-reshare-pages
Changeset: r1917:bc807f9a977e
Date: 2015-08-07 11:02 +0200
http://bitbucket.org/pypy/stmgc/changeset/bc807f9a977e/

Log:    more fixes everywhere

diff --git a/c8/stm/core.c b/c8/stm/core.c
--- a/c8/stm/core.c
+++ b/c8/stm/core.c
@@ -63,9 +63,10 @@
         stm_char *oslice = ((stm_char *)obj) + SLICE_OFFSET(undo->slice);
         uintptr_t current_page_num = ((uintptr_t)oslice) / 4096;
 
+        /* never import anything into READONLY pages */
+        assert(get_page_status_in(my_segnum, current_page_num) != 
PAGE_READONLY);
+
         if (pagenum == -1) {
-            /* XXX: what about PAGE_READONLY? check that the below line handles
-               this case correctly */
             if (get_page_status_in(my_segnum, current_page_num) != 
PAGE_ACCESSIBLE)
                 continue;
         } else if (pagenum != current_page_num) {
@@ -170,15 +171,13 @@
     assert(page_status == PAGE_NO_ACCESS
            || page_status == PAGE_READONLY);
 
-    /* make our page write-ready */
-    page_mark_accessible(my_segnum, pagenum);
+    if (page_status == PAGE_READONLY) {
+        /* make our page write-ready */
+        page_mark_accessible(my_segnum, pagenum);
 
-    if (page_status == PAGE_READONLY) {
-        /* copy from seg0 (XXX: can we depend on copy-on-write
-           of the kernel somehow?) */
-        pagecopy(get_virtual_page(my_segnum, pagenum),
-                 get_virtual_page(0, pagenum));
-
+        dprintf((" > found READONLY, make others NO_ACCESS\n"));
+        /* our READONLY copy *has* to have the current data, no
+           copy necessary */
         /* make READONLY pages in other segments NO_ACCESS */
         for (i = 1; i < NB_SEGMENTS; i++) {
             if (i == my_segnum)
@@ -193,12 +192,19 @@
     }
 
     /* find who has the most recent revision of our page */
+    /* XXX: uh, *more* recent would be enough, right? */
     int copy_from_segnum = -1;
     uint64_t most_recent_rev = 0;
+    bool was_readonly = false;
     for (i = 1; i < NB_SEGMENTS; i++) {
         if (i == my_segnum)
             continue;
 
+        if (!was_readonly && get_page_status_in(i, pagenum) == PAGE_READONLY) {
+            was_readonly = true;
+            break;
+        }
+
         struct stm_commit_log_entry_s *log_entry;
         log_entry = get_priv_segment(i)->last_commit_log_entry;
         if (get_page_status_in(i, pagenum) != PAGE_NO_ACCESS
@@ -209,10 +215,29 @@
     }
     OPT_ASSERT(copy_from_segnum != my_segnum);
 
+    if (was_readonly) {
+        assert(page_status == PAGE_NO_ACCESS);
+        /* this case could be avoided by making all NO_ACCESS to READONLY
+           when resharing pages (XXX: better?).
+           We may go from NO_ACCESS->READONLY->ACCESSIBLE on write with
+           2 SIGSEGV in a row.*/
+        dprintf((" > make a previously NO_ACCESS page READONLY\n"));
+        page_mark_readonly(my_segnum, pagenum);
+
+        release_all_privatization_locks();
+        return;
+    }
+
+    /* make our page write-ready */
+    page_mark_accessible(my_segnum, pagenum);
+
     /* account for this page now: XXX */
     /* increment_total_allocated(4096); */
 
+
     if (copy_from_segnum == -1) {
+        dprintf((" > found newly allocated page: copy from seg0\n"));
+
         /* this page is only accessible in the sharing segment seg0 so far (new
            allocation). We can thus simply mark it accessible here. */
         pagecopy(get_virtual_page(my_segnum, pagenum),
@@ -221,6 +246,8 @@
         return;
     }
 
+    dprintf((" > import data from seg %d\n", copy_from_segnum));
+
     /* before copying anything, acquire modification locks from our and
        the other segment */
     uint64_t to_lock = (1UL << copy_from_segnum);
diff --git a/c8/stm/gcpage.c b/c8/stm/gcpage.c
--- a/c8/stm/gcpage.c
+++ b/c8/stm/gcpage.c
@@ -227,10 +227,12 @@
     if (get_page_status(pagenum)->by_segment == (PAGE_ACCESSIBLE << 0))
         return;                 /* only accessible in seg0 */
 
+    /* XXX: fast-path for all pages=READONLY */
+
     long i;
     for (i = 1; i < NB_SEGMENTS; i++){
         if (get_page_status_in(i, pagenum) == PAGE_ACCESSIBLE) {
-            /* XXX: also do for PAGE_NO_ACCESS */
+            /* XXX: also do for PAGE_NO_ACCESS? */
             page_mark_readonly(i, pagenum);
         }
     }
@@ -266,10 +268,8 @@
         }
     }
 
-    /* XXXXXXXXXXXXXXX: necessary? */
-    msync(stm_object_pages + END_NURSERY_PAGE*4096,
-          NB_SHARED_PAGES*4096,
-          MS_SYNC);             /* MS_INVALIDATE| */
+    /* msync() done when validating seg0 */
+
     /* Now loop over all pages and re-share them if possible. */
     uintptr_t pagenum, endpagenum;
     pagenum = END_NURSERY_PAGE;   /* starts after the nursery */
diff --git a/c8/stm/nursery.c b/c8/stm/nursery.c
--- a/c8/stm/nursery.c
+++ b/c8/stm/nursery.c
@@ -769,6 +769,14 @@
         }
     }
 
+    /* XXXXXXX: necessary?? AFAIK it is undefined if changes
+       to the MAP_SHARED in seg0 propagate to the READONLY
+       MAP_SHARED/MAP_PRIVATE in other segments
+       TODO: only used pages */
+    msync(stm_object_pages + END_NURSERY_PAGE*4096,
+          NB_SHARED_PAGES*4096UL,
+          MS_INVALIDATE|MS_SYNC);
+
     ensure_gs_register(original_num);
 }
 
diff --git a/c8/stm/pages.c b/c8/stm/pages.c
--- a/c8/stm/pages.c
+++ b/c8/stm/pages.c
@@ -71,26 +71,15 @@
 
     dprintf(("page_mark_accessible(%lu) in seg:%ld\n", pagenum, segnum));
 
-    if (page_status == PAGE_NO_ACCESS) {
-        if (mprotect(get_virtual_page(segnum, pagenum), 4096, PROT_READ | 
PROT_WRITE)) {
-            perror("mprotect");
-            stm_fatalerror("mprotect failed! Consider running 'sysctl 
vm.max_map_count=16777216'");
-        }
-    } else {
-        /* PAGE_READONLY requires renewing the mmap to MAP_PRIVATE */
-        char *addr = get_virtual_page(segnum, pagenum);
-        char *res = mmap(addr, 4096UL,
-                         PROT_READ|PROT_WRITE,
-                         MAP_PRIVATE|MAP_NORESERVE|MAP_ANONYMOUS|MAP_FIXED,
-                         -1, 0);
-        if (res == MAP_FAILED)
-            stm_fatalerror("%s failed (mmap): %m", "page_mark_accessible");
+    if (mprotect(get_virtual_page(segnum, pagenum), 4096, PROT_READ | 
PROT_WRITE)) {
+        perror("mprotect");
+        stm_fatalerror("mprotect failed! Consider running 'sysctl 
vm.max_map_count=16777216'");
     }
 
     /* set this flag *after* we un-protected it, because XXX later */
     set_page_status_in(segnum, pagenum, PAGE_ACCESSIBLE);
     set_hint_modified_recently(pagenum);
-    dprintf(("RW(seg%ld, page%lu)\n", segnum, pagenum));
+    dprintf(("RW(seg%ld, page %lu)\n", segnum, pagenum));
 }
 
 static void page_mark_inaccessible(long segnum, uintptr_t pagenum)
@@ -102,7 +91,7 @@
 
     set_page_status_in(segnum, pagenum, PAGE_NO_ACCESS);
 
-    dprintf(("NONE(seg%ld, page%lu)\n", segnum, pagenum));
+    dprintf(("NONE(seg%ld, page %lu)\n", segnum, pagenum));
     char *addr = get_virtual_page(segnum, pagenum);
     madvise(addr, 4096, MADV_DONTNEED);
     if (mprotect(addr, 4096, PROT_NONE)) {
@@ -115,13 +104,16 @@
 static void page_mark_readonly(long segnum, uintptr_t pagenum)
 {
     /* mark readonly and share with seg0 */
-    assert(_has_mutex());
-    assert(segnum > 0 && get_page_status_in(segnum, pagenum) == 
PAGE_ACCESSIBLE);
+    assert(segnum > 0 &&
+           (get_page_status_in(segnum, pagenum) == PAGE_ACCESSIBLE
+            || get_page_status_in(segnum, pagenum) == PAGE_NO_ACCESS));
     dprintf(("page_mark_readonly(%lu) in seg:%ld\n", pagenum, segnum));
 
     char *virt_addr = get_virtual_page(segnum, pagenum);
     madvise(virt_addr, 4096UL, MADV_DONTNEED);
-    /* XXX: does it matter if SHARED or PRIVATE? */
+    /* XXX: does it matter if SHARED or PRIVATE?
+       IF MAP_SHARED, make sure page_mark_accessible doesn't simply mprotect() 
but also
+       mmap() as MAP_PRIVATE */
     char *res = mmap(virt_addr, 4096UL,
                      PROT_READ,
                      MAP_PRIVATE|MAP_FIXED|MAP_NORESERVE,
@@ -132,5 +124,5 @@
 
     set_page_status_in(segnum, pagenum, PAGE_READONLY);
 
-    dprintf(("RO(seg%ld, page%lu)\n", segnum, pagenum));
+    dprintf(("RO(seg%ld, page %lu)\n", segnum, pagenum));
 }
diff --git a/c8/stm/smallmalloc.c b/c8/stm/smallmalloc.c
--- a/c8/stm/smallmalloc.c
+++ b/c8/stm/smallmalloc.c
@@ -305,7 +305,7 @@
            inaccessible from all other segments again (except seg0) */
         uintptr_t page = (baseptr - stm_object_pages) / 4096UL;
         for (i = 1; i < NB_SEGMENTS; i++) {
-            if (get_page_status_in(i, page) == PAGE_ACCESSIBLE)
+            if (get_page_status_in(i, page) != PAGE_NO_ACCESS)
                 page_mark_inaccessible(i, page);
         }
 
diff --git a/c8/test/support.py b/c8/test/support.py
--- a/c8/test/support.py
+++ b/c8/test/support.py
@@ -760,7 +760,7 @@
     return lib._stm_get_page_status(pagenum)
 
 def stm_is_accessible_page(pagenum):
-    return stm_get_page_status() == PAGE_ACCESSIBLE
+    return stm_get_page_status(pagenum) == PAGE_ACCESSIBLE
 
 def stm_get_hint_modified_recently(pagenum):
     return lib._stm_get_hint_modified_recently(pagenum)
diff --git a/c8/test/test_resharing.py b/c8/test/test_resharing.py
--- a/c8/test/test_resharing.py
+++ b/c8/test/test_resharing.py
@@ -79,8 +79,8 @@
         self.commit_transaction()
         p1 = stm_get_obj_pages(lp1)[0]
         p2 = stm_get_obj_pages(lp2)[0]
+
         self.switch(1)
-
         self.start_transaction()
         # NO_ACCESS stays NO_ACCESS
         assert stm_get_page_status(p1) == PAGE_NO_ACCESS
@@ -97,5 +97,93 @@
         assert stm_get_page_status(p1) == PAGE_READONLY
         assert stm_get_page_status(p2) == PAGE_READONLY
 
+        self.switch(3)
+        self.start_transaction()
+        assert stm_get_page_status(p1) == PAGE_NO_ACCESS
+        assert stm_get_page_status(p2) == PAGE_NO_ACCESS
         stm_set_char(lp1, 'a')
         stm_set_char(lp2, 'b')
+        # NO_ACCESS becomes ACCESSIBLE in this segment
+        assert stm_get_page_status(p1) == PAGE_ACCESSIBLE
+        assert stm_get_page_status(p2) == PAGE_ACCESSIBLE
+
+        self.switch(0)
+        # INACCESSIBLE in others
+        assert stm_get_page_status(p1) == PAGE_NO_ACCESS
+        assert stm_get_page_status(p2) == PAGE_NO_ACCESS
+
+        self.switch(1)
+        # others
+        assert stm_get_page_status(p1) == PAGE_NO_ACCESS
+        assert stm_get_page_status(p2) == PAGE_NO_ACCESS
+
+
+
+
+    def test_resharing_more(self):
+        self.start_transaction()
+        lp1 = stm_allocate(16)
+        big = GC_LAST_SMALL_SIZE+64
+        lp2 = stm_allocate(big)
+        self.push_roots([lp1, lp2])
+        stm_minor_collect()
+        lp1, lp2 = self.pop_roots()
+        self.push_roots([lp1, lp2])
+        self.commit_transaction()
+        p1 = stm_get_obj_pages(lp1)[0]
+        p2 = stm_get_obj_pages(lp2)[0]
+
+        self.switch(1)
+        self.start_transaction()
+        # ACCESSIBLE becomes RO
+        stm_get_char(lp1)
+        stm_get_char(lp2)
+        assert stm_get_page_status(p1) == PAGE_ACCESSIBLE
+        assert stm_get_page_status(p2) == PAGE_ACCESSIBLE
+        stm_major_collect()
+        stm_major_collect()
+        stm_major_collect()
+        assert stm_get_page_status(p1) == PAGE_READONLY
+        assert stm_get_page_status(p2) == PAGE_READONLY
+
+        self.switch(0)
+        # ACCESSIBLE becomes READONLY here too
+        self.start_transaction()
+        assert stm_get_page_status(p1) == PAGE_READONLY
+        assert stm_get_page_status(p2) == PAGE_READONLY
+
+        # RO|RO|NO|NO
+
+        self.switch(2)
+        self.start_transaction()
+        # just reading makes NO_ACCESS become READONLY
+        assert stm_get_page_status(p1) == PAGE_NO_ACCESS
+        assert stm_get_page_status(p2) == PAGE_NO_ACCESS
+        stm_get_char(lp1)
+        stm_get_char(lp2)
+        assert stm_get_page_status(p1) == PAGE_READONLY
+        assert stm_get_page_status(p2) == PAGE_READONLY
+
+        # RO|RO|RO|NO
+
+        self.switch(3)
+        self.start_transaction()
+        # still INACCESSIBLE in others
+        assert stm_get_page_status(p1) == PAGE_NO_ACCESS
+        assert stm_get_page_status(p2) == PAGE_NO_ACCESS
+
+        self.switch(2)
+        # writing makes READONLY->ACCESSIBLE
+        stm_set_char(lp1, 'a')
+        stm_set_char(lp2, 'b')
+        assert stm_get_page_status(p1) == PAGE_ACCESSIBLE
+        assert stm_get_page_status(p2) == PAGE_ACCESSIBLE
+
+        self.switch(1)
+        # others go from RO->NO_ACCESS
+        assert stm_get_page_status(p1) == PAGE_NO_ACCESS
+        assert stm_get_page_status(p2) == PAGE_NO_ACCESS
+        stm_set_char(lp1, 'a')
+        stm_set_char(lp2, 'b')
+        assert stm_get_page_status(p1) == PAGE_ACCESSIBLE
+        assert stm_get_page_status(p2) == PAGE_ACCESSIBLE
_______________________________________________
pypy-commit mailing list
pypy-commit@python.org
https://mail.python.org/mailman/listinfo/pypy-commit

Reply via email to