From 63924968d133115f0653d62518ecce31e3629137 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 11 May 2021 12:18:56 +0200 Subject: [PATCH 01/26] Add debug tracing capability to isc_mem_create/isc_mem_destroy Previously, we only had capability to trace the memory gets and puts, but for debugging, it's sometimes also important to keep track how many and where do the memory contexts get created and destroyed. This commit adds such tracking capability. --- lib/isc/include/isc/mem.h | 12 ++++++------ lib/isc/mem.c | 26 +++++++++++++++++++++----- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 85973a755a..a5f003bdce 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -175,8 +175,8 @@ extern unsigned int isc_mem_defaultflags; } while (0) /*@{*/ -void -isc_mem_create(isc_mem_t **mctxp); +#define isc_mem_create(cp) ISCMEMFUNC(create)((cp)_ISC_MEM_FILELINE) +void ISCMEMFUNC(create)(isc_mem_t **_ISC_MEM_FLARG); /*!< * \brief Create a memory context. @@ -188,8 +188,8 @@ isc_mem_create(isc_mem_t **mctxp); /*@{*/ void isc_mem_attach(isc_mem_t *, isc_mem_t **); -void -isc_mem_detach(isc_mem_t **); +#define isc_mem_detach(cp) ISCMEMFUNC(detach)((cp)_ISC_MEM_FILELINE) +void ISCMEMFUNC(detach)(isc_mem_t **_ISC_MEM_FLARG); /*!< * \brief Attach to / detach from a memory context. * @@ -204,8 +204,8 @@ isc_mem_detach(isc_mem_t **); */ /*@}*/ -void -isc_mem_destroy(isc_mem_t **); +#define isc_mem_destroy(cp) ISCMEMFUNC(destroy)((cp)_ISC_MEM_FILELINE) +void ISCMEMFUNC(destroy)(isc_mem_t **_ISC_MEM_FLARG); /*%< * Destroy a memory context. */ diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 54d5411ef8..6adfd9d671 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -604,7 +604,7 @@ isc_mem_attach(isc_mem_t *source, isc_mem_t **targetp) { } void -isc_mem_detach(isc_mem_t **ctxp) { +isc__mem_detach(isc_mem_t **ctxp FLARG) { REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); isc_mem_t *ctx = *ctxp; @@ -612,6 +612,12 @@ isc_mem_detach(isc_mem_t **ctxp) { if (isc_refcount_decrement(&ctx->references) == 1) { isc_refcount_destroy(&ctx->references); +#if ISC_MEM_TRACKLINES + if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { + fprintf(stderr, "destroy mctx %p file %s line %u\n", + ctx, file, line); + } +#endif destroy(ctx); } } @@ -663,7 +669,7 @@ destroy: } void -isc_mem_destroy(isc_mem_t **ctxp) { +isc__mem_destroy(isc_mem_t **ctxp FLARG) { /* * This routine provides legacy support for callers who use mctxs * without attaching/detaching. @@ -674,6 +680,11 @@ isc_mem_destroy(isc_mem_t **ctxp) { isc_mem_t *ctx = *ctxp; #if ISC_MEM_TRACKLINES + if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { + fprintf(stderr, "destroy mctx %p file %s line %u\n", ctx, file, + line); + } + if (isc_refcount_decrement(&ctx->references) > 1) { print_active(ctx, stderr); } @@ -1343,7 +1354,6 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { void * isc__mempool_get(isc_mempool_t *mpctx FLARG) { element *item = NULL; - unsigned int i; REQUIRE(VALID_MEMPOOL(mpctx)); @@ -1366,7 +1376,7 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { * We need to dip into the well. Lock the memory * context here and fill up our free list. */ - for (i = 0; i < fillcount; i++) { + for (size_t i = 0; i < fillcount; i++) { item = mem_get(mctx, mpctx->size); mem_getstats(mctx, mpctx->size); item->next = mpctx->items; @@ -1843,8 +1853,14 @@ error: #endif /* HAVE_JSON_C */ void -isc_mem_create(isc_mem_t **mctxp) { +isc__mem_create(isc_mem_t **mctxp FLARG) { mem_create(mctxp, isc_mem_defaultflags); +#if ISC_MEM_TRACKLINES + if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { + fprintf(stderr, "create mctx %p file %s line %u\n", *mctxp, + file, line); + } +#endif /* ISC_MEM_TRACKLINES */ } void From 7f1c525625675e7e5e0953a78adefe9e21fc1462 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 11 May 2021 12:29:57 +0200 Subject: [PATCH 02/26] Compile with jemalloc to reduce memory allocator contention The jemalloc allocator is scalable high performance allocator, this is the first in the series of commits that will add jemalloc as a memory allocator for BIND 9. This commit adds configure.ac check and Makefile modifications to use jemalloc as BIND 9 allocator. --- configure.ac | 11 +++++++++++ lib/isc/Makefile.am | 2 ++ 2 files changed, 13 insertions(+) diff --git a/configure.ac b/configure.ac index 03fd0b2725..7ada1b61ee 100644 --- a/configure.ac +++ b/configure.ac @@ -1376,6 +1376,17 @@ AC_SUBST([CMOCKA_LIBS]) AM_CONDITIONAL([HAVE_CMOCKA], [test "$with_cmocka" = "yes"]) +# +# +# +AC_MSG_CHECKING([for jemalloc]) +PKG_CHECK_MODULES([JEMALLOC], [jemalloc >= 5], [] + [AC_MSG_WARN([Using jemalloc 5 is recommended]) + PKG_CHECK_MODULES([JEMALLOC], [jemalloc], [], + [AC_MSG_ERROR([jemalloc not found])])]) +AC_SUBST([JEMALLOC_CFLAGS]) +AC_SUBST([JEMALLOC_LIBS]) + # # was --with-tuning specified? # diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index deed17c9a9..1b5898817b 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -230,6 +230,7 @@ libisc_la_SOURCES = \ libisc_la_CPPFLAGS = \ $(AM_CPPFLAGS) \ + $(JEMALLOC_CPPFLAGS) \ $(LIBISC_CFLAGS) \ $(LIBUV_CFLAGS) \ $(OPENSSL_CFLAGS) \ @@ -240,6 +241,7 @@ libisc_la_LDFLAGS = \ -release "$(PACKAGE_VERSION)" libisc_la_LIBADD = \ + $(JEMALLOC_LIBS) \ $(LIBUV_LIBS) \ $(OPENSSL_LIBS) \ $(ZLIB_LIBS) From 5184384efdbd000134b412e108ff1c1e174333d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 11 May 2021 12:33:31 +0200 Subject: [PATCH 03/26] Add recommended jemalloc configuration for our load There's global variable called `malloc_conf` that can be used to configure jemalloc behaviour at the program startup. We use following configuration: * xmalloc:true - abort-on-out-of-memory enabled. * background_thread:true - Enable internal background worker threads to handle purging asynchronously. * metadata_thp:auto - allow jemalloc to use transparent huge page (THP) for internal metadata initially, but may begin to do so when metadata usage reaches certain level. * dirty_decay_ms:30000 - Approximate time in milliseconds from the creation of a set of unused dirty pages until an equivalent set of unused dirty pages is purged and/or reused. * muzzy_decay_ms:30000 - Approximate time in milliseconds from the creation of a set of unused muzzy pages until an equivalent set of unused muzzy pages is purged and/or reused. More information about the specific meaning can be found in the jemalloc manpage or online at http://jemalloc.net/jemalloc.3.html --- lib/isc/mem.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 6adfd9d671..33738b4af4 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -42,6 +42,8 @@ #include #endif /* HAVE_JSON_C */ +#include + #include "mem_p.h" #define MCTXLOCK(m) LOCK(&m->lock) @@ -442,6 +444,9 @@ default_memfree(void *ptr) { static void mem_initialize(void) { + malloc_conf = "xmalloc:true,background_thread:true,metadata_thp:auto," + "dirty_decay_ms:30000,muzzy_decay_ms:30000"; + isc_mutex_init(&contextslock); ISC_LIST_INIT(contexts); totallost = 0; From 692fd2a216954abf131c314c23280d57ff042c39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 11 May 2021 12:40:42 +0200 Subject: [PATCH 04/26] Remove default_memalloc and default_memfree Now that we have xmalloc:true enabled, we can remove our xmalloc-like wrappers around malloc and free. --- lib/isc/mem.c | 55 +++++++-------------------------------------------- 1 file changed, 7 insertions(+), 48 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 33738b4af4..70dae2c471 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -330,11 +330,6 @@ unlock: } #endif /* ISC_MEM_TRACKLINES */ -static void * -default_memalloc(size_t size); -static void -default_memfree(void *ptr); - /*! * Perform a malloc, doing memory filling and overrun detection as necessary. */ @@ -342,7 +337,7 @@ static inline void * mem_get(isc_mem_t *ctx, size_t size) { char *ret; - ret = default_memalloc(size); + ret = malloc(size); if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { if (ISC_LIKELY(ret != NULL)) { @@ -362,7 +357,7 @@ mem_put(isc_mem_t *ctx, void *mem, size_t size) { if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { memset(mem, 0xde, size); /* Mnemonic for "dead". */ } - default_memfree(mem); + free(mem); } #define stats_bucket(ctx, size) \ @@ -406,42 +401,6 @@ mem_putstats(isc_mem_t *ctx, void *ptr, size_t size) { * Private. */ -static void * -default_memalloc(size_t size) { - void *ptr; - - ptr = malloc(size); - - /* - * If the space cannot be allocated, a null pointer is returned. If the - * size of the space requested is zero, the behavior is - * implementation-defined: either a null pointer is returned, or the - * behavior is as if the size were some nonzero value, except that the - * returned pointer shall not be used to access an object. - * [ISO9899 ยง 7.22.3] - * - * [ISO9899] - * ISO/IEC WG 9899:2011: Programming languages - C. - * International Organization for Standardization, Geneva, - * Switzerland. - * http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1570.pdf - */ - - if (ptr == NULL && size != 0) { - char strbuf[ISC_STRERRORSIZE]; - strerror_r(errno, strbuf, sizeof(strbuf)); - isc_error_fatal(__FILE__, __LINE__, "malloc failed: %s", - strbuf); - } - - return (ptr); -} - -static void -default_memfree(void *ptr) { - free(ptr); -} - static void mem_initialize(void) { malloc_conf = "xmalloc:true,background_thread:true,metadata_thp:auto," @@ -480,7 +439,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { STATIC_ASSERT(ALIGNMENT_SIZE >= sizeof(size_info), "alignment size too small"); - ctx = default_memalloc(sizeof(*ctx)); + ctx = malloc(sizeof(*ctx)); *ctx = (isc_mem_t){ .magic = MEM_MAGIC, @@ -511,8 +470,8 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGRECORD) != 0)) { unsigned int i; - ctx->debuglist = default_memalloc( - (DEBUG_TABLE_COUNT * sizeof(debuglist_t))); + ctx->debuglist = + malloc((DEBUG_TABLE_COUNT * sizeof(debuglist_t))); for (i = 0; i < DEBUG_TABLE_COUNT; i++) { ISC_LIST_INIT(ctx->debuglist[i]); } @@ -564,7 +523,7 @@ destroy(isc_mem_t *ctx) { } } - default_memfree(ctx->debuglist); + free(ctx->debuglist); decrement_malloced(ctx, DEBUG_TABLE_COUNT * sizeof(debuglist_t)); } @@ -595,7 +554,7 @@ destroy(isc_mem_t *ctx) { if (ctx->checkfree) { INSIST(malloced == 0); } - default_memfree(ctx); + free(ctx); } void From 4b3d0c66009d30f5c0bc12ee128fc59f1d853f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 11 May 2021 12:59:35 +0200 Subject: [PATCH 05/26] Remove ISC_MEM_DEBUGSIZE and ISC_MEM_DEBUGRECORD The ISC_MEM_DEBUGSIZE and ISC_MEM_DEBUGCTX did sanity checks on matching size and memory context on the memory returned to the allocator. Those will no longer needed when most of the allocator will be replaced with jemalloc. --- bin/check/named-checkconf.c | 6 -- bin/dnssec/dnssec-keygen.c | 6 -- bin/dnssec/dnssec-signzone.c | 6 -- bin/dnssec/dnssec-verify.c | 6 -- bin/named/main.c | 2 - bin/tests/system/README | 2 +- bin/tests/system/additional/ns1/named.args | 2 +- bin/tests/system/allow-query/ns3/named.args | 2 +- bin/tests/system/cacheclean/ns1/named.args | 2 +- bin/tests/system/cacheclean/ns2/named.args | 2 +- bin/tests/system/delzone/ns2/named.args | 2 +- bin/tests/system/dnssec/ns6/named.args | 2 +- bin/tests/system/dnstap/ns3/named.args | 2 +- bin/tests/system/dscp/ns1/named.args | 2 +- bin/tests/system/dscp/ns2/named.args | 2 +- bin/tests/system/dscp/ns3/named.args | 2 +- bin/tests/system/dscp/ns4/named.args | 2 +- bin/tests/system/dscp/ns5/named.args | 2 +- bin/tests/system/dscp/ns6/named.args | 2 +- bin/tests/system/dscp/ns7/named.args | 2 +- bin/tests/system/dupsigs/ns1/named.args | 2 +- bin/tests/system/fetchlimit/ns3/named.args | 2 +- bin/tests/system/legacy/ns4/named.args | 2 +- bin/tests/system/legacy/ns5/named.args | 2 +- bin/tests/system/legacy/ns6/named.args | 2 +- bin/tests/system/legacy/ns7/named.args | 2 +- bin/tests/system/logfileconfig/tests.sh | 2 +- bin/tests/system/mirror/ns3/named.args | 2 +- bin/tests/system/mkeys/ns2/named.args | 2 +- bin/tests/system/mkeys/ns3/named.args | 2 +- bin/tests/system/mkeys/ns5/named1.args | 2 +- bin/tests/system/mkeys/ns5/named2.args | 2 +- bin/tests/system/mkeys/ns6/named.args | 2 +- bin/tests/system/nsupdate/ns5/named.args | 2 +- bin/tests/system/nsupdate/ns6/named.args | 2 +- bin/tests/system/resolver/tests.sh | 2 +- bin/tests/system/rpzextra/ns1/named.args | 2 +- bin/tests/system/runtime/tests.sh | 2 +- bin/tests/system/start.pl | 2 +- bin/tests/system/timeouts/ns1/named.args | 2 +- bin/tests/system/zero/ns2/named.args | 2 +- bin/tests/system/zero/ns3/named.args | 2 +- bin/tests/system/zero/ns4/named.args | 2 +- bin/tests/wire_test.c | 6 -- lib/isc/include/isc/mem.h | 10 ---- lib/isc/mem.c | 62 +-------------------- lib/isc/tls.c | 6 +- lib/isc/trampoline.c | 16 ++---- 48 files changed, 47 insertions(+), 155 deletions(-) diff --git a/bin/check/named-checkconf.c b/bin/check/named-checkconf.c index 11303e9648..d1f4e037d3 100644 --- a/bin/check/named-checkconf.c +++ b/bin/check/named-checkconf.c @@ -611,12 +611,6 @@ main(int argc, char **argv) { { isc_mem_debugging |= ISC_MEM_DEBUGUSAGE; } - if (strcasecmp(isc_commandline_argument, "size") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGSIZE; - } - if (strcasecmp(isc_commandline_argument, "mctx") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGCTX; - } break; default: break; diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index 349aa00f43..c9bf49f4fe 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -909,12 +909,6 @@ main(int argc, char **argv) { { isc_mem_debugging |= ISC_MEM_DEBUGUSAGE; } - if (strcasecmp(isc_commandline_argument, "size") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGSIZE; - } - if (strcasecmp(isc_commandline_argument, "mctx") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGCTX; - } break; default: break; diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 4329fe7f0a..e321295684 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -3335,12 +3335,6 @@ main(int argc, char *argv[]) { { isc_mem_debugging |= ISC_MEM_DEBUGUSAGE; } - if (strcasecmp(isc_commandline_argument, "size") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGSIZE; - } - if (strcasecmp(isc_commandline_argument, "mctx") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGCTX; - } break; default: break; diff --git a/bin/dnssec/dnssec-verify.c b/bin/dnssec/dnssec-verify.c index eb845ffbc8..a5ed08742f 100644 --- a/bin/dnssec/dnssec-verify.c +++ b/bin/dnssec/dnssec-verify.c @@ -209,12 +209,6 @@ main(int argc, char *argv[]) { { isc_mem_debugging |= ISC_MEM_DEBUGUSAGE; } - if (strcasecmp(isc_commandline_argument, "size") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGSIZE; - } - if (strcasecmp(isc_commandline_argument, "mctx") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGCTX; - } break; default: break; diff --git a/bin/named/main.c b/bin/named/main.c index 9fc84deef1..565246d772 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -430,8 +430,6 @@ static struct flag_def { { "trace", ISC_MEM_DEBUGTRACE, false }, { "record", ISC_MEM_DEBUGRECORD, false }, { "usage", ISC_MEM_DEBUGUSAGE, false }, - { "size", ISC_MEM_DEBUGSIZE, false }, - { "mctx", ISC_MEM_DEBUGCTX, false }, { NULL, 0, false } }, mem_context_flags[] = { { "fill", ISC_MEMFLAG_FILL, false }, { "nofill", ISC_MEMFLAG_FILL, true }, diff --git a/bin/tests/system/README b/bin/tests/system/README index c0970ebd32..f24bd0365e 100644 --- a/bin/tests/system/README +++ b/bin/tests/system/README @@ -546,7 +546,7 @@ By default, start.pl starts a "named" server with the following options: -g Runs the server in the foreground and logs everything to stderr. - -m record,size,mctx + -m record Turns on these memory usage debugging flags. -U 4 Uses four listeners. diff --git a/bin/tests/system/additional/ns1/named.args b/bin/tests/system/additional/ns1/named.args index 15aa849355..45ac93a7c7 100644 --- a/bin/tests/system/additional/ns1/named.args +++ b/bin/tests/system/additional/ns1/named.args @@ -1,2 +1,2 @@ # this server runs named with only one worker thread --m record,size,mctx -c named.conf -d 99 -D additional-ns1 -X named.lock -g -n 1 -T maxcachesize=2097152 +-m record -c named.conf -d 99 -D additional-ns1 -X named.lock -g -n 1 -T maxcachesize=2097152 diff --git a/bin/tests/system/allow-query/ns3/named.args b/bin/tests/system/allow-query/ns3/named.args index 35e99d8dd8..e875a57bc7 100644 --- a/bin/tests/system/allow-query/ns3/named.args +++ b/bin/tests/system/allow-query/ns3/named.args @@ -1,2 +1,2 @@ # this server only has 127.0.0.1 in its localhost/localnets ACLs --m record,size,mctx -c named.conf -d 99 -D allow-query-ns3 -X named.lock -g -T maxcachesize=2097152 -T fixedlocal +-m record -c named.conf -d 99 -D allow-query-ns3 -X named.lock -g -T maxcachesize=2097152 -T fixedlocal diff --git a/bin/tests/system/cacheclean/ns1/named.args b/bin/tests/system/cacheclean/ns1/named.args index 2ba9a1403d..b7ce00d2b7 100644 --- a/bin/tests/system/cacheclean/ns1/named.args +++ b/bin/tests/system/cacheclean/ns1/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 1 -D cacheclean-ns1 -X named.lock -g -T maxcachesize=2097152 +-m record -c named.conf -d 1 -D cacheclean-ns1 -X named.lock -g -T maxcachesize=2097152 diff --git a/bin/tests/system/cacheclean/ns2/named.args b/bin/tests/system/cacheclean/ns2/named.args index 1bcc5eab28..071016c00b 100644 --- a/bin/tests/system/cacheclean/ns2/named.args +++ b/bin/tests/system/cacheclean/ns2/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 3 -D cacheclean-ns2 -X named.lock -g -T maxcachesize=2097152 +-m record -c named.conf -d 3 -D cacheclean-ns2 -X named.lock -g -T maxcachesize=2097152 diff --git a/bin/tests/system/delzone/ns2/named.args b/bin/tests/system/delzone/ns2/named.args index be9241ed54..6ee0d9fe9a 100644 --- a/bin/tests/system/delzone/ns2/named.args +++ b/bin/tests/system/delzone/ns2/named.args @@ -1 +1 @@ --D delzone-ns2 -X named.lock -m record,size,mctx -c named.conf -g -U 4 -T maxcachesize=2097152 +-D delzone-ns2 -X named.lock -m record -c named.conf -g -U 4 -T maxcachesize=2097152 diff --git a/bin/tests/system/dnssec/ns6/named.args b/bin/tests/system/dnssec/ns6/named.args index 65b7dbcc93..4b96405caa 100644 --- a/bin/tests/system/dnssec/ns6/named.args +++ b/bin/tests/system/dnssec/ns6/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D dnssec-ns6 -X named.lock -g -T maxcachesize=2097152 -T nonearest -T tat=1 +-m record -c named.conf -d 99 -D dnssec-ns6 -X named.lock -g -T maxcachesize=2097152 -T nonearest -T tat=1 diff --git a/bin/tests/system/dnstap/ns3/named.args b/bin/tests/system/dnstap/ns3/named.args index fb42af211a..7937dee5fb 100644 --- a/bin/tests/system/dnstap/ns3/named.args +++ b/bin/tests/system/dnstap/ns3/named.args @@ -1,2 +1,2 @@ # Using "-n 1" allows GL #1795 to be reliably reproduced --D dnstap-ns3 -X named.lock -m record,size,mctx -c named.conf -d 99 -g -U 4 -n 1 -T maxcachesize=2097152 +-D dnstap-ns3 -X named.lock -m record -c named.conf -d 99 -g -U 4 -n 1 -T maxcachesize=2097152 diff --git a/bin/tests/system/dscp/ns1/named.args b/bin/tests/system/dscp/ns1/named.args index 0c955c704a..2a3e86a26d 100644 --- a/bin/tests/system/dscp/ns1/named.args +++ b/bin/tests/system/dscp/ns1/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D dscp-ns1 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 +-m record -c named.conf -d 99 -D dscp-ns1 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 diff --git a/bin/tests/system/dscp/ns2/named.args b/bin/tests/system/dscp/ns2/named.args index ff501a812c..7122b39537 100644 --- a/bin/tests/system/dscp/ns2/named.args +++ b/bin/tests/system/dscp/ns2/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D dscp-ns2 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 +-m record -c named.conf -d 99 -D dscp-ns2 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 diff --git a/bin/tests/system/dscp/ns3/named.args b/bin/tests/system/dscp/ns3/named.args index 3d1981fb59..60ae7f4f91 100644 --- a/bin/tests/system/dscp/ns3/named.args +++ b/bin/tests/system/dscp/ns3/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D dscp-ns3 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 +-m record -c named.conf -d 99 -D dscp-ns3 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 diff --git a/bin/tests/system/dscp/ns4/named.args b/bin/tests/system/dscp/ns4/named.args index 277a47b628..8352455c09 100644 --- a/bin/tests/system/dscp/ns4/named.args +++ b/bin/tests/system/dscp/ns4/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D dscp-ns4 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 +-m record -c named.conf -d 99 -D dscp-ns4 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 diff --git a/bin/tests/system/dscp/ns5/named.args b/bin/tests/system/dscp/ns5/named.args index c678163f37..4f0c842128 100644 --- a/bin/tests/system/dscp/ns5/named.args +++ b/bin/tests/system/dscp/ns5/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D dscp-ns5 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 +-m record -c named.conf -d 99 -D dscp-ns5 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 diff --git a/bin/tests/system/dscp/ns6/named.args b/bin/tests/system/dscp/ns6/named.args index 283cf22011..a5a278df09 100644 --- a/bin/tests/system/dscp/ns6/named.args +++ b/bin/tests/system/dscp/ns6/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D dscp-ns6 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 +-m record -c named.conf -d 99 -D dscp-ns6 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 diff --git a/bin/tests/system/dscp/ns7/named.args b/bin/tests/system/dscp/ns7/named.args index 4ccf38ea05..733a65e4f7 100644 --- a/bin/tests/system/dscp/ns7/named.args +++ b/bin/tests/system/dscp/ns7/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D dscp-ns7 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 +-m record -c named.conf -d 99 -D dscp-ns7 -X named.lock -g -U 4 -T maxcachesize=2097152 -T dscp=46 diff --git a/bin/tests/system/dupsigs/ns1/named.args b/bin/tests/system/dupsigs/ns1/named.args index 231eed4902..adf6968f86 100644 --- a/bin/tests/system/dupsigs/ns1/named.args +++ b/bin/tests/system/dupsigs/ns1/named.args @@ -1 +1 @@ --D dupsigs-ns1 -X named.lock -m record,size,mctx -c named.conf -d 99 -g -U 4 -T maxcachesize=2097152 -T sigvalinsecs +-D dupsigs-ns1 -X named.lock -m record -c named.conf -d 99 -g -U 4 -T maxcachesize=2097152 -T sigvalinsecs diff --git a/bin/tests/system/fetchlimit/ns3/named.args b/bin/tests/system/fetchlimit/ns3/named.args index 6bd3e6cf64..d3488cefe1 100644 --- a/bin/tests/system/fetchlimit/ns3/named.args +++ b/bin/tests/system/fetchlimit/ns3/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 1 -D fetchlimit-ns3 -X named.lock -g -T maxcachesize=2097152 +-m record -c named.conf -d 1 -D fetchlimit-ns3 -X named.lock -g -T maxcachesize=2097152 diff --git a/bin/tests/system/legacy/ns4/named.args b/bin/tests/system/legacy/ns4/named.args index 0fe6774329..4ccb648595 100644 --- a/bin/tests/system/legacy/ns4/named.args +++ b/bin/tests/system/legacy/ns4/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D legacy-ns4 -X named.lock -g -U 4 -T maxcachesize=2097152 -T noedns +-m record -c named.conf -d 99 -D legacy-ns4 -X named.lock -g -U 4 -T maxcachesize=2097152 -T noedns diff --git a/bin/tests/system/legacy/ns5/named.args b/bin/tests/system/legacy/ns5/named.args index 364370a18b..0c5e17a940 100644 --- a/bin/tests/system/legacy/ns5/named.args +++ b/bin/tests/system/legacy/ns5/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D legacy-ns5 -X named.lock -g -U 4 -T maxcachesize=2097152 -T noedns +-m record -c named.conf -d 99 -D legacy-ns5 -X named.lock -g -U 4 -T maxcachesize=2097152 -T noedns diff --git a/bin/tests/system/legacy/ns6/named.args b/bin/tests/system/legacy/ns6/named.args index 64e5524324..3475121784 100644 --- a/bin/tests/system/legacy/ns6/named.args +++ b/bin/tests/system/legacy/ns6/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D legacy-ns6 -X named.lock -g -U 4 -T maxcachesize=2097152 -T maxudp512 +-m record -c named.conf -d 99 -D legacy-ns6 -X named.lock -g -U 4 -T maxcachesize=2097152 -T maxudp512 diff --git a/bin/tests/system/legacy/ns7/named.args b/bin/tests/system/legacy/ns7/named.args index e491a95dc6..1da498080f 100644 --- a/bin/tests/system/legacy/ns7/named.args +++ b/bin/tests/system/legacy/ns7/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D legacy-ns7 -X named.lock -g -U 4 -T maxcachesize=2097152 -T maxudp512 +-m record -c named.conf -d 99 -D legacy-ns7 -X named.lock -g -U 4 -T maxcachesize=2097152 -T maxudp512 diff --git a/bin/tests/system/logfileconfig/tests.sh b/bin/tests/system/logfileconfig/tests.sh index e1d26a7d29..191680873e 100644 --- a/bin/tests/system/logfileconfig/tests.sh +++ b/bin/tests/system/logfileconfig/tests.sh @@ -35,7 +35,7 @@ DLFILE="named_deflog" PIDFILE="${THISDIR}/${CONFDIR}/named.pid" myRNDC="$RNDC -c ${THISDIR}/${CONFDIR}/rndc.conf" -myNAMED="$NAMED -c ${THISDIR}/${CONFDIR}/named.conf -m record,size,mctx -T nosyslog -d 99 -D logfileconfig-ns1 -X named.lock -U 4" +myNAMED="$NAMED -c ${THISDIR}/${CONFDIR}/named.conf -m record -T nosyslog -d 99 -D logfileconfig-ns1 -X named.lock -U 4" # Test given condition. If true, test again after a second. Used for testing # filesystem-dependent conditions in order to prevent false negatives caused by diff --git a/bin/tests/system/mirror/ns3/named.args b/bin/tests/system/mirror/ns3/named.args index 7519c8f587..78f3feba8f 100644 --- a/bin/tests/system/mirror/ns3/named.args +++ b/bin/tests/system/mirror/ns3/named.args @@ -1 +1 @@ --D mirror-ns3 -X named.lock -m record,size,mctx -c named.conf -d 99 -g -U 4 -T maxcachesize=2097152 -T tat=3 +-D mirror-ns3 -X named.lock -m record -c named.conf -d 99 -g -U 4 -T maxcachesize=2097152 -T tat=3 diff --git a/bin/tests/system/mkeys/ns2/named.args b/bin/tests/system/mkeys/ns2/named.args index 2f752bd79b..e588c91b48 100644 --- a/bin/tests/system/mkeys/ns2/named.args +++ b/bin/tests/system/mkeys/ns2/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D mkeys-ns2 -X named.lock -g -T maxcachesize=2097152 -T mkeytimers=5/10/20 -T tat=1 +-m record -c named.conf -d 99 -D mkeys-ns2 -X named.lock -g -T maxcachesize=2097152 -T mkeytimers=5/10/20 -T tat=1 diff --git a/bin/tests/system/mkeys/ns3/named.args b/bin/tests/system/mkeys/ns3/named.args index 2015ee509a..28396c356c 100644 --- a/bin/tests/system/mkeys/ns3/named.args +++ b/bin/tests/system/mkeys/ns3/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -D mkeys-ns3 -X named.lock -g -T maxcachesize=2097152 -T mkeytimers=5/10/20 +-m record -c named.conf -d 99 -D mkeys-ns3 -X named.lock -g -T maxcachesize=2097152 -T mkeytimers=5/10/20 diff --git a/bin/tests/system/mkeys/ns5/named1.args b/bin/tests/system/mkeys/ns5/named1.args index c4c8a55817..6eeaa29006 100644 --- a/bin/tests/system/mkeys/ns5/named1.args +++ b/bin/tests/system/mkeys/ns5/named1.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -X named.lock -g -T maxcachesize=2097152 +-m record -c named.conf -d 99 -X named.lock -g -T maxcachesize=2097152 diff --git a/bin/tests/system/mkeys/ns5/named2.args b/bin/tests/system/mkeys/ns5/named2.args index 3fd830afbb..34ac30d937 100644 --- a/bin/tests/system/mkeys/ns5/named2.args +++ b/bin/tests/system/mkeys/ns5/named2.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -X named.lock -g -T maxcachesize=2097152 -T mkeytimers=2/20/40 +-m record -c named.conf -d 99 -X named.lock -g -T maxcachesize=2097152 -T mkeytimers=2/20/40 diff --git a/bin/tests/system/mkeys/ns6/named.args b/bin/tests/system/mkeys/ns6/named.args index 65a8fcaa97..7ef5cf02f9 100644 --- a/bin/tests/system/mkeys/ns6/named.args +++ b/bin/tests/system/mkeys/ns6/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 99 -X named.lock -g -T maxcachesize=2097152 -T mkeytimers=5/10/20 +-m record -c named.conf -d 99 -X named.lock -g -T maxcachesize=2097152 -T mkeytimers=5/10/20 diff --git a/bin/tests/system/nsupdate/ns5/named.args b/bin/tests/system/nsupdate/ns5/named.args index a1ebb6f763..fc5a5cd2a7 100644 --- a/bin/tests/system/nsupdate/ns5/named.args +++ b/bin/tests/system/nsupdate/ns5/named.args @@ -1 +1 @@ --D nsupdate-ns5 -m record,size,mctx -c named.conf -d 99 -X named.lock -g -U 4 -T maxcachesize=2097152 -T fixedlocal +-D nsupdate-ns5 -m record -c named.conf -d 99 -X named.lock -g -U 4 -T maxcachesize=2097152 -T fixedlocal diff --git a/bin/tests/system/nsupdate/ns6/named.args b/bin/tests/system/nsupdate/ns6/named.args index 11e5449526..48e2bd82c6 100644 --- a/bin/tests/system/nsupdate/ns6/named.args +++ b/bin/tests/system/nsupdate/ns6/named.args @@ -1 +1 @@ --D nsupdate-ns6 -m record,size,mctx -c named.conf -d 99 -X named.lock -g -U 4 -T maxcachesize=2097152 -T fixedlocal +-D nsupdate-ns6 -m record -c named.conf -d 99 -X named.lock -g -U 4 -T maxcachesize=2097152 -T fixedlocal diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh index a958103c0a..435501c2a4 100755 --- a/bin/tests/system/resolver/tests.sh +++ b/bin/tests/system/resolver/tests.sh @@ -833,7 +833,7 @@ status=`expr $status + $ret` n=`expr $n + 1` echo_i "check logged command line ($n)" ret=0 -grep "running as: .* -m record,size,mctx " ns1/named.run > /dev/null || ret=1 +grep "running as: .* -m record " ns1/named.run > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` diff --git a/bin/tests/system/rpzextra/ns1/named.args b/bin/tests/system/rpzextra/ns1/named.args index b289924b27..970fc22639 100644 --- a/bin/tests/system/rpzextra/ns1/named.args +++ b/bin/tests/system/rpzextra/ns1/named.args @@ -1 +1 @@ - -m record,size,mctx -c named.conf -d 99 -D rpzextra-ns1 -X named.lock -U 4 -T maxcachesize=2097152 + -m record -c named.conf -d 99 -D rpzextra-ns1 -X named.lock -U 4 -T maxcachesize=2097152 diff --git a/bin/tests/system/runtime/tests.sh b/bin/tests/system/runtime/tests.sh index 5cc11015a0..ddbdc10a80 100644 --- a/bin/tests/system/runtime/tests.sh +++ b/bin/tests/system/runtime/tests.sh @@ -15,7 +15,7 @@ set -e RNDCCMD="$RNDC -c ../common/rndc.conf -p ${CONTROLPORT} -s" -NAMED_DEFAULT_ARGS="-m record,size,mctx -d 99 -g -U 4" +NAMED_DEFAULT_ARGS="-m record -d 99 -g -U 4" kill_named() { pidfile="${1}" diff --git a/bin/tests/system/start.pl b/bin/tests/system/start.pl index 89807d0cd0..307fd25d5f 100755 --- a/bin/tests/system/start.pl +++ b/bin/tests/system/start.pl @@ -267,7 +267,7 @@ sub construct_ns_command { } else { $command .= "-D $test-$server "; $command .= "-X named.lock "; - $command .= "-m record,size,mctx "; + $command .= "-m record "; foreach my $t_option( "dropedns", "ednsformerr", "ednsnotimp", "ednsrefused", diff --git a/bin/tests/system/timeouts/ns1/named.args b/bin/tests/system/timeouts/ns1/named.args index 2df2be2737..437a77f0f4 100644 --- a/bin/tests/system/timeouts/ns1/named.args +++ b/bin/tests/system/timeouts/ns1/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 1 -D timeouts-ns1 -X named.lock -g -T maxcachesize=2097152 +-m record -c named.conf -d 1 -D timeouts-ns1 -X named.lock -g -T maxcachesize=2097152 diff --git a/bin/tests/system/zero/ns2/named.args b/bin/tests/system/zero/ns2/named.args index b20594ea45..4b7aad606a 100644 --- a/bin/tests/system/zero/ns2/named.args +++ b/bin/tests/system/zero/ns2/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 1 -D zero-ns2 -X named.lock -g -T maxcachesize=2097152 +-m record -c named.conf -d 1 -D zero-ns2 -X named.lock -g -T maxcachesize=2097152 diff --git a/bin/tests/system/zero/ns3/named.args b/bin/tests/system/zero/ns3/named.args index 9d89bd697c..e6f6060089 100644 --- a/bin/tests/system/zero/ns3/named.args +++ b/bin/tests/system/zero/ns3/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 1 -D zero-ns3 -X named.lock -g -T maxcachesize=2097152 +-m record -c named.conf -d 1 -D zero-ns3 -X named.lock -g -T maxcachesize=2097152 diff --git a/bin/tests/system/zero/ns4/named.args b/bin/tests/system/zero/ns4/named.args index 09d1fe01fc..bd59f13b6c 100644 --- a/bin/tests/system/zero/ns4/named.args +++ b/bin/tests/system/zero/ns4/named.args @@ -1 +1 @@ --m record,size,mctx -c named.conf -d 1 -D zero-ns4 -X named.lock -g -T maxcachesize=2097152 +-m record -c named.conf -d 1 -D zero-ns4 -X named.lock -g -T maxcachesize=2097152 diff --git a/bin/tests/wire_test.c b/bin/tests/wire_test.c index ef86a1e7c5..7b290f8cd9 100644 --- a/bin/tests/wire_test.c +++ b/bin/tests/wire_test.c @@ -130,12 +130,6 @@ main(int argc, char *argv[]) { { isc_mem_debugging |= ISC_MEM_DEBUGUSAGE; } - if (strcasecmp(isc_commandline_argument, "size") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGSIZE; - } - if (strcasecmp(isc_commandline_argument, "mctx") == 0) { - isc_mem_debugging |= ISC_MEM_DEBUGCTX; - } break; default: break; diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index a5f003bdce..945151e0f2 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -43,8 +43,6 @@ extern unsigned int isc_mem_defaultflags; #define ISC_MEM_DEBUGTRACE 0x00000001U #define ISC_MEM_DEBUGRECORD 0x00000002U #define ISC_MEM_DEBUGUSAGE 0x00000004U -#define ISC_MEM_DEBUGSIZE 0x00000008U -#define ISC_MEM_DEBUGCTX 0x00000010U #define ISC_MEM_DEBUGALL 0x0000001FU /*!< * The variable isc_mem_debugging holds a set of flags for @@ -63,14 +61,6 @@ extern unsigned int isc_mem_defaultflags; * \li #ISC_MEM_DEBUGUSAGE * If a hi_water mark is set, print the maximum inuse memory * every time it is raised once it exceeds the hi_water mark. - * - * \li #ISC_MEM_DEBUGSIZE - * Check the size argument being passed to isc_mem_put() matches - * that passed to isc_mem_get(). - * - * \li #ISC_MEM_DEBUGCTX - * Check the mctx argument being passed to isc_mem_put() matches - * that passed to isc_mem_get(). */ /*@}*/ diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 70dae2c471..12a866e07a 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -604,28 +604,11 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { isc_mem_t *ctx = *ctxp; *ctxp = NULL; - if (ISC_UNLIKELY((isc_mem_debugging & - (ISC_MEM_DEBUGSIZE | ISC_MEM_DEBUGCTX)) != 0)) - { - if ((isc_mem_debugging & ISC_MEM_DEBUGSIZE) != 0) { - size_info *si = &(((size_info *)ptr)[-1]); - size_t oldsize = si->size - ALIGNMENT_SIZE; - if ((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0) { - oldsize -= ALIGNMENT_SIZE; - } - INSIST(oldsize == size); - } - isc__mem_free(ctx, ptr FLARG_PASS); - - goto destroy; - } - DELETE_TRACE(ctx, ptr, size, file, line); mem_putstats(ctx, ptr, size); mem_put(ctx, ptr, size); -destroy: if (isc_refcount_decrement(&ctx->references) == 1) { isc_refcount_destroy(&ctx->references); destroy(ctx); @@ -717,12 +700,6 @@ isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { void *ptr; bool call_water = false; - if (ISC_UNLIKELY((isc_mem_debugging & - (ISC_MEM_DEBUGSIZE | ISC_MEM_DEBUGCTX)) != 0)) - { - return (isc__mem_allocate(ctx, size FLARG_PASS)); - } - ptr = mem_get(ctx, size); mem_getstats(ctx, size); @@ -743,23 +720,6 @@ isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) { REQUIRE(ptr != NULL); bool call_water = false; - size_info *si; - - if (ISC_UNLIKELY((isc_mem_debugging & - (ISC_MEM_DEBUGSIZE | ISC_MEM_DEBUGCTX)) != 0)) - { - if ((isc_mem_debugging & ISC_MEM_DEBUGSIZE) != 0) { - size_t oldsize; - si = &(((size_info *)ptr)[-1]); - oldsize = si->size - ALIGNMENT_SIZE; - if ((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0) { - oldsize -= ALIGNMENT_SIZE; - } - INSIST(oldsize == size); - } - isc__mem_free(ctx, ptr FLARG_PASS); - return; - } DELETE_TRACE(ctx, ptr, size, file, line); @@ -894,16 +854,9 @@ mem_allocateunlocked(isc_mem_t *ctx, size_t size) { size_info *si; size += ALIGNMENT_SIZE; - if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0)) { - size += ALIGNMENT_SIZE; - } si = mem_get(ctx, size); - if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0)) { - si->ctx = ctx; - si++; - } si->size = size; return (&si[1]); } @@ -955,11 +908,6 @@ isc__mem_reallocate(isc_mem_t *ctx, void *ptr, size_t size FLARG) { oldsize = (((size_info *)ptr)[-1]).size; INSIST(oldsize >= ALIGNMENT_SIZE); oldsize -= ALIGNMENT_SIZE; - if (ISC_UNLIKELY((isc_mem_debugging & - ISC_MEM_DEBUGCTX) != 0)) { - INSIST(oldsize >= ALIGNMENT_SIZE); - oldsize -= ALIGNMENT_SIZE; - } copysize = (oldsize > size) ? size : oldsize; memmove(new_ptr, ptr, copysize); isc__mem_free(ctx, ptr FLARG_PASS); @@ -980,14 +928,8 @@ isc__mem_free(isc_mem_t *ctx, void *ptr FLARG) { size_t size; bool call_water = false; - if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0)) { - si = &(((size_info *)ptr)[-2]); - REQUIRE(si->ctx == ctx); - size = si[1].size; - } else { - si = &(((size_info *)ptr)[-1]); - size = si->size; - } + si = &(((size_info *)ptr)[-1]); + size = si->size; DELETE_TRACE(ctx, ptr, size, file, line); diff --git a/lib/isc/tls.c b/lib/isc/tls.c index cf947fa645..ad0cb74168 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -72,11 +72,7 @@ tls_initialize(void) { /* * We can't use isc_mem API here, because it's called too * early and when the isc_mem_debugging flags are changed - * later and ISC_MEM_DEBUGSIZE or ISC_MEM_DEBUGCTX flags are - * added, neither isc_mem_put() nor isc_mem_free() can be used - * to free up the memory allocated here because the flags were - * not set when calling isc_mem_get() or isc_mem_allocate() - * here. + * later. * * Actually, since this is a single allocation at library load * and deallocation at library unload, using the standard diff --git a/lib/isc/trampoline.c b/lib/isc/trampoline.c index 009e8d136b..5d0429dfe9 100644 --- a/lib/isc/trampoline.c +++ b/lib/isc/trampoline.c @@ -40,17 +40,13 @@ static size_t isc__trampoline_min = 1; static size_t isc__trampoline_max = 65; /* - * We can't use isc_mem API here, because it's called too - * early and when the isc_mem_debugging flags are changed - * later and ISC_MEM_DEBUGSIZE or ISC_MEM_DEBUGCTX flags are - * added, neither isc_mem_put() nor isc_mem_free() can be used - * to free up the memory allocated here because the flags were - * not set when calling isc_mem_get() or isc_mem_allocate() - * here. + * We can't use isc_mem API here, because it's called too early and the + * isc_mem_debugging flags can be changed later causing mismatch between flags + * used for isc_mem_get() and isc_mem_put(). * - * Actually, since this is a single allocation at library load - * and deallocation at library unload, using the standard - * allocator without the tracking is fine for this purpose. + * Since this is a single allocation at library load and deallocation at library + * unload, using the standard allocator without the tracking is fine for this + * single purpose. */ static isc__trampoline_t * isc__trampoline_new(int tid, isc_threadfunc_t start, isc_threadarg_t arg) { From fcc6814776c285cb64608f5ac18281fdf8a063fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 11 May 2021 14:00:12 +0200 Subject: [PATCH 06/26] Replace internal memory calls with non-standard jemalloc API The jemalloc non-standard API fits nicely with our memory contexts, so just rewrite the memory context internals to use the non-public API. There's just one caveat - since we no longer track the size of the allocation for isc_mem_allocate/isc_mem_free combination, we need to use sallocx() to get real allocation size in both allocator and deallocator because otherwise the sizes would not match. --- lib/isc/mem.c | 171 +++++++++++++++++---------------------- lib/isc/tests/mem_test.c | 6 +- 2 files changed, 75 insertions(+), 102 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 12a866e07a..0d9e7763d7 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -100,13 +100,6 @@ struct element { element *next; }; -typedef struct { - alignas(ALIGNMENT) union { - size_t size; - isc_mem_t *ctx; - }; -} size_info; - struct stats { atomic_size_t gets; atomic_size_t totalgets; @@ -263,7 +256,7 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { #endif idx = hash % DEBUG_TABLE_COUNT; - dl = malloc(sizeof(debuglink_t)); + dl = mallocx(sizeof(debuglink_t), 0); INSIST(dl != NULL); increment_malloced(mctx, sizeof(debuglink_t)); @@ -313,7 +306,7 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, if (ISC_UNLIKELY(dl->ptr == ptr)) { ISC_LIST_UNLINK(mctx->debuglist[idx], dl, link); decrement_malloced(mctx, sizeof(*dl)); - free(dl); + sdallocx(dl, sizeof(*dl), 0); goto unlock; } dl = ISC_LIST_NEXT(dl, link); @@ -337,12 +330,10 @@ static inline void * mem_get(isc_mem_t *ctx, size_t size) { char *ret; - ret = malloc(size); + ret = mallocx(size, 0); if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { - if (ISC_LIKELY(ret != NULL)) { - memset(ret, 0xbe, size); /* Mnemonic for "beef". */ - } + memset(ret, 0xbe, size); /* Mnemonic for "beef". */ } return (ret); @@ -357,7 +348,7 @@ mem_put(isc_mem_t *ctx, void *mem, size_t size) { if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { memset(mem, 0xde, size); /* Mnemonic for "dead". */ } - free(mem); + sdallocx(mem, size, 0); } #define stats_bucket(ctx, size) \ @@ -434,12 +425,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { isc_mem_t *ctx; - STATIC_ASSERT((ALIGNMENT_SIZE & (ALIGNMENT_SIZE - 1)) == 0, - "alignment size not power of 2"); - STATIC_ASSERT(ALIGNMENT_SIZE >= sizeof(size_info), - "alignment size too small"); - - ctx = malloc(sizeof(*ctx)); + ctx = mallocx(sizeof(*ctx), 0); *ctx = (isc_mem_t){ .magic = MEM_MAGIC, @@ -471,7 +457,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { unsigned int i; ctx->debuglist = - malloc((DEBUG_TABLE_COUNT * sizeof(debuglist_t))); + mallocx((DEBUG_TABLE_COUNT * sizeof(debuglist_t)), 0); for (i = 0; i < DEBUG_TABLE_COUNT; i++) { ISC_LIST_INIT(ctx->debuglist[i]); } @@ -518,12 +504,13 @@ destroy(isc_mem_t *ctx) { INSIST(!ctx->checkfree || dl->ptr == NULL); ISC_LIST_UNLINK(ctx->debuglist[i], dl, link); - free(dl); + sdallocx(dl, sizeof(*dl), 0); decrement_malloced(ctx, sizeof(*dl)); } } - free(ctx->debuglist); + sdallocx(ctx->debuglist, + (DEBUG_TABLE_COUNT * sizeof(debuglist_t)), 0); decrement_malloced(ctx, DEBUG_TABLE_COUNT * sizeof(debuglist_t)); } @@ -554,7 +541,7 @@ destroy(isc_mem_t *ctx) { if (ctx->checkfree) { INSIST(malloced == 0); } - free(ctx); + sdallocx(ctx, sizeof(*ctx), 0); } void @@ -644,6 +631,16 @@ isc__mem_destroy(isc_mem_t **ctxp FLARG) { *ctxp = NULL; } +#define CALL_HI_WATER(ctx) \ + if ((ctx->water != NULL) && hi_water(ctx)) { \ + (ctx->water)(ctx->water_arg, ISC_MEM_HIWATER); \ + } + +#define CALL_LO_WATER(ctx) \ + if ((ctx->water != NULL) && lo_water(ctx)) { \ + (ctx->water)(ctx->water_arg, ISC_MEM_LOWATER); \ + } + static inline bool hi_water(isc_mem_t *ctx) { bool call_water = false; @@ -719,18 +716,12 @@ isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(ptr != NULL); - bool call_water = false; - DELETE_TRACE(ctx, ptr, size, file, line); mem_putstats(ctx, ptr, size); mem_put(ctx, ptr, size); - call_water = lo_water(ctx); - - if (call_water && (ctx->water != NULL)) { - (ctx->water)(ctx->water_arg, ISC_MEM_LOWATER); - } + CALL_LO_WATER(ctx); } void @@ -844,34 +835,20 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { MCTXUNLOCK(ctx); } -/* - * Replacements for malloc() and free() -- they implicitly remember the - * size of the object allocated (with some additional overhead). - */ - -static void * -mem_allocateunlocked(isc_mem_t *ctx, size_t size) { - size_info *si; - - size += ALIGNMENT_SIZE; - - si = mem_get(ctx, size); - - si->size = size; - return (&si[1]); -} - void * isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); - size_info *si; + void *ptr; bool call_water = false; - si = mem_allocateunlocked(ctx, size); - mem_getstats(ctx, si[-1].size); + ptr = mem_get(ctx, size); - ADD_TRACE(ctx, si, si[-1].size, file, line); + /* Recalculate the real allocated size */ + size = sallocx(ptr, 0); + + mem_getstats(ctx, size); + ADD_TRACE(ctx, ptr, size, file, line); call_water = hi_water(ctx); @@ -879,41 +856,52 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { (ctx->water)(ctx->water_arg, ISC_MEM_HIWATER); } - return (si); + return (ptr); } void * -isc__mem_reallocate(isc_mem_t *ctx, void *ptr, size_t size FLARG) { +isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); void *new_ptr = NULL; - /* - * This function emulates the realloc(3) standard library - * function: - * - if size > 0, allocate new memory; and if ptr is non NULL, - * copy as much of the old contents to the new buffer and free - * the old one. Note that when allocation fails the original - * pointer is intact; the caller must free it. - * - if size is 0 and ptr is non NULL, simply free the given - * ptr. - * - this function returns: - * pointer to the newly allocated memory, or - * NULL if allocation fails or doesn't happen. - */ - if (size > 0U) { - new_ptr = isc__mem_allocate(ctx, size FLARG_PASS); - if (new_ptr != NULL && ptr != NULL) { - size_t oldsize, copysize; - oldsize = (((size_info *)ptr)[-1]).size; - INSIST(oldsize >= ALIGNMENT_SIZE); - oldsize -= ALIGNMENT_SIZE; - copysize = (oldsize > size) ? size : oldsize; - memmove(new_ptr, ptr, copysize); - isc__mem_free(ctx, ptr FLARG_PASS); + if (new_size == 0) { + /* + * FIXME: We should not call isc__mem_reallocate with size == 0, + * this is undefined behaviour. This code is kept only for + * backwards compatibility. + */ + isc__mem_free(ctx, old_ptr FLARG_PASS); + } else { + size_t old_size = sallocx(old_ptr, 0); + + DELETE_TRACE(ctx, old_ptr, old_size, file, line); + mem_putstats(ctx, old_ptr, old_size); + + new_ptr = rallocx(old_ptr, new_size, 0); + + if (ISC_UNLIKELY((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) { + /* Mnemonic for "beef". */ + memset(diff_ptr, 0xbe, diff_size); + } } - } else if (ptr != NULL) { - isc__mem_free(ctx, ptr FLARG_PASS); + + /* Recalculate the real allocated size */ + new_size = sallocx(new_ptr, 0); + + mem_getstats(ctx, new_size); + ADD_TRACE(ctx, new_ptr, new_size, file, line); + + /* + * We want to postpone the call to water in edge case where the + * realloc will exactly hit on the boundary of the water and we + * would call water twice. + */ + CALL_LO_WATER(ctx); + CALL_HI_WATER(ctx); } return (new_ptr); @@ -924,23 +912,14 @@ isc__mem_free(isc_mem_t *ctx, void *ptr FLARG) { REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(ptr != NULL); - size_info *si; - size_t size; - bool call_water = false; - - si = &(((size_info *)ptr)[-1]); - size = si->size; + size_t size = sallocx(ptr, 0); DELETE_TRACE(ctx, ptr, size, file, line); - mem_putstats(ctx, si, size); - mem_put(ctx, si, size); + mem_putstats(ctx, ptr, size); + mem_put(ctx, ptr, size); - call_water = lo_water(ctx); - - if (call_water && (ctx->water != NULL)) { - (ctx->water)(ctx->water_arg, ISC_MEM_LOWATER); - } + CALL_LO_WATER(ctx); } /* @@ -1562,13 +1541,11 @@ error: int isc_mem_renderxml(void *writer0) { isc_mem_t *ctx; - summarystat_t summary; + summarystat_t summary = { 0 }; uint64_t lost; int xmlrc; xmlTextWriterPtr writer = (xmlTextWriterPtr)writer0; - memset(&summary, 0, sizeof(summary)); - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "contexts")); LOCK(&contextslock); @@ -1705,13 +1682,11 @@ isc_result_t isc_mem_renderjson(void *memobj0) { isc_result_t result = ISC_R_SUCCESS; isc_mem_t *ctx; - summarystat_t summary; + summarystat_t summary = { 0 }; uint64_t lost; json_object *ctxarray, *obj; json_object *memobj = (json_object *)memobj0; - memset(&summary, 0, sizeof(summary)); - ctxarray = json_object_new_array(); CHECKMEM(ctxarray); diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index 4b42b39de0..d0838a68c4 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -181,8 +181,7 @@ isc_mem_total_test(void **state) { after = isc_mem_total(mctx2); diff = after - before; - /* 2048 +8 bytes extra for size_info */ - assert_int_equal(diff, (2048 + 8) * 100000); + assert_int_equal(diff, (2048) * 100000); /* ISC_MEMFLAG_INTERNAL */ @@ -198,8 +197,7 @@ isc_mem_total_test(void **state) { after = isc_mem_total(test_mctx); diff = after - before; - /* 2048 +8 bytes extra for size_info */ - assert_int_equal(diff, (2048 + 8) * 100000); + assert_int_equal(diff, (2048) * 100000); isc_mem_destroy(&mctx2); } From 5ab05d1696561c8169367ebd3805bd4094c24982 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 11 May 2021 14:37:18 +0200 Subject: [PATCH 07/26] Replace isc_mem_allocate() usage with isc_mem_get() in netmgr.c The isc_mem_allocate() comes with additional cost because of the memory tracking. In this commit, we replace the usage with isc_mem_get() because we track the allocated sizes anyway, so it's possible to also replace isc_mem_free() with isc_mem_put(). --- lib/isc/netmgr/netmgr.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 877fd6d664..6f2d05c74c 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -1261,7 +1261,7 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) { } if (sock->buf != NULL) { - isc_mem_free(sock->mgr->mctx, sock->buf); + isc_mem_put(sock->mgr->mctx, sock->buf, sock->buf_size); } if (sock->quota != NULL) { @@ -1279,8 +1279,10 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) { isc_astack_destroy(sock->inactivereqs); sock->magic = 0; - isc_mem_free(sock->mgr->mctx, sock->ah_frees); - isc_mem_free(sock->mgr->mctx, sock->ah_handles); + isc_mem_put(sock->mgr->mctx, sock->ah_frees, + sock->ah_size * sizeof(sock->ah_frees[0])); + isc_mem_put(sock->mgr->mctx, sock->ah_handles, + sock->ah_size * sizeof(sock->ah_handles[0])); isc_mutex_destroy(&sock->lock); isc_condition_destroy(&sock->scond); #if HAVE_LIBNGHTTP2 @@ -1492,10 +1494,10 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type, isc_nm_attach(mgr, &sock->mgr); sock->uv_handle.handle.data = sock; - sock->ah_frees = isc_mem_allocate(mgr->mctx, - sock->ah_size * sizeof(size_t)); - sock->ah_handles = isc_mem_allocate( - mgr->mctx, sock->ah_size * sizeof(isc_nmhandle_t *)); + sock->ah_frees = isc_mem_get(mgr->mctx, + sock->ah_size * sizeof(sock->ah_frees[0])); + sock->ah_handles = isc_mem_get( + mgr->mctx, sock->ah_size * sizeof(sock->ah_handles[0])); ISC_LINK_INIT(&sock->quotacb, link); for (size_t i = 0; i < 32; i++) { sock->ah_frees[i] = i; @@ -1905,12 +1907,12 @@ isc__nm_alloc_dnsbuf(isc_nmsocket_t *sock, size_t len) { if (sock->buf == NULL) { /* We don't have the buffer at all */ size_t alloc_len = len < NM_REG_BUF ? NM_REG_BUF : NM_BIG_BUF; - sock->buf = isc_mem_allocate(sock->mgr->mctx, alloc_len); + sock->buf = isc_mem_get(sock->mgr->mctx, alloc_len); sock->buf_size = alloc_len; } else { /* We have the buffer but it's too small */ - sock->buf = isc_mem_reallocate(sock->mgr->mctx, sock->buf, - NM_BIG_BUF); + isc_mem_put(sock->mgr->mctx, sock->buf, sock->buf_size); + sock->buf = isc_mem_get(sock->mgr->mctx, NM_BIG_BUF); sock->buf_size = NM_BIG_BUF; } } From fd3ceec4752210a16c089ab4efd8201c78b39642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 11 May 2021 19:54:05 +0200 Subject: [PATCH 08/26] Add debug tracing capability to isc_mempool_create/destroy Previously, we only had capability to trace the mempool gets and puts, but for debugging, it's sometimes also important to keep track how many and where do the memory pools get created and destroyed. This commit adds such tracking capability. --- lib/isc/include/isc/mem.h | 8 ++++++-- lib/isc/mem.c | 20 ++++++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 945151e0f2..ec0c98ee45 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -361,8 +361,11 @@ isc_mem_renderjson(void *memobj0); * Memory pools */ +#define isc_mempool_create(c, s, mp) \ + isc__mempool_create((c), (s), (mp)_ISC_MEM_FILELINE) void -isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp); +isc__mempool_create(isc_mem_t *mctx, size_t size, + isc_mempool_t **mpctxp _ISC_MEM_FLARG); /*%< * Create a memory pool. * @@ -381,8 +384,9 @@ isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp); *\li #ISC_R_SUCCESS -- all is well. */ +#define isc_mempool_destroy(mp) isc__mempool_destroy((mp)_ISC_MEM_FILELINE) void -isc_mempool_destroy(isc_mempool_t **mpctxp); +isc__mempool_destroy(isc_mempool_t **mpctxp _ISC_MEM_FLARG); /*%< * Destroy a memory pool. * diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 0d9e7763d7..71e9cdfa28 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -1084,7 +1084,8 @@ isc_mem_getname(isc_mem_t *ctx) { */ void -isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp) { +isc__mempool_create(isc_mem_t *mctx, size_t size, + isc_mempool_t **mpctxp FLARG) { REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(size > 0U); REQUIRE(mpctxp != NULL && *mpctxp == NULL); @@ -1117,6 +1118,13 @@ isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp) { atomic_init(&mpctx->fillcount, 1); atomic_init(&mpctx->gets, 0); +#if ISC_MEM_TRACKLINES + if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { + fprintf(stderr, "create pool %p file %s line %u mctx %p\n", + mpctx, file, line, mctx); + } +#endif /* ISC_MEM_TRACKLINES */ + *mpctxp = (isc_mempool_t *)mpctx; MCTXLOCK(mctx); @@ -1138,7 +1146,7 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name) { } void -isc_mempool_destroy(isc_mempool_t **mpctxp) { +isc__mempool_destroy(isc_mempool_t **mpctxp FLARG) { REQUIRE(mpctxp != NULL); REQUIRE(VALID_MEMPOOL(*mpctxp)); @@ -1149,6 +1157,14 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { mpctx = *mpctxp; *mpctxp = NULL; + +#if ISC_MEM_TRACKLINES + if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { + fprintf(stderr, "destroy pool %p file %s line %u mctx %p\n", + mpctx, file, line, mctx); + } +#endif + if (atomic_load_acquire(&mpctx->allocated) > 0) { UNEXPECTED_ERROR(__FILE__, __LINE__, "isc_mempool_destroy(): mempool %s " From f487c6948b65bfb34bbfac9665f375543a5f34e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 12 May 2021 21:16:17 +0200 Subject: [PATCH 09/26] Replace locked mempools with memory contexts Current mempools are kind of hybrid structures - they serve two purposes: 1. mempool with a lock is basically static sized allocator with pre-allocated free items 2. mempool without a lock is a doubly-linked list of preallocated items The first kind of usage could be easily replaced with jemalloc small sized arena objects and thread-local caches. The second usage not-so-much and we need to keep this (in libdns:message.c) for performance reasons. --- bin/dig/dighost.c | 29 +-- bin/plugins/filter-a.c | 33 +-- bin/plugins/filter-aaaa.c | 33 +-- bin/tests/system/hooks/driver/test-async.c | 9 +- lib/dns/adb.c | 155 +++---------- lib/dns/dispatch.c | 258 ++++----------------- lib/dns/tests/dispatch_test.c | 3 +- lib/isc/include/isc/mem.h | 27 --- lib/isc/mem.c | 161 ++++++------- lib/isc/netmgr/netmgr-int.h | 6 - lib/isc/netmgr/netmgr.c | 36 +-- lib/isc/tests/mem_test.c | 64 ----- 12 files changed, 161 insertions(+), 653 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 650cee5414..05c7ed5598 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -153,7 +153,6 @@ unsigned int digestbits = 0; isc_buffer_t *namebuf = NULL; dns_tsigkey_t *tsigkey = NULL; bool validated = true; -isc_mempool_t *commctx = NULL; bool debugging = false; bool debugtiming = false; bool memdebugging = false; @@ -1367,15 +1366,6 @@ setup_libs(void) { check_result(result, "dst_lib_init"); is_dst_up = true; - isc_mempool_create(mctx, COMMSIZE, &commctx); - isc_mempool_setname(commctx, "COMMPOOL"); - /* - * 6 and 2 set as reasonable parameters for 3 or 4 nameserver - * systems. - */ - isc_mempool_setfreemax(commctx, 6); - isc_mempool_setfillcount(commctx, 2); - isc_mutex_init(&lookup_lock); } @@ -1559,7 +1549,7 @@ _destroy_lookup(dig_lookup_t *lookup) { isc_buffer_free(&lookup->querysig); } if (lookup->sendspace != NULL) { - isc_mempool_put(commctx, lookup->sendspace); + isc_mem_put(mctx, lookup->sendspace, COMMSIZE); } if (lookup->tsigctx != NULL) { @@ -1645,8 +1635,8 @@ destroy_query(dig_query_t *query, const char *file, unsigned int line) { INSIST(query->recvspace != NULL); - isc_mempool_put(commctx, query->recvspace); - isc_mempool_put(commctx, query->tmpsendspace); + isc_mem_put(mctx, query->recvspace, COMMSIZE); + isc_mem_put(mctx, query->tmpsendspace, COMMSIZE); query->magic = 0; isc_mem_free(mctx, query); @@ -2087,8 +2077,8 @@ _new_query(dig_lookup_t *lookup, char *servname, char *userarg, .userarg = userarg, .first_pass = true, .warn_id = true, - .recvspace = isc_mempool_get(commctx), - .tmpsendspace = isc_mempool_get(commctx) }; + .recvspace = isc_mem_get(mctx, COMMSIZE), + .tmpsendspace = isc_mem_get(mctx, COMMSIZE) }; lookup_attach(lookup, &query->lookup); @@ -2381,10 +2371,7 @@ setup_lookup(dig_lookup_t *lookup) { check_result(result, "dns_message_settsigkey"); } - lookup->sendspace = isc_mempool_get(commctx); - if (lookup->sendspace == NULL) { - fatal("memory allocation failure"); - } + lookup->sendspace = isc_mem_get(mctx, COMMSIZE); result = dns_compress_init(&cctx, -1, mctx); check_result(result, "dns_compress_init"); @@ -4250,10 +4237,6 @@ destroy_libs(void) { clear_searchlist(); - if (commctx != NULL) { - debug("freeing commctx"); - isc_mempool_destroy(&commctx); - } if (tsigkey != NULL) { debug("freeing key %p", tsigkey); dns_tsigkey_detach(&tsigkey); diff --git a/bin/plugins/filter-a.c b/bin/plugins/filter-a.c index 4361e975f5..1c67a6cb66 100644 --- a/bin/plugins/filter-a.c +++ b/bin/plugins/filter-a.c @@ -77,12 +77,6 @@ typedef struct filter_instance { ns_plugin_t *module; isc_mem_t *mctx; - /* - * Memory pool for use with persistent data. - */ - isc_mempool_t *datapool; - isc_mutex_t plock; - /* * Hash table associating a client object with its persistent data. */ @@ -353,25 +347,9 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, cfg_line, mctx, lctx, actx)); } - isc_mempool_create(mctx, sizeof(filter_data_t), &inst->datapool); CHECK(isc_ht_init(&inst->ht, mctx, 16)); isc_mutex_init(&inst->hlock); - /* - * Fill the mempool with 1K filter_a state objects at - * a time; ideally after a single allocation, the mempool will - * have enough to handle all the simultaneous queries the system - * requires and it won't be necessary to allocate more. - * - * We don't set any limit on the number of free state objects - * so that they'll always be returned to the pool and not - * freed until the pool is destroyed on shutdown. - */ - isc_mempool_setfillcount(inst->datapool, 1024); - isc_mempool_setfreemax(inst->datapool, UINT_MAX); - isc_mutex_init(&inst->plock); - isc_mempool_associatelock(inst->datapool, &inst->plock); - /* * Set hook points in the view's hooktable. */ @@ -427,10 +405,6 @@ plugin_destroy(void **instp) { isc_ht_destroy(&inst->ht); isc_mutex_destroy(&inst->hlock); } - if (inst->datapool != NULL) { - isc_mempool_destroy(&inst->datapool); - isc_mutex_destroy(&inst->plock); - } if (inst->a_acl != NULL) { dns_acl_detach(&inst->a_acl); } @@ -512,10 +486,7 @@ client_state_create(const query_ctx_t *qctx, filter_instance_t *inst) { filter_data_t *client_state; isc_result_t result; - client_state = isc_mempool_get(inst->datapool); - if (client_state == NULL) { - return; - } + client_state = isc_mem_get(inst->mctx, sizeof(*client_state)); client_state->mode = NONE; client_state->flags = 0; @@ -542,7 +513,7 @@ client_state_destroy(const query_ctx_t *qctx, filter_instance_t *inst) { UNLOCK(&inst->hlock); RUNTIME_CHECK(result == ISC_R_SUCCESS); - isc_mempool_put(inst->datapool, client_state); + isc_mem_put(inst->mctx, client_state, sizeof(*client_state)); } /*% diff --git a/bin/plugins/filter-aaaa.c b/bin/plugins/filter-aaaa.c index f530b596ad..a00946f5f8 100644 --- a/bin/plugins/filter-aaaa.c +++ b/bin/plugins/filter-aaaa.c @@ -77,12 +77,6 @@ typedef struct filter_instance { ns_plugin_t *module; isc_mem_t *mctx; - /* - * Memory pool for use with persistent data. - */ - isc_mempool_t *datapool; - isc_mutex_t plock; - /* * Hash table associating a client object with its persistent data. */ @@ -356,25 +350,9 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, cfg_line, mctx, lctx, actx)); } - isc_mempool_create(mctx, sizeof(filter_data_t), &inst->datapool); CHECK(isc_ht_init(&inst->ht, mctx, 16)); isc_mutex_init(&inst->hlock); - /* - * Fill the mempool with 1K filter_aaaa state objects at - * a time; ideally after a single allocation, the mempool will - * have enough to handle all the simultaneous queries the system - * requires and it won't be necessary to allocate more. - * - * We don't set any limit on the number of free state objects - * so that they'll always be returned to the pool and not - * freed until the pool is destroyed on shutdown. - */ - isc_mempool_setfillcount(inst->datapool, 1024); - isc_mempool_setfreemax(inst->datapool, UINT_MAX); - isc_mutex_init(&inst->plock); - isc_mempool_associatelock(inst->datapool, &inst->plock); - /* * Set hook points in the view's hooktable. */ @@ -430,10 +408,6 @@ plugin_destroy(void **instp) { isc_ht_destroy(&inst->ht); isc_mutex_destroy(&inst->hlock); } - if (inst->datapool != NULL) { - isc_mempool_destroy(&inst->datapool); - isc_mutex_destroy(&inst->plock); - } if (inst->aaaa_acl != NULL) { dns_acl_detach(&inst->aaaa_acl); } @@ -515,10 +489,7 @@ client_state_create(const query_ctx_t *qctx, filter_instance_t *inst) { filter_data_t *client_state; isc_result_t result; - client_state = isc_mempool_get(inst->datapool); - if (client_state == NULL) { - return; - } + client_state = isc_mem_get(inst->mctx, sizeof(*client_state)); client_state->mode = NONE; client_state->flags = 0; @@ -545,7 +516,7 @@ client_state_destroy(const query_ctx_t *qctx, filter_instance_t *inst) { UNLOCK(&inst->hlock); RUNTIME_CHECK(result == ISC_R_SUCCESS); - isc_mempool_put(inst->datapool, client_state); + isc_mem_put(inst->mctx, client_state, sizeof(*client_state)); } /*% diff --git a/bin/tests/system/hooks/driver/test-async.c b/bin/tests/system/hooks/driver/test-async.c index f1669e9fe9..9ea950af46 100644 --- a/bin/tests/system/hooks/driver/test-async.c +++ b/bin/tests/system/hooks/driver/test-async.c @@ -53,7 +53,6 @@ typedef struct async_instance { ns_plugin_t *module; isc_mem_t *mctx; - isc_mempool_t *datapool; isc_ht_t *ht; isc_mutex_t hlock; isc_log_t *lctx; @@ -146,7 +145,6 @@ plugin_register(const char *parameters, const void *cfg, const char *cfg_file, *inst = (async_instance_t){ .mctx = NULL }; isc_mem_attach(mctx, &inst->mctx); - isc_mempool_create(mctx, sizeof(state_t), &inst->datapool); CHECK(isc_ht_init(&inst->ht, mctx, 16)); isc_mutex_init(&inst->hlock); @@ -194,9 +192,6 @@ plugin_destroy(void **instp) { isc_ht_destroy(&inst->ht); isc_mutex_destroy(&inst->hlock); } - if (inst->datapool != NULL) { - isc_mempool_destroy(&inst->datapool); - } isc_mem_putanddetach(&inst->mctx, inst, sizeof(*inst)); *instp = NULL; @@ -230,7 +225,7 @@ client_state_create(const query_ctx_t *qctx, async_instance_t *inst) { state_t *state = NULL; isc_result_t result; - state = isc_mempool_get(inst->datapool); + state = isc_mem_get(inst->mctx, sizeof(*state)); if (state == NULL) { return; } @@ -257,7 +252,7 @@ client_state_destroy(const query_ctx_t *qctx, async_instance_t *inst) { UNLOCK(&inst->hlock); RUNTIME_CHECK(result == ISC_R_SUCCESS); - isc_mempool_put(inst->datapool, state); + isc_mem_put(inst->mctx, state, sizeof(*state)); } static ns_hookresult_t diff --git a/lib/dns/adb.c b/lib/dns/adb.c index 37cd3dbdcd..defb75c44c 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -112,14 +112,8 @@ struct dns_adb { unsigned int irefcnt; unsigned int erefcnt; - isc_mutex_t mplock; - isc_mempool_t *nmp; /*%< dns_adbname_t */ - isc_mempool_t *nhmp; /*%< dns_adbnamehook_t */ - isc_mempool_t *limp; /*%< dns_adblameinfo_t */ - isc_mempool_t *emp; /*%< dns_adbentry_t */ - isc_mempool_t *ahmp; /*%< dns_adbfind_t */ - isc_mempool_t *aimp; /*%< dns_adbaddrinfo_t */ - isc_mempool_t *afmp; /*%< dns_adbfetch_t */ + isc_refcount_t ahrefcnt; + isc_refcount_t nhrefcnt; /*! * Bucketized locks and lists for names. @@ -624,12 +618,6 @@ grow_entries(isc_task_t *task, isc_event_t *ev) { newentrylocks = isc_mem_get(adb->mctx, sizeof(*newentrylocks) * n); newentry_sd = isc_mem_get(adb->mctx, sizeof(*newentry_sd) * n); newentry_refcnt = isc_mem_get(adb->mctx, sizeof(*newentry_refcnt) * n); - if (newentries == NULL || newdeadentries == NULL || - newentrylocks == NULL || newentry_sd == NULL || - newentry_refcnt == NULL) - { - goto cleanup; - } /* * Initialise the new resources. @@ -795,11 +783,6 @@ grow_names(isc_task_t *task, isc_event_t *ev) { newnamelocks = isc_mem_get(adb->mctx, sizeof(*newnamelocks) * n); newname_sd = isc_mem_get(adb->mctx, sizeof(*newname_sd) * n); newname_refcnt = isc_mem_get(adb->mctx, sizeof(*newname_refcnt) * n); - if (newnames == NULL || newdeadnames == NULL || newnamelocks == NULL || - newname_sd == NULL || newname_refcnt == NULL) - { - goto cleanup; - } /* * Initialise the new resources. @@ -1734,10 +1717,7 @@ static inline dns_adbname_t * new_adbname(dns_adb_t *adb, const dns_name_t *dnsname) { dns_adbname_t *name; - name = isc_mempool_get(adb->nmp); - if (name == NULL) { - return (NULL); - } + name = isc_mem_get(adb->mctx, sizeof(*name)); dns_name_init(&name->name, NULL); dns_name_dup(dnsname, adb->mctx, &name->name); @@ -1795,7 +1775,7 @@ free_adbname(dns_adb_t *adb, dns_adbname_t **name) { n->magic = 0; dns_name_free(&n->name, adb->mctx); - isc_mempool_put(adb->nmp, n); + isc_mem_put(adb->mctx, n, sizeof(*n)); LOCK(&adb->namescntlock); adb->namescnt--; dec_adbstats(adb, dns_adbstats_namescnt); @@ -1806,10 +1786,8 @@ static inline dns_adbnamehook_t * new_adbnamehook(dns_adb_t *adb, dns_adbentry_t *entry) { dns_adbnamehook_t *nh; - nh = isc_mempool_get(adb->nhmp); - if (nh == NULL) { - return (NULL); - } + nh = isc_mem_get(adb->mctx, sizeof(*nh)); + isc_refcount_increment0(&adb->nhrefcnt); nh->magic = DNS_ADBNAMEHOOK_MAGIC; nh->entry = entry; @@ -1830,7 +1808,9 @@ free_adbnamehook(dns_adb_t *adb, dns_adbnamehook_t **namehook) { INSIST(!ISC_LINK_LINKED(nh, plink)); nh->magic = 0; - isc_mempool_put(adb->nhmp, nh); + + isc_refcount_decrement(&adb->nhrefcnt); + isc_mem_put(adb->mctx, nh, sizeof(*nh)); } static inline dns_adblameinfo_t * @@ -1838,10 +1818,7 @@ new_adblameinfo(dns_adb_t *adb, const dns_name_t *qname, dns_rdatatype_t qtype) { dns_adblameinfo_t *li; - li = isc_mempool_get(adb->limp); - if (li == NULL) { - return (NULL); - } + li = isc_mem_get(adb->mctx, sizeof(*li)); dns_name_init(&li->qname, NULL); dns_name_dup(qname, adb->mctx, &li->qname); @@ -1867,17 +1844,14 @@ free_adblameinfo(dns_adb_t *adb, dns_adblameinfo_t **lameinfo) { li->magic = 0; - isc_mempool_put(adb->limp, li); + isc_mem_put(adb->mctx, li, sizeof(*li)); } static inline dns_adbentry_t * new_adbentry(dns_adb_t *adb) { dns_adbentry_t *e; - e = isc_mempool_get(adb->emp); - if (e == NULL) { - return (NULL); - } + e = isc_mem_get(adb->mctx, sizeof(*e)); e->magic = DNS_ADBENTRY_MAGIC; e->lock_bucket = DNS_ADB_INVALIDBUCKET; @@ -1944,7 +1918,7 @@ free_adbentry(dns_adb_t *adb, dns_adbentry_t **entry) { li = ISC_LIST_HEAD(e->lameinfo); } - isc_mempool_put(adb->emp, e); + isc_mem_put(adb->mctx, e, sizeof(*e)); LOCK(&adb->entriescntlock); adb->entriescnt--; dec_adbstats(adb, dns_adbstats_entriescnt); @@ -1955,10 +1929,8 @@ static inline dns_adbfind_t * new_adbfind(dns_adb_t *adb) { dns_adbfind_t *h; - h = isc_mempool_get(adb->ahmp); - if (h == NULL) { - return (NULL); - } + h = isc_mem_get(adb->mctx, sizeof(*h)); + isc_refcount_increment0(&adb->ahrefcnt); /* * Public members. @@ -1993,10 +1965,7 @@ static inline dns_adbfetch_t * new_adbfetch(dns_adb_t *adb) { dns_adbfetch_t *f; - f = isc_mempool_get(adb->afmp); - if (f == NULL) { - return (NULL); - } + f = isc_mem_get(adb->mctx, sizeof(*f)); f->magic = 0; f->fetch = NULL; @@ -2022,7 +1991,7 @@ free_adbfetch(dns_adb_t *adb, dns_adbfetch_t **fetch) { dns_rdataset_disassociate(&f->rdataset); } - isc_mempool_put(adb->afmp, f); + isc_mem_put(adb->mctx, f, sizeof(*f)); } static inline bool @@ -2042,7 +2011,9 @@ free_adbfind(dns_adb_t *adb, dns_adbfind_t **findp) { find->magic = 0; isc_mutex_destroy(&find->lock); - isc_mempool_put(adb->ahmp, find); + + isc_refcount_decrement(&adb->ahrefcnt); + isc_mem_put(adb->mctx, find, sizeof(*find)); return (dec_adb_irefcnt(adb)); } @@ -2055,10 +2026,7 @@ static inline dns_adbaddrinfo_t * new_adbaddrinfo(dns_adb_t *adb, dns_adbentry_t *entry, in_port_t port) { dns_adbaddrinfo_t *ai; - ai = isc_mempool_get(adb->aimp); - if (ai == NULL) { - return (NULL); - } + ai = isc_mem_get(adb->mctx, sizeof(*ai)); ai->magic = DNS_ADBADDRINFO_MAGIC; ai->sockaddr = entry->sockaddr; @@ -2085,7 +2053,7 @@ free_adbaddrinfo(dns_adb_t *adb, dns_adbaddrinfo_t **ainfo) { ai->magic = 0; - isc_mempool_put(adb->aimp, ai); + isc_mem_put(adb->mctx, ai, sizeof(*ai)); } /* @@ -2539,14 +2507,6 @@ destroy(dns_adb_t *adb) { isc_task_detach(&adb->excl); } - isc_mempool_destroy(&adb->nmp); - isc_mempool_destroy(&adb->nhmp); - isc_mempool_destroy(&adb->limp); - isc_mempool_destroy(&adb->emp); - isc_mempool_destroy(&adb->ahmp); - isc_mempool_destroy(&adb->aimp); - isc_mempool_destroy(&adb->afmp); - isc_mutexblock_destroy(adb->entrylocks, adb->nentries); isc_mem_put(adb->mctx, adb->entries, sizeof(*adb->entries) * adb->nentries); @@ -2572,7 +2532,6 @@ destroy(dns_adb_t *adb) { isc_mutex_destroy(&adb->reflock); isc_mutex_destroy(&adb->lock); - isc_mutex_destroy(&adb->mplock); isc_mutex_destroy(&adb->overmemlock); isc_mutex_destroy(&adb->entriescntlock); isc_mutex_destroy(&adb->namescntlock); @@ -2608,13 +2567,6 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr, adb->magic = 0; adb->erefcnt = 1; adb->irefcnt = 0; - adb->nmp = NULL; - adb->nhmp = NULL; - adb->limp = NULL; - adb->emp = NULL; - adb->ahmp = NULL; - adb->aimp = NULL; - adb->afmp = NULL; adb->task = NULL; adb->excl = NULL; adb->mctx = NULL; @@ -2670,7 +2622,6 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr, isc_mem_attach(mem, &adb->mctx); isc_mutex_init(&adb->lock); - isc_mutex_init(&adb->mplock); isc_mutex_init(&adb->reflock); isc_mutex_init(&adb->overmemlock); isc_mutex_init(&adb->entriescntlock); @@ -2680,10 +2631,6 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr, do { \ (adb)->el = isc_mem_get((adb)->mctx, \ sizeof(*(adb)->el) * (adb)->nentries); \ - if ((adb)->el == NULL) { \ - result = ISC_R_NOMEMORY; \ - goto fail1; \ - } \ } while (0) ALLOCENTRY(adb, entries); ALLOCENTRY(adb, deadentries); @@ -2696,10 +2643,6 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr, do { \ (adb)->el = isc_mem_get((adb)->mctx, \ sizeof(*(adb)->el) * (adb)->nnames); \ - if ((adb)->el == NULL) { \ - result = ISC_R_NOMEMORY; \ - goto fail1; \ - } \ } while (0) ALLOCNAME(adb, names); ALLOCNAME(adb, deadnames); @@ -2730,27 +2673,8 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr, } isc_mutexblock_init(adb->entrylocks, adb->nentries); - /* - * Memory pools - */ -#define MPINIT(t, p, n) \ - do { \ - isc_mempool_create(mem, sizeof(t), &(p)); \ - isc_mempool_setfreemax((p), FREE_ITEMS); \ - isc_mempool_setfillcount((p), FILL_COUNT); \ - isc_mempool_setname((p), n); \ - isc_mempool_associatelock((p), &adb->mplock); \ - } while (0) - - MPINIT(dns_adbname_t, adb->nmp, "adbname"); - MPINIT(dns_adbnamehook_t, adb->nhmp, "adbnamehook"); - MPINIT(dns_adblameinfo_t, adb->limp, "adblameinfo"); - MPINIT(dns_adbentry_t, adb->emp, "adbentry"); - MPINIT(dns_adbfind_t, adb->ahmp, "adbfind"); - MPINIT(dns_adbaddrinfo_t, adb->aimp, "adbaddrinfo"); - MPINIT(dns_adbfetch_t, adb->afmp, "adbfetch"); - -#undef MPINIT + isc_refcount_init(&adb->ahrefcnt, 0); + isc_refcount_init(&adb->nhrefcnt, 0); /* * Allocate an internal task. @@ -2786,7 +2710,6 @@ fail2: isc_mutexblock_destroy(adb->entrylocks, adb->nentries); isc_mutexblock_destroy(adb->namelocks, adb->nnames); -fail1: /* clean up only allocated memory */ if (adb->entries != NULL) { isc_mem_put(adb->mctx, adb->entries, sizeof(*adb->entries) * adb->nentries); @@ -2827,33 +2750,11 @@ fail1: /* clean up only allocated memory */ isc_mem_put(adb->mctx, adb->name_refcnt, sizeof(*adb->name_refcnt) * adb->nnames); } - if (adb->nmp != NULL) { - isc_mempool_destroy(&adb->nmp); - } - if (adb->nhmp != NULL) { - isc_mempool_destroy(&adb->nhmp); - } - if (adb->limp != NULL) { - isc_mempool_destroy(&adb->limp); - } - if (adb->emp != NULL) { - isc_mempool_destroy(&adb->emp); - } - if (adb->ahmp != NULL) { - isc_mempool_destroy(&adb->ahmp); - } - if (adb->aimp != NULL) { - isc_mempool_destroy(&adb->aimp); - } - if (adb->afmp != NULL) { - isc_mempool_destroy(&adb->afmp); - } isc_mutex_destroy(&adb->namescntlock); isc_mutex_destroy(&adb->entriescntlock); isc_mutex_destroy(&adb->overmemlock); isc_mutex_destroy(&adb->reflock); - isc_mutex_destroy(&adb->mplock); isc_mutex_destroy(&adb->lock); if (adb->excl != NULL) { isc_task_detach(&adb->excl); @@ -2918,7 +2819,7 @@ dns_adb_whenshutdown(dns_adb_t *adb, isc_task_t *task, isc_event_t **eventp) { zeroirefcnt = (adb->irefcnt == 0); if (adb->shutting_down && zeroirefcnt && - isc_mempool_getallocated(adb->ahmp) == 0) + isc_refcount_current(&adb->ahrefcnt) == 0) { /* * We're already shutdown. Send the event. @@ -3523,9 +3424,11 @@ dump_adb(dns_adb_t *adb, FILE *f, bool debug, isc_stdtime_t now) { fprintf(f, "; [plain success/timeout]\n;\n"); if (debug) { LOCK(&adb->reflock); - fprintf(f, "; addr %p, erefcnt %u, irefcnt %u, finds out %u\n", + fprintf(f, + "; addr %p, erefcnt %u, irefcnt %u, finds out " + "%" PRIuFAST32 "\n", adb, adb->erefcnt, adb->irefcnt, - isc_mempool_getallocated(adb->nhmp)); + isc_refcount_current(&adb->nhrefcnt)); UNLOCK(&adb->reflock); } diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index f226d156bf..168dc37fa1 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -74,17 +74,7 @@ struct dns_dispatchmgr { unsigned int buffersize; /*%< size of each buffer */ unsigned int maxbuffers; /*%< max buffers */ - /* Locked internally. */ - isc_mutex_t depool_lock; - isc_mempool_t *depool; /*%< pool for dispatch events */ - isc_mutex_t rpool_lock; - isc_mempool_t *rpool; /*%< pool for replies */ - isc_mutex_t dpool_lock; - isc_mempool_t *dpool; /*%< dispatch allocations */ - isc_mutex_t bpool_lock; - isc_mempool_t *bpool; /*%< pool for buffers */ - isc_mutex_t spool_lock; - isc_mempool_t *spool; /*%< pool for dispsocks */ + isc_refcount_t irefs; /*% * Locked by qid->lock if qid exists; otherwise, can be used without @@ -206,8 +196,7 @@ struct dns_dispatch { unsigned int maxrequests; /*%< max requests */ isc_event_t *ctlevent; - isc_mutex_t sepool_lock; - isc_mempool_t *sepool; /*%< pool for socket events */ + isc_mem_t *sepool; /*%< pool for socket events */ /*% Locked by mgr->lock. */ ISC_LINK(dns_dispatch_t) link; @@ -232,7 +221,6 @@ struct dns_dispatch { dns_tcpmsg_t tcpmsg; /*%< for tcp streams */ dns_qid_t *qid; dispportlist_t *port_table; /*%< hold ports 'owned' by us */ - isc_mempool_t *portpool; /*%< port table entries */ }; #define QID_MAGIC ISC_MAGIC('Q', 'i', 'd', ' ') @@ -547,8 +535,7 @@ destroy_disp(isc_task_t *task, isc_event_t *event) { disp->socket, disp->task[0]); /* XXXX */ if (disp->sepool != NULL) { - isc_mempool_destroy(&disp->sepool); - isc_mutex_destroy(&disp->sepool_lock); + isc_mem_destroy(&disp->sepool); } if (disp->socket != NULL) { @@ -601,10 +588,7 @@ new_portentry(dns_dispatch_t *disp, in_port_t port) { REQUIRE(disp->port_table != NULL); - portentry = isc_mempool_get(disp->portpool); - if (portentry == NULL) { - return (portentry); - } + portentry = isc_mem_get(disp->mgr->mctx, sizeof(*portentry)); portentry->port = port; isc_refcount_init(&portentry->refs, 1); @@ -633,7 +617,7 @@ deref_portentry(dns_dispatch_t *disp, dispportentry_t **portentryp) { ISC_LIST_UNLINK(disp->port_table[portentry->port % DNS_DISPATCH_PORTTABLESIZE], portentry, link); - isc_mempool_put(disp->portpool, portentry); + isc_mem_put(disp->mgr->mctx, portentry, sizeof(*portentry)); } } @@ -703,10 +687,7 @@ get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest, sock = dispsock->socket; dispsock->socket = NULL; } else { - dispsock = isc_mempool_get(mgr->spool); - if (dispsock == NULL) { - return (ISC_R_NOMEMORY); - } + dispsock = isc_mem_get(mgr->mctx, sizeof(*dispsock)); disp->nsockets++; dispsock->socket = NULL; @@ -832,7 +813,7 @@ destroy_dispsocket(dns_dispatch_t *disp, dispsocket_t **dispsockp) { if (dispsock->task != NULL) { isc_task_detach(&dispsock->task); } - isc_mempool_put(disp->mgr->spool, dispsock); + isc_mem_put(disp->mgr->mctx, dispsock, sizeof(*dispsock)); } /*% @@ -911,7 +892,7 @@ entry_search(dns_qid_t *qid, const isc_sockaddr_t *dest, dns_messageid_t id, static void free_buffer(dns_dispatch_t *disp, void *buf, unsigned int len) { - isc_mempool_t *bpool; + unsigned int buffersize; INSIST(buf != NULL && len != 0); switch (disp->socktype) { @@ -925,9 +906,9 @@ free_buffer(dns_dispatch_t *disp, void *buf, unsigned int len) { INSIST(disp->mgr->buffers > 0); INSIST(len == disp->mgr->buffersize); disp->mgr->buffers--; - bpool = disp->mgr->bpool; + buffersize = disp->mgr->buffersize; UNLOCK(&disp->mgr->buffer_lock); - isc_mempool_put(bpool, buf); + isc_mem_put(disp->mgr->mctx, buf, buffersize); break; default: INSIST(0); @@ -937,34 +918,25 @@ free_buffer(dns_dispatch_t *disp, void *buf, unsigned int len) { static void * allocate_udp_buffer(dns_dispatch_t *disp) { - isc_mempool_t *bpool; - void *temp; + unsigned int buffersize; LOCK(&disp->mgr->buffer_lock); if (disp->mgr->buffers >= disp->mgr->maxbuffers) { UNLOCK(&disp->mgr->buffer_lock); return (NULL); } - bpool = disp->mgr->bpool; + buffersize = disp->mgr->buffersize; disp->mgr->buffers++; UNLOCK(&disp->mgr->buffer_lock); - temp = isc_mempool_get(bpool); - - if (temp == NULL) { - LOCK(&disp->mgr->buffer_lock); - disp->mgr->buffers--; - UNLOCK(&disp->mgr->buffer_lock); - } - - return (temp); + return (isc_mem_get(disp->mgr->mctx, buffersize)); } static inline void free_sevent(isc_event_t *ev) { - isc_mempool_t *pool = ev->ev_destroy_arg; + isc_mem_t *pool = ev->ev_destroy_arg; isc_socketevent_t *sev = (isc_socketevent_t *)ev; - isc_mempool_put(pool, sev); + isc_mem_put(pool, sev, sizeof(*sev)); } static inline isc_socketevent_t * @@ -973,10 +945,7 @@ allocate_sevent(dns_dispatch_t *disp, isc_socket_t *sock, isc_eventtype_t type, isc_socketevent_t *ev; void *deconst_arg; - ev = isc_mempool_get(disp->sepool); - if (ev == NULL) { - return (NULL); - } + ev = isc_mem_get(disp->sepool, sizeof(*ev)); DE_CONST(arg, deconst_arg); ISC_EVENT_INIT(ev, sizeof(*ev), 0, NULL, type, action, deconst_arg, sock, free_sevent, disp->sepool); @@ -999,17 +968,16 @@ free_devent(dns_dispatch_t *disp, dns_dispatchevent_t *ev) { return; } - isc_mempool_put(disp->mgr->depool, ev); + isc_refcount_decrement(&disp->mgr->irefs); + isc_mem_put(disp->mgr->mctx, ev, sizeof(*ev)); } static inline dns_dispatchevent_t * allocate_devent(dns_dispatch_t *disp) { dns_dispatchevent_t *ev; - ev = isc_mempool_get(disp->mgr->depool); - if (ev == NULL) { - return (NULL); - } + ev = isc_mem_get(disp->mgr->mctx, sizeof(*ev)); + isc_refcount_increment0(&disp->mgr->irefs); ISC_EVENT_INIT(ev, sizeof(*ev), 0, NULL, 0, NULL, NULL, NULL, NULL, NULL); @@ -1627,25 +1595,15 @@ startrecv(dns_dispatch_t *disp, dispsocket_t *dispsock) { static bool destroy_mgr_ok(dns_dispatchmgr_t *mgr) { mgr_log(mgr, LVL(90), - "destroy_mgr_ok: shuttingdown=%d, listnonempty=%d, " - "depool=%d, rpool=%d, dpool=%d", - MGR_IS_SHUTTINGDOWN(mgr), !ISC_LIST_EMPTY(mgr->list), - isc_mempool_getallocated(mgr->depool), - isc_mempool_getallocated(mgr->rpool), - isc_mempool_getallocated(mgr->dpool)); + "destroy_mgr_ok: shuttingdown=%d, listnonempty=%d, ", + MGR_IS_SHUTTINGDOWN(mgr), !ISC_LIST_EMPTY(mgr->list)); if (!MGR_IS_SHUTTINGDOWN(mgr)) { return (false); } if (!ISC_LIST_EMPTY(mgr->list)) { return (false); } - if (isc_mempool_getallocated(mgr->depool) != 0) { - return (false); - } - if (isc_mempool_getallocated(mgr->rpool) != 0) { - return (false); - } - if (isc_mempool_getallocated(mgr->dpool) != 0) { + if (isc_refcount_current(&mgr->irefs) != 0) { return (false); } @@ -1666,22 +1624,6 @@ destroy_mgr(dns_dispatchmgr_t **mgrp) { isc_mutex_destroy(&mgr->lock); mgr->state = 0; - isc_mempool_destroy(&mgr->depool); - isc_mempool_destroy(&mgr->rpool); - isc_mempool_destroy(&mgr->dpool); - if (mgr->bpool != NULL) { - isc_mempool_destroy(&mgr->bpool); - } - if (mgr->spool != NULL) { - isc_mempool_destroy(&mgr->spool); - } - - isc_mutex_destroy(&mgr->spool_lock); - isc_mutex_destroy(&mgr->bpool_lock); - isc_mutex_destroy(&mgr->dpool_lock); - isc_mutex_destroy(&mgr->rpool_lock); - isc_mutex_destroy(&mgr->depool_lock); - if (mgr->qid != NULL) { qid_destroy(mgr->mctx, &mgr->qid); } @@ -1791,61 +1733,17 @@ dns_dispatchmgr_create(isc_mem_t *mctx, dns_dispatchmgr_t **mgrp) { REQUIRE(mgrp != NULL && *mgrp == NULL); mgr = isc_mem_get(mctx, sizeof(dns_dispatchmgr_t)); + *mgr = (dns_dispatchmgr_t){ 0 }; - mgr->mctx = NULL; isc_mem_attach(mctx, &mgr->mctx); - mgr->blackhole = NULL; - mgr->stats = NULL; - isc_mutex_init(&mgr->lock); isc_mutex_init(&mgr->buffer_lock); - isc_mutex_init(&mgr->depool_lock); - isc_mutex_init(&mgr->rpool_lock); - isc_mutex_init(&mgr->dpool_lock); - isc_mutex_init(&mgr->bpool_lock); - isc_mutex_init(&mgr->spool_lock); - mgr->depool = NULL; - isc_mempool_create(mgr->mctx, sizeof(dns_dispatchevent_t), - &mgr->depool); + isc_refcount_init(&mgr->irefs, 0); - mgr->rpool = NULL; - isc_mempool_create(mgr->mctx, sizeof(dns_dispentry_t), &mgr->rpool); - - mgr->dpool = NULL; - isc_mempool_create(mgr->mctx, sizeof(dns_dispatch_t), &mgr->dpool); - - isc_mempool_setname(mgr->depool, "dispmgr_depool"); - isc_mempool_setmaxalloc(mgr->depool, 32768); - isc_mempool_setfreemax(mgr->depool, 32768); - isc_mempool_associatelock(mgr->depool, &mgr->depool_lock); - isc_mempool_setfillcount(mgr->depool, 32); - - isc_mempool_setname(mgr->rpool, "dispmgr_rpool"); - isc_mempool_setmaxalloc(mgr->rpool, 32768); - isc_mempool_setfreemax(mgr->rpool, 32768); - isc_mempool_associatelock(mgr->rpool, &mgr->rpool_lock); - isc_mempool_setfillcount(mgr->rpool, 32); - - isc_mempool_setname(mgr->dpool, "dispmgr_dpool"); - isc_mempool_setmaxalloc(mgr->dpool, 32768); - isc_mempool_setfreemax(mgr->dpool, 32768); - isc_mempool_associatelock(mgr->dpool, &mgr->dpool_lock); - isc_mempool_setfillcount(mgr->dpool, 32); - - mgr->buffers = 0; - mgr->buffersize = 0; - mgr->maxbuffers = 0; - mgr->bpool = NULL; - mgr->spool = NULL; - mgr->qid = NULL; - mgr->state = 0; ISC_LIST_INIT(mgr->list); - mgr->v4ports = NULL; - mgr->v6ports = NULL; - mgr->nv4ports = 0; - mgr->nv6ports = 0; + mgr->magic = DNS_DISPATCHMGR_MAGIC; result = create_default_portset(mctx, &v4portset); @@ -1870,14 +1768,6 @@ dns_dispatchmgr_create(isc_mem_t *mctx, dns_dispatchmgr_t **mgrp) { return (ISC_R_SUCCESS); kill_dpool: - isc_mempool_destroy(&mgr->dpool); - isc_mempool_destroy(&mgr->rpool); - isc_mempool_destroy(&mgr->depool); - isc_mutex_destroy(&mgr->spool_lock); - isc_mutex_destroy(&mgr->bpool_lock); - isc_mutex_destroy(&mgr->dpool_lock); - isc_mutex_destroy(&mgr->rpool_lock); - isc_mutex_destroy(&mgr->depool_lock); isc_mutex_destroy(&mgr->buffer_lock); isc_mutex_destroy(&mgr->lock); isc_mem_putanddetach(&mctx, mgr, sizeof(dns_dispatchmgr_t)); @@ -1965,6 +1855,7 @@ dns_dispatchmgr_setudp(dns_dispatchmgr_t *mgr, unsigned int buffersize, REQUIRE(maxbuffers > 0); REQUIRE(buckets < 2097169); /* next prime > 65536 * 32 */ REQUIRE(increment > buckets); + UNUSED(maxrequests); /* * Keep some number of items around. This should be a config @@ -1985,48 +1876,15 @@ dns_dispatchmgr_setudp(dns_dispatchmgr_t *mgr, unsigned int buffersize, LOCK(&mgr->buffer_lock); - /* Create or adjust buffer pool */ - if (mgr->bpool != NULL) { - /* - * We only increase the maxbuffers to avoid accidental buffer - * shortage. Ideally we'd separate the manager-wide maximum - * from per-dispatch limits and respect the latter within the - * global limit. But at this moment that's deemed to be - * overkilling and isn't worth additional implementation - * complexity. - */ - if (maxbuffers > mgr->maxbuffers) { - isc_mempool_setmaxalloc(mgr->bpool, maxbuffers); - isc_mempool_setfreemax(mgr->bpool, maxbuffers); - mgr->maxbuffers = maxbuffers; - } - } else { - isc_mempool_create(mgr->mctx, buffersize, &mgr->bpool); - isc_mempool_setname(mgr->bpool, "dispmgr_bpool"); - isc_mempool_setmaxalloc(mgr->bpool, maxbuffers); - isc_mempool_setfreemax(mgr->bpool, maxbuffers); - isc_mempool_associatelock(mgr->bpool, &mgr->bpool_lock); - isc_mempool_setfillcount(mgr->bpool, 32); + if (maxbuffers > mgr->maxbuffers) { + mgr->maxbuffers = maxbuffers; } /* Create or adjust socket pool */ - if (mgr->spool != NULL) { - if (maxrequests < DNS_DISPATCH_POOLSOCKS * 2) { - isc_mempool_setmaxalloc(mgr->spool, - DNS_DISPATCH_POOLSOCKS * 2); - isc_mempool_setfreemax(mgr->spool, - DNS_DISPATCH_POOLSOCKS * 2); - } + if (mgr->qid != NULL) { UNLOCK(&mgr->buffer_lock); return (ISC_R_SUCCESS); } - isc_mempool_create(mgr->mctx, sizeof(dispsocket_t), &mgr->spool); - - isc_mempool_setname(mgr->spool, "dispmgr_spool"); - isc_mempool_setmaxalloc(mgr->spool, maxrequests); - isc_mempool_setfreemax(mgr->spool, maxrequests); - isc_mempool_associatelock(mgr->spool, &mgr->spool_lock); - isc_mempool_setfillcount(mgr->spool, 32); result = qid_allocate(mgr, buckets, increment, &mgr->qid, true); if (result != ISC_R_SUCCESS) { @@ -2039,10 +1897,6 @@ dns_dispatchmgr_setudp(dns_dispatchmgr_t *mgr, unsigned int buffersize, return (ISC_R_SUCCESS); cleanup: - isc_mempool_destroy(&mgr->bpool); - if (mgr->spool != NULL) { - isc_mempool_destroy(&mgr->spool); - } UNLOCK(&mgr->buffer_lock); return (result); } @@ -2308,10 +2162,8 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, unsigned int maxrequests, * the options that are controlled by tcp vs. udp, etc. */ - disp = isc_mempool_get(mgr->dpool); - if (disp == NULL) { - return (ISC_R_NOMEMORY); - } + disp = isc_mem_get(mgr->mctx, sizeof(*disp)); + isc_refcount_increment0(&mgr->irefs); disp->magic = 0; disp->mgr = mgr; @@ -2335,7 +2187,6 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, unsigned int maxrequests, ISC_LIST_INIT(disp->inactivesockets); disp->nsockets = 0; disp->port_table = NULL; - disp->portpool = NULL; disp->dscp = -1; isc_mutex_init(&disp->lock); @@ -2356,7 +2207,8 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, unsigned int maxrequests, */ kill_lock: isc_mutex_destroy(&disp->lock); - isc_mempool_put(mgr->dpool, disp); + isc_refcount_decrement(&mgr->irefs); + isc_mem_put(mgr->mctx, disp, sizeof(*disp)); return (result); } @@ -2387,7 +2239,8 @@ dispatch_free(dns_dispatch_t **dispp) { INSIST(ISC_LIST_EMPTY(disp->activesockets)); INSIST(ISC_LIST_EMPTY(disp->inactivesockets)); - isc_mempool_put(mgr->depool, disp->failsafe_ev); + isc_refcount_decrement(&mgr->irefs); + isc_mem_put(mgr->mctx, disp->failsafe_ev, sizeof(*disp->failsafe_ev)); disp->failsafe_ev = NULL; if (disp->qid != NULL) { @@ -2403,14 +2256,11 @@ dispatch_free(dns_dispatch_t **dispp) { DNS_DISPATCH_PORTTABLESIZE); } - if (disp->portpool != NULL) { - isc_mempool_destroy(&disp->portpool); - } - disp->mgr = NULL; isc_mutex_destroy(&disp->lock); disp->magic = 0; - isc_mempool_put(mgr->dpool, disp); + isc_refcount_decrement(&mgr->irefs); + isc_mem_put(mgr->mctx, disp, sizeof(*disp)); } isc_result_t @@ -2886,11 +2736,6 @@ dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, for (i = 0; i < DNS_DISPATCH_PORTTABLESIZE; i++) { ISC_LIST_INIT(disp->port_table[i]); } - - isc_mempool_create(mgr->mctx, sizeof(dispportentry_t), - &disp->portpool); - isc_mempool_setname(disp->portpool, "disp_portpool"); - isc_mempool_setfreemax(disp->portpool, 128); } disp->socket = sock; disp->local = *localaddr; @@ -2918,15 +2763,8 @@ dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, destroy_disp, disp, sizeof(isc_event_t)); disp->sepool = NULL; - isc_mempool_create(mgr->mctx, sizeof(isc_socketevent_t), &disp->sepool); - - isc_mutex_init(&disp->sepool_lock); - - isc_mempool_setname(disp->sepool, "disp_sepool"); - isc_mempool_setmaxalloc(disp->sepool, 32768); - isc_mempool_setfreemax(disp->sepool, 32768); - isc_mempool_associatelock(disp->sepool, &disp->sepool_lock); - isc_mempool_setfillcount(disp->sepool, 16); + isc_mem_create(&disp->sepool); + isc_mem_setname(disp->sepool, "disp_sepool"); attributes &= ~DNS_DISPATCHATTR_TCP; attributes |= DNS_DISPATCHATTR_UDP; @@ -3142,14 +2980,8 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, return (ISC_R_NOMORE); } - res = isc_mempool_get(disp->mgr->rpool); - if (res == NULL) { - if (dispsocket != NULL) { - destroy_dispsocket(disp, &dispsocket); - } - UNLOCK(&disp->lock); - return (ISC_R_NOMEMORY); - } + res = isc_mem_get(disp->mgr->mctx, sizeof(*res)); + isc_refcount_increment0(&disp->mgr->irefs); disp->refcount++; disp->requests++; @@ -3204,7 +3036,8 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, UNLOCK(&disp->lock); isc_task_detach(&res->task); - isc_mempool_put(disp->mgr->rpool, res); + isc_refcount_decrement(&disp->mgr->irefs); + isc_mem_put(disp->mgr->mctx, res, sizeof(*res)); return (result); } } @@ -3392,7 +3225,8 @@ dns_dispatch_removeresponse(dns_dispentry_t **resp, ev = ISC_LIST_HEAD(res->items); } res->magic = 0; - isc_mempool_put(disp->mgr->rpool, res); + isc_refcount_decrement(&disp->mgr->irefs); + isc_mem_put(disp->mgr->mctx, res, sizeof(*res)); if (disp->shutting_down == 1) { do_cancel(disp); } else { diff --git a/lib/dns/tests/dispatch_test.c b/lib/dns/tests/dispatch_test.c index 7bd0712d56..05fd4c59d2 100644 --- a/lib/dns/tests/dispatch_test.c +++ b/lib/dns/tests/dispatch_test.c @@ -275,8 +275,7 @@ dispatch_getnext(void **state) { isc_sockaddr_fromin(&local, &ina, 0); attrs = DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_UDP; result = dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, &local, - 512, 6, 1024, 17, 19, attrs, attrs, - &dispatch); + 512, 6, 1024, 17, 19, attrs, &dispatch); assert_int_equal(result, ISC_R_SUCCESS); /* diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index ec0c98ee45..f129285f90 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -406,33 +406,6 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name); *\li name != NULL; */ -void -isc_mempool_associatelock(isc_mempool_t *mpctx, isc_mutex_t *lock); -/*%< - * Associate a lock with this memory pool. - * - * This lock is used when getting or putting items using this memory - *pool, and it is also used to set or get internal state via the - *isc_mempool_get*() and isc_mempool_set*() set of functions. - * - * Multiple pools can each share a single lock. For instance, if - *"manager" type object contained pools for various sizes of events, and - *each of these pools used a common lock. Note that this lock must - *NEVER be used by other than mempool routines once it is given to a - *pool, since that can easily cause double locking. - * - * Requires: - * - *\li mpctpx is a valid pool. - * - *\li lock != NULL. - * - *\li No previous lock is assigned to this pool. - * - *\li The lock is initialized before calling this function via the - *normal means of doing that. - */ - /* * The following functions get/set various parameters. Note that due to * the unlocked nature of pools these are potentially random values diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 71e9cdfa28..9f3a6143d9 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -48,14 +48,6 @@ #define MCTXLOCK(m) LOCK(&m->lock) #define MCTXUNLOCK(m) UNLOCK(&m->lock) -#define MPCTXLOCK(mp) \ - if (mp->lock != NULL) { \ - LOCK(mp->lock); \ - } -#define MPCTXUNLOCK(mp) \ - if (mp->lock != NULL) { \ - UNLOCK(mp->lock); \ - } #ifndef ISC_MEM_DEBUGGING #define ISC_MEM_DEBUGGING 0 @@ -158,18 +150,15 @@ struct isc_mem { struct isc_mempool { /* always unlocked */ unsigned int magic; - isc_mutex_t *lock; /*%< optional lock */ - isc_mem_t *mctx; /*%< our memory context */ - /*%< locked via the memory context's lock */ + isc_mem_t *mctx; /*%< our memory context */ ISC_LINK(isc_mempool_t) link; /*%< next pool in this mem context */ - /*%< optionally locked from here down */ - element *items; /*%< low water item list */ - size_t size; /*%< size of each item on this pool */ - atomic_size_t maxalloc; /*%< max number of items allowed */ - atomic_size_t allocated; /*%< # of items currently given out */ - atomic_size_t freecount; /*%< # of items on reserved list */ - atomic_size_t freemax; /*%< # of items allowed on free list */ - atomic_size_t fillcount; /*%< # of items to fetch on each fill */ + element *items; /*%< low water item list */ + size_t size; /*%< size of each item on this pool */ + atomic_size_t maxalloc; /*%< max number of items allowed */ + atomic_size_t allocated; /*%< # of items currently given out */ + atomic_size_t freecount; /*%< # of items on reserved list */ + atomic_size_t freemax; /*%< # of items allowed on free list */ + atomic_size_t fillcount; /*%< # of items to fetch on each fill */ /*%< Stats only. */ atomic_size_t gets; /*%< # of requests to this pool */ /*%< Debugging only. */ @@ -209,6 +198,7 @@ static inline size_t increment_malloced(isc_mem_t *ctx, size_t size) { size_t malloced = atomic_fetch_add_relaxed(&ctx->malloced, size) + size; size_t maxmalloced = atomic_load_acquire(&ctx->maxmalloced); + if (malloced > maxmalloced) { atomic_compare_exchange_strong(&ctx->maxmalloced, &maxmalloced, malloced); @@ -230,7 +220,7 @@ decrement_malloced(isc_mem_t *ctx, size_t size) { */ static void add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { - debuglink_t *dl; + debuglink_t *dl = NULL; uint32_t hash; uint32_t idx; @@ -275,7 +265,7 @@ unlock: static void delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, const char *file, unsigned int line) { - debuglink_t *dl; + debuglink_t *dl = NULL; uint32_t hash; uint32_t idx; @@ -328,9 +318,7 @@ unlock: */ static inline void * mem_get(isc_mem_t *ctx, size_t size) { - char *ret; - - ret = mallocx(size, 0); + char *ret = mallocx(size, 0); if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { memset(ret, 0xbe, size); /* Mnemonic for "beef". */ @@ -421,9 +409,9 @@ isc__mem_shutdown(void) { static void mem_create(isc_mem_t **ctxp, unsigned int flags) { - REQUIRE(ctxp != NULL && *ctxp == NULL); + isc_mem_t *ctx = NULL; - isc_mem_t *ctx; + REQUIRE(ctxp != NULL && *ctxp == NULL); ctx = mallocx(sizeof(*ctx), 0); @@ -556,9 +544,11 @@ isc_mem_attach(isc_mem_t *source, isc_mem_t **targetp) { void isc__mem_detach(isc_mem_t **ctxp FLARG) { + isc_mem_t *ctx = NULL; + REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); - isc_mem_t *ctx = *ctxp; + ctx = *ctxp; *ctxp = NULL; if (isc_refcount_decrement(&ctx->references) == 1) { @@ -585,10 +575,12 @@ isc__mem_detach(isc_mem_t **ctxp FLARG) { void isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { + isc_mem_t *ctx = NULL; + REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); REQUIRE(ptr != NULL); - isc_mem_t *ctx = *ctxp; + ctx = *ctxp; *ctxp = NULL; DELETE_TRACE(ctx, ptr, size, file, line); @@ -604,6 +596,8 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { void isc__mem_destroy(isc_mem_t **ctxp FLARG) { + isc_mem_t *ctx = NULL; + /* * This routine provides legacy support for callers who use mctxs * without attaching/detaching. @@ -611,7 +605,8 @@ isc__mem_destroy(isc_mem_t **ctxp FLARG) { REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); - isc_mem_t *ctx = *ctxp; + ctx = *ctxp; + *ctxp = NULL; #if ISC_MEM_TRACKLINES if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { @@ -692,11 +687,11 @@ lo_water(isc_mem_t *ctx) { void * isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { - REQUIRE(VALID_CONTEXT(ctx)); - - void *ptr; + void *ptr = NULL; bool call_water = false; + REQUIRE(VALID_CONTEXT(ctx)); + ptr = mem_get(ctx, size); mem_getstats(ctx, size); @@ -778,9 +773,9 @@ print_active(isc_mem_t *mctx, FILE *out) { */ void isc_mem_stats(isc_mem_t *ctx, FILE *out) { - REQUIRE(VALID_CONTEXT(ctx)); + isc_mempool_t *pool = NULL; - isc_mempool_t *pool; + REQUIRE(VALID_CONTEXT(ctx)); MCTXLOCK(ctx); @@ -823,8 +818,7 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { atomic_load_relaxed(&pool->freecount), atomic_load_relaxed(&pool->freemax), atomic_load_relaxed(&pool->fillcount), - atomic_load_relaxed(&pool->gets), - (pool->lock == NULL ? "N" : "Y")); + atomic_load_relaxed(&pool->gets), "N"); pool = ISC_LIST_NEXT(pool, link); } @@ -837,11 +831,11 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { void * isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { - REQUIRE(VALID_CONTEXT(ctx)); - - void *ptr; + void *ptr = NULL; bool call_water = false; + REQUIRE(VALID_CONTEXT(ctx)); + ptr = mem_get(ctx, size); /* Recalculate the real allocated size */ @@ -861,11 +855,13 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { void * isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size FLARG) { - REQUIRE(VALID_CONTEXT(ctx)); - void *new_ptr = NULL; - if (new_size == 0) { + REQUIRE(VALID_CONTEXT(ctx)); + + if (old_ptr == NULL) { + new_ptr = isc__mem_allocate(ctx, new_size FLARG_PASS); + } else if (new_size == 0) { /* * FIXME: We should not call isc__mem_reallocate with size == 0, * this is undefined behaviour. This code is kept only for @@ -909,10 +905,12 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size FLARG) { void isc__mem_free(isc_mem_t *ctx, void *ptr FLARG) { + size_t size; + REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(ptr != NULL); - size_t size = sallocx(ptr, 0); + size = sallocx(ptr, 0); DELETE_TRACE(ctx, ptr, size, file, line); @@ -928,12 +926,12 @@ isc__mem_free(isc_mem_t *ctx, void *ptr FLARG) { char * isc__mem_strdup(isc_mem_t *mctx, const char *s FLARG) { + size_t len; + char *ns = NULL; + REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(s != NULL); - size_t len; - char *ns; - len = strlen(s) + 1; ns = isc__mem_allocate(mctx, len FLARG_PASS); @@ -947,12 +945,12 @@ isc__mem_strdup(isc_mem_t *mctx, const char *s FLARG) { char * isc__mem_strndup(isc_mem_t *mctx, const char *s, size_t size FLARG) { + size_t len; + char *ns = NULL; + REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(s != NULL); - size_t len; - char *ns; - len = strlen(s) + 1; if (len > size) { len = size; @@ -1016,12 +1014,12 @@ isc_mem_maxmalloced(isc_mem_t *ctx) { void isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg, size_t hiwater, size_t lowater) { - REQUIRE(VALID_CONTEXT(ctx)); - REQUIRE(hiwater >= lowater); - bool callwater = false; isc_mem_water_t oldwater; - void *oldwater_arg; + void *oldwater_arg = NULL; + + REQUIRE(VALID_CONTEXT(ctx)); + REQUIRE(hiwater >= lowater); MCTXLOCK(ctx); oldwater = ctx->water; @@ -1086,12 +1084,12 @@ isc_mem_getname(isc_mem_t *ctx) { void isc__mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp FLARG) { + isc_mempool_t *mpctx = NULL; + REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(size > 0U); REQUIRE(mpctxp != NULL && *mpctxp == NULL); - isc_mempool_t *mpctx; - /* * Mempools are stored as a linked list of element. */ @@ -1138,26 +1136,23 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(name != NULL); - MPCTXLOCK(mpctx); - strlcpy(mpctx->name, name, sizeof(mpctx->name)); - - MPCTXUNLOCK(mpctx); } void isc__mempool_destroy(isc_mempool_t **mpctxp FLARG) { + isc_mempool_t *mpctx = NULL; + isc_mem_t *mctx = NULL; + element *item = NULL; + REQUIRE(mpctxp != NULL); REQUIRE(VALID_MEMPOOL(*mpctxp)); - isc_mempool_t *mpctx; - isc_mem_t *mctx; - isc_mutex_t *lock; - element *item; - mpctx = *mpctxp; *mpctxp = NULL; + mctx = mpctx->mctx; + #if ISC_MEM_TRACKLINES if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { fprintf(stderr, "destroy pool %p file %s line %u mctx %p\n", @@ -1173,14 +1168,6 @@ isc__mempool_destroy(isc_mempool_t **mpctxp FLARG) { } REQUIRE(atomic_load_acquire(&mpctx->allocated) == 0); - mctx = mpctx->mctx; - - lock = mpctx->lock; - - if (lock != NULL) { - LOCK(lock); - } - /* * Return any items on the free list */ @@ -1205,19 +1192,6 @@ isc__mempool_destroy(isc_mempool_t **mpctxp FLARG) { mpctx->magic = 0; isc_mem_put(mpctx->mctx, mpctx, sizeof(isc_mempool_t)); - - if (lock != NULL) { - UNLOCK(lock); - } -} - -void -isc_mempool_associatelock(isc_mempool_t *mpctx, isc_mutex_t *lock) { - REQUIRE(VALID_MEMPOOL(mpctx)); - REQUIRE(lock != NULL); - REQUIRE(mpctx->lock == NULL); - - mpctx->lock = lock; } #if __SANITIZE_ADDRESS__ @@ -1225,8 +1199,8 @@ void * isc__mempool_get(isc_mempool_t *mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); - size_t allocated = atomic_fetch_add_release(&mpctx->allocated, 1); - size_t maxalloc = atomic_load_acquire(&mpctx->maxalloc); + allocated = atomic_fetch_add_release(&mpctx->allocated, 1); + maxalloc = atomic_load_acquire(&mpctx->maxalloc); /* * Don't let the caller go over quota. @@ -1255,11 +1229,13 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { void * isc__mempool_get(isc_mempool_t *mpctx FLARG) { element *item = NULL; + size_t allocated; + size_t maxalloc; REQUIRE(VALID_MEMPOOL(mpctx)); - size_t allocated = atomic_fetch_add_release(&mpctx->allocated, 1); - size_t maxalloc = atomic_load_acquire(&mpctx->maxalloc); + allocated = atomic_fetch_add_release(&mpctx->allocated, 1); + maxalloc = atomic_load_acquire(&mpctx->maxalloc); /* * Don't let the caller go over quota @@ -1269,7 +1245,6 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { return (NULL); } - MPCTXLOCK(mpctx); if (ISC_UNLIKELY(mpctx->items == NULL)) { isc_mem_t *mctx = mpctx->mctx; size_t fillcount = atomic_load_acquire(&mpctx->fillcount); @@ -1294,8 +1269,6 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { ADD_TRACE(mpctx->mctx, item, mpctx->size, file, line); - MPCTXUNLOCK(mpctx); - return (item); } @@ -1327,14 +1300,10 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { /* * Otherwise, attach it to our free list and bump the counter. */ - MPCTXLOCK(mpctx); - item = (element *)mem; item->next = mpctx->items; mpctx->items = item; atomic_fetch_add_relaxed(&mpctx->freecount, 1); - - MPCTXUNLOCK(mpctx); } #endif /* __SANITIZE_ADDRESS__ */ diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 68891d425e..dcc2cb9e24 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -669,12 +669,6 @@ struct isc_nm { isc_stats_t *stats; - isc_mempool_t *reqpool; - isc_mutex_t reqlock; - - isc_mempool_t *evpool; - isc_mutex_t evlock; - uint_fast32_t workers_running; atomic_uint_fast32_t workers_paused; atomic_uint_fast32_t maxudp; diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 6f2d05c74c..b2c59f4800 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -281,21 +281,6 @@ isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { atomic_init(&mgr->keepalive, 30000); atomic_init(&mgr->advertised, 30000); - isc_mutex_init(&mgr->reqlock); - isc_mempool_create(mgr->mctx, sizeof(isc__nm_uvreq_t), &mgr->reqpool); - isc_mempool_setname(mgr->reqpool, "nm_reqpool"); - isc_mempool_setfreemax(mgr->reqpool, 4096); - isc_mempool_associatelock(mgr->reqpool, &mgr->reqlock); - isc_mempool_setfillcount(mgr->reqpool, 32); - - isc_mutex_init(&mgr->evlock); - isc_mempool_create(mgr->mctx, sizeof(isc__netievent_storage_t), - &mgr->evpool); - isc_mempool_setname(mgr->evpool, "nm_evpool"); - isc_mempool_setfreemax(mgr->evpool, 4096); - isc_mempool_associatelock(mgr->evpool, &mgr->evlock); - isc_mempool_setfillcount(mgr->evpool, 32); - isc_barrier_init(&mgr->pausing, workers); isc_barrier_init(&mgr->resuming, workers); @@ -377,14 +362,14 @@ nm_destroy(isc_nm_t **mgr0) { /* Empty the async event queues */ while ((ievent = DEQUEUE_PRIORITY_NETIEVENT(worker)) != NULL) { - isc_mempool_put(mgr->evpool, ievent); + isc_mem_put(mgr->mctx, ievent, sizeof(*ievent)); } INSIST(DEQUEUE_PRIVILEGED_NETIEVENT(worker) == NULL); INSIST(DEQUEUE_TASK_NETIEVENT(worker) == NULL); while ((ievent = DEQUEUE_PRIORITY_NETIEVENT(worker)) != NULL) { - isc_mempool_put(mgr->evpool, ievent); + isc_mem_put(mgr->mctx, ievent, sizeof(*ievent)); } isc_condition_destroy(&worker->cond_prio); @@ -413,12 +398,6 @@ nm_destroy(isc_nm_t **mgr0) { isc_condition_destroy(&mgr->wkpausecond); isc_mutex_destroy(&mgr->lock); - isc_mempool_destroy(&mgr->evpool); - isc_mutex_destroy(&mgr->evlock); - - isc_mempool_destroy(&mgr->reqpool); - isc_mutex_destroy(&mgr->reqlock); - isc_mem_put(mgr->mctx, mgr->workers, mgr->nworkers * sizeof(isc__networker_t)); isc_mem_putanddetach(&mgr->mctx, mgr, sizeof(*mgr)); @@ -1038,7 +1017,8 @@ process_queue(isc__networker_t *worker, netievent_type_t type) { void * isc__nm_get_netievent(isc_nm_t *mgr, isc__netievent_type type) { - isc__netievent_storage_t *event = isc_mempool_get(mgr->evpool); + isc__netievent_storage_t *event = isc_mem_get(mgr->mctx, + sizeof(*event)); *event = (isc__netievent_storage_t){ .ni.type = type }; return (event); @@ -1046,7 +1026,7 @@ isc__nm_get_netievent(isc_nm_t *mgr, isc__netievent_type type) { void isc__nm_put_netievent(isc_nm_t *mgr, void *ievent) { - isc_mempool_put(mgr->evpool, ievent); + isc_mem_put(mgr->mctx, ievent, sizeof(isc__netievent_storage_t)); } NETIEVENT_SOCKET_DEF(tcpclose); @@ -1273,7 +1253,7 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) { isc_astack_destroy(sock->inactivehandles); while ((uvreq = isc_astack_pop(sock->inactivereqs)) != NULL) { - isc_mempool_put(sock->mgr->reqpool, uvreq); + isc_mem_put(sock->mgr->mctx, uvreq, sizeof(*uvreq)); } isc_astack_destroy(sock->inactivereqs); @@ -2428,7 +2408,7 @@ isc___nm_uvreq_get(isc_nm_t *mgr, isc_nmsocket_t *sock FLARG) { } if (req == NULL) { - req = isc_mempool_get(mgr->reqpool); + req = isc_mem_get(mgr->mctx, sizeof(*req)); } *req = (isc__nm_uvreq_t){ .magic = 0 }; @@ -2464,7 +2444,7 @@ isc___nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock FLARG) { if (!isc__nmsocket_active(sock) || !isc_astack_trypush(sock->inactivereqs, req)) { - isc_mempool_put(sock->mgr->reqpool, req); + isc_mem_put(sock->mgr->mctx, req, sizeof(*req)); } if (handle != NULL) { diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index d0838a68c4..d930971c73 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -428,68 +428,6 @@ isc_mem_benchmark(void **state) { (nthreads * ITERS * NUM_ITEMS) / (t / 1000000.0)); } -static isc_threadresult_t -mempool_thread(isc_threadarg_t arg) { - isc_mempool_t *mp = (isc_mempool_t *)arg; - void *items[NUM_ITEMS]; - - for (int i = 0; i < ITERS; i++) { - for (int j = 0; j < NUM_ITEMS; j++) { - items[j] = isc_mempool_get(mp); - } - for (int j = 0; j < NUM_ITEMS; j++) { - isc_mempool_put(mp, items[j]); - } - } - - return ((isc_threadresult_t)0); -} - -static void -isc_mempool_benchmark(void **state) { - int nthreads = ISC_MAX(ISC_MIN(isc_os_ncpus(), 32), 1); - isc_thread_t threads[32]; - isc_time_t ts1, ts2; - double t; - isc_result_t result; - isc_mempool_t *mp = NULL; - isc_mutex_t mplock; - - isc_mutex_init(&mplock); - - isc_mempool_create(test_mctx, ITEM_SIZE, &mp); - - isc_mempool_associatelock(mp, &mplock); - - isc_mempool_setfreemax(mp, 32768); - isc_mempool_setfillcount(mp, ISC_MAX(NUM_ITEMS / nthreads, 1)); - - UNUSED(state); - - result = isc_time_now(&ts1); - assert_int_equal(result, ISC_R_SUCCESS); - - for (int i = 0; i < nthreads; i++) { - isc_thread_create(mempool_thread, mp, &threads[i]); - } - for (int i = 0; i < nthreads; i++) { - isc_thread_join(threads[i], NULL); - } - - result = isc_time_now(&ts2); - assert_int_equal(result, ISC_R_SUCCESS); - - t = isc_time_microdiff(&ts2, &ts1); - - printf("[ TIME ] isc_mempool_benchmark: " - "%d isc_mempool_{get,put} calls, %f seconds, %f calls/second\n", - nthreads * ITERS * NUM_ITEMS, t / 1000000.0, - (nthreads * ITERS * NUM_ITEMS) / (t / 1000000.0)); - - isc_mempool_destroy(&mp); - isc_mutex_destroy(&mplock); -} - #endif /* __SANITIZE_THREAD */ /* @@ -509,8 +447,6 @@ main(void) { #if !defined(__SANITIZE_THREAD__) cmocka_unit_test_setup_teardown(isc_mem_benchmark, _setup, _teardown), - cmocka_unit_test_setup_teardown(isc_mempool_benchmark, _setup, - _teardown), #endif /* __SANITIZE_THREAD__ */ #if ISC_MEM_TRACKLINES cmocka_unit_test_setup_teardown(isc_mem_noflags_test, _setup, From 7cbfbc8faae0042227e19ec5892f57b6d7cca922 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 12 May 2021 23:27:15 +0200 Subject: [PATCH 10/26] Clean up the dns_dispatch_getudp API Cleanup unused parts of dns_dispatch_getudp API, remove dns_dispatch_getudp_dup() function and related code. --- bin/named/server.c | 24 +--- bin/nsupdate/nsupdate.c | 9 +- bin/tests/system/pipelined/pipequeries.c | 6 +- bin/tests/system/tkey/keycreate.c | 7 +- bin/tests/system/tkey/keydelete.c | 7 +- bin/tools/mdig.c | 8 +- lib/dns/client.c | 10 +- lib/dns/dispatch.c | 159 ++--------------------- lib/dns/include/dns/dispatch.h | 11 +- lib/dns/request.c | 9 +- lib/dns/resolver.c | 8 +- lib/dns/tests/dispatch_test.c | 2 +- lib/dns/tests/resolver_test.c | 2 +- 13 files changed, 34 insertions(+), 228 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index d444e38f20..adff7f55cc 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -1260,7 +1260,7 @@ get_view_querysource_dispatch(const cfg_obj_t **maps, int af, isc_result_t result = ISC_R_FAILURE; dns_dispatch_t *disp; isc_sockaddr_t sa; - unsigned int attrs, attrmask; + unsigned int attrs; const cfg_obj_t *obj = NULL; unsigned int maxdispatchbuffers = UDPBUFFERS; isc_dscp_t dscp = -1; @@ -1331,17 +1331,10 @@ get_view_querysource_dispatch(const cfg_obj_t **maps, int af, } } - attrmask = 0; - attrmask |= DNS_DISPATCHATTR_UDP; - attrmask |= DNS_DISPATCHATTR_TCP; - attrmask |= DNS_DISPATCHATTR_IPV4; - attrmask |= DNS_DISPATCHATTR_IPV6; - disp = NULL; - result = dns_dispatch_getudp(named_g_dispatchmgr, named_g_socketmgr, - named_g_taskmgr, &sa, 4096, - maxdispatchbuffers, 32768, 16411, 16433, - attrs, attrmask, &disp); + result = dns_dispatch_getudp( + named_g_dispatchmgr, named_g_socketmgr, named_g_taskmgr, &sa, + 4096, maxdispatchbuffers, 32768, 16411, 16433, attrs, &disp); if (result != ISC_R_SUCCESS) { isc_sockaddr_t any; char buf[ISC_SOCKADDR_FORMATSIZE]; @@ -10514,7 +10507,7 @@ named_add_reserved_dispatch(named_server_t *server, in_port_t port; char addrbuf[ISC_SOCKADDR_FORMATSIZE]; isc_result_t result; - unsigned int attrs, attrmask; + unsigned int attrs; REQUIRE(NAMED_SERVER_VALID(server)); @@ -10554,16 +10547,11 @@ named_add_reserved_dispatch(named_server_t *server, result = ISC_R_NOTIMPLEMENTED; goto cleanup; } - attrmask = 0; - attrmask |= DNS_DISPATCHATTR_UDP; - attrmask |= DNS_DISPATCHATTR_TCP; - attrmask |= DNS_DISPATCHATTR_IPV4; - attrmask |= DNS_DISPATCHATTR_IPV6; result = dns_dispatch_getudp(named_g_dispatchmgr, named_g_socketmgr, named_g_taskmgr, &dispatch->addr, 4096, UDPBUFFERS, 32768, 16411, 16433, attrs, - attrmask, &dispatch->dispatch); + &dispatch->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup; } diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index 16be26a7fa..dfe4721be7 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -804,7 +804,7 @@ static void setup_system(void) { isc_result_t result; isc_sockaddr_t bind_any, bind_any6; - unsigned int attrs, attrmask; + unsigned int attrs; isc_sockaddrlist_t *nslist; isc_logconfig_t *logconfig = NULL; irs_resconf_t *resconf = NULL; @@ -939,9 +939,6 @@ setup_system(void) { set_source_ports(dispatchmgr); - attrmask = DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_TCP; - attrmask |= DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_IPV6; - if (have_ipv6) { attrs = DNS_DISPATCHATTR_UDP; attrs |= DNS_DISPATCHATTR_MAKEQUERY; @@ -949,7 +946,7 @@ setup_system(void) { isc_sockaddr_any6(&bind_any6); result = dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, &bind_any6, PACKETSIZE, 4, 2, 3, 5, - attrs, attrmask, &dispatchv6); + attrs, &dispatchv6); check_result(result, "dns_dispatch_getudp (v6)"); } @@ -960,7 +957,7 @@ setup_system(void) { isc_sockaddr_any(&bind_any); result = dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, &bind_any, PACKETSIZE, 4, 2, 3, 5, - attrs, attrmask, &dispatchv4); + attrs, &dispatchv4); check_result(result, "dns_dispatch_getudp (v4)"); } diff --git a/bin/tests/system/pipelined/pipequeries.c b/bin/tests/system/pipelined/pipequeries.c index c96afd16e7..b4595c7eff 100644 --- a/bin/tests/system/pipelined/pipequeries.c +++ b/bin/tests/system/pipelined/pipequeries.c @@ -211,7 +211,7 @@ main(int argc, char *argv[]) { isc_timermgr_t *timermgr = NULL; isc_socketmgr_t *socketmgr = NULL; dns_dispatchmgr_t *dispatchmgr = NULL; - unsigned int attrs, attrmask; + unsigned int attrs; dns_dispatch_t *dispatchv4; dns_view_t *view; uint16_t port = PORT; @@ -284,12 +284,10 @@ main(int argc, char *argv[]) { attrs = DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_MAKEQUERY | DNS_DISPATCHATTR_IPV4; - attrmask = DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_TCP | - DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_IPV6; dispatchv4 = NULL; RUNCHECK(dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, have_src ? &srcaddr : &bind_any, 4096, 4, - 2, 3, 5, attrs, attrmask, &dispatchv4)); + 2, 3, 5, attrs, &dispatchv4)); requestmgr = NULL; RUNCHECK(dns_requestmgr_create(mctx, timermgr, socketmgr, taskmgr, dispatchmgr, dispatchv4, NULL, diff --git a/bin/tests/system/tkey/keycreate.c b/bin/tests/system/tkey/keycreate.c index b72b38056a..291c861867 100644 --- a/bin/tests/system/tkey/keycreate.c +++ b/bin/tests/system/tkey/keycreate.c @@ -198,7 +198,7 @@ main(int argc, char *argv[]) { isc_timermgr_t *timermgr = NULL; isc_socketmgr_t *socketmgr = NULL; isc_socket_t *sock = NULL; - unsigned int attrs, attrmask; + unsigned int attrs; isc_sockaddr_t bind_any; dns_dispatchmgr_t *dispatchmgr = NULL; dns_dispatch_t *dispatchv4 = NULL; @@ -245,12 +245,9 @@ main(int argc, char *argv[]) { isc_sockaddr_any(&bind_any); attrs = DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_MAKEQUERY | DNS_DISPATCHATTR_IPV4; - attrmask = DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_TCP | - DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_IPV6; dispatchv4 = NULL; RUNCHECK(dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, &bind_any, - 4096, 4, 2, 3, 5, attrs, attrmask, - &dispatchv4)); + 4096, 4, 2, 3, 5, attrs, &dispatchv4)); requestmgr = NULL; RUNCHECK(dns_requestmgr_create(mctx, timermgr, socketmgr, taskmgr, dispatchmgr, dispatchv4, NULL, diff --git a/bin/tests/system/tkey/keydelete.c b/bin/tests/system/tkey/keydelete.c index 891b9cd04f..5871e12032 100644 --- a/bin/tests/system/tkey/keydelete.c +++ b/bin/tests/system/tkey/keydelete.c @@ -142,7 +142,7 @@ main(int argc, char **argv) { isc_timermgr_t *timermgr = NULL; isc_socketmgr_t *socketmgr = NULL; isc_socket_t *sock = NULL; - unsigned int attrs, attrmask; + unsigned int attrs; isc_sockaddr_t bind_any; dns_dispatchmgr_t *dispatchmgr = NULL; dns_dispatch_t *dispatchv4 = NULL; @@ -188,12 +188,9 @@ main(int argc, char **argv) { isc_sockaddr_any(&bind_any); attrs = DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_MAKEQUERY | DNS_DISPATCHATTR_IPV4; - attrmask = DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_TCP | - DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_IPV6; dispatchv4 = NULL; RUNCHECK(dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, &bind_any, - 4096, 4, 2, 3, 5, attrs, attrmask, - &dispatchv4)); + 4096, 4, 2, 3, 5, attrs, &dispatchv4)); requestmgr = NULL; RUNCHECK(dns_requestmgr_create(mctx, timermgr, socketmgr, taskmgr, dispatchmgr, dispatchv4, NULL, diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index 0052916977..e2ca8ca0f0 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -2071,7 +2071,7 @@ main(int argc, char *argv[]) { isc_timermgr_t *timermgr = NULL; isc_socketmgr_t *socketmgr = NULL; dns_dispatchmgr_t *dispatchmgr = NULL; - unsigned int attrs, attrmask; + unsigned int attrs; dns_dispatch_t *dispatchvx = NULL; dns_view_t *view = NULL; int ns; @@ -2139,14 +2139,10 @@ main(int argc, char *argv[]) { isc_sockaddr_any6(&bind_any); attrs |= DNS_DISPATCHATTR_IPV6; } - attrmask = DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_TCP | - DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_IPV6; dispatchvx = NULL; RUNCHECK(dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, have_src ? &srcaddr : &bind_any, 4096, 100, - 100, 17, 19, attrs, attrmask, - &dispatchvx)); - + 100, 17, 19, attrs, &dispatchvx)); RUNCHECK(dns_requestmgr_create( mctx, timermgr, socketmgr, taskmgr, dispatchmgr, have_ipv4 ? dispatchvx : NULL, have_ipv6 ? dispatchvx : NULL, diff --git a/lib/dns/client.c b/lib/dns/client.c index f3eb236fb8..88244727f1 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -201,7 +201,7 @@ getudpdispatch(int family, dns_dispatchmgr_t *dispatchmgr, isc_socketmgr_t *socketmgr, isc_taskmgr_t *taskmgr, bool is_shared, dns_dispatch_t **dispp, const isc_sockaddr_t *localaddr) { - unsigned int attrs, attrmask; + unsigned int attrs; dns_dispatch_t *disp; unsigned buffersize, maxbuffers, maxrequests, buckets, increment; isc_result_t result; @@ -220,11 +220,6 @@ getudpdispatch(int family, dns_dispatchmgr_t *dispatchmgr, INSIST(0); ISC_UNREACHABLE(); } - attrmask = 0; - attrmask |= DNS_DISPATCHATTR_UDP; - attrmask |= DNS_DISPATCHATTR_TCP; - attrmask |= DNS_DISPATCHATTR_IPV4; - attrmask |= DNS_DISPATCHATTR_IPV6; if (localaddr == NULL) { isc_sockaddr_anyofpf(&anyaddr, family); @@ -240,8 +235,7 @@ getudpdispatch(int family, dns_dispatchmgr_t *dispatchmgr, disp = NULL; result = dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, localaddr, buffersize, maxbuffers, maxrequests, - buckets, increment, attrs, attrmask, - &disp); + buckets, increment, attrs, &disp); if (result == ISC_R_SUCCESS) { *dispp = disp; } diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index 168dc37fa1..177695184e 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -1991,101 +1991,6 @@ unlock: #define ATTRMATCH(_a1, _a2, _mask) (((_a1) & (_mask)) == ((_a2) & (_mask))) -static bool -local_addr_match(dns_dispatch_t *disp, const isc_sockaddr_t *addr) { - isc_sockaddr_t sockaddr; - isc_result_t result; - - REQUIRE(disp->socket != NULL); - - if (addr == NULL) { - return (true); - } - - /* - * Don't match wildcard ports unless the port is available in the - * current configuration. - */ - if (isc_sockaddr_getport(addr) == 0 && - isc_sockaddr_getport(&disp->local) == 0 && - !portavailable(disp->mgr, disp->socket, NULL)) - { - return (false); - } - - /* - * Check if we match the binding . - * Wildcard ports match/fail here. - */ - if (isc_sockaddr_equal(&disp->local, addr)) { - return (true); - } - if (isc_sockaddr_getport(addr) == 0) { - return (false); - } - - /* - * Check if we match a bound wildcard port . - */ - if (!isc_sockaddr_eqaddr(&disp->local, addr)) { - return (false); - } - result = isc_socket_getsockname(disp->socket, &sockaddr); - if (result != ISC_R_SUCCESS) { - return (false); - } - - return (isc_sockaddr_equal(&sockaddr, addr)); -} - -/* - * Requires mgr be locked. - * - * No dispatcher can be locked by this thread when calling this function. - * - * - * NOTE: - * If a matching dispatcher is found, it is locked after this function - * returns, and must be unlocked by the caller. - */ -static isc_result_t -dispatch_find(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *local, - unsigned int attributes, unsigned int mask, - dns_dispatch_t **dispp) { - dns_dispatch_t *disp; - isc_result_t result; - - /* - * Make certain that we will not match a private or exclusive dispatch. - */ - attributes &= ~(DNS_DISPATCHATTR_PRIVATE | DNS_DISPATCHATTR_EXCLUSIVE); - mask |= (DNS_DISPATCHATTR_PRIVATE | DNS_DISPATCHATTR_EXCLUSIVE); - - disp = ISC_LIST_HEAD(mgr->list); - while (disp != NULL) { - LOCK(&disp->lock); - if ((disp->shutting_down == 0) && - ATTRMATCH(disp->attributes, attributes, mask) && - local_addr_match(disp, local)) - { - break; - } - UNLOCK(&disp->lock); - disp = ISC_LIST_NEXT(disp, link); - } - - if (disp == NULL) { - result = ISC_R_NOTFOUND; - goto out; - } - - *dispp = disp; - result = ISC_R_SUCCESS; -out: - - return (result); -} - static isc_result_t qid_allocate(dns_dispatchmgr_t *mgr, unsigned int buckets, unsigned int increment, dns_qid_t **qidp, bool needsocktable) { @@ -2450,13 +2355,12 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, } isc_result_t -dns_dispatch_getudp_dup(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, - isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, - unsigned int buffersize, unsigned int maxbuffers, - unsigned int maxrequests, unsigned int buckets, - unsigned int increment, unsigned int attributes, - unsigned int mask, dns_dispatch_t **dispp, - dns_dispatch_t *dup_dispatch) { +dns_dispatch_getudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, + isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, + unsigned int buffersize, unsigned int maxbuffers, + unsigned int maxrequests, unsigned int buckets, + unsigned int increment, unsigned int attributes, + dns_dispatch_t **dispp) { isc_result_t result; dns_dispatch_t *disp = NULL; @@ -2484,46 +2388,12 @@ dns_dispatch_getudp_dup(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, goto createudp; } - /* - * See if we have a dispatcher that matches. - */ - if (dup_dispatch == NULL) { - result = dispatch_find(mgr, localaddr, attributes, mask, &disp); - if (result == ISC_R_SUCCESS) { - disp->refcount++; - - if (disp->maxrequests < maxrequests) { - disp->maxrequests = maxrequests; - } - - if ((disp->attributes & DNS_DISPATCHATTR_NOLISTEN) == - 0 && - (attributes & DNS_DISPATCHATTR_NOLISTEN) != 0) - { - disp->attributes |= DNS_DISPATCHATTR_NOLISTEN; - if (disp->recv_pending != 0) { - isc_socket_cancel(disp->socket, - disp->task[0], - ISC_SOCKCANCEL_RECV); - } - } - - UNLOCK(&disp->lock); - UNLOCK(&mgr->lock); - - *dispp = disp; - - return (ISC_R_SUCCESS); - } - } - createudp: /* * Nope, create one. */ - result = dispatch_createudp( - mgr, sockmgr, taskmgr, localaddr, maxrequests, attributes, - &disp, dup_dispatch == NULL ? NULL : dup_dispatch->socket); + result = dispatch_createudp(mgr, sockmgr, taskmgr, localaddr, + maxrequests, attributes, &disp, NULL); if (result != ISC_R_SUCCESS) { UNLOCK(&mgr->lock); @@ -2536,19 +2406,6 @@ createudp: return (ISC_R_SUCCESS); } -isc_result_t -dns_dispatch_getudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, - isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, - unsigned int buffersize, unsigned int maxbuffers, - unsigned int maxrequests, unsigned int buckets, - unsigned int increment, unsigned int attributes, - unsigned int mask, dns_dispatch_t **dispp) { - return (dns_dispatch_getudp_dup(mgr, sockmgr, taskmgr, localaddr, - buffersize, maxbuffers, maxrequests, - buckets, increment, attributes, mask, - dispp, NULL)); -} - /* * mgr should be locked. */ diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h index c82b6c6ce4..8ed9601d16 100644 --- a/lib/dns/include/dns/dispatch.h +++ b/lib/dns/include/dns/dispatch.h @@ -229,16 +229,7 @@ dns_dispatch_getudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, unsigned int buffersize, unsigned int maxbuffers, unsigned int maxrequests, unsigned int buckets, unsigned int increment, unsigned int attributes, - unsigned int mask, dns_dispatch_t **dispp); - -isc_result_t -dns_dispatch_getudp_dup(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, - isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, - unsigned int buffersize, unsigned int maxbuffers, - unsigned int maxrequests, unsigned int buckets, - unsigned int increment, unsigned int attributes, - unsigned int mask, dns_dispatch_t **dispp, - dns_dispatch_t *dup); + dns_dispatch_t **dispp); /*%< * Attach to existing dns_dispatch_t if one is found with dns_dispatchmgr_find, * otherwise create a new UDP dispatch. diff --git a/lib/dns/request.c b/lib/dns/request.c index f2c3d3d94d..1311de4027 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -593,7 +593,7 @@ static isc_result_t find_udp_dispatch(dns_requestmgr_t *requestmgr, const isc_sockaddr_t *srcaddr, const isc_sockaddr_t *destaddr, dns_dispatch_t **dispatchp) { dns_dispatch_t *disp = NULL; - unsigned int attrs, attrmask; + unsigned int attrs; if (srcaddr == NULL) { switch (isc_sockaddr_pf(destaddr)) { @@ -628,15 +628,10 @@ find_udp_dispatch(dns_requestmgr_t *requestmgr, const isc_sockaddr_t *srcaddr, default: return (ISC_R_NOTIMPLEMENTED); } - attrmask = 0; - attrmask |= DNS_DISPATCHATTR_UDP; - attrmask |= DNS_DISPATCHATTR_TCP; - attrmask |= DNS_DISPATCHATTR_IPV4; - attrmask |= DNS_DISPATCHATTR_IPV6; return (dns_dispatch_getudp(requestmgr->dispatchmgr, requestmgr->socketmgr, requestmgr->taskmgr, srcaddr, 4096, 32768, 32768, 16411, 16433, - attrs, attrmask, dispatchp)); + attrs, dispatchp)); } static isc_result_t diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 9772ae2628..847ead38a8 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2158,7 +2158,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, */ } else { if (have_addr) { - unsigned int attrs, attrmask; + unsigned int attrs; attrs = DNS_DISPATCHATTR_UDP; switch (isc_sockaddr_pf(&addr)) { case AF_INET: @@ -2173,14 +2173,10 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, result = ISC_R_NOTIMPLEMENTED; goto cleanup_query; } - attrmask = DNS_DISPATCHATTR_UDP; - attrmask |= DNS_DISPATCHATTR_TCP; - attrmask |= DNS_DISPATCHATTR_IPV4; - attrmask |= DNS_DISPATCHATTR_IPV6; result = dns_dispatch_getudp( res->dispatchmgr, res->socketmgr, res->taskmgr, &addr, 4096, 20000, 32768, 16411, 16433, attrs, - attrmask, &query->dispatch); + &query->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup_query; } diff --git a/lib/dns/tests/dispatch_test.c b/lib/dns/tests/dispatch_test.c index 05fd4c59d2..a425091bf6 100644 --- a/lib/dns/tests/dispatch_test.c +++ b/lib/dns/tests/dispatch_test.c @@ -77,7 +77,7 @@ make_dispatchset(unsigned int ndisps) { isc_sockaddr_any(&any); attrs = DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_UDP; result = dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, &any, 512, - 6, 1024, 17, 19, attrs, attrs, &disp); + 6, 1024, 17, 19, attrs, &disp); if (result != ISC_R_SUCCESS) { return (result); } diff --git a/lib/dns/tests/resolver_test.c b/lib/dns/tests/resolver_test.c index 66f7c96b83..c115c34d8c 100644 --- a/lib/dns/tests/resolver_test.c +++ b/lib/dns/tests/resolver_test.c @@ -58,7 +58,7 @@ _setup(void **state) { isc_sockaddr_any(&local); result = dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, &local, - 4096, 100, 100, 100, 500, 0, 0, &dispatch); + 4096, 100, 100, 100, 500, 0, &dispatch); assert_int_equal(result, ISC_R_SUCCESS); return (0); From 62d06a4987e44c0004b4fd1fa16193d79692fdd0 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 14 May 2021 02:35:12 -0700 Subject: [PATCH 11/26] initialize state object in test-async driver the hooks system test was failing due to a block of memory not having been zeroed after allocation. --- bin/tests/system/hooks/driver/test-async.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bin/tests/system/hooks/driver/test-async.c b/bin/tests/system/hooks/driver/test-async.c index 9ea950af46..f3deee2b83 100644 --- a/bin/tests/system/hooks/driver/test-async.c +++ b/bin/tests/system/hooks/driver/test-async.c @@ -226,9 +226,7 @@ client_state_create(const query_ctx_t *qctx, async_instance_t *inst) { isc_result_t result; state = isc_mem_get(inst->mctx, sizeof(*state)); - if (state == NULL) { - return; - } + *state = (state_t){ .async = false }; LOCK(&inst->hlock); result = isc_ht_add(inst->ht, (const unsigned char *)&qctx->client, From efb385ecdcfd3213b3bb739a3dcb9e431690e559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 13 May 2021 00:29:11 +0200 Subject: [PATCH 12/26] Clean up isc_mempool API - isc_mempool_get() can no longer fail; when there are no more objects in the pool, more are always allocated. checking for NULL return is no longer necessary. - the isc_mempool_setmaxalloc() and isc_mempool_getmaxalloc() functions are no longer used and have been removed. --- cocci/isc_mempool_get_never_fail.spatch | 41 +++++++++++++++++++ lib/dns/message.c | 11 ------ lib/isc/include/isc/mem.h | 20 +--------- lib/isc/mem.c | 52 ++++--------------------- lib/isc/tests/mem_test.c | 7 ---- 5 files changed, 49 insertions(+), 82 deletions(-) create mode 100644 cocci/isc_mempool_get_never_fail.spatch diff --git a/cocci/isc_mempool_get_never_fail.spatch b/cocci/isc_mempool_get_never_fail.spatch new file mode 100644 index 0000000000..232ff9b369 --- /dev/null +++ b/cocci/isc_mempool_get_never_fail.spatch @@ -0,0 +1,41 @@ +@@ +statement S; +expression V; +@@ + +V = isc_mempool_get(...); +- if (V == NULL) S + +@@ +type T; +statement S; +expression V; +@@ + +V = (T *)isc_mempool_get(...); +- if (V == NULL) S + +@@ +statement S; +expression V; +@@ + +if (V == NULL) V = isc_mempool_get(...); +- if (V == NULL) S + +@@ +statement S1, S2; +expression V; +@@ + +V = isc_mempool_get(...); +- if (V == NULL) S1 else { S2 } ++ S2 + +@@ +type T; +expression V, E1, E2; +@@ + +- V = (T)isc_mempool_get(E1, E2); ++ V = isc_mempool_get(E1, E2); diff --git a/lib/dns/message.c b/lib/dns/message.c index ad21bb17b9..976f3e78ef 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -1076,10 +1076,6 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, goto cleanup; } rdataset = isc_mempool_get(msg->rdspool); - if (rdataset == NULL) { - result = ISC_R_NOMEMORY; - goto cleanup; - } /* * Convert rdatalist to rdataset, and attach the latter to @@ -1516,10 +1512,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx, if (result == ISC_R_NOTFOUND) { rdataset = isc_mempool_get(msg->rdspool); - if (rdataset == NULL) { - result = ISC_R_NOMEMORY; - goto cleanup; - } free_rdataset = true; rdatalist = newrdatalist(msg); @@ -2558,9 +2550,6 @@ dns_message_gettemprdataset(dns_message_t *msg, dns_rdataset_t **item) { REQUIRE(item != NULL && *item == NULL); *item = isc_mempool_get(msg->rdspool); - if (*item == NULL) { - return (ISC_R_NOMEMORY); - } dns_rdataset_init(*item); return (ISC_R_SUCCESS); diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index f129285f90..fde46200a5 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -375,7 +375,6 @@ isc__mempool_create(isc_mem_t *mctx, size_t size, *\li mpctxp != NULL and *mpctxp == NULL * * Defaults: - *\li maxalloc = UINT_MAX *\li freemax = 1 *\li fillcount = 1 * @@ -412,9 +411,7 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name); *unless the imposed externally provided locking protocols are followed. * * Also note that the quota limits will not always take immediate - *effect. For instance, setting "maxalloc" to a number smaller than the - *currently allocated count is permitted. New allocations will be - *refused until the count drops below this threshold. + * effect. * * All functions require (in addition to other requirements): * mpctx is a valid memory pool @@ -438,21 +435,6 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx); * Returns current size of the free list. */ -unsigned int -isc_mempool_getmaxalloc(isc_mempool_t *mpctx); -/*!< - * Returns the maximum allowed number of allocations. - */ - -void -isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit); -/*%< - * Sets the maximum allowed number of allocations. - * - * Additional requirements: - *\li limit > 0 - */ - unsigned int isc_mempool_getallocated(isc_mempool_t *mpctx); /*%< diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 9f3a6143d9..52e2b998ea 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -154,7 +154,6 @@ struct isc_mempool { ISC_LINK(isc_mempool_t) link; /*%< next pool in this mem context */ element *items; /*%< low water item list */ size_t size; /*%< size of each item on this pool */ - atomic_size_t maxalloc; /*%< max number of items allowed */ atomic_size_t allocated; /*%< # of items currently given out */ atomic_size_t freecount; /*%< # of items on reserved list */ atomic_size_t freemax; /*%< # of items allowed on free list */ @@ -805,15 +804,14 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { pool = ISC_LIST_HEAD(ctx->pools); if (pool != NULL) { fprintf(out, "[Pool statistics]\n"); - fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %10s %1s\n", - "name", "size", "maxalloc", "allocated", "freecount", - "freemax", "fillcount", "gets", "L"); + fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %1s\n", "name", + "size", "allocated", "freecount", "freemax", + "fillcount", "gets", "L"); } while (pool != NULL) { fprintf(out, "%15s %10zu %10zu %10zu %10zu %10zu %10zu %10zu %s\n", - pool->name, pool->size, - atomic_load_relaxed(&pool->maxalloc), + pool->name, pool->size, (size_t)0, atomic_load_relaxed(&pool->allocated), atomic_load_relaxed(&pool->freecount), atomic_load_relaxed(&pool->freemax), @@ -1109,7 +1107,6 @@ isc__mempool_create(isc_mem_t *mctx, size_t size, .size = size, }; - atomic_init(&mpctx->maxalloc, SIZE_MAX); atomic_init(&mpctx->allocated, 0); atomic_init(&mpctx->freecount, 0); atomic_init(&mpctx->freemax, 1); @@ -1199,17 +1196,7 @@ void * isc__mempool_get(isc_mempool_t *mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); - allocated = atomic_fetch_add_release(&mpctx->allocated, 1); - maxalloc = atomic_load_acquire(&mpctx->maxalloc); - - /* - * Don't let the caller go over quota. - */ - if (ISC_UNLIKELY(allocated >= maxalloc)) { - atomic_fetch_sub_release(&mpctx->allocated, 1); - return (NULL); - } - + (void)atomic_fetch_add_relaxed(&mpctx->allocated, 1); atomic_fetch_add_relaxed(&mpctx->gets, 1); return (isc__mem_get(mpctx->mctx, mpctx->size FLARG_PASS)); @@ -1220,8 +1207,7 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(mem != NULL); - INSIST(atomic_fetch_sub_release(&mpctx->allocated, 1) > 0); - + atomic_fetch_sub_relaxed(&mpctx->allocated, 1); isc__mem_put(mpctx->mctx, mem, mpctx->size FLARG_PASS); } @@ -1234,16 +1220,7 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); - allocated = atomic_fetch_add_release(&mpctx->allocated, 1); - maxalloc = atomic_load_acquire(&mpctx->maxalloc); - - /* - * Don't let the caller go over quota - */ - if (ISC_UNLIKELY(allocated >= maxalloc)) { - atomic_fetch_sub_release(&mpctx->allocated, 1); - return (NULL); - } + (void)atomic_fetch_add_release(&mpctx->allocated, 1); if (ISC_UNLIKELY(mpctx->items == NULL)) { isc_mem_t *mctx = mpctx->mctx; @@ -1333,21 +1310,6 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx) { return (atomic_load_relaxed(&mpctx->freecount)); } -void -isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit) { - REQUIRE(VALID_MEMPOOL(mpctx)); - REQUIRE(limit > 0); - - atomic_store_release(&mpctx->maxalloc, limit); -} - -unsigned int -isc_mempool_getmaxalloc(isc_mempool_t *mpctx) { - REQUIRE(VALID_MEMPOOL(mpctx)); - - return (atomic_load_relaxed(&mpctx->maxalloc)); -} - unsigned int isc_mempool_getallocated(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index d930971c73..c9602927f9 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -82,7 +82,6 @@ isc_mem_test(void **state) { isc_mempool_setfreemax(mp1, MP1_FREEMAX); isc_mempool_setfillcount(mp1, MP1_FILLCNT); - isc_mempool_setmaxalloc(mp1, MP1_MAXALLOC); /* * Allocate MP1_MAXALLOC items from the pool. This is our max. @@ -92,12 +91,6 @@ isc_mem_test(void **state) { assert_non_null(items1[i]); } - /* - * Try to allocate one more. This should fail. - */ - tmp = isc_mempool_get(mp1); - assert_null(tmp); - /* * Free the first 11 items. Verify that there are 10 free items on * the free list (which is our max). From e75436017096d22f6b66e6cc940a9116c7a488e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 14 May 2021 15:13:33 +0200 Subject: [PATCH 13/26] Remove atomic thread synchronization from the memory hot-path This commit refactors the hi/lo-water related code to remove contention on the hot path in the memory allocator. --- lib/isc/mem.c | 81 ++++++++++++++++++++++++++++----------------------- 1 file changed, 45 insertions(+), 36 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 52e2b998ea..ed30d1aa2b 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -196,7 +196,7 @@ print_active(isc_mem_t *ctx, FILE *out); static inline size_t increment_malloced(isc_mem_t *ctx, size_t size) { size_t malloced = atomic_fetch_add_relaxed(&ctx->malloced, size) + size; - size_t maxmalloced = atomic_load_acquire(&ctx->maxmalloced); + size_t maxmalloced = atomic_load_relaxed(&ctx->maxmalloced); if (malloced > maxmalloced) { atomic_compare_exchange_strong(&ctx->maxmalloced, &maxmalloced, @@ -208,7 +208,7 @@ increment_malloced(isc_mem_t *ctx, size_t size) { static inline size_t decrement_malloced(isc_mem_t *ctx, size_t size) { - size_t malloced = atomic_fetch_sub_release(&ctx->malloced, size) - size; + size_t malloced = atomic_fetch_sub_relaxed(&ctx->malloced, size) - size; return (malloced); } @@ -637,51 +637,62 @@ isc__mem_destroy(isc_mem_t **ctxp FLARG) { static inline bool hi_water(isc_mem_t *ctx) { - bool call_water = false; - size_t inuse = atomic_load_acquire(&ctx->inuse); - size_t maxinuse = atomic_load_acquire(&ctx->maxinuse); - size_t hi_water = atomic_load_acquire(&ctx->hi_water); + size_t inuse; + size_t maxinuse; + size_t hi_water = atomic_load_relaxed(&ctx->hi_water); - if (hi_water != 0U && inuse > hi_water) { - atomic_store(&ctx->is_overmem, true); - if (!atomic_load_acquire(&ctx->hi_called)) { - call_water = true; - } + if (hi_water == 0) { + return (false); } + + inuse = atomic_load_acquire(&ctx->inuse); + if (inuse <= hi_water) { + return (false); + } + + maxinuse = atomic_load_acquire(&ctx->maxinuse); if (inuse > maxinuse) { (void)atomic_compare_exchange_strong(&ctx->maxinuse, &maxinuse, inuse); - if (hi_water != 0U && inuse > hi_water && - (isc_mem_debugging & ISC_MEM_DEBUGUSAGE) != 0) - { + if ((isc_mem_debugging & ISC_MEM_DEBUGUSAGE) != 0) { fprintf(stderr, "maxinuse = %lu\n", (unsigned long)inuse); } } - return (call_water); -} - -/* - * The check against ctx->lo_water == 0 is for the condition - * when the context was pushed over hi_water but then had - * isc_mem_setwater() called with 0 for hi_water and lo_water. - */ -static inline bool -lo_water(isc_mem_t *ctx) { - bool call_water = false; - size_t inuse = atomic_load_acquire(&ctx->inuse); - size_t lo_water = atomic_load_acquire(&ctx->lo_water); - - if ((inuse < lo_water) || (lo_water == 0U)) { - atomic_store(&ctx->is_overmem, false); - if (atomic_load_acquire(&ctx->hi_called)) { - call_water = true; - } + if (atomic_load_acquire(&ctx->hi_called)) { + return (false); } - return (call_water); + /* We are over water (for the first time) */ + atomic_store_release(&ctx->is_overmem, true); + + return (true); +} + +static inline bool +lo_water(isc_mem_t *ctx) { + size_t inuse; + size_t lo_water = atomic_load_relaxed(&ctx->lo_water); + + if (lo_water == 0) { + return (false); + } + + inuse = atomic_load_acquire(&ctx->inuse); + if (inuse >= lo_water) { + return (false); + } + + if (!atomic_load_acquire(&ctx->hi_called)) { + return (false); + } + + /* We are no longer overmem */ + atomic_store(&ctx->is_overmem, false); + + return (true); } void * @@ -1215,8 +1226,6 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { void * isc__mempool_get(isc_mempool_t *mpctx FLARG) { element *item = NULL; - size_t allocated; - size_t maxalloc; REQUIRE(VALID_MEMPOOL(mpctx)); From 2ce0de699528c8d505adfde37a916b1742e5562f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 19 May 2021 17:18:22 -0700 Subject: [PATCH 14/26] Remove error checks in dns_message for mem allocations Removed error checks for several functions that can no longer fail due to failed memory allocation. --- lib/dns/include/dns/message.h | 4 ---- lib/dns/message.c | 29 +---------------------------- 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index fb114e70ab..2d15c481c9 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -899,7 +899,6 @@ dns_message_gettempname(dns_message_t *msg, dns_name_t **item); * * Returns: *\li #ISC_R_SUCCESS -- All is well. - *\li #ISC_R_NOMEMORY -- No item can be allocated. */ isc_result_t @@ -916,7 +915,6 @@ dns_message_gettemprdata(dns_message_t *msg, dns_rdata_t **item); * * Returns: *\li #ISC_R_SUCCESS -- All is well. - *\li #ISC_R_NOMEMORY -- No item can be allocated. */ isc_result_t @@ -934,7 +932,6 @@ dns_message_gettemprdataset(dns_message_t *msg, dns_rdataset_t **item); * * Returns: *\li #ISC_R_SUCCESS -- All is well. - *\li #ISC_R_NOMEMORY -- No item can be allocated. */ isc_result_t @@ -951,7 +948,6 @@ dns_message_gettemprdatalist(dns_message_t *msg, dns_rdatalist_t **item); * * Returns: *\li #ISC_R_SUCCESS -- All is well. - *\li #ISC_R_NOMEMORY -- No item can be allocated. */ void diff --git a/lib/dns/message.c b/lib/dns/message.c index 976f3e78ef..ad36e1887b 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -295,10 +295,6 @@ newrdata(dns_message_t *msg) { if (rdata == NULL) { msgblock = msgblock_allocate(msg->mctx, sizeof(dns_rdata_t), RDATA_COUNT); - if (msgblock == NULL) { - return (NULL); - } - ISC_LIST_APPEND(msg->rdatas, msgblock, link); rdata = msgblock_get(msgblock, dns_rdata_t); @@ -329,19 +325,12 @@ newrdatalist(dns_message_t *msg) { if (rdatalist == NULL) { msgblock = msgblock_allocate(msg->mctx, sizeof(dns_rdatalist_t), RDATALIST_COUNT); - if (msgblock == NULL) { - return (NULL); - } - ISC_LIST_APPEND(msg->rdatalists, msgblock, link); rdatalist = msgblock_get(msgblock, dns_rdatalist_t); } out: - if (rdatalist != NULL) { - dns_rdatalist_init(rdatalist); - } - + dns_rdatalist_init(rdatalist); return (rdatalist); } @@ -355,10 +344,6 @@ newoffsets(dns_message_t *msg) { if (offsets == NULL) { msgblock = msgblock_allocate(msg->mctx, sizeof(dns_offsets_t), OFFSET_COUNT); - if (msgblock == NULL) { - return (NULL); - } - ISC_LIST_APPEND(msg->offsets, msgblock, link); offsets = msgblock_get(msgblock, dns_offsets_t); @@ -2523,9 +2508,6 @@ dns_message_gettempname(dns_message_t *msg, dns_name_t **item) { REQUIRE(item != NULL && *item == NULL); fn = isc_mempool_get(msg->namepool); - if (fn == NULL) { - return (ISC_R_NOMEMORY); - } *item = dns_fixedname_initname(fn); return (ISC_R_SUCCESS); @@ -2537,10 +2519,6 @@ dns_message_gettemprdata(dns_message_t *msg, dns_rdata_t **item) { REQUIRE(item != NULL && *item == NULL); *item = newrdata(msg); - if (*item == NULL) { - return (ISC_R_NOMEMORY); - } - return (ISC_R_SUCCESS); } @@ -2550,7 +2528,6 @@ dns_message_gettemprdataset(dns_message_t *msg, dns_rdataset_t **item) { REQUIRE(item != NULL && *item == NULL); *item = isc_mempool_get(msg->rdspool); - dns_rdataset_init(*item); return (ISC_R_SUCCESS); } @@ -2561,10 +2538,6 @@ dns_message_gettemprdatalist(dns_message_t *msg, dns_rdatalist_t **item) { REQUIRE(item != NULL && *item == NULL); *item = newrdatalist(msg); - if (*item == NULL) { - return (ISC_R_NOMEMORY); - } - return (ISC_R_SUCCESS); } From 6591786102dfcbef14c8b81a38f4bd2d63ac1fce Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 22 May 2021 10:40:00 -0700 Subject: [PATCH 15/26] document the dependency on jemalloc updated README and PLATFORMS with new text on build requirements. --- PLATFORMS.md | 19 ++++++++++++++----- README.md | 14 +++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/PLATFORMS.md b/PLATFORMS.md index 405871a331..6a3d052dbd 100644 --- a/PLATFORMS.md +++ b/PLATFORMS.md @@ -12,8 +12,13 @@ In general, this version of BIND will build and run on any POSIX-compliant system with a C11-compliant C compiler, BSD-style sockets with RFC-compliant -IPv6 support, POSIX-compliant threads, the `libuv` asynchronous I/O library, -the OpenSSL cryptography library, and the `nghttp2` HTTP/2 library. +IPv6 support, and POSIX-compliant threads, plus the following mandatory +libraries: + +- `libuv` for asynchronous I/O operations and event loops +- `libssl` and `libcrpyto` from OpenSSL for cryptography +- `libjemalloc` for memory allocation +- `libnghttp2` for HTTP/2 The following C11 features are used in BIND 9: @@ -31,13 +36,17 @@ some of the older systems listed below, you will have to install an updated updated packages. The other option is to build and install `libuv` from source. -Certain optional BIND features have additional library dependencies: +Certain optional BIND features have additional library dependencies. +These include: * `libfstrm` and `libprotobuf-c` for DNSTAP -* `libidn2` for internationalized domain name conversion. +* `libidn2` for display of internationalized domain names in `dig` +* `libjson-c` for JSON statistics * `libmaxminddb` for geolocation * `libnghttp2` for DNS over HTTPS -* `libxml2` and `libjson-c` for statistics channel +* `libxml2` for XML statistics +* `libz` for compression of the HTTP statistics channel +* `readline` for line editing in `nsupdate` and `nslookup` ISC regularly tests BIND on many operating systems and architectures, but lacks the resources to test all of them. Consequently, ISC is only able to diff --git a/README.md b/README.md index cce47bc687..f85910ddbb 100644 --- a/README.md +++ b/README.md @@ -125,13 +125,13 @@ including your patch as an attachment, preferably generated by At a minimum, BIND requires a Unix or Linux system with an ANSI C compiler, basic POSIX support, and a 64-bit integer type. BIND also requires the -`libuv` asynchronous I/O library, the `nghttp2` HTTP/2 library, and a -cryptography provider library such as OpenSSL or a hardware service -module supporting PKCS#11. On Linux, BIND requires the `libcap` library -to set process privileges, though this requirement can be overridden by -disabling capability support at compile time. See [Compile-time -options](#opts) below for details on other libraries that may be -required to support optional features. +`libuv` asynchronous I/O library, the `nghttp2` HTTP/2 library, the +`jemalloc` memory allocation library, and the OpenSSL cryptography +library. On Linux, BIND requires the `libcap` library to set process +privileges, though this requirement can be overridden by disabling +capability support at compile time. See [Compile-time options](#opts) +below for details on other libraries that may be required to support +optional features. Successful builds have been observed on many versions of Linux and Unix, including RHEL/CentOS, Fedora, Debian, Ubuntu, SLES, openSUSE, From 68a28cbc0aeb3a613b0f15e3d7f8484490ae89ee Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 24 May 2021 12:40:15 -0700 Subject: [PATCH 16/26] update the "memory" section of the developer doc Information about memory allocation was outdated. --- doc/dev/dev.md | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/doc/dev/dev.md b/doc/dev/dev.md index 235bc2b49e..9a765090c2 100644 --- a/doc/dev/dev.md +++ b/doc/dev/dev.md @@ -483,10 +483,12 @@ or simply by running `isc_buffer_init()` on the region's base pointer. #### Memory management -BIND manages its own memory internally via "memory contexts". Multiple +BIND tracks its memory usage internally via "memory contexts". Multiple separate memory contexts can be created for the use of different modules or subcomponents, and each can have its own size limits and tuning parameters -and maintain its own statistics, allocations and free lists. +and maintain its own statistics, allocations and free lists. Memory +allocation is based on the `jemalloc` library on platforms where the library +is available. The memory system helps with diagnosis of common coding errors such as memory leaks and use after free. Newly allocated memory is populated with @@ -499,13 +501,6 @@ To create a basic memory context, use: isc_mem_t *mctx = NULL; isc_mem_create(&mctx); -(The zeroes are tuning parameters, `max_size` and `target_size`: Any -allocations smaller than `max_size` will be satisfied by getting -blocks of size `target_size` from the operating system's memory -allocator and breaking them up into pieces, while larger allocations -will call the system allocator directly. These parameters are rarely -used.) - When holding a persistent reference to a memory context it is advisable to increment its reference counter using `isc_mem_attach()`. Do not just copy an `mctx` pointer; this may lead to a shutdown race in which the @@ -567,16 +562,14 @@ The function `isc_mem_strdup()` -- a version of `strdup()` that uses memory contexts -- will also return memory that can be freed with `isc_mem_free()`. -Every allocation and deallocation requires a memory context lock to be -acquired. This will cause performance problems if you write code that -allocates and deallocates memory frequently. Whenever possible, -inner loop functions should be passed static buffers rather than allocating -memory. - In cases where small fixed-size blocks of memory may be needed frequently, the `isc_mempool` API can be used. This creates a standing pool of blocks of a specified size which can be passed out and returned without the need -for locking the entire memory context. +for a new memory allocation; this can improve performance in tight inner +loops. + +None of these allocation functions, including `isc_mempool_get()`, can +fail. If no memory is available for allocation, the program will abort. #### Lists From e20cc41e56905fbd32c7824f7cf8bdf6a5db6d1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 25 May 2021 12:46:00 +0200 Subject: [PATCH 17/26] Use system allocator when jemalloc is unavailable This commit adds support for systems where the jemalloc library is not available as a package, here's the quick summary: * On Linux - the jemalloc is usually available as a package, if configured --without-jemalloc, the shim would be used around malloc(), free(), realloc() and malloc_usable_size() * On macOS - the jemalloc is available from homebrew or macports, if configured --without-jemalloc, the shim would be used around malloc(), free(), realloc() and malloc_size() * On FreeBSD - the jemalloc is *the* system allocator, we just need to check for header to get access to non-standard API * On NetBSD - the jemalloc is *the* system allocator, we just need to check for header to get access to non-standard API * On a system hostile to users and developers (read OpenBSD) - the jemalloc API is emulated by using ((size_t *)ptr)[-1] field to hold the size information. The OpenBSD developers care only for themselves, so why should we care about speed on OpenBSD? --- configure.ac | 38 ++++++++++--- lib/isc/Makefile.am | 1 + lib/isc/jemalloc_shim.h | 118 ++++++++++++++++++++++++++++++++++++++++ lib/isc/mem.c | 11 ++++ m4/ax_jemalloc.m4 | 53 ++++++++++++++++++ util/copyrights | 1 + 6 files changed, 214 insertions(+), 8 deletions(-) create mode 100644 lib/isc/jemalloc_shim.h create mode 100644 m4/ax_jemalloc.m4 diff --git a/configure.ac b/configure.ac index 7ada1b61ee..3aa9cc57cb 100644 --- a/configure.ac +++ b/configure.ac @@ -1377,15 +1377,26 @@ AC_SUBST([CMOCKA_LIBS]) AM_CONDITIONAL([HAVE_CMOCKA], [test "$with_cmocka" = "yes"]) # +# Compile with jemalloc (either provided as package or wired in the system on FreeBSD and NetBSD) # -# -AC_MSG_CHECKING([for jemalloc]) -PKG_CHECK_MODULES([JEMALLOC], [jemalloc >= 5], [] - [AC_MSG_WARN([Using jemalloc 5 is recommended]) - PKG_CHECK_MODULES([JEMALLOC], [jemalloc], [], - [AC_MSG_ERROR([jemalloc not found])])]) -AC_SUBST([JEMALLOC_CFLAGS]) -AC_SUBST([JEMALLOC_LIBS]) +# [pairwise: --with-jemalloc=detect, --with-jemalloc=yes, --without-jemalloc] +AC_ARG_WITH([jemalloc], + [AS_HELP_STRING([--with-jemalloc=detect],[enable jemalloc memory allocator (default is detect)])], + [],[with_jemalloc=detect]) + +AS_CASE([$with_jemalloc], + [no],[], + [yes],[AX_CHECK_JEMALLOC( + [AC_DEFINE([HAVE_JEMALLOC], [1], [Define to 1 if jemalloc is available])], + [AC_MSG_ERROR([jemalloc not found])])], + [AX_CHECK_JEMALLOC( + [AC_DEFINE([HAVE_JEMALLOC], [1], [Define to 1 if jemalloc is available]) + with_jemalloc=yes], + [AC_MSG_WARN([jemalloc not found; performance will be reduced]) + with_jemalloc=no])]) + +AS_IF([test "$with_jemalloc" = "no"], + [AC_CHECK_FUNCS([malloc_size malloc_usable_size])]) # # was --with-tuning specified? @@ -1698,6 +1709,9 @@ report() { echo "Configuration summary:" echo "-------------------------------------------------------------------------------" echo "Optional features enabled:" + if test "yes" = "$with_jemalloc"; then + echo " Memory allocator: jemalloc" + fi if test "yes" = "$enable_full_report" -o "standard" = "$with_locktype"; then echo " Mutex lock type: $with_locktype" fi @@ -1754,6 +1768,14 @@ report() { echo "-------------------------------------------------------------------------------" echo "Features disabled or unavailable on this platform:" + if test "no" = "$with_jemalloc"; then + echo " Memory allocator: system" + echo " WARNING: This is not a recommended configuration" + echo " WARNING: Using system memory allocator causes" + echo " WARNING: reduced performance and increased memory" + echo " WARNING: fragmentation. Installing jemalloc >= 4.0.0" + echo " WARNING: memory allocator is strongly recommended." + fi test "small" = "$with_tuning" || echo " Small-system tuning (--with-tuning)" test "no" = "$enable_dnstap" && \ diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index 1b5898817b..5f90634b3a 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -167,6 +167,7 @@ libisc_la_SOURCES = \ httpd.c \ interfaceiter.c \ iterated_hash.c \ + jemalloc_shim.h \ lex.c \ lib.c \ log.c \ diff --git a/lib/isc/jemalloc_shim.h b/lib/isc/jemalloc_shim.h new file mode 100644 index 0000000000..11f8099661 --- /dev/null +++ b/lib/isc/jemalloc_shim.h @@ -0,0 +1,118 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +#pragma once + +#if !defined(HAVE_JEMALLOC) + +#include + +const char *malloc_conf = NULL; + +#if defined(HAVE_MALLOC_SIZE) || defined(HAVE_MALLOC_USABLE_SIZE) + +#include + +static inline void * +mallocx(size_t size, int flags) { + UNUSED(flags); + void *__ptr = malloc(size); + REQUIRE(__ptr != NULL); + return (__ptr); +} + +static inline void +sdallocx(void *ptr, size_t size, int flags) { + UNUSED(size); + UNUSED(flags); + + free(ptr); +} + +static inline void * +rallocx(void *ptr, size_t size, int flags) { + UNUSED(flags); + REQUIRE(size != 0); + + void *__ptr = realloc(ptr, size); + REQUIRE(__ptr != NULL); + + return (__ptr); +} + +#ifdef HAVE_MALLOC_SIZE + +#include + +static inline size_t +sallocx(void *ptr, int flags) { + UNUSED(flags); + + return (malloc_size(ptr)); +} + +#elif HAVE_MALLOC_USABLE_SIZE + +#include + +static inline size_t +sallocx(void *ptr, int flags) { + UNUSED(flags); + + return (malloc_usable_size(ptr)); +} + +#endif /* HAVE_MALLOC_SIZE */ + +#else /* defined(HAVE_MALLOC_SIZE) || defined (HAVE_MALLOC_USABLE_SIZE) */ + +#include + +static inline void * +mallocx(size_t size, int flags) { + UNUSED(flags); + + size_t *__ptr = malloc(size + sizeof(size_t)); + REQUIRE(__ptr != NULL); + __ptr[0] = size; + + return (&__ptr[1]); +} + +static inline void +sdallocx(void *ptr, size_t size, int flags) { + UNUSED(size); + UNUSED(flags); + + free(&((size_t *)ptr)[-1]); +} + +static inline size_t +sallocx(void *ptr, int flags) { + UNUSED(flags); + + return (((size_t *)ptr)[-1]); +} + +static inline void * +rallocx(void *ptr, size_t size, int flags) { + UNUSED(flags); + + size_t *__ptr = realloc(&((size_t *)ptr)[-1], size); + REQUIRE(__ptr != NULL); + __ptr[0] = size; + + return (&__ptr[1]); +} + +#endif /* defined(HAVE_MALLOC_SIZE) || defined (HAVE_MALLOC_USABLE_SIZE) */ + +#endif /* !defined(HAVE_JEMALLOC) */ diff --git a/lib/isc/mem.c b/lib/isc/mem.c index ed30d1aa2b..f023737098 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -42,8 +42,19 @@ #include #endif /* HAVE_JSON_C */ +#if defined(HAVE_MALLOC_NP_H) +#include +#elif defined(HAVE_JEMALLOC) #include +#if JEMALLOC_VERSION_MAJOR < 4 +#define sdallocx(ptr, size, flags) dallocx(ptr, flags) +#endif /* JEMALLOC_VERSION_MAJOR < 4 */ + +#else +#include "jemalloc_shim.h" +#endif + #include "mem_p.h" #define MCTXLOCK(m) LOCK(&m->lock) diff --git a/m4/ax_jemalloc.m4 b/m4/ax_jemalloc.m4 new file mode 100644 index 0000000000..3d7b463568 --- /dev/null +++ b/m4/ax_jemalloc.m4 @@ -0,0 +1,53 @@ +# =========================================================================== +# https://gitlab.isc.org/isc-projects/autoconf-archive/ax_jemalloc.html +# =========================================================================== +# +# SYNOPSIS +# +# AX_CHECK_JEMALLOC([, ACTION-IF-FOUND[, ACTION-IF-NOT-FOUND]]) +# +# DESCRIPTION +# +# Test for the jemalloc library in a path +# +# LICENSE +# +# Copyright (c) 2021 Internet Systems Consortium +# +# Copying and distribution of this file, with or without modification, are +# permitted in any medium without royalty provided the copyright notice +# and this notice are preserved. This file is offered as-is, without any +# warranty. + +#serial 1 + +# +AC_DEFUN([AX_CHECK_JEMALLOC], [ + found=false + PKG_CHECK_MODULES( + [JEMALLOC], [jemalloc], + [ + found=true + ], [ + AC_CHECK_HEADERS([malloc_np.h jemalloc/jemalloc.h], + [ + save_LIBS="$LIBS" + save_LDFLAGS="$LDFLAGS" + save_CPPFLAGS="$CPPFLAGS" + AC_SEARCH_LIBS([mallocx], [jemalloc], + [ + found=true + AS_IF([test "$ac_cv_search_mallocx" != "none required"], + [JEMALLOC_LIBS="$ac_cv_search_mallocx"]) + ]) + CPPFLAGS="$save_CPPFLAGS" + LDFLAGS="$save_LDFLAGS" + LIBS="$save_LIBS" + ]) + ]) + + AS_IF([$found], [$1], [$2]) + + AC_SUBST([JEMALLOC_CFLAGS]) + AC_SUBST([JEMALLOC_LIBS]) +]) diff --git a/util/copyrights b/util/copyrights index f982fbae20..68aede72b4 100644 --- a/util/copyrights +++ b/util/copyrights @@ -1787,6 +1787,7 @@ ./lib/isc/include/pkcs11/pkcs11.h X 2019,2020,2021 ./lib/isc/interfaceiter.c C 1999,2000,2001,2002,2003,2004,2005,2007,2008,2014,2016,2017,2018,2019,2020,2021 ./lib/isc/iterated_hash.c C 2006,2008,2009,2016,2018,2019,2020,2021 +./lib/isc/jemalloc_shim.h C 2021 ./lib/isc/lex.c C 1998,1999,2000,2001,2002,2003,2004,2005,2007,2013,2014,2015,2016,2017,2018,2019,2020,2021 ./lib/isc/lib.c C 1999,2000,2001,2004,2005,2007,2009,2013,2014,2015,2016,2018,2019,2020,2021 ./lib/isc/log.c C 1999,2000,2001,2002,2003,2004,2005,2006,2007,2009,2011,2012,2013,2014,2016,2017,2018,2019,2020,2021 From 798333d456eb79011907ef19e08fc2ebe4bbd67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 10 Jun 2021 10:18:24 +0200 Subject: [PATCH 18/26] Allow size == 0 in isc_mem_{get,allocate,reallocate} Calls to jemalloc extended API with size == 0 ends up in undefined behaviour. This commit makes the isc_mem_get() and friends calls more POSIX aligned: If size is 0, either a null pointer or a unique pointer that can be successfully passed to free() shall be returned. We picked the easier route (which have been already supported in the old code) and return NULL on calls to the API where size == 0. --- lib/isc/mem.c | 57 +++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index f023737098..2b81d02385 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -589,6 +589,7 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); REQUIRE(ptr != NULL); + REQUIRE(size > 0); ctx = *ctxp; *ctxp = NULL; @@ -713,9 +714,11 @@ isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); - ptr = mem_get(ctx, size); - mem_getstats(ctx, size); + if (ISC_LIKELY(size != 0)) { + ptr = mem_get(ctx, size); + } + mem_getstats(ctx, size); ADD_TRACE(ctx, ptr, size, file, line); call_water = hi_water(ctx); @@ -730,12 +733,15 @@ isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { void isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); - REQUIRE(ptr != NULL); + REQUIRE(ISC_LIKELY(ptr != NULL && size != 0) || + ISC_UNLIKELY(ptr == NULL && size == 0)); DELETE_TRACE(ctx, ptr, size, file, line); mem_putstats(ctx, ptr, size); - mem_put(ctx, ptr, size); + if (ISC_LIKELY(ptr != NULL)) { + mem_put(ctx, ptr, size); + } CALL_LO_WATER(ctx); } @@ -856,10 +862,12 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); - ptr = mem_get(ctx, size); + if (ISC_LIKELY(size != 0)) { + ptr = mem_get(ctx, size); - /* Recalculate the real allocated size */ - size = sallocx(ptr, 0); + /* Recalculate the real allocated size */ + size = sallocx(ptr, 0); + } mem_getstats(ctx, size); ADD_TRACE(ctx, ptr, size, file, line); @@ -879,14 +887,9 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); - if (old_ptr == NULL) { + if (ISC_UNLIKELY(old_ptr == NULL)) { new_ptr = isc__mem_allocate(ctx, new_size FLARG_PASS); - } else if (new_size == 0) { - /* - * FIXME: We should not call isc__mem_reallocate with size == 0, - * this is undefined behaviour. This code is kept only for - * backwards compatibility. - */ + } else if (ISC_UNLIKELY(new_size == 0)) { isc__mem_free(ctx, old_ptr FLARG_PASS); } else { size_t old_size = sallocx(old_ptr, 0); @@ -912,9 +915,9 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size FLARG) { ADD_TRACE(ctx, new_ptr, new_size, file, line); /* - * We want to postpone the call to water in edge case where the - * realloc will exactly hit on the boundary of the water and we - * would call water twice. + * We want to postpone the call to water in edge case + * where the realloc will exactly hit on the boundary of + * the water and we would call water twice. */ CALL_LO_WATER(ctx); CALL_HI_WATER(ctx); @@ -925,17 +928,20 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size FLARG) { void isc__mem_free(isc_mem_t *ctx, void *ptr FLARG) { - size_t size; + size_t size = 0; REQUIRE(VALID_CONTEXT(ctx)); - REQUIRE(ptr != NULL); - size = sallocx(ptr, 0); + if (ISC_LIKELY(ptr != NULL)) { + size = sallocx(ptr, 0); + } DELETE_TRACE(ctx, ptr, size, file, line); mem_putstats(ctx, ptr, size); - mem_put(ctx, ptr, size); + if (ISC_LIKELY(ptr != NULL)) { + mem_put(ctx, ptr, size); + } CALL_LO_WATER(ctx); } @@ -956,9 +962,7 @@ isc__mem_strdup(isc_mem_t *mctx, const char *s FLARG) { ns = isc__mem_allocate(mctx, len FLARG_PASS); - if (ns != NULL) { - strlcpy(ns, s, len); - } + strlcpy(ns, s, len); return (ns); } @@ -970,6 +974,7 @@ isc__mem_strndup(isc_mem_t *mctx, const char *s, size_t size FLARG) { REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(s != NULL); + REQUIRE(size != 0); len = strlen(s) + 1; if (len > size) { @@ -978,9 +983,7 @@ isc__mem_strndup(isc_mem_t *mctx, const char *s, size_t size FLARG) { ns = isc__mem_allocate(mctx, len FLARG_PASS); - if (ns != NULL) { - strlcpy(ns, s, len); - } + strlcpy(ns, s, len); return (ns); } From 6f162e8aa465d98f9452c26aa0019ca635596943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 7 Jul 2021 16:05:48 +0200 Subject: [PATCH 19/26] Rewrite isc_mem water to use single atomic exchange operation This commit refactors the water mechanism in the isc_mem API to use single pointer to a water_t structure that can be swapped with atomic_exchange operation instead of having four different values (water, water_arg, hi_water, lo_water) in the flat namespace. This reduces the need for locking and prevents a race when water and water_arg could be desynchronized. --- lib/isc/mem.c | 138 ++++++++++++++++++++++++++------------------------ 1 file changed, 73 insertions(+), 65 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 2b81d02385..f5e72b0386 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -108,6 +108,13 @@ struct stats { atomic_size_t totalgets; }; +typedef struct water { + isc_mem_water_t water; + void *water_arg; + size_t hi_water; + size_t lo_water; +} water_t; + #define MEM_MAGIC ISC_MAGIC('M', 'e', 'm', 'C') #define VALID_CONTEXT(c) ISC_MAGIC_VALID(c, MEM_MAGIC) @@ -138,12 +145,9 @@ struct isc_mem { atomic_size_t maxinuse; atomic_size_t malloced; atomic_size_t maxmalloced; - atomic_size_t hi_water; - atomic_size_t lo_water; atomic_bool hi_called; atomic_bool is_overmem; - isc_mem_water_t water; - void *water_arg; + atomic_uintptr_t water; ISC_LIST(isc_mempool_t) pools; unsigned int poolcnt; @@ -439,8 +443,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { atomic_init(&ctx->maxinuse, 0); atomic_init(&ctx->malloced, sizeof(*ctx)); atomic_init(&ctx->maxmalloced, sizeof(*ctx)); - atomic_init(&ctx->hi_water, 0); - atomic_init(&ctx->lo_water, 0); + atomic_init(&ctx->water, (uintptr_t)NULL); atomic_init(&ctx->hi_called, false); atomic_init(&ctx->is_overmem, false); @@ -479,12 +482,19 @@ static void destroy(isc_mem_t *ctx) { unsigned int i; size_t malloced; + water_t *water; LOCK(&contextslock); ISC_LIST_UNLINK(contexts, ctx, link); totallost += isc_mem_inuse(ctx); UNLOCK(&contextslock); + water = (water_t *)atomic_exchange(&ctx->water, (uintptr_t)NULL); + if (water != NULL) { + sdallocx(water, sizeof(*water), 0); + decrement_malloced(ctx, sizeof(water_t)); + } + ctx->magic = 0; INSIST(ISC_LIST_EMPTY(ctx->pools)); @@ -637,28 +647,33 @@ isc__mem_destroy(isc_mem_t **ctxp FLARG) { *ctxp = NULL; } -#define CALL_HI_WATER(ctx) \ - if ((ctx->water != NULL) && hi_water(ctx)) { \ - (ctx->water)(ctx->water_arg, ISC_MEM_HIWATER); \ +#define CALL_HI_WATER(ctx) \ + { \ + water_t *water = (water_t *)atomic_load_relaxed(&ctx->water); \ + if (water != NULL && hi_water(ctx, water)) { \ + (water->water)(water->water_arg, ISC_MEM_HIWATER); \ + } \ } -#define CALL_LO_WATER(ctx) \ - if ((ctx->water != NULL) && lo_water(ctx)) { \ - (ctx->water)(ctx->water_arg, ISC_MEM_LOWATER); \ +#define CALL_LO_WATER(ctx) \ + { \ + water_t *water = (water_t *)atomic_load_relaxed(&ctx->water); \ + if ((water != NULL) && lo_water(ctx, water)) { \ + (water->water)(water->water_arg, ISC_MEM_LOWATER); \ + } \ } static inline bool -hi_water(isc_mem_t *ctx) { +hi_water(isc_mem_t *ctx, water_t *water) { size_t inuse; size_t maxinuse; - size_t hi_water = atomic_load_relaxed(&ctx->hi_water); - if (hi_water == 0) { + if (water->hi_water == 0) { return (false); } inuse = atomic_load_acquire(&ctx->inuse); - if (inuse <= hi_water) { + if (inuse <= water->hi_water) { return (false); } @@ -684,16 +699,15 @@ hi_water(isc_mem_t *ctx) { } static inline bool -lo_water(isc_mem_t *ctx) { +lo_water(isc_mem_t *ctx, water_t *water) { size_t inuse; - size_t lo_water = atomic_load_relaxed(&ctx->lo_water); - if (lo_water == 0) { + if (water->lo_water == 0) { return (false); } inuse = atomic_load_acquire(&ctx->inuse); - if (inuse >= lo_water) { + if (inuse >= water->lo_water) { return (false); } @@ -702,7 +716,7 @@ lo_water(isc_mem_t *ctx) { } /* We are no longer overmem */ - atomic_store(&ctx->is_overmem, false); + atomic_store_release(&ctx->is_overmem, false); return (true); } @@ -710,7 +724,6 @@ lo_water(isc_mem_t *ctx) { void * isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { void *ptr = NULL; - bool call_water = false; REQUIRE(VALID_CONTEXT(ctx)); @@ -721,11 +734,7 @@ isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { mem_getstats(ctx, size); ADD_TRACE(ctx, ptr, size, file, line); - call_water = hi_water(ctx); - - if (call_water && (ctx->water != NULL)) { - (ctx->water)(ctx->water_arg, ISC_MEM_HIWATER); - } + CALL_HI_WATER(ctx); return (ptr); } @@ -858,7 +867,6 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { void * isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { void *ptr = NULL; - bool call_water = false; REQUIRE(VALID_CONTEXT(ctx)); @@ -872,11 +880,7 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { mem_getstats(ctx, size); ADD_TRACE(ctx, ptr, size, file, line); - call_water = hi_water(ctx); - - if (call_water && (ctx->water != NULL)) { - (ctx->water)(ctx->water_arg, ISC_MEM_HIWATER); - } + CALL_HI_WATER(ctx); return (ptr); } @@ -1037,40 +1041,40 @@ isc_mem_maxmalloced(isc_mem_t *ctx) { void isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg, size_t hiwater, size_t lowater) { - bool callwater = false; - isc_mem_water_t oldwater; - void *oldwater_arg = NULL; + water_t *oldwater; + water_t *newwater = NULL; REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(hiwater >= lowater); - MCTXLOCK(ctx); - oldwater = ctx->water; - oldwater_arg = ctx->water_arg; - if (water == NULL) { - callwater = atomic_load_acquire(&ctx->hi_called); - ctx->water = NULL; - ctx->water_arg = NULL; - atomic_store_release(&ctx->hi_water, 0); - atomic_store_release(&ctx->lo_water, 0); - } else { - if (atomic_load_acquire(&ctx->hi_called) && - (ctx->water != water || ctx->water_arg != water_arg || - atomic_load_acquire(&ctx->inuse) < lowater || - lowater == 0U)) - { - callwater = true; - } - ctx->water = water; - ctx->water_arg = water_arg; - atomic_store_release(&ctx->hi_water, hiwater); - atomic_store_release(&ctx->lo_water, lowater); - } - MCTXUNLOCK(ctx); + if (water != NULL) { + newwater = mallocx(sizeof(*newwater), 0); + increment_malloced(ctx, sizeof(*newwater)); - if (callwater && oldwater != NULL) { - (oldwater)(oldwater_arg, ISC_MEM_LOWATER); + *newwater = (water_t){ + .water = water, + .water_arg = water_arg, + .hi_water = hiwater, + .lo_water = lowater, + }; } + oldwater = (water_t *)atomic_exchange(&ctx->water, (uintptr_t)newwater); + + if (oldwater == NULL) { + return; + } + + INSIST(oldwater->water != NULL); + + if (atomic_load_acquire(&ctx->hi_called) && + (oldwater->water != water || oldwater->water_arg != water_arg || + atomic_load_acquire(&ctx->inuse) < lowater || lowater == 0U)) + { + (oldwater->water)(oldwater->water_arg, ISC_MEM_LOWATER); + } + + decrement_malloced(ctx, sizeof(*oldwater)); + sdallocx(oldwater, sizeof(*oldwater), 0); } bool @@ -1426,6 +1430,7 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { REQUIRE(VALID_CONTEXT(ctx)); int xmlrc; + water_t *water; MCTXLOCK(ctx); @@ -1489,15 +1494,16 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { summary->contextsize += ctx->poolcnt * sizeof(isc_mempool_t); TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "hiwater")); + water = (water_t *)atomic_load_relaxed(&ctx->water); TRY0(xmlTextWriterWriteFormatString( writer, "%" PRIu64 "", - (uint64_t)atomic_load_relaxed(&ctx->hi_water))); + (water != NULL) ? (uint64_t)water->hi_water : 0)); TRY0(xmlTextWriterEndElement(writer)); /* hiwater */ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "lowater")); TRY0(xmlTextWriterWriteFormatString( writer, "%" PRIu64 "", - (uint64_t)atomic_load_relaxed(&ctx->lo_water))); + (water != NULL) ? (uint64_t)water->lo_water : 0)); TRY0(xmlTextWriterEndElement(writer)); /* lowater */ TRY0(xmlTextWriterEndElement(writer)); /* context */ @@ -1576,6 +1582,7 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { json_object *ctxobj, *obj; char buf[1024]; + water_t *water; MCTXLOCK(ctx); @@ -1635,11 +1642,12 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { summary->contextsize += ctx->poolcnt * sizeof(isc_mempool_t); - obj = json_object_new_int64(atomic_load_relaxed(&ctx->hi_water)); + water = (water_t *)atomic_load_relaxed(&ctx->water); + obj = json_object_new_int64((water != NULL) ? water->hi_water : 0); CHECKMEM(obj); json_object_object_add(ctxobj, "hiwater", obj); - obj = json_object_new_int64(atomic_load_relaxed(&ctx->lo_water)); + obj = json_object_new_int64((water != NULL) ? water->lo_water : 0); CHECKMEM(obj); json_object_object_add(ctxobj, "lowater", obj); From d3676a1fc5c90f0500e2bc0445fc92b190a18189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 9 Jul 2021 11:19:17 +0200 Subject: [PATCH 20/26] Disable jemalloc on softhsm2.4 branch It was discovered that softhsm2.4 has a bug that causes invalid free() call to be called when unloading libsofthsm.so.2 library. The native PKCS#11 API is scheduled to removed in the 9.17+ release, we could safely just disable jemalloc for this particular build. --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e328317ce6..ea7bd423f2 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -998,7 +998,7 @@ gcc:softhsm2.4: variables: CC: gcc CFLAGS: "${CFLAGS_COMMON}" - EXTRA_CONFIGURE: "--enable-native-pkcs11 --with-pkcs11=/usr/lib/softhsm/libsofthsm2.so" + EXTRA_CONFIGURE: "--without-jemalloc --enable-native-pkcs11 --with-pkcs11=/usr/lib/softhsm/libsofthsm2.so" <<: *base_image <<: *build_job From 63b06571b99ef89831d5612c029df82bea52a049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 9 Jul 2021 11:44:44 +0200 Subject: [PATCH 21/26] Use isc_mem_get() and isc_mem_put() in isc_mem_total test Previously, the isc_mem_allocate() and isc_mem_free() would be used for isc_mem_total test, but since we now use the real allocation size (sallocx, malloc_size, malloc_usable_size) to track the allocation size, it's impossible to get the test value right. Changing the test to use isc_mem_get() and isc_mem_put() will use the exact size provided, so the test would work again on all the platforms even when jemalloc is not being used. --- lib/isc/tests/mem_test.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index c9602927f9..e848200e2a 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -167,8 +167,8 @@ isc_mem_total_test(void **state) { for (i = 0; i < 100000; i++) { void *ptr; - ptr = isc_mem_allocate(mctx2, 2048); - isc_mem_free(mctx2, ptr); + ptr = isc_mem_get(mctx2, 2048); + isc_mem_put(mctx2, ptr, 2048); } after = isc_mem_total(mctx2); @@ -183,8 +183,8 @@ isc_mem_total_test(void **state) { for (i = 0; i < 100000; i++) { void *ptr; - ptr = isc_mem_allocate(test_mctx, 2048); - isc_mem_free(test_mctx, ptr); + ptr = isc_mem_get(test_mctx, 2048); + isc_mem_put(test_mctx, ptr, 2048); } after = isc_mem_total(test_mctx); From c11a401add14d58eb520da15551ba246d3ff2307 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Fri, 9 Jul 2021 14:01:22 +0300 Subject: [PATCH 22/26] Do not use atomic variables in isc_mempool_t As now mempool objects intended to be used in a thread-local manner, there is no point in using atomic here. --- lib/isc/mem.c | 78 +++++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index f5e72b0386..2e7a4126cd 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -169,14 +169,14 @@ struct isc_mempool { ISC_LINK(isc_mempool_t) link; /*%< next pool in this mem context */ element *items; /*%< low water item list */ size_t size; /*%< size of each item on this pool */ - atomic_size_t allocated; /*%< # of items currently given out */ - atomic_size_t freecount; /*%< # of items on reserved list */ - atomic_size_t freemax; /*%< # of items allowed on free list */ - atomic_size_t fillcount; /*%< # of items to fetch on each fill */ + size_t allocated; /*%< # of items currently given out */ + size_t freecount; /*%< # of items on reserved list */ + size_t freemax; /*%< # of items allowed on free list */ + size_t fillcount; /*%< # of items to fetch on each fill */ /*%< Stats only. */ - atomic_size_t gets; /*%< # of requests to this pool */ - /*%< Debugging only. */ - char name[16]; /*%< printed name in stats reports */ + size_t gets; /*%< # of requests to this pool */ + /*%< Debugging only. */ + char name[16]; /*%< printed name in stats reports */ }; /* @@ -848,12 +848,9 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { while (pool != NULL) { fprintf(out, "%15s %10zu %10zu %10zu %10zu %10zu %10zu %10zu %s\n", - pool->name, pool->size, (size_t)0, - atomic_load_relaxed(&pool->allocated), - atomic_load_relaxed(&pool->freecount), - atomic_load_relaxed(&pool->freemax), - atomic_load_relaxed(&pool->fillcount), - atomic_load_relaxed(&pool->gets), "N"); + pool->name, pool->size, (size_t)0, pool->allocated, + pool->freecount, pool->freemax, pool->fillcount, + pool->gets, "N"); pool = ISC_LIST_NEXT(pool, link); } @@ -1134,14 +1131,10 @@ isc__mempool_create(isc_mem_t *mctx, size_t size, .magic = MEMPOOL_MAGIC, .mctx = mctx, .size = size, + .freemax = 1, + .fillcount = 1, }; - atomic_init(&mpctx->allocated, 0); - atomic_init(&mpctx->freecount, 0); - atomic_init(&mpctx->freemax, 1); - atomic_init(&mpctx->fillcount, 1); - atomic_init(&mpctx->gets, 0); - #if ISC_MEM_TRACKLINES if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { fprintf(stderr, "create pool %p file %s line %u mctx %p\n", @@ -1186,19 +1179,20 @@ isc__mempool_destroy(isc_mempool_t **mpctxp FLARG) { } #endif - if (atomic_load_acquire(&mpctx->allocated) > 0) { + if (mpctx->allocated > 0) { UNEXPECTED_ERROR(__FILE__, __LINE__, "isc_mempool_destroy(): mempool %s " "leaked memory", mpctx->name); } - REQUIRE(atomic_load_acquire(&mpctx->allocated) == 0); + REQUIRE(mpctx->allocated == 0); /* * Return any items on the free list */ while (mpctx->items != NULL) { - INSIST(atomic_fetch_sub_release(&mpctx->freecount, 1) > 0); + INSIST(mpctx->freecount > 0); + mpctx->freecount--; item = mpctx->items; mpctx->items = item->next; @@ -1225,8 +1219,8 @@ void * isc__mempool_get(isc_mempool_t *mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); - (void)atomic_fetch_add_relaxed(&mpctx->allocated, 1); - atomic_fetch_add_relaxed(&mpctx->gets, 1); + mpctx->allocated++; + mpctx->gets++; return (isc__mem_get(mpctx->mctx, mpctx->size FLARG_PASS)); } @@ -1236,7 +1230,7 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(mem != NULL); - atomic_fetch_sub_relaxed(&mpctx->allocated, 1); + mpctx->allocated--; isc__mem_put(mpctx->mctx, mem, mpctx->size FLARG_PASS); } @@ -1247,11 +1241,11 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); - (void)atomic_fetch_add_release(&mpctx->allocated, 1); + mpctx->allocated++; if (ISC_UNLIKELY(mpctx->items == NULL)) { isc_mem_t *mctx = mpctx->mctx; - size_t fillcount = atomic_load_acquire(&mpctx->fillcount); + const size_t fillcount = mpctx->fillcount; /* * We need to dip into the well. Lock the memory * context here and fill up our free list. @@ -1261,15 +1255,18 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { mem_getstats(mctx, mpctx->size); item->next = mpctx->items; mpctx->items = item; - atomic_fetch_add_relaxed(&mpctx->freecount, 1); + mpctx->freecount++; } } item = mpctx->items; + INSIST(item != NULL); + mpctx->items = item->next; - INSIST(atomic_fetch_sub_release(&mpctx->freecount, 1) > 0); - atomic_fetch_add_relaxed(&mpctx->gets, 1); + INSIST(mpctx->freecount > 0); + mpctx->freecount--; + mpctx->gets++; ADD_TRACE(mpctx->mctx, item, mpctx->size, file, line); @@ -1285,10 +1282,11 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { REQUIRE(mem != NULL); isc_mem_t *mctx = mpctx->mctx; - size_t freecount = atomic_load_acquire(&mpctx->freecount); - size_t freemax = atomic_load_acquire(&mpctx->freemax); + const size_t freecount = mpctx->freecount; + const size_t freemax = mpctx->freemax; - INSIST(atomic_fetch_sub_release(&mpctx->allocated, 1) > 0); + INSIST(mpctx->allocated > 0); + mpctx->allocated--; DELETE_TRACE(mctx, mem, mpctx->size, file, line); @@ -1307,7 +1305,7 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { item = (element *)mem; item->next = mpctx->items; mpctx->items = item; - atomic_fetch_add_relaxed(&mpctx->freecount, 1); + mpctx->freecount++; } #endif /* __SANITIZE_ADDRESS__ */ @@ -1320,28 +1318,28 @@ void isc_mempool_setfreemax(isc_mempool_t *mpctx, unsigned int limit) { REQUIRE(VALID_MEMPOOL(mpctx)); - atomic_store_release(&mpctx->freemax, limit); + mpctx->freemax = limit; } unsigned int isc_mempool_getfreemax(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - return (atomic_load_acquire(&mpctx->freemax)); + return (mpctx->freemax); } unsigned int isc_mempool_getfreecount(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - return (atomic_load_relaxed(&mpctx->freecount)); + return (mpctx->freecount); } unsigned int isc_mempool_getallocated(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - return (atomic_load_relaxed(&mpctx->allocated)); + return (mpctx->allocated); } void @@ -1349,14 +1347,14 @@ isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(limit > 0); - atomic_store_release(&mpctx->fillcount, limit); + mpctx->fillcount = limit; } unsigned int isc_mempool_getfillcount(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - return (atomic_load_relaxed(&mpctx->fillcount)); + return (mpctx->fillcount); } /* From 3673abc53c22ba8fee59b51d830322f04852c6dd Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Fri, 9 Jul 2021 14:24:33 +0300 Subject: [PATCH 23/26] Use restrict and const in isc_mempool_t This commit makes add restrict and const modifiers to some variables to aid compiler to do its optimizations. --- lib/isc/include/isc/mem.h | 19 +++++++++--------- lib/isc/mem.c | 41 +++++++++++++++++++++------------------ 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index fde46200a5..bd72f86549 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -364,7 +364,7 @@ isc_mem_renderjson(void *memobj0); #define isc_mempool_create(c, s, mp) \ isc__mempool_create((c), (s), (mp)_ISC_MEM_FILELINE) void -isc__mempool_create(isc_mem_t *mctx, size_t size, +isc__mempool_create(isc_mem_t *restrict mctx, const size_t element_size, isc_mempool_t **mpctxp _ISC_MEM_FLARG); /*%< * Create a memory pool. @@ -385,7 +385,7 @@ isc__mempool_create(isc_mem_t *mctx, size_t size, #define isc_mempool_destroy(mp) isc__mempool_destroy((mp)_ISC_MEM_FILELINE) void -isc__mempool_destroy(isc_mempool_t **mpctxp _ISC_MEM_FLARG); +isc__mempool_destroy(isc_mempool_t **restrict mpctxp _ISC_MEM_FLARG); /*%< * Destroy a memory pool. * @@ -395,7 +395,7 @@ isc__mempool_destroy(isc_mempool_t **mpctxp _ISC_MEM_FLARG); */ void -isc_mempool_setname(isc_mempool_t *mpctx, const char *name); +isc_mempool_setname(isc_mempool_t *restrict mpctx, const char *name); /*%< * Associate a name with a memory pool. At most 15 characters may be *used. @@ -418,38 +418,39 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name); */ unsigned int -isc_mempool_getfreemax(isc_mempool_t *mpctx); +isc_mempool_getfreemax(isc_mempool_t *restrict mpctx); /*%< * Returns the maximum allowed size of the free list. */ void -isc_mempool_setfreemax(isc_mempool_t *mpctx, unsigned int limit); +isc_mempool_setfreemax(isc_mempool_t *restrict mpctx, const unsigned int limit); /*%< * Sets the maximum allowed size of the free list. */ unsigned int -isc_mempool_getfreecount(isc_mempool_t *mpctx); +isc_mempool_getfreecount(isc_mempool_t *restrict mpctx); /*%< * Returns current size of the free list. */ unsigned int -isc_mempool_getallocated(isc_mempool_t *mpctx); +isc_mempool_getallocated(isc_mempool_t *restrict mpctx); /*%< * Returns the number of items allocated from this pool. */ unsigned int -isc_mempool_getfillcount(isc_mempool_t *mpctx); +isc_mempool_getfillcount(isc_mempool_t *restrict mpctx); /*%< * Returns the number of items allocated as a block from the parent * memory context when the free list is empty. */ void -isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit); +isc_mempool_setfillcount(isc_mempool_t *restrict mpctx, + const unsigned int limit); /*%< * Sets the fillcount. * diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 2e7a4126cd..4e92e54c97 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -1106,9 +1106,10 @@ isc_mem_getname(isc_mem_t *ctx) { */ void -isc__mempool_create(isc_mem_t *mctx, size_t size, - isc_mempool_t **mpctxp FLARG) { - isc_mempool_t *mpctx = NULL; +isc__mempool_create(isc_mem_t *restrict mctx, const size_t element_size, + isc_mempool_t **restrict mpctxp FLARG) { + isc_mempool_t *restrict mpctx = NULL; + size_t size = element_size; REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(size > 0U); @@ -1151,7 +1152,7 @@ isc__mempool_create(isc_mem_t *mctx, size_t size, } void -isc_mempool_setname(isc_mempool_t *mpctx, const char *name) { +isc_mempool_setname(isc_mempool_t *restrict mpctx, const char *name) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(name != NULL); @@ -1159,10 +1160,10 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name) { } void -isc__mempool_destroy(isc_mempool_t **mpctxp FLARG) { - isc_mempool_t *mpctx = NULL; +isc__mempool_destroy(isc_mempool_t **restrict mpctxp FLARG) { + isc_mempool_t *restrict mpctx = NULL; isc_mem_t *mctx = NULL; - element *item = NULL; + element *restrict item = NULL; REQUIRE(mpctxp != NULL); REQUIRE(VALID_MEMPOOL(*mpctxp)); @@ -1216,7 +1217,7 @@ isc__mempool_destroy(isc_mempool_t **mpctxp FLARG) { #if __SANITIZE_ADDRESS__ void * -isc__mempool_get(isc_mempool_t *mpctx FLARG) { +isc__mempool_get(isc_mempool_t *restrict mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); mpctx->allocated++; @@ -1226,7 +1227,7 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { } void -isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { +isc__mempool_put(isc_mempool_t *restrict mpctx, void *mem FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(mem != NULL); @@ -1236,8 +1237,8 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { #else /* __SANITIZE_ADDRESS__ */ void * -isc__mempool_get(isc_mempool_t *mpctx FLARG) { - element *item = NULL; +isc__mempool_get(isc_mempool_t *restrict mpctx FLARG) { + element *restrict item = NULL; REQUIRE(VALID_MEMPOOL(mpctx)); @@ -1275,8 +1276,8 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { /* coverity[+free : arg-1] */ void -isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { - element *item = NULL; +isc__mempool_put(isc_mempool_t *restrict mpctx, void *mem FLARG) { + element *restrict item = NULL; REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(mem != NULL); @@ -1315,35 +1316,37 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { */ void -isc_mempool_setfreemax(isc_mempool_t *mpctx, unsigned int limit) { +isc_mempool_setfreemax(isc_mempool_t *restrict mpctx, + const unsigned int limit) { REQUIRE(VALID_MEMPOOL(mpctx)); mpctx->freemax = limit; } unsigned int -isc_mempool_getfreemax(isc_mempool_t *mpctx) { +isc_mempool_getfreemax(isc_mempool_t *restrict mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); return (mpctx->freemax); } unsigned int -isc_mempool_getfreecount(isc_mempool_t *mpctx) { +isc_mempool_getfreecount(isc_mempool_t *restrict mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); return (mpctx->freecount); } unsigned int -isc_mempool_getallocated(isc_mempool_t *mpctx) { +isc_mempool_getallocated(isc_mempool_t *restrict mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); return (mpctx->allocated); } void -isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit) { +isc_mempool_setfillcount(isc_mempool_t *restrict mpctx, + unsigned int const limit) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(limit > 0); @@ -1351,7 +1354,7 @@ isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit) { } unsigned int -isc_mempool_getfillcount(isc_mempool_t *mpctx) { +isc_mempool_getfillcount(isc_mempool_t *restrict mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); return (mpctx->fillcount); From ca228ec3e5b4aeff1a097d375a988100583edf9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 9 Jul 2021 13:30:43 +0200 Subject: [PATCH 24/26] Disable jemalloc for Address and Thread Sanitizers The Address and Thread Sanitizers both intercept the malloc calls and using the extended jemalloc API interferes with that. This commit disables the use of jemalloc for both ASAN and TSAN enabled builds to eliminate both false positives and false negatives. --- .gitlab-ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ea7bd423f2..b107e9ff2c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -864,7 +864,7 @@ gcc:asan: CC: gcc CFLAGS: "${CFLAGS_COMMON} -fsanitize=address,undefined" LDFLAGS: "-fsanitize=address,undefined" - EXTRA_CONFIGURE: "--with-libidn2" + EXTRA_CONFIGURE: "--with-libidn2 --without-jemalloc" <<: *base_image <<: *build_job @@ -891,7 +891,7 @@ clang:asan: CC: ${CLANG} CFLAGS: "${CFLAGS_COMMON} -fsanitize=address,undefined" LDFLAGS: "-fsanitize=address,undefined" - EXTRA_CONFIGURE: "--with-libidn2" + EXTRA_CONFIGURE: "--with-libidn2 --without-jemalloc" <<: *base_image <<: *build_job @@ -922,7 +922,7 @@ gcc:tsan: CC: gcc CFLAGS: "${CFLAGS_COMMON} -fsanitize=thread" LDFLAGS: "-fsanitize=thread" - EXTRA_CONFIGURE: "--with-libidn2 --enable-pthread-rwlock" + EXTRA_CONFIGURE: "--with-libidn2 --enable-pthread-rwlock --without-jemalloc" system:gcc:tsan: variables: @@ -949,7 +949,7 @@ clang:tsan: CC: "${CLANG}" CFLAGS: "${CFLAGS_COMMON} -fsanitize=thread" LDFLAGS: "-fsanitize=thread" - EXTRA_CONFIGURE: "--with-libidn2 --enable-pthread-rwlock" + EXTRA_CONFIGURE: "--with-libidn2 --enable-pthread-rwlock --without-jemalloc" system:clang:tsan: variables: From 9c3bebc26f943566a685d343fcc32a0402c11f9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 9 Jul 2021 14:35:00 +0200 Subject: [PATCH 25/26] Properly disable the "water" in isc_mem The proper way how to disable the water limit in the isc_mem context is to call: isc_mem_setwater(ctx, NULL, NULL, 0, 0); this ensures that the old water callback is called with ISC_MEM_LOWATER if the callback was called with ISC_MEM_HIWATER before. Historically, there were some places where the limits were disabled by calling: isc_mem_setwater(ctx, water, water_arg, 0, 0); which would also call the old callback, but it also causes the water_t to be allocated and extra check to be executed because water callback is not NULL. This commits unifies the calls to disable water to the preferred form. --- lib/dns/adb.c | 4 ++-- lib/dns/cache.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dns/adb.c b/lib/dns/adb.c index defb75c44c..6b1bddad63 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -2869,7 +2869,7 @@ dns_adb_shutdown(dns_adb_t *adb) { if (!adb->shutting_down) { adb->shutting_down = true; - isc_mem_setwater(adb->mctx, water, adb, 0, 0); + isc_mem_setwater(adb->mctx, NULL, NULL, 0, 0); /* * Isolate shutdown_names and shutdown_entries calls. */ @@ -4699,7 +4699,7 @@ dns_adb_setadbsize(dns_adb_t *adb, size_t size) { lowater = size - (size >> 2); /* Approximately 3/4ths. */ if (size == 0U || hiwater == 0U || lowater == 0U) { - isc_mem_setwater(adb->mctx, water, adb, 0, 0); + isc_mem_setwater(adb->mctx, NULL, NULL, 0, 0); } else { isc_mem_setwater(adb->mctx, water, adb, hiwater, lowater); } diff --git a/lib/dns/cache.c b/lib/dns/cache.c index 9935275d8f..bf73300c05 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -950,7 +950,7 @@ dns_cache_setcachesize(dns_cache_t *cache, size_t size) { /* * Disable cache memory limiting. */ - isc_mem_setwater(cache->mctx, water, cache, 0, 0); + isc_mem_setwater(cache->mctx, NULL, NULL, 0, 0); } else { /* * Establish new cache memory limits (either for the first From d40d1fd5909b44e70254874b7913eaf110faf7ea Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Sat, 22 May 2021 10:43:20 -0700 Subject: [PATCH 26/26] Add CHANGES and release notes for [GL #2433] --- CHANGES | 5 +++++ doc/notes/notes-current.rst | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/CHANGES b/CHANGES index 319af7b76a..594c850699 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +5676. [func] Memory allocation has been substantially refactored, + and is now based on the memory allocation API + provided by 'libjemalloc'. This is now a build + dependency for BIND. [GL #2433] + 5675. [bug] Improve BIND's compatibility with DoH clients by ignoring an "Accept" HTTP header value. [GL !5246] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 0ef55db54c..58b24f4ffb 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -51,6 +51,11 @@ Feature Changes configuration option ``--disable-doh``. This allows BIND 9 to be compiled without libnghttp2 library. [GL #2478] +- Memory allocation has been substantially refactored, and is now based on + the memory allocation API provided by the `jemalloc` library on platforms + where it is available. This library is now recommended for building BIND 9. + :gl:`#2433` + Bug Fixes ~~~~~~~~~