From 74e5f5c6cf69f494b6a9765fc90d38465953e76a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 25 Jun 2025 08:25:41 +0200 Subject: [PATCH 1/5] Fix implicit headers when using isc/overflow.h header In jemalloc_shim.h, we relied on including implicitly instead of explicitly and same was happening inside isc/overflow.h - the stdbool.h (for bool type) was being included implicitly instead of explicitly. --- lib/isc/include/isc/overflow.h | 2 ++ lib/isc/jemalloc_shim.h | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/isc/include/isc/overflow.h b/lib/isc/include/isc/overflow.h index c272cfa879..62727513ad 100644 --- a/lib/isc/include/isc/overflow.h +++ b/lib/isc/include/isc/overflow.h @@ -13,6 +13,8 @@ #pragma once +#include + #include /* diff --git a/lib/isc/jemalloc_shim.h b/lib/isc/jemalloc_shim.h index 1f6ef0fc44..a29d7c0ecd 100644 --- a/lib/isc/jemalloc_shim.h +++ b/lib/isc/jemalloc_shim.h @@ -16,8 +16,10 @@ #if !defined(HAVE_JEMALLOC) #include +#include #include +#include #include const char *malloc_conf = NULL; @@ -26,8 +28,6 @@ const char *malloc_conf = NULL; #define MALLOCX_TCACHE_NONE (0) #define MALLOCX_ARENA(a) (0) -#include - typedef union { size_t size; max_align_t __alignment; From c6828bcf8fd0c0bdd07d3b477ee575a4dff01135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 4 Jun 2025 10:35:57 +0200 Subject: [PATCH 2/5] Delete jemalloc arena support from isc_mem The jemalloc arena in isc_mem was added to solve runaway memory problem for outgoing TCP connections. In the end, this was a red herring and the jemalloc arena code is now unused (via e28266bf). Remove the support for jemalloc memory arenas as we can restore this at any time if we need it ever again, but right now it's just a dead code. --- lib/isc/include/isc/mem.h | 34 ---------- lib/isc/jemalloc_shim.h | 4 +- lib/isc/mem.c | 133 -------------------------------------- 3 files changed, 1 insertion(+), 170 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index fd4238f67b..820c766944 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -219,40 +219,6 @@ isc__mem_create(const char *name, isc_mem_t **_ISC_MEM_FLARG); * mctxp != NULL && *mctxp == NULL */ /*@}*/ -#define isc_mem_create_arena(name, cp) \ - isc__mem_create_arena((name), (cp)_ISC_MEM_FILELINE) -void -isc__mem_create_arena(const char *name, isc_mem_t **_ISC_MEM_FLARG); -/*!< - * \brief Create a memory context that routs all its operations to a - * dedicated jemalloc arena (when available). When jemalloc is not - * available, the function is, effectively, an alias to - * isc_mem_create(). - * - * Requires: - * mctxp != NULL && *mctxp == NULL */ -/*@}*/ - -isc_result_t -isc_mem_arena_set_muzzy_decay_ms(isc_mem_t *mctx, const ssize_t decay_ms); - -isc_result_t -isc_mem_arena_set_dirty_decay_ms(isc_mem_t *mctx, const ssize_t decay_ms); -/*!< - * \brief These two functions set the given parameters on the - * jemalloc arena associated with the memory context (if there is - * one). When jemalloc is not available, these are no-op. - * - * NOTE: The "muzzy_decay_ms" and "dirty_decay_ms" are the most common - * parameters to adjust when the defaults do not work well (per the - * official jemalloc tuning guide: - * https://github.com/jemalloc/jemalloc/blob/dev/TUNING.md). - * - * Requires: - * mctx - a valid memory context. - */ -/*@}*/ - #if ISC_MEM_TRACE #define isc_mem_ref(ptr) isc_mem__ref(ptr, __func__, __FILE__, __LINE__) #define isc_mem_unref(ptr) isc_mem__unref(ptr, __func__, __FILE__, __LINE__) diff --git a/lib/isc/jemalloc_shim.h b/lib/isc/jemalloc_shim.h index a29d7c0ecd..55bab911d1 100644 --- a/lib/isc/jemalloc_shim.h +++ b/lib/isc/jemalloc_shim.h @@ -24,9 +24,7 @@ const char *malloc_conf = NULL; -#define MALLOCX_ZERO ((int)0x40) -#define MALLOCX_TCACHE_NONE (0) -#define MALLOCX_ARENA(a) (0) +#define MALLOCX_ZERO ((int)0x40) typedef union { size_t size; diff --git a/lib/isc/mem.c b/lib/isc/mem.c index d8d67fa766..00137c14d0 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -64,8 +64,6 @@ unsigned int isc_mem_debugging = ISC_MEM_DEBUGGING; unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT; -#define ISC_MEM_ILLEGAL_ARENA (UINT_MAX) - volatile void *isc__mem_malloc = mallocx; /* @@ -73,8 +71,6 @@ volatile void *isc__mem_malloc = mallocx; */ #define ZERO_ALLOCATION_SIZE sizeof(void *) -#define ALIGNMENT 8U /*%< must be a power of 2 */ -#define ALIGNMENT_SIZE sizeof(size_info) #define DEBUG_TABLE_COUNT 512U /* @@ -118,7 +114,6 @@ struct isc_mem { unsigned int magic; unsigned int flags; unsigned int jemalloc_flags; - unsigned int jemalloc_arena; unsigned int debugging; isc_mutex_t lock; bool checkfree; @@ -377,48 +372,6 @@ mem_putstats(isc_mem_t *ctx, size_t size) { * Private. */ -static bool -mem_jemalloc_arena_create(unsigned int *pnew_arenano) { - REQUIRE(pnew_arenano != NULL); - -#ifdef JEMALLOC_API_SUPPORTED - unsigned int arenano = 0; - size_t len = sizeof(arenano); - int res = 0; - - res = mallctl("arenas.create", &arenano, &len, NULL, 0); - if (res != 0) { - return false; - } - - *pnew_arenano = arenano; - - return true; -#else - *pnew_arenano = ISC_MEM_ILLEGAL_ARENA; - return true; -#endif /* JEMALLOC_API_SUPPORTED */ -} - -static bool -mem_jemalloc_arena_destroy(unsigned int arenano) { -#ifdef JEMALLOC_API_SUPPORTED - int res = 0; - char buf[256] = { 0 }; - - (void)snprintf(buf, sizeof(buf), "arena.%u.destroy", arenano); - res = mallctl(buf, NULL, NULL, NULL, 0); - if (res != 0) { - return false; - } - - return true; -#else - UNUSED(arenano); - return true; -#endif /* JEMALLOC_API_SUPPORTED */ -} - void isc__mem_initialize(void) { /* @@ -465,7 +418,6 @@ mem_create(const char *name, isc_mem_t **ctxp, unsigned int debugging, .debugging = debugging, .flags = flags, .jemalloc_flags = jemalloc_flags, - .jemalloc_arena = ISC_MEM_ILLEGAL_ARENA, .checkfree = true, .name = strdup(name), }; @@ -509,8 +461,6 @@ mem_create(const char *name, isc_mem_t **ctxp, unsigned int debugging, static void mem_destroy(isc_mem_t *ctx) { - unsigned int arena_no; - isc_refcount_destroy(&ctx->references); LOCK(&contextslock); @@ -519,8 +469,6 @@ mem_destroy(isc_mem_t *ctx) { ctx->magic = 0; - arena_no = ctx->jemalloc_arena; - INSIST(ISC_LIST_EMPTY(ctx->pools)); #if ISC_MEM_TRACKLINES @@ -552,10 +500,6 @@ mem_destroy(isc_mem_t *ctx) { INSIST(atomic_load(&ctx->inuse) == 0); } sdallocx(ctx, sizeof(*ctx), ctx->jemalloc_flags); - - if (arena_no != ISC_MEM_ILLEGAL_ARENA) { - RUNTIME_CHECK(mem_jemalloc_arena_destroy(arena_no) == true); - } } #if ISC_MEM_TRACE @@ -1416,83 +1360,6 @@ isc__mem_create(const char *name, isc_mem_t **mctxp FLARG) { #endif /* ISC_MEM_TRACKLINES */ } -void -isc__mem_create_arena(const char *name, isc_mem_t **mctxp FLARG) { - unsigned int arena_no = ISC_MEM_ILLEGAL_ARENA; - - RUNTIME_CHECK(mem_jemalloc_arena_create(&arena_no)); - - /* - * We use MALLOCX_TCACHE_NONE to bypass the tcache and route - * allocations directly to the arena. That is a recommendation - * from jemalloc developers: - * - * https://github.com/jemalloc/jemalloc/issues/2483#issuecomment-1698173849 - */ - mem_create(name, mctxp, isc_mem_debugging, isc_mem_defaultflags, - arena_no == ISC_MEM_ILLEGAL_ARENA - ? 0 - : MALLOCX_ARENA(arena_no) | MALLOCX_TCACHE_NONE); - (*mctxp)->jemalloc_arena = arena_no; -#if ISC_MEM_TRACKLINES - if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { - fprintf(stderr, - "create mctx %p func %s file %s line %u " - "for jemalloc arena %u\n", - *mctxp, func, file, line, arena_no); - } -#endif /* ISC_MEM_TRACKLINES */ -} - -#ifdef JEMALLOC_API_SUPPORTED -static bool -jemalloc_set_ssize_value(const char *valname, ssize_t newval) { - int ret; - - ret = mallctl(valname, NULL, NULL, &newval, sizeof(newval)); - return ret == 0; -} -#endif /* JEMALLOC_API_SUPPORTED */ - -static isc_result_t -mem_set_arena_ssize_value(isc_mem_t *mctx, const char *arena_valname, - const ssize_t newval) { - REQUIRE(VALID_CONTEXT(mctx)); -#ifdef JEMALLOC_API_SUPPORTED - bool ret; - char buf[256] = { 0 }; - - if (mctx->jemalloc_arena == ISC_MEM_ILLEGAL_ARENA) { - return ISC_R_UNEXPECTED; - } - - (void)snprintf(buf, sizeof(buf), "arena.%u.%s", mctx->jemalloc_arena, - arena_valname); - - ret = jemalloc_set_ssize_value(buf, newval); - - if (!ret) { - return ISC_R_FAILURE; - } - - return ISC_R_SUCCESS; -#else - UNUSED(arena_valname); - UNUSED(newval); - return ISC_R_NOTIMPLEMENTED; -#endif -} - -isc_result_t -isc_mem_arena_set_muzzy_decay_ms(isc_mem_t *mctx, const ssize_t decay_ms) { - return mem_set_arena_ssize_value(mctx, "muzzy_decay_ms", decay_ms); -} - -isc_result_t -isc_mem_arena_set_dirty_decay_ms(isc_mem_t *mctx, const ssize_t decay_ms) { - return mem_set_arena_ssize_value(mctx, "dirty_decay_ms", decay_ms); -} - void isc__mem_printactive(isc_mem_t *ctx, FILE *file) { #if ISC_MEM_TRACKLINES From d1427e9fa808aae795997ccf8a1e2292bb41bc48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 4 Jun 2025 10:19:35 +0200 Subject: [PATCH 3/5] Add and use MALLOCX_ZERO_GET() macro to jemalloc_shim.h Pull MALLOCX_ZERO_GET() macro to align the usage with the jemalloc jemalloc/internal/jemalloc_internal_types.h header. --- lib/isc/jemalloc_shim.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/isc/jemalloc_shim.h b/lib/isc/jemalloc_shim.h index 55bab911d1..ef892f0318 100644 --- a/lib/isc/jemalloc_shim.h +++ b/lib/isc/jemalloc_shim.h @@ -24,7 +24,13 @@ const char *malloc_conf = NULL; -#define MALLOCX_ZERO ((int)0x40) +/* + * The MALLOCX_ZERO and MALLOCX_ZERO_GET macros were taken literal from + * jemalloc_macros.h and jemalloc_internal_types.h headers respectively. + */ + +#define MALLOCX_ZERO ((int)0x40) +#define MALLOCX_ZERO_GET(flags) ((bool)(flags & MALLOCX_ZERO)) typedef union { size_t size; @@ -42,7 +48,7 @@ mallocx(size_t size, int flags) { si->size = size; ptr = &si[1]; - if ((flags & MALLOCX_ZERO) != 0) { + if (MALLOCX_ZERO_GET(flags)) { memset(ptr, 0, size); } @@ -68,7 +74,7 @@ rallocx(void *ptr, size_t size, int flags) { size_info *si = realloc(&(((size_info *)ptr)[-1]), size + sizeof(*si)); INSIST(si != NULL); - if ((flags & MALLOCX_ZERO) != 0 && size > si->size) { + if (MALLOCX_ZERO_GET(flags) && size > si->size) { memset((uint8_t *)si + sizeof(*si) + si->size, 0, size - si->size); } From 560047307d7ac72fd8281e206b10dda7fe6cbdbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 4 Jun 2025 17:43:34 +0200 Subject: [PATCH 4/5] Remove .hi_called member of isc_mem_t structure The .hi_called member was dead structure member and it hasn't been used since the overmem callback has been removed in commit 14bdd21e0a7ad5f115bb2427d4f88fe7a84e9324. --- lib/isc/mem.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 00137c14d0..bb2805c1b4 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -120,7 +120,6 @@ struct isc_mem { isc_refcount_t references; char *name; atomic_size_t inuse; - atomic_bool hi_called; atomic_bool is_overmem; atomic_size_t hi_water; atomic_size_t lo_water; @@ -428,7 +427,6 @@ mem_create(const char *name, isc_mem_t **ctxp, unsigned int debugging, atomic_init(&ctx->inuse, 0); atomic_init(&ctx->hi_water, 0); atomic_init(&ctx->lo_water, 0); - atomic_init(&ctx->hi_called, false); atomic_init(&ctx->is_overmem, false); ISC_LIST_INIT(ctx->pools); From f689dc22973f528d375d8e197c9259c2d508c261 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 5 Jun 2025 12:19:43 +0200 Subject: [PATCH 5/5] Don't use ssize_t for storing difference between sizes As POSIX guarantees only that the type ssize_t shall be capable of storing values at least in the range [-1, {SSIZE_MAX}], it can't be used to calculate the difference between two memory sizes. Change the logic for junk filling to test whether the new size is larger than old size and then use size_t as the result will be always positive. --- lib/isc/mem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index bb2805c1b4..e7421e012e 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -339,9 +339,9 @@ mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, if ((flags & ISC__MEM_ZERO) == 0 && (ctx->flags & ISC_MEMFLAG_FILL) != 0) { - ssize_t diff_size = new_size - old_size; - void *diff_ptr = (uint8_t *)new_ptr + old_size; - if (diff_size > 0) { + if (new_size > old_size) { + size_t diff_size = new_size - old_size; + void *diff_ptr = (uint8_t *)new_ptr + old_size; /* Mnemonic for "beef". */ memset(diff_ptr, 0xbe, diff_size); }