From 99fc0717e653277c3d7fe77fe84316ad47381936 Mon Sep 17 00:00:00 2001 From: David Goldblatt Date: Sat, 5 Dec 2020 15:58:31 -0800 Subject: [PATCH] psset: Reconceptualize insertion/removal. Really, this isn't a functional change, just a naming change. We start thinking of pageslabs as being always in the psset. What we used to think of as removal is now thought of as being in the psset, but in the process of being updated (and therefore, unavalable for serving new allocations). This is in preparation of subsequent changes to support deferred purging; allocations will still be in the psset for the purposes of choosing when to purge, but not for purposes of allocation/deallocation. --- include/jemalloc/internal/hpdata.h | 18 +++++++---- include/jemalloc/internal/psset.h | 10 ++++-- src/hpa.c | 35 ++++++++++++-------- src/hpdata.c | 23 ++++++------- src/psset.c | 52 +++++++++++++++--------------- test/unit/hpdata.c | 6 ++++ test/unit/psset.c | 33 ++++++++++++------- 7 files changed, 106 insertions(+), 71 deletions(-) diff --git a/include/jemalloc/internal/hpdata.h b/include/jemalloc/internal/hpdata.h index f8001586..2e2e1d8e 100644 --- a/include/jemalloc/internal/hpdata.h +++ b/include/jemalloc/internal/hpdata.h @@ -44,8 +44,13 @@ struct hpdata_s { bool h_mid_purge; bool h_mid_hugify; - /* Whether or not the hpdata is a the psset. */ - bool h_in_psset; + /* + * Whether or not the hpdata is being updated in the psset (i.e. if + * there has been a psset_update_begin call issued without a matching + * psset_update_end call). Eventually this will expand to other types + * of updates. + */ + bool h_updating; union { /* When nonempty, used by the psset bins. */ @@ -123,13 +128,14 @@ hpdata_mid_hugify_get(const hpdata_t *hpdata) { } static inline bool -hpdata_in_psset_get(const hpdata_t *hpdata) { - return hpdata->h_in_psset; +hpdata_updating_get(const hpdata_t *hpdata) { + return hpdata->h_updating; } static inline void -hpdata_in_psset_set(hpdata_t *hpdata, bool in_psset) { - hpdata->h_in_psset = in_psset; +hpdata_updating_set(hpdata_t *hpdata, bool updating) { + assert(updating != hpdata->h_updating); + hpdata->h_updating = updating; } static inline size_t diff --git a/include/jemalloc/internal/psset.h b/include/jemalloc/internal/psset.h index fef0468e..a7c9a8b6 100644 --- a/include/jemalloc/internal/psset.h +++ b/include/jemalloc/internal/psset.h @@ -64,10 +64,14 @@ struct psset_s { void psset_init(psset_t *psset); void psset_stats_accum(psset_stats_t *dst, psset_stats_t *src); -void psset_insert(psset_t *psset, hpdata_t *ps); -void psset_remove(psset_t *psset, hpdata_t *ps); +/* + * Begin or end updating the given pageslab's metadata. While the pageslab is + * being updated, it won't be returned from psset_fit calls. + */ +void psset_update_begin(psset_t *psset, hpdata_t *ps); +void psset_update_end(psset_t *psset, hpdata_t *ps); /* Analogous to the eset_fit; pick a hpdata to serve the request. */ -hpdata_t *psset_fit(psset_t *psset, size_t size); +hpdata_t *psset_pick_alloc(psset_t *psset, size_t size); #endif /* JEMALLOC_INTERNAL_PSSET_H */ diff --git a/src/hpa.c b/src/hpa.c index 822e3bac..6a4f2a6d 100644 --- a/src/hpa.c +++ b/src/hpa.c @@ -333,14 +333,14 @@ hpa_try_alloc_no_grow(tsdn_t *tsdn, hpa_shard_t *shard, size_t size, bool *oom) } assert(edata_arena_ind_get(edata) == shard->ind); - hpdata_t *ps = psset_fit(&shard->psset, size); + hpdata_t *ps = psset_pick_alloc(&shard->psset, size); if (ps == NULL) { edata_cache_small_put(tsdn, &shard->ecs, edata); malloc_mutex_unlock(tsdn, &shard->mtx); return NULL; } - psset_remove(&shard->psset, ps); + psset_update_begin(&shard->psset, ps); void *addr = hpdata_reserve_alloc(ps, size); edata_init(edata, shard->ind, addr, size, /* slab */ false, SC_NSIZES, /* sn */ 0, extent_state_active, /* zeroed */ false, @@ -365,7 +365,7 @@ hpa_try_alloc_no_grow(tsdn_t *tsdn, hpa_shard_t *shard, size_t size, bool *oom) * require some sort of prepare + commit functionality that's a * little much to deal with for now. */ - psset_insert(&shard->psset, ps); + psset_update_end(&shard->psset, ps); edata_cache_small_put(tsdn, &shard->ecs, edata); malloc_mutex_unlock(tsdn, &shard->mtx); *oom = true; @@ -377,7 +377,7 @@ hpa_try_alloc_no_grow(tsdn_t *tsdn, hpa_shard_t *shard, size_t size, bool *oom) hpdata_hugify_begin(ps); shard->stats.nhugifies++; } - psset_insert(&shard->psset, ps); + psset_update_end(&shard->psset, ps); malloc_mutex_unlock(tsdn, &shard->mtx); if (hugify) { @@ -409,9 +409,9 @@ hpa_try_alloc_no_grow(tsdn_t *tsdn, hpa_shard_t *shard, size_t size, bool *oom) * hugified. Undo our operation, taking care to meet * the precondition that the ps isn't in the psset. */ - psset_remove(&shard->psset, ps); + psset_update_begin(&shard->psset, ps); hpa_purge(tsdn, shard, ps); - psset_insert(&shard->psset, ps); + psset_update_end(&shard->psset, ps); } malloc_mutex_unlock(tsdn, &shard->mtx); } @@ -455,6 +455,15 @@ hpa_alloc_psset(tsdn_t *tsdn, hpa_shard_t *shard, size_t size) { /* We got the new edata; allocate from it. */ malloc_mutex_lock(tsdn, &shard->mtx); + /* + * This will go away soon. The psset doesn't draw a distinction between + * pageslab removal and updating. If this is a new pageslab, we pretend + * that it's an old one that's been getting updated. + */ + if (!hpdata_updating_get(ps)) { + hpdata_updating_set(ps, true); + } + edata = edata_cache_small_get(tsdn, &shard->ecs); if (edata == NULL) { shard->stats.nevictions++; @@ -500,7 +509,7 @@ hpa_alloc_psset(tsdn_t *tsdn, hpa_shard_t *shard, size_t size) { hpa_handle_ps_eviction(tsdn, shard, ps); return NULL; } - psset_insert(&shard->psset, ps); + psset_update_end(&shard->psset, ps); malloc_mutex_unlock(tsdn, &shard->mtx); malloc_mutex_unlock(tsdn, &shard->grow_mtx); @@ -615,7 +624,7 @@ hpa_dalloc(tsdn_t *tsdn, pai_t *self, edata_t *edata) { * psset and we can do our metadata update. The other thread is * in charge of reinserting the ps, so we're done. */ - assert(!hpdata_in_psset_get(ps)); + assert(hpdata_updating_get(ps)); hpdata_unreserve(ps, unreserve_addr, unreserve_size); malloc_mutex_unlock(tsdn, &shard->mtx); return; @@ -624,15 +633,15 @@ hpa_dalloc(tsdn_t *tsdn, pai_t *self, edata_t *edata) { * No other thread is purging, and the ps is non-empty, so it should be * in the psset. */ - assert(hpdata_in_psset_get(ps)); - psset_remove(&shard->psset, ps); + assert(!hpdata_updating_get(ps)); + psset_update_begin(&shard->psset, ps); hpdata_unreserve(ps, unreserve_addr, unreserve_size); if (!hpa_should_purge(shard, ps)) { /* * This should be the common case; no other thread is purging, * and we won't purge either. */ - psset_insert(&shard->psset, ps); + psset_update_end(&shard->psset, ps); malloc_mutex_unlock(tsdn, &shard->mtx); return; } @@ -648,7 +657,7 @@ hpa_dalloc(tsdn_t *tsdn, pai_t *self, edata_t *edata) { malloc_mutex_unlock(tsdn, &shard->mtx); hpa_handle_ps_eviction(tsdn, shard, ps); } else { - psset_insert(&shard->psset, ps); + psset_update_end(&shard->psset, ps); malloc_mutex_unlock(tsdn, &shard->mtx); } } @@ -669,7 +678,7 @@ hpa_shard_assert_stats_empty(psset_bin_stats_t *bin_stats) { static void hpa_assert_empty(tsdn_t *tsdn, hpa_shard_t *shard, psset_t *psset) { malloc_mutex_assert_owner(tsdn, &shard->mtx); - hpdata_t *ps = psset_fit(psset, PAGE); + hpdata_t *ps = psset_pick_alloc(psset, PAGE); assert(ps == NULL); for (int huge = 0; huge <= 1; huge++) { hpa_shard_assert_stats_empty(&psset->stats.full_slabs[huge]); diff --git a/src/hpdata.c b/src/hpdata.c index e2a0b37f..0af7da0c 100644 --- a/src/hpdata.c +++ b/src/hpdata.c @@ -24,7 +24,7 @@ hpdata_init(hpdata_t *hpdata, void *addr, uint64_t age) { hpdata->h_huge = false; hpdata->h_mid_purge = false; hpdata->h_mid_hugify = false; - hpdata->h_in_psset = false; + hpdata->h_updating = false; hpdata_longest_free_range_set(hpdata, HUGEPAGE_PAGES); hpdata->h_nactive = 0; fb_init(hpdata->active_pages, HUGEPAGE_PAGES); @@ -37,7 +37,7 @@ hpdata_init(hpdata_t *hpdata, void *addr, uint64_t age) { void * hpdata_reserve_alloc(hpdata_t *hpdata, size_t sz) { hpdata_assert_consistent(hpdata); - assert(!hpdata_in_psset_get(hpdata)); + assert(hpdata->h_updating); assert((sz & PAGE_MASK) == 0); size_t npages = sz >> LG_PAGE; assert(npages <= hpdata_longest_free_range_get(hpdata)); @@ -118,7 +118,7 @@ hpdata_reserve_alloc(hpdata_t *hpdata, size_t sz) { void hpdata_unreserve(hpdata_t *hpdata, void *addr, size_t sz) { hpdata_assert_consistent(hpdata); - assert(!hpdata->h_in_psset); + assert(hpdata->h_updating); assert(((uintptr_t)addr & PAGE_MASK) == 0); assert((sz & PAGE_MASK) == 0); size_t begin = ((uintptr_t)addr - (uintptr_t)hpdata_addr_get(hpdata)) @@ -147,7 +147,7 @@ hpdata_unreserve(hpdata_t *hpdata, void *addr, size_t sz) { void hpdata_purge_begin(hpdata_t *hpdata, hpdata_purge_state_t *purge_state) { hpdata_assert_consistent(hpdata); - assert(!hpdata->h_in_psset); + assert(hpdata->h_updating); assert(!hpdata->h_mid_purge); assert(!hpdata->h_mid_hugify); hpdata->h_mid_purge = true; @@ -185,7 +185,7 @@ hpdata_purge_next(hpdata_t *hpdata, hpdata_purge_state_t *purge_state, * a consistent state. */ assert(hpdata->h_mid_purge); - assert(!hpdata->h_in_psset); + assert(hpdata->h_updating); /* Should have dehugified already (if necessary). */ assert(!hpdata->h_huge); assert(!hpdata->h_mid_hugify); @@ -215,7 +215,7 @@ hpdata_purge_next(hpdata_t *hpdata, hpdata_purge_state_t *purge_state, void hpdata_purge_end(hpdata_t *hpdata, hpdata_purge_state_t *purge_state) { hpdata_assert_consistent(hpdata); - assert(!hpdata->h_in_psset); + assert(hpdata->h_updating); assert(hpdata->h_mid_purge); assert(!hpdata->h_mid_hugify); hpdata->h_mid_purge = false; @@ -236,7 +236,7 @@ hpdata_purge_end(hpdata_t *hpdata, hpdata_purge_state_t *purge_state) { void hpdata_hugify_begin(hpdata_t *hpdata) { hpdata_assert_consistent(hpdata); - assert(!hpdata_in_psset_get(hpdata)); + assert(hpdata->h_updating); assert(!hpdata->h_mid_purge); assert(!hpdata->h_mid_hugify); hpdata->h_mid_hugify = true; @@ -250,10 +250,10 @@ void hpdata_hugify_end(hpdata_t *hpdata) { hpdata_assert_consistent(hpdata); /* - * This is the exception to the "no metadata tweaks while in the psset" - * rule. + * This is the exception to the "no-metadata updates without informing + * the psset first" rule; this assert would be incorrect. */ - /* assert(!hpdata_in_psset_get(hpdata)); */ + /* assert(hpdata->h_updating); */ assert(!hpdata->h_mid_purge); assert(hpdata->h_mid_hugify); hpdata->h_mid_hugify = false; @@ -263,7 +263,8 @@ hpdata_hugify_end(hpdata_t *hpdata) { void hpdata_dehugify(hpdata_t *hpdata) { hpdata_assert_consistent(hpdata); - assert(!hpdata_in_psset_get(hpdata)); + assert(hpdata->h_updating); + assert(hpdata->h_updating); assert(hpdata->h_mid_purge); assert(!hpdata->h_mid_hugify); hpdata->h_huge = false; diff --git a/src/psset.c b/src/psset.c index a09913c5..22564605 100644 --- a/src/psset.c +++ b/src/psset.c @@ -79,11 +79,33 @@ psset_hpdata_heap_insert(psset_t *psset, pszind_t pind, hpdata_t *ps) { } void -psset_insert(psset_t *psset, hpdata_t *ps) { +psset_update_begin(psset_t *psset, hpdata_t *ps) { + hpdata_assert_consistent(ps); + assert(!hpdata_updating_get(ps)); + hpdata_updating_set(ps, true); + + size_t longest_free_range = hpdata_longest_free_range_get(ps); + + if (longest_free_range == 0) { + psset_bin_stats_remove(psset->stats.full_slabs, ps); + return; + } + + pszind_t pind = sz_psz2ind(sz_psz_quantize_floor( + longest_free_range << LG_PAGE)); + assert(pind < PSSET_NPSIZES); + psset_hpdata_heap_remove(psset, pind, ps); + if (hpdata_age_heap_empty(&psset->pageslabs[pind])) { + bitmap_set(psset->bitmap, &psset_bitmap_info, (size_t)pind); + } +} + +void +psset_update_end(psset_t *psset, hpdata_t *ps) { assert(!hpdata_empty(ps)); hpdata_assert_consistent(ps); - assert(!hpdata_in_psset_get(ps)); - hpdata_in_psset_set(ps, true); + assert(hpdata_updating_get(ps)); + hpdata_updating_set(ps, false); size_t longest_free_range = hpdata_longest_free_range_get(ps); if (longest_free_range == 0) { @@ -105,30 +127,8 @@ psset_insert(psset_t *psset, hpdata_t *ps) { psset_hpdata_heap_insert(psset, pind, ps); } -void -psset_remove(psset_t *psset, hpdata_t *ps) { - hpdata_assert_consistent(ps); - assert(hpdata_in_psset_get(ps)); - hpdata_in_psset_set(ps, false); - - size_t longest_free_range = hpdata_longest_free_range_get(ps); - - if (longest_free_range == 0) { - psset_bin_stats_remove(psset->stats.full_slabs, ps); - return; - } - - pszind_t pind = sz_psz2ind(sz_psz_quantize_floor( - longest_free_range << LG_PAGE)); - assert(pind < PSSET_NPSIZES); - psset_hpdata_heap_remove(psset, pind, ps); - if (hpdata_age_heap_empty(&psset->pageslabs[pind])) { - bitmap_set(psset->bitmap, &psset_bitmap_info, (size_t)pind); - } -} - hpdata_t * -psset_fit(psset_t *psset, size_t size) { +psset_pick_alloc(psset_t *psset, size_t size) { pszind_t min_pind = sz_psz2ind(sz_psz_quantize_ceil(size)); pszind_t pind = (pszind_t)bitmap_ffu(psset->bitmap, &psset_bitmap_info, (size_t)min_pind); diff --git a/test/unit/hpdata.c b/test/unit/hpdata.c index 688911a6..cf7b89fd 100644 --- a/test/unit/hpdata.c +++ b/test/unit/hpdata.c @@ -7,6 +7,8 @@ TEST_BEGIN(test_reserve_alloc) { hpdata_t hpdata; hpdata_init(&hpdata, HPDATA_ADDR, HPDATA_AGE); + hpdata_updating_set(&hpdata, true); + /* Allocating a page at a time, we should do first fit. */ for (size_t i = 0; i < HUGEPAGE_PAGES; i++) { expect_true(hpdata_consistent(&hpdata), ""); @@ -59,6 +61,8 @@ TEST_BEGIN(test_purge_simple) { hpdata_t hpdata; hpdata_init(&hpdata, HPDATA_ADDR, HPDATA_AGE); + hpdata_updating_set(&hpdata, true); + void *alloc = hpdata_reserve_alloc(&hpdata, HUGEPAGE_PAGES / 2 * PAGE); expect_ptr_eq(alloc, HPDATA_ADDR, ""); @@ -107,6 +111,7 @@ TEST_END TEST_BEGIN(test_purge_intervening_dalloc) { hpdata_t hpdata; hpdata_init(&hpdata, HPDATA_ADDR, HPDATA_AGE); + hpdata_updating_set(&hpdata, true); /* Allocate the first 3/4 of the pages. */ void *alloc = hpdata_reserve_alloc(&hpdata, 3 * HUGEPAGE_PAGES / 4 * PAGE); @@ -160,6 +165,7 @@ TEST_END TEST_BEGIN(test_hugify) { hpdata_t hpdata; hpdata_init(&hpdata, HPDATA_ADDR, HPDATA_AGE); + hpdata_updating_set(&hpdata, true); void *alloc = hpdata_reserve_alloc(&hpdata, HUGEPAGE / 2); expect_ptr_eq(alloc, HPDATA_ADDR, ""); diff --git a/test/unit/psset.c b/test/unit/psset.c index 88014445..2043e4eb 100644 --- a/test/unit/psset.c +++ b/test/unit/psset.c @@ -19,41 +19,50 @@ static void test_psset_alloc_new(psset_t *psset, hpdata_t *ps, edata_t *r_edata, size_t size) { hpdata_assert_empty(ps); + + /* + * As in hpa.c; pretend that the ps is already in the psset and just + * being updated, until we implement true insert/removal support. + */ + if (!hpdata_updating_get(ps)) { + hpdata_updating_set(ps, true); + } + void *addr = hpdata_reserve_alloc(ps, size); edata_init(r_edata, edata_arena_ind_get(r_edata), addr, size, /* slab */ false, SC_NSIZES, /* sn */ 0, extent_state_active, /* zeroed */ false, /* committed */ true, EXTENT_PAI_HPA, EXTENT_NOT_HEAD); edata_ps_set(r_edata, ps); - psset_insert(psset, ps); + psset_update_end(psset, ps); } static bool test_psset_alloc_reuse(psset_t *psset, edata_t *r_edata, size_t size) { - hpdata_t *ps = psset_fit(psset, size); + hpdata_t *ps = psset_pick_alloc(psset, size); if (ps == NULL) { return true; } - psset_remove(psset, ps); + psset_update_begin(psset, ps); void *addr = hpdata_reserve_alloc(ps, size); edata_init(r_edata, edata_arena_ind_get(r_edata), addr, size, /* slab */ false, SC_NSIZES, /* sn */ 0, extent_state_active, /* zeroed */ false, /* committed */ true, EXTENT_PAI_HPA, EXTENT_NOT_HEAD); edata_ps_set(r_edata, ps); - psset_insert(psset, ps); + psset_update_end(psset, ps); return false; } static hpdata_t * test_psset_dalloc(psset_t *psset, edata_t *edata) { hpdata_t *ps = edata_ps_get(edata); - psset_remove(psset, ps); + psset_update_begin(psset, ps); hpdata_unreserve(ps, edata_addr_get(edata), edata_size_get(edata)); if (hpdata_empty(ps)) { return ps; } else { - psset_insert(psset, ps); + psset_update_end(psset, ps); return NULL; } } @@ -390,9 +399,9 @@ TEST_BEGIN(test_stats) { test_psset_alloc_new(&psset, &pageslab, &alloc[0], PAGE); stats_expect(&psset, 1); - psset_remove(&psset, &pageslab); + psset_update_begin(&psset, &pageslab); stats_expect(&psset, 0); - psset_insert(&psset, &pageslab); + psset_update_end(&psset, &pageslab); stats_expect(&psset, 1); } TEST_END @@ -490,7 +499,7 @@ TEST_BEGIN(test_insert_remove) { worse_alloc); /* Remove better; should still be able to alloc from worse. */ - psset_remove(&psset, &pageslab); + psset_update_begin(&psset, &pageslab); err = test_psset_alloc_reuse(&psset, &worse_alloc[HUGEPAGE_PAGES - 1], PAGE); expect_false(err, "Removal should still leave an empty page"); @@ -504,7 +513,7 @@ TEST_BEGIN(test_insert_remove) { */ ps = test_psset_dalloc(&psset, &worse_alloc[HUGEPAGE_PAGES - 1]); expect_ptr_null(ps, "Incorrect eviction of nonempty pageslab"); - psset_insert(&psset, &pageslab); + psset_update_end(&psset, &pageslab); err = test_psset_alloc_reuse(&psset, &alloc[HUGEPAGE_PAGES - 1], PAGE); expect_false(err, "psset should be nonempty"); expect_ptr_eq(&pageslab, edata_ps_get(&alloc[HUGEPAGE_PAGES - 1]), @@ -514,8 +523,8 @@ TEST_BEGIN(test_insert_remove) { */ ps = test_psset_dalloc(&psset, &alloc[HUGEPAGE_PAGES - 1]); expect_ptr_null(ps, "Incorrect eviction"); - psset_remove(&psset, &pageslab); - psset_remove(&psset, &worse_pageslab); + psset_update_begin(&psset, &pageslab); + psset_update_begin(&psset, &worse_pageslab); err = test_psset_alloc_reuse(&psset, &alloc[HUGEPAGE_PAGES - 1], PAGE); expect_true(err, "psset should be empty, but an alloc succeeded"); }