From 3ecc3c84862ef3e66b20be8213b0301c06c692cc Mon Sep 17 00:00:00 2001 From: Jason Evans Date: Sun, 29 Jan 2017 21:32:39 -0800 Subject: [PATCH] Fix/refactor tcaches synchronization. Synchronize tcaches with tcaches_mtx rather than ctl_mtx. Add missing synchronization for tcache flushing. This bug was introduced by 1cb181ed632e7573fb4eab194e4d216867222d27 (Implement explicit tcache support.), which was first released in 4.0.0. --- include/jemalloc/internal/private_symbols.txt | 3 + include/jemalloc/internal/tcache.h | 3 + include/jemalloc/internal/witness.h | 21 +-- src/ctl.c | 4 +- src/jemalloc.c | 3 + src/tcache.c | 120 ++++++++++++++---- 6 files changed, 113 insertions(+), 41 deletions(-) diff --git a/include/jemalloc/internal/private_symbols.txt b/include/jemalloc/internal/private_symbols.txt index 4dfe442c..6111eac8 100644 --- a/include/jemalloc/internal/private_symbols.txt +++ b/include/jemalloc/internal/private_symbols.txt @@ -529,6 +529,9 @@ tcache_flush tcache_get tcache_get_hard tcache_maxclass +tcache_prefork +tcache_postfork_child +tcache_postfork_parent tcache_salloc tcache_stats_merge tcaches diff --git a/include/jemalloc/internal/tcache.h b/include/jemalloc/internal/tcache.h index 01ba062d..5fe5ebfa 100644 --- a/include/jemalloc/internal/tcache.h +++ b/include/jemalloc/internal/tcache.h @@ -149,6 +149,9 @@ bool tcaches_create(tsd_t *tsd, unsigned *r_ind); void tcaches_flush(tsd_t *tsd, unsigned ind); void tcaches_destroy(tsd_t *tsd, unsigned ind); bool tcache_boot(tsdn_t *tsdn); +void tcache_prefork(tsdn_t *tsdn); +void tcache_postfork_parent(tsdn_t *tsdn); +void tcache_postfork_child(tsdn_t *tsdn); #endif /* JEMALLOC_H_EXTERNS */ /******************************************************************************/ diff --git a/include/jemalloc/internal/witness.h b/include/jemalloc/internal/witness.h index dfd827f7..e64e56eb 100644 --- a/include/jemalloc/internal/witness.h +++ b/include/jemalloc/internal/witness.h @@ -14,19 +14,20 @@ typedef int witness_comp_t (const witness_t *, const witness_t *); #define WITNESS_RANK_INIT 1U #define WITNESS_RANK_CTL 1U -#define WITNESS_RANK_ARENAS 2U +#define WITNESS_RANK_TCACHES 2U +#define WITNESS_RANK_ARENAS 3U -#define WITNESS_RANK_PROF_DUMP 3U -#define WITNESS_RANK_PROF_BT2GCTX 4U -#define WITNESS_RANK_PROF_TDATAS 5U -#define WITNESS_RANK_PROF_TDATA 6U -#define WITNESS_RANK_PROF_GCTX 7U +#define WITNESS_RANK_PROF_DUMP 4U +#define WITNESS_RANK_PROF_BT2GCTX 5U +#define WITNESS_RANK_PROF_TDATAS 6U +#define WITNESS_RANK_PROF_TDATA 7U +#define WITNESS_RANK_PROF_GCTX 8U -#define WITNESS_RANK_ARENA 8U -#define WITNESS_RANK_ARENA_CHUNKS 9U -#define WITNESS_RANK_ARENA_NODE_CACHE 10 +#define WITNESS_RANK_ARENA 9U +#define WITNESS_RANK_ARENA_CHUNKS 10U +#define WITNESS_RANK_ARENA_NODE_CACHE 11U -#define WITNESS_RANK_BASE 11U +#define WITNESS_RANK_BASE 12U #define WITNESS_RANK_LEAF 0xffffffffU #define WITNESS_RANK_ARENA_BIN WITNESS_RANK_LEAF diff --git a/src/ctl.c b/src/ctl.c index bc78b205..1e62e2d3 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -1476,7 +1476,6 @@ tcache_create_ctl(tsd_t *tsd, const size_t *mib, size_t miblen, void *oldp, if (!config_tcache) return (ENOENT); - malloc_mutex_lock(tsd_tsdn(tsd), &ctl_mtx); READONLY(); if (tcaches_create(tsd, &tcache_ind)) { ret = EFAULT; @@ -1486,8 +1485,7 @@ tcache_create_ctl(tsd_t *tsd, const size_t *mib, size_t miblen, void *oldp, ret = 0; label_return: - malloc_mutex_unlock(tsd_tsdn(tsd), &ctl_mtx); - return (ret); + return ret; } static int diff --git a/src/jemalloc.c b/src/jemalloc.c index 92813b62..a376e143 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -2828,6 +2828,7 @@ _malloc_prefork(void) witness_prefork(tsd); /* Acquire all mutexes in a safe order. */ ctl_prefork(tsd_tsdn(tsd)); + tcache_prefork(tsd_tsdn(tsd)); malloc_mutex_prefork(tsd_tsdn(tsd), &arenas_lock); prof_prefork0(tsd_tsdn(tsd)); for (i = 0; i < 3; i++) { @@ -2887,6 +2888,7 @@ _malloc_postfork(void) } prof_postfork_parent(tsd_tsdn(tsd)); malloc_mutex_postfork_parent(tsd_tsdn(tsd), &arenas_lock); + tcache_postfork_parent(tsd_tsdn(tsd)); ctl_postfork_parent(tsd_tsdn(tsd)); } @@ -2911,6 +2913,7 @@ jemalloc_postfork_child(void) } prof_postfork_child(tsd_tsdn(tsd)); malloc_mutex_postfork_child(tsd_tsdn(tsd), &arenas_lock); + tcache_postfork_child(tsd_tsdn(tsd)); ctl_postfork_child(tsd_tsdn(tsd)); } diff --git a/src/tcache.c b/src/tcache.c index 21540ff4..e3b04be6 100644 --- a/src/tcache.c +++ b/src/tcache.c @@ -21,6 +21,9 @@ static unsigned tcaches_past; /* Head of singly linked list tracking available tcaches elements. */ static tcaches_t *tcaches_avail; +/* Protects tcaches{,_past,_avail}. */ +static malloc_mutex_t tcaches_mtx; + /******************************************************************************/ size_t @@ -444,29 +447,56 @@ tcache_stats_merge(tsdn_t *tsdn, tcache_t *tcache, arena_t *arena) } } -bool -tcaches_create(tsd_t *tsd, unsigned *r_ind) -{ - arena_t *arena; - tcache_t *tcache; - tcaches_t *elm; +static bool +tcaches_create_prep(tsd_t *tsd) { + bool err; + + malloc_mutex_lock(tsd_tsdn(tsd), &tcaches_mtx); if (tcaches == NULL) { tcaches = base_alloc(tsd_tsdn(tsd), sizeof(tcache_t *) * (MALLOCX_TCACHE_MAX+1)); - if (tcaches == NULL) - return (true); + if (tcaches == NULL) { + err = true; + goto label_return; + } } - if (tcaches_avail == NULL && tcaches_past > MALLOCX_TCACHE_MAX) - return (true); - arena = arena_ichoose(tsd, NULL); - if (unlikely(arena == NULL)) - return (true); - tcache = tcache_create(tsd_tsdn(tsd), arena); - if (tcache == NULL) - return (true); + if (tcaches_avail == NULL && tcaches_past > MALLOCX_TCACHE_MAX) { + err = true; + goto label_return; + } + err = false; +label_return: + malloc_mutex_unlock(tsd_tsdn(tsd), &tcaches_mtx); + return err; +} + +bool +tcaches_create(tsd_t *tsd, unsigned *r_ind) { + bool err; + arena_t *arena; + tcache_t *tcache; + tcaches_t *elm; + + if (tcaches_create_prep(tsd)) { + err = true; + goto label_return; + } + + arena = arena_ichoose(tsd, NULL); + if (unlikely(arena == NULL)) { + err = true; + goto label_return; + } + tcache = tcache_create(tsd_tsdn(tsd), arena); + if (tcache == NULL) { + err = true; + goto label_return; + } + + malloc_mutex_lock(tsd_tsdn(tsd), &tcaches_mtx); if (tcaches_avail != NULL) { elm = tcaches_avail; tcaches_avail = tcaches_avail->next; @@ -478,41 +508,50 @@ tcaches_create(tsd_t *tsd, unsigned *r_ind) *r_ind = tcaches_past; tcaches_past++; } + malloc_mutex_unlock(tsd_tsdn(tsd), &tcaches_mtx); - return (false); + err = false; +label_return: + malloc_mutex_assert_not_owner(tsd_tsdn(tsd), &tcaches_mtx); + return err; } static void -tcaches_elm_flush(tsd_t *tsd, tcaches_t *elm) -{ +tcaches_elm_flush(tsd_t *tsd, tcaches_t *elm) { + malloc_mutex_assert_owner(tsd_tsdn(tsd), &tcaches_mtx); - if (elm->tcache == NULL) + if (elm->tcache == NULL) { return; + } tcache_destroy(tsd, elm->tcache); elm->tcache = NULL; } void -tcaches_flush(tsd_t *tsd, unsigned ind) -{ - +tcaches_flush(tsd_t *tsd, unsigned ind) { + malloc_mutex_lock(tsd_tsdn(tsd), &tcaches_mtx); tcaches_elm_flush(tsd, &tcaches[ind]); + malloc_mutex_unlock(tsd_tsdn(tsd), &tcaches_mtx); } void -tcaches_destroy(tsd_t *tsd, unsigned ind) -{ - tcaches_t *elm = &tcaches[ind]; +tcaches_destroy(tsd_t *tsd, unsigned ind) { + tcaches_t *elm; + + malloc_mutex_lock(tsd_tsdn(tsd), &tcaches_mtx); + elm = &tcaches[ind]; tcaches_elm_flush(tsd, elm); elm->next = tcaches_avail; tcaches_avail = elm; + malloc_mutex_unlock(tsd_tsdn(tsd), &tcaches_mtx); } bool -tcache_boot(tsdn_t *tsdn) -{ +tcache_boot(tsdn_t *tsdn) { unsigned i; + cassert(config_tcache); + /* * If necessary, clamp opt_lg_tcache_max, now that large_maxclass is * known. @@ -524,6 +563,10 @@ tcache_boot(tsdn_t *tsdn) else tcache_maxclass = (ZU(1) << opt_lg_tcache_max); + if (malloc_mutex_init(&tcaches_mtx, "tcaches", WITNESS_RANK_TCACHES)) { + return true; + } + nhbins = size2index(tcache_maxclass) + 1; /* Initialize tcache_bin_info. */ @@ -553,3 +596,24 @@ tcache_boot(tsdn_t *tsdn) return (false); } + +void +tcache_prefork(tsdn_t *tsdn) { + if (!config_prof && opt_tcache) { + malloc_mutex_prefork(tsdn, &tcaches_mtx); + } +} + +void +tcache_postfork_parent(tsdn_t *tsdn) { + if (!config_prof && opt_tcache) { + malloc_mutex_postfork_parent(tsdn, &tcaches_mtx); + } +} + +void +tcache_postfork_child(tsdn_t *tsdn) { + if (!config_prof && opt_tcache) { + malloc_mutex_postfork_child(tsdn, &tcaches_mtx); + } +}