From d90655390f5192d53723023667b57453ba23e676 Mon Sep 17 00:00:00 2001 From: Alex Lapenkou Date: Mon, 22 Nov 2021 16:57:56 -0800 Subject: [PATCH] San: Create a function for committing and zeroing Committing and zeroing an extent is usually done together, hence a new function. --- include/jemalloc/internal/extent.h | 4 ++- src/extent.c | 56 +++++++++++++++++++----------- src/san_bump.c | 19 +++------- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/include/jemalloc/internal/extent.h b/include/jemalloc/internal/extent.h index 1660f45f..7336e8b8 100644 --- a/include/jemalloc/internal/extent.h +++ b/include/jemalloc/internal/extent.h @@ -43,7 +43,7 @@ void extent_dalloc_wrapper(tsdn_t *tsdn, pac_t *pac, ehooks_t *ehooks, void extent_destroy_wrapper(tsdn_t *tsdn, pac_t *pac, ehooks_t *ehooks, edata_t *edata); bool extent_commit_wrapper(tsdn_t *tsdn, ehooks_t *ehooks, edata_t *edata, - size_t offset, size_t length, bool growing_retained); + size_t offset, size_t length); bool extent_decommit_wrapper(tsdn_t *tsdn, ehooks_t *ehooks, edata_t *edata, size_t offset, size_t length); bool extent_purge_lazy_wrapper(tsdn_t *tsdn, ehooks_t *ehooks, edata_t *edata, @@ -55,6 +55,8 @@ edata_t *extent_split_wrapper(tsdn_t *tsdn, pac_t *pac, bool holding_core_locks); bool extent_merge_wrapper(tsdn_t *tsdn, pac_t *pac, ehooks_t *ehooks, edata_t *a, edata_t *b); +bool extent_commit_zero(tsdn_t *tsdn, ehooks_t *ehooks, edata_t *edata, + bool commit, bool zero, bool growing_retained); size_t extent_sn_next(pac_t *pac); bool extent_boot(void); diff --git a/src/extent.c b/src/extent.c index 6fabcc7d..4bbbff38 100644 --- a/src/extent.c +++ b/src/extent.c @@ -604,26 +604,20 @@ extent_recycle(tsdn_t *tsdn, pac_t *pac, ehooks_t *ehooks, ecache_t *ecache, return NULL; } - if (*commit && !edata_committed_get(edata)) { - if (extent_commit_impl(tsdn, ehooks, edata, 0, - edata_size_get(edata), growing_retained)) { - extent_record(tsdn, pac, ehooks, ecache, edata); - return NULL; - } - } - - if (edata_committed_get(edata)) { - *commit = true; - } - assert(edata_state_get(edata) == extent_state_active); - - if (zero) { - void *addr = edata_base_get(edata); - if (!edata_zeroed_get(edata)) { - size_t size = edata_size_get(edata); - ehooks_zero(tsdn, ehooks, addr, size); - } + if (extent_commit_zero(tsdn, ehooks, edata, *commit, zero, + growing_retained)) { + extent_record(tsdn, pac, ehooks, ecache, edata); + return NULL; + } + if (edata_committed_get(edata)) { + /* + * This reverses the purpose of this variable - previously it + * was treated as an input parameter, now it turns into an + * output parameter, reporting if the edata has actually been + * committed. + */ + *commit = true; } return edata; } @@ -1106,9 +1100,9 @@ extent_commit_impl(tsdn_t *tsdn, ehooks_t *ehooks, edata_t *edata, bool extent_commit_wrapper(tsdn_t *tsdn, ehooks_t *ehooks, edata_t *edata, - size_t offset, size_t length, bool growing_retained) { + size_t offset, size_t length) { return extent_commit_impl(tsdn, ehooks, edata, offset, length, - growing_retained); + /* growing_retained */ false); } bool @@ -1287,6 +1281,26 @@ extent_merge_wrapper(tsdn_t *tsdn, pac_t *pac, ehooks_t *ehooks, /* holding_core_locks */ false); } +bool +extent_commit_zero(tsdn_t *tsdn, ehooks_t *ehooks, edata_t *edata, + bool commit, bool zero, bool growing_retained) { + witness_assert_depth_to_rank(tsdn_witness_tsdp_get(tsdn), + WITNESS_RANK_CORE, growing_retained ? 1 : 0); + + if (commit && !edata_committed_get(edata)) { + if (extent_commit_impl(tsdn, ehooks, edata, 0, + edata_size_get(edata), growing_retained)) { + return true; + } + } + if (zero && !edata_zeroed_get(edata)) { + void *addr = edata_base_get(edata); + size_t size = edata_size_get(edata); + ehooks_zero(tsdn, ehooks, addr, size); + } + return false; +} + bool extent_boot(void) { assert(sizeof(slab_data_t) >= sizeof(e_prof_info_t)); diff --git a/src/san_bump.c b/src/san_bump.c index 1a94e55d..88897455 100644 --- a/src/san_bump.c +++ b/src/san_bump.c @@ -68,20 +68,11 @@ san_bump_alloc(tsdn_t *tsdn, san_bump_alloc_t* sba, pac_t *pac, san_guard_pages(tsdn, ehooks, edata, pac->emap, /* left */ false, /* right */ true, /* remap */ true); - if (!edata_committed_get(edata)) { - if (extent_commit_wrapper(tsdn, ehooks, edata, 0, - edata_size_get(edata), true)) { - extent_record(tsdn, pac, ehooks, &pac->ecache_retained, - edata); - return NULL; - } - edata_committed_set(edata, true); - } - if (zero && !edata_zeroed_get(edata)) { - void *addr = edata_base_get(edata); - size_t size = edata_size_get(edata); - ehooks_zero(tsdn, ehooks, addr, size); - edata_zeroed_set(edata, true); + if (extent_commit_zero(tsdn, ehooks, edata, /* commit */ true, zero, + /* growing_retained */ false)) { + extent_record(tsdn, pac, ehooks, &pac->ecache_retained, + edata); + return NULL; } if (config_prof) {