Author: Remi Meier <remi.me...@inf.ethz.ch> Branch: c8-new-page-handling Changeset: r1484:df56f7c667fa Date: 2014-10-22 17:20 +0200 http://bitbucket.org/pypy/stmgc/changeset/df56f7c667fa/
Log: try adding back some synchronization diff --git a/c8/stm/core.c b/c8/stm/core.c --- a/c8/stm/core.c +++ b/c8/stm/core.c @@ -18,6 +18,9 @@ char *src_segment_base = (from_segnum >= 0 ? get_segment_base(from_segnum) : NULL); + assert(IMPLY(from_segnum >= 0, get_priv_segment(from_segnum)->modification_lock)); + assert(STM_PSEGMENT->modification_lock); + for (; undo < end; undo++) { object_t *obj = undo->object; stm_char *oslice = ((stm_char *)obj) + SLICE_OFFSET(undo->slice); @@ -70,8 +73,8 @@ { /* looks at all bk copies of objects overlapping page 'pagenum' and copies the part in 'pagenum' back to the current segment */ - dprintf(("copy_bk_objs_in_page_from(%d, %lu, %d)\n", - from_segnum, pagenum, only_if_not_modified)); + dprintf(("copy_bk_objs_in_page_from(%d, %ld, %d)\n", + from_segnum, (long)pagenum, only_if_not_modified)); struct list_s *list = get_priv_segment(from_segnum)->modified_old_objects; struct stm_undo_s *undo = (struct stm_undo_s *)list->items; @@ -85,6 +88,7 @@ struct stm_commit_log_entry_s *from, struct stm_commit_log_entry_s *to) { + assert(STM_PSEGMENT->modification_lock); assert(from->rev_num >= to->rev_num); /* walk BACKWARDS the commit log and update the page 'pagenum', initially at revision 'from', until we reach the revision 'to'. */ @@ -150,10 +154,11 @@ assert(shared_ref_count == 1); /* make our page private */ - page_privatize_in(STM_SEGMENT->segment_num, pagenum); + page_privatize_in(my_segnum, pagenum); assert(get_page_status_in(my_segnum, pagenum) == PAGE_PRIVATE); - acquire_modified_objs_lock(copy_from_segnum); + uint64_t to_lock = (1UL << copy_from_segnum)| (1UL << my_segnum); + acquire_modification_lock_set(to_lock); pagecopy((char*)(get_virt_page_of(my_segnum, pagenum) * 4096UL), (char*)(get_virt_page_of(copy_from_segnum, pagenum) * 4096UL)); @@ -167,8 +172,6 @@ OPT_ASSERT(src_version->rev_num == most_recent_rev); target_version = STM_PSEGMENT->last_commit_log_entry; - release_modified_objs_lock(copy_from_segnum); - release_all_privatization_locks(); dprintf(("handle_segfault_in_page: rev %lu to rev %lu\n", src_version->rev_num, target_version->rev_num)); @@ -178,6 +181,9 @@ */ if (src_version->rev_num > target_version->rev_num) go_to_the_past(pagenum, src_version, target_version); + + release_modification_lock_set(to_lock); + release_all_privatization_locks(); } static void _signal_handler(int sig, siginfo_t *siginfo, void *context) @@ -245,102 +251,97 @@ static void _stm_validate(void *free_if_abort) { + dprintf(("_stm_validate(%p)\n", free_if_abort)); /* go from last known entry in commit log to the most current one and apply all changes done by other transactions. Abort if we read one of the committed objs. */ - struct stm_commit_log_entry_s *cl = STM_PSEGMENT->last_commit_log_entry; - struct stm_commit_log_entry_s *next_cl; + struct stm_commit_log_entry_s *first_cl = STM_PSEGMENT->last_commit_log_entry; + struct stm_commit_log_entry_s *next_cl, *last_cl, *cl; int my_segnum = STM_SEGMENT->segment_num; /* Don't check this 'cl'. This entry is already checked */ if (STM_PSEGMENT->transaction_state == TS_INEVITABLE) { - assert(cl->next == (void*)-1); + assert(first_cl->next == (void*)-1); return; } - /* We need this lock to prevent a segfault handler in a different thread - from seeing inconsistent data. It could also be done by carefully - ordering things, but later. */ - acquire_modified_objs_lock(my_segnum); - - /* XXXXXXXXXX: we shouldn't be able to update pages while someone else copies - from our pages (signal handler / import objs) */ - - bool needs_abort = false; - uint64_t segment_copied_from = 0; + /* Find the set of segments we need to copy from and lock them: */ + uint64_t segments_to_lock = 1UL << my_segnum; + cl = first_cl; while ((next_cl = cl->next) != NULL) { if (next_cl == (void *)-1) { - /* there is an inevitable transaction running */ - release_modified_objs_lock(my_segnum); #if STM_TESTS - needs_abort = true; - goto complete_work_and_possibly_abort; + if (free_if_abort != (void *)-1) + free(free_if_abort); + stm_abort_transaction(); #endif - abort(); /* XXX non-busy wait here - or: XXX do we always need to wait? we should just - break out of this loop and let the caller handle it - if it wants to */ - - _stm_collectable_safe_point(); - acquire_modified_objs_lock(my_segnum); - continue; + /* only go this far when validating */ + break; } assert(next_cl->rev_num > cl->rev_num); cl = next_cl; - /*int srcsegnum = cl->segment_num; - OPT_ASSERT(srcsegnum >= 0 && srcsegnum < NB_SEGMENTS);*/ + if (cl->written_count) { + segments_to_lock |= (1UL << cl->segment_num); + } + } + last_cl = cl; + acquire_modification_lock_set(segments_to_lock); - if (!needs_abort) { - struct stm_undo_s *undo = cl->written; - struct stm_undo_s *end = cl->written + cl->written_count; - for (; undo < end; undo++) { - if (_stm_was_read(undo->object)) { - /* first reset all modified objects from the backup - copies as soon as the first conflict is detected; - then we will proceed below to update our segment from - the old (but unmodified) version to the newer version. - */ - release_modified_objs_lock(my_segnum); - reset_modified_from_backup_copies(my_segnum); - acquire_modified_objs_lock(my_segnum); - needs_abort = true; - break; + + /* import objects from first_cl to last_cl: */ + bool needs_abort = false; + if (first_cl != last_cl) { + uint64_t segment_really_copied_from = 0UL; + + cl = first_cl; + while ((cl = cl->next) != NULL) { + if (!needs_abort) { + struct stm_undo_s *undo = cl->written; + struct stm_undo_s *end = cl->written + cl->written_count; + for (; undo < end; undo++) { + if (_stm_was_read(undo->object)) { + /* first reset all modified objects from the backup + copies as soon as the first conflict is detected; + then we will proceed below to update our segment from + the old (but unmodified) version to the newer version. + */ + reset_modified_from_backup_copies(my_segnum); + needs_abort = true; + break; + } } } + + if (cl->written_count) { + struct stm_undo_s *undo = cl->written; + struct stm_undo_s *end = cl->written + cl->written_count; + + segment_really_copied_from |= (1UL << cl->segment_num); + import_objects(cl->segment_num, -1, undo, end); + } + + /* last fully validated entry */ + STM_PSEGMENT->last_commit_log_entry = cl; } - if (cl->written_count) { - struct stm_undo_s *undo = cl->written; - struct stm_undo_s *end = cl->written + cl->written_count; - - segment_copied_from |= (1UL << cl->segment_num); - import_objects(cl->segment_num, -1, undo, end); + OPT_ASSERT(segment_really_copied_from < (1 << NB_SEGMENTS)); + int segnum; + for (segnum = 0; segnum < NB_SEGMENTS; segnum++) { + if (segment_really_copied_from & (1UL << segnum)) { + /* here we can actually have our own modified version, so + make sure to only copy things that are not modified in our + segment... (if we do not abort) */ + copy_bk_objs_in_page_from( + segnum, -1, /* any page */ + !needs_abort); /* if we abort, we still want to copy everything */ + } } - - /* last fully validated entry */ - STM_PSEGMENT->last_commit_log_entry = cl; } - release_modified_objs_lock(my_segnum); - -#if STM_TESTS - complete_work_and_possibly_abort:; -#endif - /* XXXXXXXXXXXXXXXX CORRECT LOCKING NEEDED XXXXXXXXXXXXXXXXXXXXXX */ - int segnum; - for (segnum = 0; segment_copied_from != 0; segnum++) { - if (segment_copied_from & (1UL << segnum)) { - segment_copied_from &= ~(1UL << segnum); - /* here we can actually have our own modified version, so - make sure to only copy things that are not modified in our - segment... (if we do not abort) */ - copy_bk_objs_in_page_from( - segnum, -1, /* any page */ - !needs_abort); /* if we abort, we still want to copy everything */ - } - } + /* done with modifications */ + release_modification_lock_set(segments_to_lock); if (needs_abort) { if (free_if_abort != (void *)-1) @@ -411,10 +412,10 @@ _validate_and_attach(new); } - acquire_modified_objs_lock(STM_SEGMENT->segment_num); + acquire_modification_lock(STM_SEGMENT->segment_num); list_clear(STM_PSEGMENT->modified_old_objects); STM_PSEGMENT->last_commit_log_entry = new; - release_modified_objs_lock(STM_SEGMENT->segment_num); + release_modification_lock(STM_SEGMENT->segment_num); } /* ############# STM ############# */ @@ -516,7 +517,7 @@ if 'obj' is merely an overflow object. FIX ME, likely by copying the overflow number logic from c7. */ - acquire_modified_objs_lock(STM_SEGMENT->segment_num); + acquire_modification_lock(STM_SEGMENT->segment_num); uintptr_t slice_sz; uintptr_t in_page_offset = (uintptr_t)obj % 4096UL; uintptr_t remaining_obj_sz = obj_size; @@ -543,7 +544,7 @@ } OPT_ASSERT(remaining_obj_sz == 0); - release_modified_objs_lock(STM_SEGMENT->segment_num); + release_modification_lock(STM_SEGMENT->segment_num); /* done fiddling with protection and privatization */ release_all_privatization_locks(); @@ -734,7 +735,7 @@ #pragma push_macro("STM_SEGMENT") #undef STM_PSEGMENT #undef STM_SEGMENT - acquire_modified_objs_lock(segment_num); + assert(get_priv_segment(segment_num)->modification_lock); struct stm_priv_segment_info_s *pseg = get_priv_segment(segment_num); struct list_s *list = pseg->modified_old_objects; @@ -764,8 +765,6 @@ check_all_write_barrier_flags(pseg->pub.segment_base, list); list_clear(list); - - release_modified_objs_lock(segment_num); #pragma pop_macro("STM_SEGMENT") #pragma pop_macro("STM_PSEGMENT") } @@ -790,7 +789,9 @@ long bytes_in_nursery = throw_away_nursery(pseg); + acquire_modification_lock(segment_num); reset_modified_from_backup_copies(segment_num); + release_modification_lock(segment_num); stm_thread_local_t *tl = pseg->pub.running_thread; #ifdef STM_NO_AUTOMATIC_SETJMP diff --git a/c8/stm/core.h b/c8/stm/core.h --- a/c8/stm/core.h +++ b/c8/stm/core.h @@ -52,7 +52,10 @@ struct stm_priv_segment_info_s { struct stm_segment_info_s pub; - uint8_t modified_objs_lock; + /* lock protecting from concurrent modification of + 'modified_old_objects', page-revision-changes, ... + Always acquired in global order of segments to avoid deadlocks. */ + uint8_t modification_lock; /* All the old objects (older than the current transaction) that the current transaction attempts to modify. This is used to @@ -190,17 +193,6 @@ spinlock_release(get_priv_segment(segnum)->privatization_lock); } -static inline void acquire_modified_objs_lock(int segnum) -{ - spinlock_acquire(get_priv_segment(segnum)->modified_objs_lock); -} - -static inline void release_modified_objs_lock(int segnum) -{ - spinlock_release(get_priv_segment(segnum)->modified_objs_lock); -} - - static inline bool all_privatization_locks_acquired() { #ifndef NDEBUG @@ -230,3 +222,53 @@ release_privatization_lock(l); } } + + + +/* Modification locks are used to prevent copying from a segment + where either the revision of some pages is inconsistent with the + rest, or the modified_old_objects list is being modified (bk_copys). + + Lock ordering: acquire privatization lock around acquiring a set + of modification locks! +*/ + +static inline void acquire_modification_lock(int segnum) +{ + spinlock_acquire(get_priv_segment(segnum)->modification_lock); +} + +static inline void release_modification_lock(int segnum) +{ + spinlock_release(get_priv_segment(segnum)->modification_lock); +} + +static inline void acquire_modification_lock_set(uint64_t seg_set) +{ + assert(NB_SEGMENTS <= 64); + OPT_ASSERT(seg_set < (1 << NB_SEGMENTS)); + + /* acquire locks in global order */ + int i; + for (i = 0; i < NB_SEGMENTS; i++) { + if ((seg_set & (1 << i)) == 0) + continue; + + spinlock_acquire(get_priv_segment(i)->modification_lock); + } +} + +static inline void release_modification_lock_set(uint64_t seg_set) +{ + assert(NB_SEGMENTS <= 64); + OPT_ASSERT(seg_set < (1 << NB_SEGMENTS)); + + int i; + for (i = 0; i < NB_SEGMENTS; i++) { + if ((seg_set & (1 << i)) == 0) + continue; + + assert(get_priv_segment(i)->modification_lock); + spinlock_release(get_priv_segment(i)->modification_lock); + } +} diff --git a/c8/test/support.py b/c8/test/support.py --- a/c8/test/support.py +++ b/c8/test/support.py @@ -285,7 +285,7 @@ ], undef_macros=['NDEBUG'], include_dirs=[parent_dir], - extra_compile_args=['-g', '-O0', '-Wall', '-ferror-limit=1'], + extra_compile_args=['-g', '-O0', '-Wall', '-Werror', '-ferror-limit=1'], extra_link_args=['-g', '-lrt'], force_generic_engine=True) _______________________________________________ pypy-commit mailing list pypy-commit@python.org https://mail.python.org/mailman/listinfo/pypy-commit