From d6eb8ac8f30745b06744ad5cb2988a392c4448cd Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Thu, 22 Jun 2017 15:36:41 -0700 Subject: [PATCH] Set reentrancy when invoking customized extent hooks. Customized extent hooks may malloc / free thus trigger reentry. Support this behavior by adding reentrancy on hook functions. --- include/jemalloc/internal/base_externs.h | 2 +- src/arena.c | 4 +- src/base.c | 39 ++++++----- src/extent.c | 84 +++++++++++++++++++++--- test/unit/base.c | 15 ++--- 5 files changed, 109 insertions(+), 35 deletions(-) diff --git a/include/jemalloc/internal/base_externs.h b/include/jemalloc/internal/base_externs.h index 0a1114f4..a4fd5ac7 100644 --- a/include/jemalloc/internal/base_externs.h +++ b/include/jemalloc/internal/base_externs.h @@ -3,7 +3,7 @@ base_t *b0get(void); base_t *base_new(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks); -void base_delete(base_t *base); +void base_delete(tsdn_t *tsdn, base_t *base); extent_hooks_t *base_extent_hooks_get(base_t *base); extent_hooks_t *base_extent_hooks_set(base_t *base, extent_hooks_t *extent_hooks); diff --git a/src/arena.c b/src/arena.c index f9b0f685..4e3bd6c1 100644 --- a/src/arena.c +++ b/src/arena.c @@ -1257,7 +1257,7 @@ arena_destroy(tsd_t *tsd, arena_t *arena) { * Destroy the base allocator, which manages all metadata ever mapped by * this arena. */ - base_delete(arena->base); + base_delete(tsd_tsdn(tsd), arena->base); } static extent_t * @@ -2061,7 +2061,7 @@ arena_new(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks) { return arena; label_error: if (ind != 0) { - base_delete(base); + base_delete(tsdn, base); } return NULL; } diff --git a/src/base.c b/src/base.c index 8e1544fd..22c94338 100644 --- a/src/base.c +++ b/src/base.c @@ -15,7 +15,7 @@ static base_t *b0; /******************************************************************************/ static void * -base_map(extent_hooks_t *extent_hooks, unsigned ind, size_t size) { +base_map(tsdn_t *tsdn, extent_hooks_t *extent_hooks, unsigned ind, size_t size) { void *addr; bool zero = true; bool commit = true; @@ -25,15 +25,18 @@ base_map(extent_hooks_t *extent_hooks, unsigned ind, size_t size) { if (extent_hooks == &extent_hooks_default) { addr = extent_alloc_mmap(NULL, size, PAGE, &zero, &commit); } else { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); addr = extent_hooks->alloc(extent_hooks, NULL, size, PAGE, &zero, &commit, ind); + post_reentrancy(tsdn_tsd(tsdn)); } return addr; } static void -base_unmap(extent_hooks_t *extent_hooks, unsigned ind, void *addr, +base_unmap(tsdn_t *tsdn, extent_hooks_t *extent_hooks, unsigned ind, void *addr, size_t size) { /* * Cascade through dalloc, decommit, purge_forced, and purge_lazy, @@ -61,27 +64,32 @@ base_unmap(extent_hooks_t *extent_hooks, unsigned ind, void *addr, /* Nothing worked. This should never happen. */ not_reached(); } else { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); if (extent_hooks->dalloc != NULL && !extent_hooks->dalloc(extent_hooks, addr, size, true, ind)) { - return; + goto label_done; } if (extent_hooks->decommit != NULL && !extent_hooks->decommit(extent_hooks, addr, size, 0, size, ind)) { - return; + goto label_done; } if (extent_hooks->purge_forced != NULL && !extent_hooks->purge_forced(extent_hooks, addr, size, 0, size, ind)) { - return; + goto label_done; } if (extent_hooks->purge_lazy != NULL && !extent_hooks->purge_lazy(extent_hooks, addr, size, 0, size, ind)) { - return; + goto label_done; } /* Nothing worked. That's the application's problem. */ + label_done: + post_reentrancy(tsdn_tsd(tsdn)); + return; } } @@ -157,7 +165,7 @@ base_extent_bump_alloc(tsdn_t *tsdn, base_t *base, extent_t *extent, * On success a pointer to the initialized base_block_t header is returned. */ static base_block_t * -base_block_alloc(extent_hooks_t *extent_hooks, unsigned ind, +base_block_alloc(tsdn_t *tsdn, extent_hooks_t *extent_hooks, unsigned ind, pszind_t *pind_last, size_t *extent_sn_next, size_t size, size_t alignment) { alignment = ALIGNMENT_CEILING(alignment, QUANTUM); @@ -179,7 +187,7 @@ base_block_alloc(extent_hooks_t *extent_hooks, unsigned ind, size_t next_block_size = HUGEPAGE_CEILING(sz_pind2sz(pind_next)); size_t block_size = (min_block_size > next_block_size) ? min_block_size : next_block_size; - base_block_t *block = (base_block_t *)base_map(extent_hooks, ind, + base_block_t *block = (base_block_t *)base_map(tsdn, extent_hooks, ind, block_size); if (block == NULL) { return NULL; @@ -207,8 +215,9 @@ base_extent_alloc(tsdn_t *tsdn, base_t *base, size_t size, size_t alignment) { * called. */ malloc_mutex_unlock(tsdn, &base->mtx); - base_block_t *block = base_block_alloc(extent_hooks, base_ind_get(base), - &base->pind_last, &base->extent_sn_next, size, alignment); + base_block_t *block = base_block_alloc(tsdn, extent_hooks, + base_ind_get(base), &base->pind_last, &base->extent_sn_next, size, + alignment); malloc_mutex_lock(tsdn, &base->mtx); if (block == NULL) { return NULL; @@ -234,8 +243,8 @@ base_t * base_new(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks) { pszind_t pind_last = 0; size_t extent_sn_next = 0; - base_block_t *block = base_block_alloc(extent_hooks, ind, &pind_last, - &extent_sn_next, sizeof(base_t), QUANTUM); + base_block_t *block = base_block_alloc(tsdn, extent_hooks, ind, + &pind_last, &extent_sn_next, sizeof(base_t), QUANTUM); if (block == NULL) { return NULL; } @@ -249,7 +258,7 @@ base_new(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks) { atomic_store_p(&base->extent_hooks, extent_hooks, ATOMIC_RELAXED); if (malloc_mutex_init(&base->mtx, "base", WITNESS_RANK_BASE, malloc_mutex_rank_exclusive)) { - base_unmap(extent_hooks, ind, block, block->size); + base_unmap(tsdn, extent_hooks, ind, block, block->size); return NULL; } base->pind_last = pind_last; @@ -272,13 +281,13 @@ base_new(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks) { } void -base_delete(base_t *base) { +base_delete(tsdn_t *tsdn, base_t *base) { extent_hooks_t *extent_hooks = base_extent_hooks_get(base); base_block_t *next = base->blocks; do { base_block_t *block = next; next = block->next; - base_unmap(extent_hooks, base_ind_get(base), block, + base_unmap(tsdn, extent_hooks, base_ind_get(base), block, block->size); } while (next != NULL); } diff --git a/src/extent.c b/src/extent.c index f31ed32e..4b66dd88 100644 --- a/src/extent.c +++ b/src/extent.c @@ -1073,9 +1073,12 @@ extent_grow_retained(tsdn_t *tsdn, arena_t *arena, &zeroed, &committed, (dss_prec_t)atomic_load_u( &arena->dss_prec, ATOMIC_RELAXED)); } else { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); ptr = (*r_extent_hooks)->alloc(*r_extent_hooks, NULL, alloc_size, PAGE, &zeroed, &committed, arena_ind_get(arena)); + post_reentrancy(tsdn_tsd(tsdn)); } extent_init(extent, arena, ptr, alloc_size, false, NSIZES, @@ -1247,8 +1250,11 @@ extent_alloc_wrapper_hard(tsdn_t *tsdn, arena_t *arena, addr = extent_alloc_default_impl(tsdn, arena, new_addr, esize, alignment, zero, commit); } else { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); addr = (*r_extent_hooks)->alloc(*r_extent_hooks, new_addr, esize, alignment, zero, commit, arena_ind_get(arena)); + post_reentrancy(tsdn_tsd(tsdn)); } if (addr == NULL) { extent_dalloc(tsdn, arena, extent); @@ -1486,10 +1492,13 @@ extent_dalloc_wrapper_try(tsdn_t *tsdn, arena_t *arena, err = extent_dalloc_default_impl(extent_base_get(extent), extent_size_get(extent)); } else { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); err = ((*r_extent_hooks)->dalloc == NULL || (*r_extent_hooks)->dalloc(*r_extent_hooks, extent_base_get(extent), extent_size_get(extent), extent_committed_get(extent), arena_ind_get(arena))); + post_reentrancy(tsdn_tsd(tsdn)); } if (!err) { @@ -1515,6 +1524,10 @@ extent_dalloc_wrapper(tsdn_t *tsdn, arena_t *arena, } extent_reregister(tsdn, extent); + if (*r_extent_hooks != &extent_hooks_default) { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); + } /* Try to decommit; purge if that fails. */ bool zeroed; if (!extent_committed_get(extent)) { @@ -1536,6 +1549,9 @@ extent_dalloc_wrapper(tsdn_t *tsdn, arena_t *arena, } else { zeroed = false; } + if (*r_extent_hooks != &extent_hooks_default) { + post_reentrancy(tsdn_tsd(tsdn)); + } extent_zeroed_set(extent, zeroed); if (config_prof) { @@ -1579,9 +1595,12 @@ extent_destroy_wrapper(tsdn_t *tsdn, arena_t *arena, extent_destroy_default_impl(extent_base_get(extent), extent_size_get(extent)); } else if ((*r_extent_hooks)->destroy != NULL) { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); (*r_extent_hooks)->destroy(*r_extent_hooks, extent_base_get(extent), extent_size_get(extent), extent_committed_get(extent), arena_ind_get(arena)); + post_reentrancy(tsdn_tsd(tsdn)); } extent_dalloc(tsdn, arena, extent); @@ -1602,9 +1621,16 @@ extent_commit_impl(tsdn_t *tsdn, arena_t *arena, WITNESS_RANK_CORE, growing_retained ? 1 : 0); extent_hooks_assure_initialized(arena, r_extent_hooks); + if (*r_extent_hooks != &extent_hooks_default) { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); + } bool err = ((*r_extent_hooks)->commit == NULL || (*r_extent_hooks)->commit(*r_extent_hooks, extent_base_get(extent), extent_size_get(extent), offset, length, arena_ind_get(arena))); + if (*r_extent_hooks != &extent_hooks_default) { + post_reentrancy(tsdn_tsd(tsdn)); + } extent_committed_set(extent, extent_committed_get(extent) || !err); return err; } @@ -1633,10 +1659,17 @@ extent_decommit_wrapper(tsdn_t *tsdn, arena_t *arena, extent_hooks_assure_initialized(arena, r_extent_hooks); + if (*r_extent_hooks != &extent_hooks_default) { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); + } bool err = ((*r_extent_hooks)->decommit == NULL || (*r_extent_hooks)->decommit(*r_extent_hooks, extent_base_get(extent), extent_size_get(extent), offset, length, arena_ind_get(arena))); + if (*r_extent_hooks != &extent_hooks_default) { + post_reentrancy(tsdn_tsd(tsdn)); + } extent_committed_set(extent, extent_committed_get(extent) && err); return err; } @@ -1663,10 +1696,23 @@ extent_purge_lazy_impl(tsdn_t *tsdn, arena_t *arena, WITNESS_RANK_CORE, growing_retained ? 1 : 0); extent_hooks_assure_initialized(arena, r_extent_hooks); - return ((*r_extent_hooks)->purge_lazy == NULL || - (*r_extent_hooks)->purge_lazy(*r_extent_hooks, + + if ((*r_extent_hooks)->purge_lazy == NULL) { + return true; + } + + if (*r_extent_hooks != &extent_hooks_default) { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); + } + bool err = (*r_extent_hooks)->purge_lazy(*r_extent_hooks, extent_base_get(extent), extent_size_get(extent), offset, length, - arena_ind_get(arena))); + arena_ind_get(arena)); + if (*r_extent_hooks != &extent_hooks_default) { + post_reentrancy(tsdn_tsd(tsdn)); + } + + return err; } bool @@ -1699,10 +1745,21 @@ extent_purge_forced_impl(tsdn_t *tsdn, arena_t *arena, WITNESS_RANK_CORE, growing_retained ? 1 : 0); extent_hooks_assure_initialized(arena, r_extent_hooks); - return ((*r_extent_hooks)->purge_forced == NULL || - (*r_extent_hooks)->purge_forced(*r_extent_hooks, + + if ((*r_extent_hooks)->purge_forced == NULL) { + return true; + } + if (*r_extent_hooks != &extent_hooks_default) { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); + } + bool err = (*r_extent_hooks)->purge_forced(*r_extent_hooks, extent_base_get(extent), extent_size_get(extent), offset, length, - arena_ind_get(arena))); + arena_ind_get(arena)); + if (*r_extent_hooks != &extent_hooks_default) { + post_reentrancy(tsdn_tsd(tsdn)); + } + return err; } bool @@ -1771,9 +1828,17 @@ extent_split_impl(tsdn_t *tsdn, arena_t *arena, extent_lock2(tsdn, extent, trail); - if ((*r_extent_hooks)->split(*r_extent_hooks, extent_base_get(extent), + if (*r_extent_hooks != &extent_hooks_default) { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); + } + bool err = (*r_extent_hooks)->split(*r_extent_hooks, extent_base_get(extent), size_a + size_b, size_a, size_b, extent_committed_get(extent), - arena_ind_get(arena))) { + arena_ind_get(arena)); + if (*r_extent_hooks != &extent_hooks_default) { + post_reentrancy(tsdn_tsd(tsdn)); + } + if (err) { goto label_error_c; } @@ -1843,10 +1908,13 @@ extent_merge_impl(tsdn_t *tsdn, arena_t *arena, err = extent_merge_default_impl(extent_base_get(a), extent_base_get(b)); } else { + assert(!tsdn_null(tsdn)); + pre_reentrancy(tsdn_tsd(tsdn)); err = (*r_extent_hooks)->merge(*r_extent_hooks, extent_base_get(a), extent_size_get(a), extent_base_get(b), extent_size_get(b), extent_committed_get(a), arena_ind_get(arena)); + post_reentrancy(tsdn_tsd(tsdn)); } if (err) { diff --git a/test/unit/base.c b/test/unit/base.c index 5dc42f0a..7fa24ac0 100644 --- a/test/unit/base.c +++ b/test/unit/base.c @@ -27,11 +27,10 @@ static extent_hooks_t hooks_not_null = { }; TEST_BEGIN(test_base_hooks_default) { - tsdn_t *tsdn; base_t *base; size_t allocated0, allocated1, resident, mapped; - tsdn = tsdn_fetch(); + tsdn_t *tsdn = tsd_tsdn(tsd_fetch()); base = base_new(tsdn, 0, (extent_hooks_t *)&extent_hooks_default); if (config_stats) { @@ -49,13 +48,12 @@ TEST_BEGIN(test_base_hooks_default) { "At least 42 bytes were allocated by base_alloc()"); } - base_delete(base); + base_delete(tsdn, base); } TEST_END TEST_BEGIN(test_base_hooks_null) { extent_hooks_t hooks_orig; - tsdn_t *tsdn; base_t *base; size_t allocated0, allocated1, resident, mapped; @@ -68,7 +66,7 @@ TEST_BEGIN(test_base_hooks_null) { memcpy(&hooks_orig, &hooks, sizeof(extent_hooks_t)); memcpy(&hooks, &hooks_null, sizeof(extent_hooks_t)); - tsdn = tsdn_fetch(); + tsdn_t *tsdn = tsd_tsdn(tsd_fetch()); base = base_new(tsdn, 0, &hooks); assert_ptr_not_null(base, "Unexpected base_new() failure"); @@ -87,7 +85,7 @@ TEST_BEGIN(test_base_hooks_null) { "At least 42 bytes were allocated by base_alloc()"); } - base_delete(base); + base_delete(tsdn, base); memcpy(&hooks, &hooks_orig, sizeof(extent_hooks_t)); } @@ -95,7 +93,6 @@ TEST_END TEST_BEGIN(test_base_hooks_not_null) { extent_hooks_t hooks_orig; - tsdn_t *tsdn; base_t *base; void *p, *q, *r, *r_exp; @@ -108,7 +105,7 @@ TEST_BEGIN(test_base_hooks_not_null) { memcpy(&hooks_orig, &hooks, sizeof(extent_hooks_t)); memcpy(&hooks, &hooks_not_null, sizeof(extent_hooks_t)); - tsdn = tsdn_fetch(); + tsdn_t *tsdn = tsd_tsdn(tsd_fetch()); did_alloc = false; base = base_new(tsdn, 0, &hooks); assert_ptr_not_null(base, "Unexpected base_new() failure"); @@ -200,7 +197,7 @@ TEST_BEGIN(test_base_hooks_not_null) { called_dalloc = called_destroy = called_decommit = called_purge_lazy = called_purge_forced = false; - base_delete(base); + base_delete(tsdn, base); assert_true(called_dalloc, "Expected dalloc call"); assert_true(!called_destroy, "Unexpected destroy call"); assert_true(called_decommit, "Expected decommit call");