From 982c10de3566f38628770e57c62d1a6cdc5a09f9 Mon Sep 17 00:00:00 2001 From: David Goldblatt Date: Thu, 8 Mar 2018 16:34:17 -0800 Subject: [PATCH] TSD: Make all state access happen through a function. Shortly, tsd state will be atomic and have some complicated enough logic down the state-setting path that we should be aware of it. --- include/jemalloc/internal/atomic.h | 2 + .../internal/jemalloc_internal_inlines_a.h | 2 +- include/jemalloc/internal/tsd.h | 37 ++++++++++++------- src/tsd.c | 30 +++++++-------- test/unit/tsd.c | 4 +- 5 files changed, 43 insertions(+), 32 deletions(-) diff --git a/include/jemalloc/internal/atomic.h b/include/jemalloc/internal/atomic.h index a184e465..bb751cfc 100644 --- a/include/jemalloc/internal/atomic.h +++ b/include/jemalloc/internal/atomic.h @@ -66,6 +66,8 @@ JEMALLOC_GENERATE_INT_ATOMICS(size_t, zu, LG_SIZEOF_PTR) JEMALLOC_GENERATE_INT_ATOMICS(ssize_t, zd, LG_SIZEOF_PTR) +JEMALLOC_GENERATE_INT_ATOMICS(uint8_t, u8, 0) + JEMALLOC_GENERATE_INT_ATOMICS(uint32_t, u32, 2) #ifdef JEMALLOC_ATOMIC_U64 diff --git a/include/jemalloc/internal/jemalloc_internal_inlines_a.h b/include/jemalloc/internal/jemalloc_internal_inlines_a.h index c6a1f7eb..6577a4f2 100644 --- a/include/jemalloc/internal/jemalloc_internal_inlines_a.h +++ b/include/jemalloc/internal/jemalloc_internal_inlines_a.h @@ -156,7 +156,7 @@ pre_reentrancy(tsd_t *tsd, arena_t *arena) { if (fast) { /* Prepare slow path for reentrancy. */ tsd_slow_update(tsd); - assert(tsd->state == tsd_state_nominal_slow); + assert(tsd_state_get(tsd) == tsd_state_nominal_slow); } } diff --git a/include/jemalloc/internal/tsd.h b/include/jemalloc/internal/tsd.h index 0b9841aa..aa64d937 100644 --- a/include/jemalloc/internal/tsd.h +++ b/include/jemalloc/internal/tsd.h @@ -107,9 +107,6 @@ enum { tsd_state_uninitialized = 5 }; -/* Manually limit tsd_state_t to a single byte. */ -typedef uint8_t tsd_state_t; - /* The actual tsd. */ struct tsd_s { /* @@ -117,13 +114,25 @@ struct tsd_s { * module. Access any thread-local state through the getters and * setters below. */ - tsd_state_t state; + + /* We manually limit the state to just a single byte. */ + uint8_t state; #define O(n, t, nt) \ t use_a_getter_or_setter_instead_##n; MALLOC_TSD #undef O }; +JEMALLOC_ALWAYS_INLINE uint8_t +tsd_state_get(tsd_t *tsd) { + return tsd->state; +} + +JEMALLOC_ALWAYS_INLINE void +tsd_state_set(tsd_t *tsd, uint8_t state) { + tsd->state = state; +} + /* * Wrapper around tsd_t that makes it possible to avoid implicit conversion * between tsd_t and tsdn_t, where tsdn_t is "nullable" and has to be @@ -191,10 +200,10 @@ MALLOC_TSD #define O(n, t, nt) \ JEMALLOC_ALWAYS_INLINE t * \ tsd_##n##p_get(tsd_t *tsd) { \ - assert(tsd->state == tsd_state_nominal || \ - tsd->state == tsd_state_nominal_slow || \ - tsd->state == tsd_state_reincarnated || \ - tsd->state == tsd_state_minimal_initialized); \ + assert(tsd_state_get(tsd) == tsd_state_nominal || \ + tsd_state_get(tsd) == tsd_state_nominal_slow || \ + tsd_state_get(tsd) == tsd_state_reincarnated || \ + tsd_state_get(tsd) == tsd_state_minimal_initialized); \ return tsd_##n##p_get_unsafe(tsd); \ } MALLOC_TSD @@ -229,8 +238,8 @@ MALLOC_TSD #define O(n, t, nt) \ JEMALLOC_ALWAYS_INLINE void \ tsd_##n##_set(tsd_t *tsd, t val) { \ - assert(tsd->state != tsd_state_reincarnated && \ - tsd->state != tsd_state_minimal_initialized); \ + assert(tsd_state_get(tsd) != tsd_state_reincarnated && \ + tsd_state_get(tsd) != tsd_state_minimal_initialized); \ *tsd_##n##p_get(tsd) = val; \ } MALLOC_TSD @@ -244,7 +253,7 @@ tsd_assert_fast(tsd_t *tsd) { JEMALLOC_ALWAYS_INLINE bool tsd_fast(tsd_t *tsd) { - bool fast = (tsd->state == tsd_state_nominal); + bool fast = (tsd_state_get(tsd) == tsd_state_nominal); if (fast) { tsd_assert_fast(tsd); } @@ -261,7 +270,7 @@ tsd_fetch_impl(bool init, bool minimal) { } assert(tsd != NULL); - if (unlikely(tsd->state != tsd_state_nominal)) { + if (unlikely(tsd_state_get(tsd) != tsd_state_nominal)) { return tsd_fetch_slow(tsd, minimal); } assert(tsd_fast(tsd)); @@ -281,7 +290,7 @@ JEMALLOC_ALWAYS_INLINE tsd_t * tsd_internal_fetch(void) { tsd_t *tsd = tsd_fetch_min(); /* Use reincarnated state to prevent full initialization. */ - tsd->state = tsd_state_reincarnated; + tsd_state_set(tsd, tsd_state_reincarnated); return tsd; } @@ -293,7 +302,7 @@ tsd_fetch(void) { static inline bool tsd_nominal(tsd_t *tsd) { - return (tsd->state <= tsd_state_nominal_max); + return (tsd_state_get(tsd) <= tsd_state_nominal_max); } JEMALLOC_ALWAYS_INLINE tsdn_t * diff --git a/src/tsd.c b/src/tsd.c index c1430682..f3320ebb 100644 --- a/src/tsd.c +++ b/src/tsd.c @@ -56,9 +56,9 @@ tsd_slow_update(tsd_t *tsd) { if (tsd_nominal(tsd)) { if (malloc_slow || !tsd_tcache_enabled_get(tsd) || tsd_reentrancy_level_get(tsd) > 0) { - tsd->state = tsd_state_nominal_slow; + tsd_state_set(tsd, tsd_state_nominal_slow); } else { - tsd->state = tsd_state_nominal; + tsd_state_set(tsd, tsd_state_nominal); } } } @@ -97,8 +97,8 @@ assert_tsd_data_cleanup_done(tsd_t *tsd) { static bool tsd_data_init_nocleanup(tsd_t *tsd) { - assert(tsd->state == tsd_state_reincarnated || - tsd->state == tsd_state_minimal_initialized); + assert(tsd_state_get(tsd) == tsd_state_reincarnated || + tsd_state_get(tsd) == tsd_state_minimal_initialized); /* * During reincarnation, there is no guarantee that the cleanup function * will be called (deallocation may happen after all tsd destructors). @@ -117,27 +117,27 @@ tsd_t * tsd_fetch_slow(tsd_t *tsd, bool minimal) { assert(!tsd_fast(tsd)); - if (tsd->state == tsd_state_nominal_slow) { + if (tsd_state_get(tsd) == tsd_state_nominal_slow) { /* On slow path but no work needed. */ assert(malloc_slow || !tsd_tcache_enabled_get(tsd) || tsd_reentrancy_level_get(tsd) > 0 || *tsd_arenas_tdata_bypassp_get(tsd)); - } else if (tsd->state == tsd_state_uninitialized) { + } else if (tsd_state_get(tsd) == tsd_state_uninitialized) { if (!minimal) { - tsd->state = tsd_state_nominal; + tsd_state_set(tsd, tsd_state_nominal); tsd_slow_update(tsd); /* Trigger cleanup handler registration. */ tsd_set(tsd); tsd_data_init(tsd); } else { - tsd->state = tsd_state_minimal_initialized; + tsd_state_set(tsd, tsd_state_minimal_initialized); tsd_set(tsd); tsd_data_init_nocleanup(tsd); } - } else if (tsd->state == tsd_state_minimal_initialized) { + } else if (tsd_state_get(tsd) == tsd_state_minimal_initialized) { if (!minimal) { /* Switch to fully initialized. */ - tsd->state = tsd_state_nominal; + tsd_state_set(tsd, tsd_state_nominal); assert(*tsd_reentrancy_levelp_get(tsd) >= 1); (*tsd_reentrancy_levelp_get(tsd))--; tsd_slow_update(tsd); @@ -145,12 +145,12 @@ tsd_fetch_slow(tsd_t *tsd, bool minimal) { } else { assert_tsd_data_cleanup_done(tsd); } - } else if (tsd->state == tsd_state_purgatory) { - tsd->state = tsd_state_reincarnated; + } else if (tsd_state_get(tsd) == tsd_state_purgatory) { + tsd_state_set(tsd, tsd_state_reincarnated); tsd_set(tsd); tsd_data_init_nocleanup(tsd); } else { - assert(tsd->state == tsd_state_reincarnated); + assert(tsd_state_get(tsd) == tsd_state_reincarnated); } return tsd; @@ -214,7 +214,7 @@ void tsd_cleanup(void *arg) { tsd_t *tsd = (tsd_t *)arg; - switch (tsd->state) { + switch (tsd_state_get(tsd)) { case tsd_state_uninitialized: /* Do nothing. */ break; @@ -232,7 +232,7 @@ tsd_cleanup(void *arg) { case tsd_state_nominal: case tsd_state_nominal_slow: tsd_do_data_cleanup(tsd); - tsd->state = tsd_state_purgatory; + tsd_state_set(tsd, tsd_state_purgatory); tsd_set(tsd); break; case tsd_state_purgatory: diff --git a/test/unit/tsd.c b/test/unit/tsd.c index 6c479139..3379891b 100644 --- a/test/unit/tsd.c +++ b/test/unit/tsd.c @@ -98,11 +98,11 @@ thd_start_reincarnated(void *arg) { tsd_cleanup((void *)tsd); assert_ptr_null(*tsd_arenap_get_unsafe(tsd), "TSD arena should have been cleared."); - assert_u_eq(tsd->state, tsd_state_purgatory, + assert_u_eq(tsd_state_get(tsd), tsd_state_purgatory, "TSD state should be purgatory\n"); free(p); - assert_u_eq(tsd->state, tsd_state_reincarnated, + assert_u_eq(tsd_state_get(tsd), tsd_state_reincarnated, "TSD state should be reincarnated\n"); p = mallocx(1, MALLOCX_TCACHE_NONE); assert_ptr_not_null(p, "Unexpected malloc() failure");