From cdc201f402616f89d478d868b44c8ff1b7462e55 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Wed, 26 Sep 2018 10:12:06 +0300 Subject: [PATCH] Use AO_or in async_set_pht_entry_from_index if available * blacklst.c (backlist_set_pht_entry_from_index): New macro; move comment from gc_priv.h. * blacklst.c (GC_add_to_black_list_normal, GC_add_to_black_list_stack): Use backlist_set_pht_entry_from_index() instead of set_pht_entry_from_index_concurrent(). * include/private/gc_priv.h (set_pht_entry_from_index): Add outermost parentheses; cast result to void. * include/private/gc_priv.h [THREADS] (set_pht_entry_from_index_concurrent): Define only if AO_HAVE_or; update comment. * include/private/gc_priv.h [THREADS] (GC_acquire_dirty_lock, GC_release_dirty_lock): Define to no-op if set_pht_entry_from_index_concurrent is defined. * include/private/gc_priv.h [THREADS && set_pht_entry_from_index_concurrent] (GC_fault_handler_lock): Do not declare. * os_dep.c [!GC_DISABLE_INCREMENTAL && THREADS && set_pht_entry_from_index_concurrent] (async_set_pht_entry_from_index): Redirect to set_pht_entry_from_index_concurrent; do not define GC_fault_handler_lock variable. * os_dep.c [!GC_DISABLE_INCREMENTAL && THREADS && !set_pht_entry_from_index_concurrent && AO_HAVE_test_and_set_acquire] (async_set_pht_entry_from_index): Remove GC_ATTR_NO_SANITIZE_THREAD (because AO_HAVE_or should be defined in case of TSan). --- blacklst.c | 16 ++++++++++++++-- include/private/gc_priv.h | 19 ++++++++----------- os_dep.c | 4 +++- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/blacklst.c b/blacklst.c index 452a340..31511ae 100644 --- a/blacklst.c +++ b/blacklst.c @@ -175,6 +175,18 @@ GC_INNER void GC_unpromote_black_lists(void) GC_copy_bl(GC_old_stack_bl, GC_incomplete_stack_bl); } +#if defined(set_pht_entry_from_index_concurrent) && defined(PARALLEL_MARK) \ + && defined(THREAD_SANITIZER) +# define backlist_set_pht_entry_from_index(db, index) \ + set_pht_entry_from_index_concurrent(db, index) +#else + /* It is safe to set a bit in a blacklist even without */ + /* synchronization, the only drawback is that we might have */ + /* to redo blacklisting sometimes. */ +# define backlist_set_pht_entry_from_index(bl, index) \ + set_pht_entry_from_index(bl, index) +#endif + /* P is not a valid pointer reference, but it falls inside */ /* the plausible heap bounds. */ /* Add it to the normal incomplete black list if appropriate. */ @@ -193,7 +205,7 @@ GC_INNER void GC_unpromote_black_lists(void) GC_print_blacklisted_ptr(p, source, "normal"); } # endif - set_pht_entry_from_index_concurrent(GC_incomplete_normal_bl, index); + backlist_set_pht_entry_from_index(GC_incomplete_normal_bl, index); } /* else this is probably just an interior pointer to an allocated */ /* object, and isn't worth black listing. */ } @@ -214,7 +226,7 @@ GC_INNER void GC_unpromote_black_lists(void) GC_print_blacklisted_ptr(p, source, "stack"); } # endif - set_pht_entry_from_index_concurrent(GC_incomplete_stack_bl, index); + backlist_set_pht_entry_from_index(GC_incomplete_stack_bl, index); } } diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 53ec41c..728e74e 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -960,20 +960,16 @@ typedef word page_hash_table[PHT_SIZE]; # define get_pht_entry_from_index(bl, index) \ (((bl)[divWORDSZ(index)] >> modWORDSZ(index)) & 1) # define set_pht_entry_from_index(bl, index) \ - (bl)[divWORDSZ(index)] |= (word)1 << modWORDSZ(index) + (void)((bl)[divWORDSZ(index)] |= (word)1 << modWORDSZ(index)) -/* And, one more version for GC_add_to_black_list_normal/stack. */ -/* The latter ones are invoked (indirectly) by GC_do_local_mark. */ -#if defined(PARALLEL_MARK) && defined(THREAD_SANITIZER) +#if defined(THREADS) && defined(AO_HAVE_or) + /* And, one more version for GC_add_to_black_list_normal/stack */ + /* (invoked indirectly by GC_do_local_mark) and */ + /* async_set_pht_entry_from_index (invoked by GC_dirty or the write */ + /* fault handler). */ # define set_pht_entry_from_index_concurrent(bl, index) \ AO_or((volatile AO_t *)&(bl)[divWORDSZ(index)], \ (AO_t)((word)1 << modWORDSZ(index))) -#else - /* It is safe to set a bit in a blacklist even without */ - /* synchronization, the only drawback is that we might have */ - /* to redo blacklisting sometimes. */ -# define set_pht_entry_from_index_concurrent(bl, index) \ - set_pht_entry_from_index(bl, index) #endif @@ -2320,7 +2316,8 @@ GC_EXTERN signed_word GC_bytes_found; /* protected by GC_write_cs. */ # endif -# if defined(GC_DISABLE_INCREMENTAL) +# if defined(GC_DISABLE_INCREMENTAL) \ + || defined(set_pht_entry_from_index_concurrent) # define GC_acquire_dirty_lock() (void)0 # define GC_release_dirty_lock() (void)0 # else diff --git a/os_dep.c b/os_dep.c index c2ae775..6eee61a 100644 --- a/os_dep.c +++ b/os_dep.c @@ -2962,13 +2962,15 @@ GC_API GC_push_other_roots_proc GC_CALL GC_get_push_other_roots(void) # ifndef THREADS # define async_set_pht_entry_from_index(db, index) \ set_pht_entry_from_index(db, index) +# elif defined(set_pht_entry_from_index_concurrent) +# define async_set_pht_entry_from_index(db, index) \ + set_pht_entry_from_index_concurrent(db, index) # elif defined(AO_HAVE_test_and_set_acquire) /* We need to lock around the bitmap update (in the write fault */ /* handler or GC_dirty) in order to avoid the risk of losing a bit. */ /* We do this with a test-and-set spin lock if possible. */ GC_INNER volatile AO_TS_t GC_fault_handler_lock = AO_TS_INITIALIZER; - GC_ATTR_NO_SANITIZE_THREAD static void async_set_pht_entry_from_index(volatile page_hash_table db, size_t index) { -- 2.7.4