diff --git a/CHANGES b/CHANGES index 6df4547eb3..a8d92f29b5 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6240. [bug] Use dedicated per-worker thread jemalloc memory + arenas for send buffers allocation to reduce memory + consumption and avoid lock contention. [GL #4038] + 6239. [func] Deprecate the 'dnssec-must-be-secure' option. [GL #3700] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index a57ad29ee5..d641a266f8 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -68,6 +68,11 @@ Bug Fixes - The value of :any:`stale-refresh-time` was set to zero after ``rndc flush``. This has been fixed. :gl:`#4278` +- BIND could consume more memory than it needs. That has been fixed by + using specialised jemalloc memory arenas dedicated to sending buffers. It + allowed us to optimize the process of returning memory pages back to + the operating system. :gl:`#4038` + Known Issues ~~~~~~~~~~~~ diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 73196afbe2..560ad74d88 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -197,6 +197,42 @@ isc__mem_create(isc_mem_t **_ISC_MEM_FLARG); * mctxp != NULL && *mctxp == NULL */ /*@}*/ +#define isc_mem_create_arena(cp) isc__mem_create_arena((cp)_ISC_MEM_FILELINE) +void +isc__mem_create_arena(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. + */ +/*@}*/ + +void +isc_mem_attach(isc_mem_t *, isc_mem_t **); + /*@{*/ void isc_mem_attach(isc_mem_t *, isc_mem_t **); diff --git a/lib/isc/jemalloc_shim.h b/lib/isc/jemalloc_shim.h index 94df92418b..ef7641e7f6 100644 --- a/lib/isc/jemalloc_shim.h +++ b/lib/isc/jemalloc_shim.h @@ -22,7 +22,9 @@ const char *malloc_conf = NULL; -#define MALLOCX_ZERO ((int)0x40) +#define MALLOCX_ZERO ((int)0x40) +#define MALLOCX_TCACHE_NONE (0) +#define MALLOCX_ARENA(a) (0) #if defined(HAVE_MALLOC_SIZE) || defined(HAVE_MALLOC_USABLE_SIZE) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 9d382c98cf..6f5466daa6 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -72,6 +72,8 @@ unsigned int isc_mem_debugging = ISC_MEM_DEBUGGING; unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT; +#define ISC_MEM_ILLEGAL_ARENA (UINT_MAX) + /* * Constants. */ @@ -122,6 +124,8 @@ static isc_mutex_t contextslock; 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; @@ -225,7 +229,7 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { #endif idx = hash % DEBUG_TABLE_COUNT; - dl = mallocx(sizeof(debuglink_t), 0); + dl = mallocx(sizeof(debuglink_t), mctx->jemalloc_flags); INSIST(dl != NULL); ISC_LINK_INIT(dl, link); @@ -273,7 +277,7 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, while (dl != NULL) { if (dl->ptr == ptr) { ISC_LIST_UNLINK(mctx->debuglist[idx], dl, link); - sdallocx(dl, sizeof(*dl), 0); + sdallocx(dl, sizeof(*dl), mctx->jemalloc_flags); goto unlock; } dl = ISC_LIST_NEXT(dl, link); @@ -303,7 +307,7 @@ mem_get(isc_mem_t *ctx, size_t size, int flags) { ADJUST_ZERO_ALLOCATION_SIZE(size); - ret = mallocx(size, flags); + ret = mallocx(size, flags | ctx->jemalloc_flags); INSIST(ret != NULL); if ((flags & ISC__MEM_ZERO) == 0 && @@ -326,7 +330,7 @@ mem_put(isc_mem_t *ctx, void *mem, size_t size, int flags) { if ((ctx->flags & ISC_MEMFLAG_FILL) != 0) { memset(mem, 0xde, size); /* Mnemonic for "dead". */ } - sdallocx(mem, size, flags); + sdallocx(mem, size, flags | ctx->jemalloc_flags); } static void * @@ -336,7 +340,7 @@ mem_realloc(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, ADJUST_ZERO_ALLOCATION_SIZE(new_size); - new_ptr = rallocx(old_ptr, new_size, flags); + new_ptr = rallocx(old_ptr, new_size, flags | ctx->jemalloc_flags); INSIST(new_ptr != NULL); if ((flags & ISC__MEM_ZERO) == 0 && @@ -379,6 +383,48 @@ 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 */ +} + static void mem_initialize(void) { /* @@ -410,18 +456,21 @@ isc__mem_shutdown(void) { } static void -mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { +mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags, + unsigned int jemalloc_flags) { isc_mem_t *ctx = NULL; REQUIRE(ctxp != NULL && *ctxp == NULL); - ctx = malloc(sizeof(*ctx)); + ctx = mallocx(sizeof(*ctx), jemalloc_flags); INSIST(ctx != NULL); *ctx = (isc_mem_t){ .magic = MEM_MAGIC, .debugging = debugging, .flags = flags, + .jemalloc_flags = jemalloc_flags, + .jemalloc_arena = ISC_MEM_ILLEGAL_ARENA, .checkfree = true, }; @@ -440,7 +489,9 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { if ((ctx->debugging & ISC_MEM_DEBUGRECORD) != 0) { unsigned int i; - ctx->debuglist = calloc(DEBUG_TABLE_COUNT, sizeof(debuglist_t)); + ctx->debuglist = mallocx( + ISC_CHECKED_MUL(DEBUG_TABLE_COUNT, sizeof(debuglist_t)), + jemalloc_flags); INSIST(ctx->debuglist != NULL); for (i = 0; i < DEBUG_TABLE_COUNT; i++) { @@ -462,12 +513,15 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags) { static void destroy(isc_mem_t *ctx) { + unsigned int arena_no; LOCK(&contextslock); ISC_LIST_UNLINK(contexts, ctx, link); UNLOCK(&contextslock); ctx->magic = 0; + arena_no = ctx->jemalloc_arena; + INSIST(ISC_LIST_EMPTY(ctx->pools)); #if ISC_MEM_TRACKLINES @@ -483,11 +537,14 @@ destroy(isc_mem_t *ctx) { INSIST(!ctx->checkfree || dl->ptr == NULL); ISC_LIST_UNLINK(ctx->debuglist[i], dl, link); - sdallocx(dl, sizeof(*dl), 0); + sdallocx(dl, sizeof(*dl), ctx->jemalloc_flags); } } - free(ctx->debuglist); + sdallocx( + ctx->debuglist, + ISC_CHECKED_MUL(DEBUG_TABLE_COUNT, sizeof(debuglist_t)), + ctx->jemalloc_flags); } #endif /* if ISC_MEM_TRACKLINES */ @@ -496,7 +553,11 @@ destroy(isc_mem_t *ctx) { if (ctx->checkfree) { INSIST(atomic_load(&ctx->inuse) == 0); } - free(ctx); + sdallocx(ctx, sizeof(*ctx), ctx->jemalloc_flags); + + if (arena_no != ISC_MEM_ILLEGAL_ARENA) { + RUNTIME_CHECK(mem_jemalloc_arena_destroy(arena_no) == true); + } } void @@ -806,7 +867,7 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size, int flags FLARG) { ptr = mem_get(ctx, size, flags); /* Recalculate the real allocated size */ - size = sallocx(ptr, flags); + size = sallocx(ptr, flags | ctx->jemalloc_flags); mem_getstats(ctx, size); ADD_TRACE(ctx, ptr, size, file, line); @@ -859,7 +920,7 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size, } else if (new_size == 0) { isc__mem_free(ctx, old_ptr, flags FLARG_PASS); } else { - size_t old_size = sallocx(old_ptr, flags); + size_t old_size = sallocx(old_ptr, flags | ctx->jemalloc_flags); DELETE_TRACE(ctx, old_ptr, old_size, file, line); mem_putstats(ctx, old_size); @@ -867,7 +928,7 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size, new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size, flags); /* Recalculate the real allocated size */ - new_size = sallocx(new_ptr, flags); + new_size = sallocx(new_ptr, flags | ctx->jemalloc_flags); mem_getstats(ctx, new_size); ADD_TRACE(ctx, new_ptr, new_size, file, line); @@ -891,7 +952,7 @@ isc__mem_free(isc_mem_t *ctx, void *ptr, int flags FLARG) { REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(ptr != NULL); - size = sallocx(ptr, flags); + size = sallocx(ptr, flags | ctx->jemalloc_flags); DELETE_TRACE(ctx, ptr, size, file, line); @@ -1542,7 +1603,7 @@ error: void isc__mem_create(isc_mem_t **mctxp FLARG) { - mem_create(mctxp, isc_mem_debugging, isc_mem_defaultflags); + mem_create(mctxp, isc_mem_debugging, isc_mem_defaultflags, 0); #if ISC_MEM_TRACKLINES if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { fprintf(stderr, "create mctx %p file %s line %u\n", *mctxp, @@ -1551,6 +1612,83 @@ isc__mem_create(isc_mem_t **mctxp FLARG) { #endif /* ISC_MEM_TRACKLINES */ } +void +isc__mem_create_arena(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(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 file %s line %u for jemalloc arena " + "%u\n", + *mctxp, 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 diff --git a/lib/ns/client.c b/lib/ns/client.c index f13ff04c76..58d5c48624 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -334,7 +334,7 @@ client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer, if (TCP_CLIENT(client)) { INSIST(client->tcpbuf == NULL); - client->tcpbuf = isc_mem_get(client->manager->mctx, + client->tcpbuf = isc_mem_get(client->manager->send_mctx, NS_CLIENT_TCP_BUFFER_SIZE); client->tcpbuf_size = NS_CLIENT_TCP_BUFFER_SIZE; data = client->tcpbuf; @@ -371,9 +371,9 @@ client_sendpkg(ns_client_t *client, isc_buffer_t *buffer) { if (isc_buffer_base(buffer) == client->tcpbuf) { size_t used = isc_buffer_usedlength(buffer); - client->tcpbuf = - isc_mem_creget(client->manager->mctx, client->tcpbuf, - client->tcpbuf_size, used, sizeof(char)); + client->tcpbuf = isc_mem_creget( + client->manager->send_mctx, client->tcpbuf, + client->tcpbuf_size, used, sizeof(char)); client->tcpbuf_size = used; r.base = client->tcpbuf; r.length = used; @@ -449,7 +449,7 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) { return; done: if (client->tcpbuf != NULL) { - isc_mem_put(client->manager->mctx, client->tcpbuf, + isc_mem_put(client->manager->send_mctx, client->tcpbuf, client->tcpbuf_size); } @@ -733,7 +733,7 @@ renderend: cleanup: if (client->tcpbuf != NULL) { - isc_mem_put(client->manager->mctx, client->tcpbuf, + isc_mem_put(client->manager->send_mctx, client->tcpbuf, client->tcpbuf_size); } @@ -1621,7 +1621,7 @@ ns__client_reset_cb(void *client0) { ns_client_endrequest(client); if (client->tcpbuf != NULL) { - isc_mem_put(client->manager->mctx, client->tcpbuf, + isc_mem_put(client->manager->send_mctx, client->tcpbuf, client->tcpbuf_size); } @@ -1658,7 +1658,8 @@ ns__client_put_cb(void *client0) { client->magic = 0; - isc_mem_put(manager->mctx, client->sendbuf, NS_CLIENT_SEND_BUFFER_SIZE); + isc_mem_put(manager->send_mctx, client->sendbuf, + NS_CLIENT_SEND_BUFFER_SIZE); if (client->opt != NULL) { INSIST(dns_rdataset_isassociated(client->opt)); dns_rdataset_disassociate(client->opt); @@ -2308,7 +2309,7 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { dns_message_create(client->manager->mctx, DNS_MESSAGE_INTENTPARSE, &client->message); - client->sendbuf = isc_mem_get(client->manager->mctx, + client->sendbuf = isc_mem_get(client->manager->send_mctx, NS_CLIENT_SEND_BUFFER_SIZE); /* * Set magic earlier than usual because ns_query_init() @@ -2356,7 +2357,7 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) { cleanup: if (client->sendbuf != NULL) { - isc_mem_put(client->manager->mctx, client->sendbuf, + isc_mem_put(client->manager->send_mctx, client->sendbuf, NS_CLIENT_SEND_BUFFER_SIZE); } @@ -2392,6 +2393,8 @@ clientmgr_destroy_cb(void *arg) { ns_server_detach(&manager->sctx); + isc_mem_detach(&manager->send_mctx); + isc_mem_putanddetach(&manager->mctx, manager, sizeof(*manager)); } @@ -2424,6 +2427,61 @@ ns_clientmgr_create(ns_server_t *sctx, isc_loopmgr_t *loopmgr, isc_refcount_init(&manager->references, 1); ns_server_attach(sctx, &manager->sctx); + /* + * We create specialised per-worker memory context specifically + * dedicated and tuned for allocating send buffers as it is a very + * common operation. Not doing so may result in excessive memory + * use in certain workloads. + * + * Please see this thread for more details: + * + * https://github.com/jemalloc/jemalloc/issues/2483 + * + * In particular, this information from the jemalloc developers is + * of the most interest: + * + * https://github.com/jemalloc/jemalloc/issues/2483#issuecomment-1639019699 + * https://github.com/jemalloc/jemalloc/issues/2483#issuecomment-1698173849 + * + * In essence, we use the following memory management strategy: + * + * 1. We use a per-worker memory arena for send buffers memory + * allocation to reduce lock contention (In reality, we create a + * per-client manager arena, but we have one client manager per + * worker). + * + * 2. The automatically created arenas settings remain unchanged + * and may be controlled by users (e.g. by setting the + * "MALLOC_CONF" variable). + * + * 3. We attune the arenas to not use dirty pages cache as the + * cache would have a poor reuse rate, and that is known to + * significantly contribute to excessive memory use. + * + * 4. There is no strict need for the dirty cache, as there is a + * per arena bin for each allocation size, so because we initially + * allocate strictly 64K per send buffer (enough for a DNS + * message), allocations would get directed to one bin (an "object + * pool" or a "slab") maintained within an arena. That is, there + * is an object pool already, specifically to optimise for the + * case of frequent allocations of objects of the given size. The + * object pool should suffice our needs, as we will end up + * recycling the objects from there without the need to back it by + * an additional layer of dirty pages cache. The dirty pages cache + * would have worked better in the case when there are more + * allocation bins involved due to a higher reuse rate (the case + * of a more "generic" memory management). + */ + isc_mem_create_arena(&manager->send_mctx); + isc_mem_setname(manager->send_mctx, "sendbufs"); + (void)isc_mem_arena_set_dirty_decay_ms(manager->send_mctx, 0); + /* + * Disable muzzy pages cache too, as versions < 5.2.0 have it + * enabled by default. The muzzy pages cache goes right below the + * dirty pages cache and backs it. + */ + (void)isc_mem_arena_set_muzzy_decay_ms(manager->send_mctx, 0); + manager->magic = MANAGER_MAGIC; MTRACE("create"); diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index 111e107d58..43cd72d1c8 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -144,6 +144,7 @@ struct ns_clientmgr { unsigned int magic; isc_mem_t *mctx; + isc_mem_t *send_mctx; ns_server_t *sctx; isc_refcount_t references; uint32_t tid; diff --git a/lib/ns/server.c b/lib/ns/server.c index bd17a9baaa..d853848a76 100644 --- a/lib/ns/server.c +++ b/lib/ns/server.c @@ -55,6 +55,11 @@ ns_server_create(isc_mem_t *mctx, ns_matchview_t matchingview, isc_mem_attach(mctx, &sctx->mctx); + /* + * See here for more details: + * https://github.com/jemalloc/jemalloc/issues/2483 + */ + isc_refcount_init(&sctx->references, 1); isc_quota_init(&sctx->xfroutquota, 10);