diff --git a/configure.ac b/configure.ac index 9f8311cc..462f509f 100644 --- a/configure.ac +++ b/configure.ac @@ -1086,7 +1086,7 @@ if test "x${maps_coalesce}" = "x1" ; then AC_DEFINE([JEMALLOC_MAPS_COALESCE], [ ]) fi -dnl Indicate whether to retain memory (rather than using) munmap()) by default. +dnl Indicate whether to retain memory (rather than using munmap()) by default. if test "x$default_retain" = "x1" ; then AC_DEFINE([JEMALLOC_RETAIN], [ ]) fi diff --git a/doc/jemalloc.xml.in b/doc/jemalloc.xml.in index fa65c39b..d1b2e334 100644 --- a/doc/jemalloc.xml.in +++ b/doc/jemalloc.xml.in @@ -1605,6 +1605,7 @@ typedef extent_hooks_s extent_hooks_t; struct extent_hooks_s { extent_alloc_t *alloc; extent_dalloc_t *dalloc; + extent_destroy_t *destroy; extent_commit_t *commit; extent_decommit_t *decommit; extent_purge_t *purge_lazy; @@ -1681,6 +1682,25 @@ struct extent_hooks_s { remains mapped, in the same commit state, and available for future use, in which case it will be automatically retained for later reuse. + + typedef void (extent_destroy_t) + extent_hooks_t *extent_hooks + void *addr + size_t size + bool committed + unsigned arena_ind + + + + An extent destruction function conforms to the + extent_destroy_t type and unconditionally destroys an + extent at given addr and + size with + committed/decommited memory as indicated, on + behalf of arena arena_ind. This function may be + called to destroy retained extents during arena destruction (see arena.<i>.destroy). + typedef bool (extent_commit_t) extent_hooks_t *extent_hooks diff --git a/include/jemalloc/internal/extent_externs.h b/include/jemalloc/internal/extent_externs.h index 58e57e70..c4fe8425 100644 --- a/include/jemalloc/internal/extent_externs.h +++ b/include/jemalloc/internal/extent_externs.h @@ -40,10 +40,10 @@ extent_t *extent_alloc_wrapper(tsdn_t *tsdn, arena_t *arena, extent_hooks_t **r_extent_hooks, void *new_addr, size_t size, size_t pad, size_t alignment, bool slab, szind_t szind, bool *zero, bool *commit); void extent_dalloc_gap(tsdn_t *tsdn, arena_t *arena, extent_t *extent); -bool extent_dalloc_wrapper_try(tsdn_t *tsdn, arena_t *arena, - extent_hooks_t **r_extent_hooks, extent_t *extent); void extent_dalloc_wrapper(tsdn_t *tsdn, arena_t *arena, extent_hooks_t **r_extent_hooks, extent_t *extent); +void extent_destroy_wrapper(tsdn_t *tsdn, arena_t *arena, + extent_hooks_t **r_extent_hooks, extent_t *extent); bool extent_commit_wrapper(tsdn_t *tsdn, arena_t *arena, extent_hooks_t **r_extent_hooks, extent_t *extent, size_t offset, size_t length); diff --git a/include/jemalloc/internal/private_symbols.txt b/include/jemalloc/internal/private_symbols.txt index 50590957..eb9b3010 100644 --- a/include/jemalloc/internal/private_symbols.txt +++ b/include/jemalloc/internal/private_symbols.txt @@ -154,8 +154,8 @@ extent_dalloc extent_dalloc_gap extent_dalloc_mmap extent_dalloc_wrapper -extent_dalloc_wrapper_try extent_decommit_wrapper +extent_destroy_wrapper extent_dss_boot extent_dss_mergeable extent_dss_prec_get diff --git a/include/jemalloc/jemalloc_typedefs.h.in b/include/jemalloc/jemalloc_typedefs.h.in index 91b5a8dc..1a588743 100644 --- a/include/jemalloc/jemalloc_typedefs.h.in +++ b/include/jemalloc/jemalloc_typedefs.h.in @@ -16,6 +16,14 @@ typedef void *(extent_alloc_t)(extent_hooks_t *, void *, size_t, size_t, bool *, typedef bool (extent_dalloc_t)(extent_hooks_t *, void *, size_t, bool, unsigned); +/* + * void + * extent_destroy(extent_hooks_t *extent_hooks, void *addr, size_t size, + * bool committed, unsigned arena_ind); + */ +typedef void (extent_destroy_t)(extent_hooks_t *, void *, size_t, bool, + unsigned); + /* * bool * extent_commit(extent_hooks_t *extent_hooks, void *addr, size_t size, @@ -59,6 +67,7 @@ typedef bool (extent_merge_t)(extent_hooks_t *, void *, size_t, void *, size_t, struct extent_hooks_s { extent_alloc_t *alloc; extent_dalloc_t *dalloc; + extent_destroy_t *destroy; extent_commit_t *commit; extent_decommit_t *decommit; extent_purge_t *purge_lazy; diff --git a/src/arena.c b/src/arena.c index 2c7cea08..edbd875f 100644 --- a/src/arena.c +++ b/src/arena.c @@ -1138,21 +1138,19 @@ arena_reset(tsd_t *tsd, arena_t *arena) { static void arena_destroy_retained(tsdn_t *tsdn, arena_t *arena) { /* - * Iterate over the retained extents and blindly attempt to deallocate - * them. This gives the extent allocator underlying the extent hooks an - * opportunity to unmap all retained memory without having to keep its - * own metadata structures, but if deallocation fails, that is the - * application's decision/problem. In practice, retained extents are - * leaked here if opt_retain unless the application provided custom - * extent hooks, so best practice is to either disable retain (and avoid - * dss for arenas to be destroyed), or provide custom extent hooks that - * either unmap retained extents or track them for later use. + * Iterate over the retained extents and destroy them. This gives the + * extent allocator underlying the extent hooks an opportunity to unmap + * all retained memory without having to keep its own metadata + * structures. In practice, virtual memory for dss-allocated extents is + * leaked here, so best practice is to avoid dss for arenas to be + * destroyed, or provide custom extent hooks that track retained + * dss-based extents for later reuse. */ extent_hooks_t *extent_hooks = extent_hooks_get(arena); extent_t *extent; while ((extent = extents_evict(tsdn, arena, &extent_hooks, &arena->extents_retained, 0)) != NULL) { - extent_dalloc_wrapper_try(tsdn, arena, &extent_hooks, extent); + extent_destroy_wrapper(tsdn, arena, &extent_hooks, extent); } } @@ -1169,7 +1167,7 @@ arena_destroy(tsd_t *tsd, arena_t *arena) { */ assert(extents_npages_get(&arena->extents_dirty) == 0); - /* Attempt to deallocate retained memory. */ + /* Deallocate retained memory. */ arena_destroy_retained(tsd_tsdn(tsd), arena); /* diff --git a/src/extent.c b/src/extent.c index bc17711c..1b284535 100644 --- a/src/extent.c +++ b/src/extent.c @@ -19,6 +19,8 @@ static void *extent_alloc_default(extent_hooks_t *extent_hooks, unsigned arena_ind); static bool extent_dalloc_default(extent_hooks_t *extent_hooks, void *addr, size_t size, bool committed, unsigned arena_ind); +static void extent_destroy_default(extent_hooks_t *extent_hooks, void *addr, + size_t size, bool committed, unsigned arena_ind); static bool extent_commit_default(extent_hooks_t *extent_hooks, void *addr, size_t size, size_t offset, size_t length, unsigned arena_ind); static bool extent_decommit_default(extent_hooks_t *extent_hooks, @@ -43,6 +45,7 @@ static bool extent_merge_default(extent_hooks_t *extent_hooks, void *addr_a, const extent_hooks_t extent_hooks_default = { extent_alloc_default, extent_dalloc_default, + extent_destroy_default, extent_commit_default, extent_decommit_default #ifdef PAGES_CAN_PURGE_LAZY @@ -1366,7 +1369,7 @@ extent_dalloc_default(extent_hooks_t *extent_hooks, void *addr, size_t size, return extent_dalloc_default_impl(addr, size); } -bool +static bool extent_dalloc_wrapper_try(tsdn_t *tsdn, arena_t *arena, extent_hooks_t **r_extent_hooks, extent_t *extent) { bool err; @@ -1443,6 +1446,48 @@ extent_dalloc_wrapper(tsdn_t *tsdn, arena_t *arena, extent); } +static void +extent_destroy_default_impl(void *addr, size_t size) { + if (!have_dss || !extent_in_dss(addr)) { + pages_unmap(addr, size); + } +} + +static void +extent_destroy_default(extent_hooks_t *extent_hooks, void *addr, size_t size, + bool committed, unsigned arena_ind) { + assert(extent_hooks == &extent_hooks_default); + + extent_destroy_default_impl(addr, size); +} + +void +extent_destroy_wrapper(tsdn_t *tsdn, arena_t *arena, + extent_hooks_t **r_extent_hooks, extent_t *extent) { + assert(extent_base_get(extent) != NULL); + assert(extent_size_get(extent) != 0); + witness_assert_depth_to_rank(tsdn, WITNESS_RANK_CORE, 0); + + /* Deregister first to avoid a race with other allocating threads. */ + extent_deregister(tsdn, extent); + + extent_addr_set(extent, extent_base_get(extent)); + + extent_hooks_assure_initialized(arena, r_extent_hooks); + /* Try to destroy; silently fail otherwise. */ + if (*r_extent_hooks == &extent_hooks_default) { + /* Call directly to propagate tsdn. */ + extent_destroy_default_impl(extent_base_get(extent), + extent_size_get(extent)); + } else if ((*r_extent_hooks)->destroy != NULL) { + (*r_extent_hooks)->destroy(*r_extent_hooks, + extent_base_get(extent), extent_size_get(extent), + extent_committed_get(extent), arena_ind_get(arena)); + } + + extent_dalloc(tsdn, arena, extent); +} + static bool extent_commit_default(extent_hooks_t *extent_hooks, void *addr, size_t size, size_t offset, size_t length, unsigned arena_ind) { diff --git a/test/include/test/extent_hooks.h b/test/include/test/extent_hooks.h index 96fee103..ea012857 100644 --- a/test/include/test/extent_hooks.h +++ b/test/include/test/extent_hooks.h @@ -8,6 +8,8 @@ static void *extent_alloc_hook(extent_hooks_t *extent_hooks, void *new_addr, unsigned arena_ind); static bool extent_dalloc_hook(extent_hooks_t *extent_hooks, void *addr, size_t size, bool committed, unsigned arena_ind); +static void extent_destroy_hook(extent_hooks_t *extent_hooks, void *addr, + size_t size, bool committed, unsigned arena_ind); static bool extent_commit_hook(extent_hooks_t *extent_hooks, void *addr, size_t size, size_t offset, size_t length, unsigned arena_ind); static bool extent_decommit_hook(extent_hooks_t *extent_hooks, void *addr, @@ -27,6 +29,7 @@ static extent_hooks_t *default_hooks; static extent_hooks_t hooks = { extent_alloc_hook, extent_dalloc_hook, + extent_destroy_hook, extent_commit_hook, extent_decommit_hook, extent_purge_lazy_hook, @@ -38,6 +41,7 @@ static extent_hooks_t hooks = { /* Control whether hook functions pass calls through to default hooks. */ static bool try_alloc = true; static bool try_dalloc = true; +static bool try_destroy = true; static bool try_commit = true; static bool try_decommit = true; static bool try_purge_lazy = true; @@ -48,6 +52,7 @@ static bool try_merge = true; /* Set to false prior to operations, then introspect after operations. */ static bool called_alloc; static bool called_dalloc; +static bool called_destroy; static bool called_commit; static bool called_decommit; static bool called_purge_lazy; @@ -58,6 +63,7 @@ static bool called_merge; /* Set to false prior to operations, then introspect after operations. */ static bool did_alloc; static bool did_dalloc; +static bool did_destroy; static bool did_commit; static bool did_decommit; static bool did_purge_lazy; @@ -115,6 +121,24 @@ extent_dalloc_hook(extent_hooks_t *extent_hooks, void *addr, size_t size, return err; } +static void +extent_destroy_hook(extent_hooks_t *extent_hooks, void *addr, size_t size, + bool committed, unsigned arena_ind) { + TRACE_HOOK("%s(extent_hooks=%p, addr=%p, size=%zu, committed=%s, " + "arena_ind=%u)\n", __func__, extent_hooks, addr, size, committed ? + "true" : "false", arena_ind); + assert_ptr_eq(extent_hooks, &hooks, + "extent_hooks should be same as pointer used to set hooks"); + assert_ptr_eq(extent_hooks->destroy, extent_destroy_hook, + "Wrong hook function"); + called_destroy = true; + if (!try_destroy) { + return; + } + default_hooks->destroy(default_hooks, addr, size, committed, 0); + did_destroy = true; +} + static bool extent_commit_hook(extent_hooks_t *extent_hooks, void *addr, size_t size, size_t offset, size_t length, unsigned arena_ind) { diff --git a/test/unit/arena_reset.c b/test/unit/arena_reset.c index 5d6c1a77..d1698325 100644 --- a/test/unit/arena_reset.c +++ b/test/unit/arena_reset.c @@ -278,6 +278,7 @@ static extent_hooks_t hooks_orig; static extent_hooks_t hooks_unmap = { extent_alloc_hook, extent_dalloc_unmap, /* dalloc */ + extent_destroy_hook, extent_commit_hook, extent_decommit_hook, extent_purge_lazy_hook, diff --git a/test/unit/base.c b/test/unit/base.c index f498394e..5dc42f0a 100644 --- a/test/unit/base.c +++ b/test/unit/base.c @@ -5,6 +5,7 @@ static extent_hooks_t hooks_null = { extent_alloc_hook, NULL, /* dalloc */ + NULL, /* destroy */ NULL, /* commit */ NULL, /* decommit */ NULL, /* purge_lazy */ @@ -16,6 +17,7 @@ static extent_hooks_t hooks_null = { static extent_hooks_t hooks_not_null = { extent_alloc_hook, extent_dalloc_hook, + extent_destroy_hook, NULL, /* commit */ extent_decommit_hook, extent_purge_lazy_hook, @@ -59,6 +61,7 @@ TEST_BEGIN(test_base_hooks_null) { extent_hooks_prep(); try_dalloc = false; + try_destroy = true; try_decommit = false; try_purge_lazy = false; try_purge_forced = false; @@ -98,6 +101,7 @@ TEST_BEGIN(test_base_hooks_not_null) { extent_hooks_prep(); try_dalloc = false; + try_destroy = true; try_decommit = false; try_purge_lazy = false; try_purge_forced = false; @@ -194,15 +198,17 @@ TEST_BEGIN(test_base_hooks_not_null) { } } - called_dalloc = called_decommit = called_purge_lazy = + called_dalloc = called_destroy = called_decommit = called_purge_lazy = called_purge_forced = false; base_delete(base); assert_true(called_dalloc, "Expected dalloc call"); + assert_true(!called_destroy, "Unexpected destroy call"); assert_true(called_decommit, "Expected decommit call"); assert_true(called_purge_lazy, "Expected purge_lazy call"); assert_true(called_purge_forced, "Expected purge_forced call"); try_dalloc = true; + try_destroy = true; try_decommit = true; try_purge_lazy = true; try_purge_forced = true;