diff --git a/include/jemalloc/internal/background_thread.h b/include/jemalloc/internal/background_thread.h index 9947a0e9..a4cf094a 100644 --- a/include/jemalloc/internal/background_thread.h +++ b/include/jemalloc/internal/background_thread.h @@ -76,6 +76,25 @@ bool background_thread_stats_read( tsdn_t *tsdn, background_thread_stats_t *stats); void background_thread_ctl_init(tsdn_t *tsdn); +/* + * Arena-reset lifecycle bracket (owned by the bg-thread module so the + * started<->paused state machine is mutated only inside the module). begin() + * acquires the global background_thread_lock and RETURNS WITH IT HELD; + * finish() releases it. The lock intentionally spans the whole arena-reset + * body across the two calls. Pass arena_ind (not an arena_t *): the destroy + * path frees the arena before calling finish(). + */ +void background_thread_arena_reset_begin(tsd_t *tsd, unsigned arena_ind); +void background_thread_arena_reset_finish(tsd_t *tsd, unsigned arena_ind); + +/* + * Serialize a rare admin operation against the background thread by holding the + * per-arena info mutex (have_background_thread-gated). Used by + * arena_set_extent_hooks to fence pa_shard_disable_hpa. + */ +void background_thread_serialize_lock(tsd_t *tsd, unsigned arena_ind); +void background_thread_serialize_unlock(tsd_t *tsd, unsigned arena_ind); + #ifdef JEMALLOC_PTHREAD_CREATE_WRAPPER extern int pthread_create_wrapper(pthread_t *__restrict, const pthread_attr_t *, void *(*)(void *), void *__restrict); diff --git a/include/jemalloc/internal/background_thread_inlines.h b/include/jemalloc/internal/background_thread_inlines.h index ba6e6644..e02b25de 100644 --- a/include/jemalloc/internal/background_thread_inlines.h +++ b/include/jemalloc/internal/background_thread_inlines.h @@ -22,12 +22,6 @@ background_thread_enabled_set(tsdn_t *tsdn, bool state) { background_thread_enabled_set_impl(state); } -JEMALLOC_ALWAYS_INLINE background_thread_info_t * -arena_background_thread_info_get(arena_t *arena) { - unsigned arena_ind = arena_ind_get(arena); - return &background_thread_info[arena_ind % max_background_threads]; -} - JEMALLOC_ALWAYS_INLINE background_thread_info_t * background_thread_info_get(size_t ind) { return &background_thread_info[ind % max_background_threads]; diff --git a/src/arena.c b/src/arena.c index 3e4a2ea3..273bfe22 100644 --- a/src/arena.c +++ b/src/arena.c @@ -1427,17 +1427,17 @@ arena_get_ehooks(const arena_t *arena) { extent_hooks_t * arena_set_extent_hooks( tsd_t *tsd, arena_t *arena, extent_hooks_t *extent_hooks) { - background_thread_info_t *info; - if (have_background_thread) { - info = arena_background_thread_info_get(arena); - malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx); - } + /* + * Serialize disabling the HPA against the background thread via the + * module-owned bracket (have_background_thread-gated internally) rather + * than reaching into info->mtx directly. + */ + unsigned arena_ind = arena_ind_get(arena); + background_thread_serialize_lock(tsd, arena_ind); /* No using the HPA now that we have the custom hooks. */ pa_shard_disable_hpa(tsd_tsdn(tsd), &arena->pa_shard); extent_hooks_t *ret = base_extent_hooks_set(arena->base, extent_hooks); - if (have_background_thread) { - malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx); - } + background_thread_serialize_unlock(tsd, arena_ind); return ret; } diff --git a/src/background_thread.c b/src/background_thread.c index 9c843ede..d1822a76 100644 --- a/src/background_thread.c +++ b/src/background_thread.c @@ -34,6 +34,72 @@ size_t max_background_threads; /* Thread info per-index. */ background_thread_info_t *background_thread_info; +/******************************************************************************/ +/* + * Config-independent lifecycle/state-ownership helpers. Defined + * unconditionally and gated at runtime on have_background_thread, so callers in + * ctl.c / arena.c never touch background_thread_lock or info->state directly; + * they compile to runtime no-ops when !have_background_thread. + */ + +void +background_thread_arena_reset_begin(tsd_t *tsd, unsigned arena_ind) { + /* Temporarily disable the background thread during arena reset. */ + if (have_background_thread) { + /* + * Hold background_thread_lock across the whole arena-reset + * body (acquired here, released in _finish) so a concurrent + * background_threads_enable() cannot start a background + * thread mid-reset. This must happen whenever the feature + * is compiled in (have_background_thread), even when the + * thread is not currently enabled; the state transition + * below is separately gated on background_thread_enabled(). + */ + malloc_mutex_lock(tsd_tsdn(tsd), &background_thread_lock); + if (background_thread_enabled()) { + background_thread_info_t *info = + background_thread_info_get(arena_ind); + assert(info->state == background_thread_started); + malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx); + info->state = background_thread_paused; + malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx); + } + } +} + +void +background_thread_arena_reset_finish(tsd_t *tsd, unsigned arena_ind) { + if (have_background_thread) { + if (background_thread_enabled()) { + background_thread_info_t *info = + background_thread_info_get(arena_ind); + assert(info->state == background_thread_paused); + malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx); + info->state = background_thread_started; + malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx); + } + malloc_mutex_unlock(tsd_tsdn(tsd), &background_thread_lock); + } +} + +void +background_thread_serialize_lock(tsd_t *tsd, unsigned arena_ind) { + if (have_background_thread) { + background_thread_info_t *info = + background_thread_info_get(arena_ind); + malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx); + } +} + +void +background_thread_serialize_unlock(tsd_t *tsd, unsigned arena_ind) { + if (have_background_thread) { + background_thread_info_t *info = + background_thread_info_get(arena_ind); + malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx); + } +} + /******************************************************************************/ #ifdef JEMALLOC_PTHREAD_CREATE_WRAPPER diff --git a/src/ctl.c b/src/ctl.c index 6a12e957..0727588c 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -2798,37 +2798,6 @@ arena_i_reset_destroy_helper(tsd_t *tsd, const size_t *mib, size_t miblen, return ret; } -static void -arena_reset_prepare_background_thread(tsd_t *tsd, unsigned arena_ind) { - /* Temporarily disable the background thread during arena reset. */ - if (have_background_thread) { - malloc_mutex_lock(tsd_tsdn(tsd), &background_thread_lock); - if (background_thread_enabled()) { - background_thread_info_t *info = - background_thread_info_get(arena_ind); - assert(info->state == background_thread_started); - malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx); - info->state = background_thread_paused; - malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx); - } - } -} - -static void -arena_reset_finish_background_thread(tsd_t *tsd, unsigned arena_ind) { - if (have_background_thread) { - if (background_thread_enabled()) { - background_thread_info_t *info = - background_thread_info_get(arena_ind); - assert(info->state == background_thread_paused); - malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx); - info->state = background_thread_started; - malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx); - } - malloc_mutex_unlock(tsd_tsdn(tsd), &background_thread_lock); - } -} - static int arena_i_reset_ctl(tsd_t *tsd, const size_t *mib, size_t miblen, void *oldp, size_t *oldlenp, void *newp, size_t newlen) { @@ -2842,9 +2811,9 @@ arena_i_reset_ctl(tsd_t *tsd, const size_t *mib, size_t miblen, void *oldp, return ret; } - arena_reset_prepare_background_thread(tsd, arena_ind); + background_thread_arena_reset_begin(tsd, arena_ind); arena_reset(tsd, arena); - arena_reset_finish_background_thread(tsd, arena_ind); + background_thread_arena_reset_finish(tsd, arena_ind); return ret; } @@ -2871,7 +2840,7 @@ arena_i_destroy_ctl(tsd_t *tsd, const size_t *mib, size_t miblen, void *oldp, goto label_return; } - arena_reset_prepare_background_thread(tsd, arena_ind); + background_thread_arena_reset_begin(tsd, arena_ind); /* Merge stats after resetting and purging arena. */ arena_reset(tsd, arena); arena_decay(tsd_tsdn(tsd), arena, false, true); @@ -2885,7 +2854,7 @@ arena_i_destroy_ctl(tsd_t *tsd, const size_t *mib, size_t miblen, void *oldp, /* Record arena index for later recycling via arenas.create. */ ql_elm_new(ctl_arena, destroyed_link); ql_tail_insert(&ctl_arenas->destroyed, ctl_arena, destroyed_link); - arena_reset_finish_background_thread(tsd, arena_ind); + background_thread_arena_reset_finish(tsd, arena_ind); assert(ret == 0); label_return: