From 55ace5d3aa4cb8c1b70a94679e1dd6d716dfd8bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 4 Feb 2021 20:11:20 +0100 Subject: [PATCH 01/13] Remove the internal memory allocator The internal memory allocator had an extra code to keep a list of blocks for small size allocation. This would help to reduce the interactions with the system malloc as the memory would be already allocated from the system, but there's an extra cost associated with that - all the allocations/deallocations must be locked, effectively eliminating any optimizations in the system allocator targeted at multi-threaded applications. While the isc_mem API is still using locks pretty heavily, this is a first step into reducing the memory allocation/deallocation contention. --- .gitlab-ci.yml | 10 +- bin/named/main.c | 3 +- bin/tests/system/start.pl | 2 +- lib/isc/include/isc/mem.h | 34 +-- lib/isc/mem.c | 431 ++------------------------------------ 5 files changed, 29 insertions(+), 451 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4bcdae3baa..d0dc0fc3bc 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -896,7 +896,7 @@ unit:gcc:focal:amd64: gcc:asan: variables: CC: gcc - CFLAGS: "${CFLAGS_COMMON} -fsanitize=address,undefined -DISC_MEM_USE_INTERNAL_MALLOC=0" + CFLAGS: "${CFLAGS_COMMON} -fsanitize=address,undefined" LDFLAGS: "-fsanitize=address,undefined" EXTRA_CONFIGURE: "--with-libidn2" <<: *base_image @@ -923,7 +923,7 @@ unit:gcc:asan: clang:asan: variables: CC: ${CLANG} - CFLAGS: "${CFLAGS_COMMON} -fsanitize=address,undefined -DISC_MEM_USE_INTERNAL_MALLOC=0" + CFLAGS: "${CFLAGS_COMMON} -fsanitize=address,undefined" LDFLAGS: "-fsanitize=address,undefined" EXTRA_CONFIGURE: "--with-libidn2" <<: *base_image @@ -954,7 +954,7 @@ gcc:tsan: <<: *build_job variables: CC: gcc - CFLAGS: "${CFLAGS_COMMON} -fsanitize=thread -DISC_MEM_USE_INTERNAL_MALLOC=0" + CFLAGS: "${CFLAGS_COMMON} -fsanitize=thread" LDFLAGS: "-fsanitize=thread" EXTRA_CONFIGURE: "--with-libidn2 --enable-pthread-rwlock" @@ -981,7 +981,7 @@ clang:tsan: <<: *build_job variables: CC: "${CLANG}" - CFLAGS: "${CFLAGS_COMMON} -fsanitize=thread -DISC_MEM_USE_INTERNAL_MALLOC=0" + CFLAGS: "${CFLAGS_COMMON} -fsanitize=thread" LDFLAGS: "-fsanitize=thread" EXTRA_CONFIGURE: "--with-libidn2 --enable-pthread-rwlock" @@ -1008,7 +1008,7 @@ unit:clang:tsan: gcc:mutexatomics: variables: CC: gcc - CFLAGS: "${CFLAGS_COMMON} -DISC_MEM_USE_INTERNAL_MALLOC=0" + CFLAGS: "${CFLAGS_COMMON}" EXTRA_CONFIGURE: "--with-libidn2 --enable-mutex-atomics" <<: *base_image <<: *build_job diff --git a/bin/named/main.c b/bin/named/main.c index 13d44dda17..9b7074caa8 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -440,8 +440,7 @@ static struct flag_def { { "size", ISC_MEM_DEBUGSIZE, false }, { "mctx", ISC_MEM_DEBUGCTX, false }, { NULL, 0, false } }, - mem_context_flags[] = { { "external", ISC_MEMFLAG_INTERNAL, true }, - { "fill", ISC_MEMFLAG_FILL, false }, + mem_context_flags[] = { { "fill", ISC_MEMFLAG_FILL, false }, { "nofill", ISC_MEMFLAG_FILL, true }, { NULL, 0, false } }; diff --git a/bin/tests/system/start.pl b/bin/tests/system/start.pl index 54d345069f..791e65d56c 100755 --- a/bin/tests/system/start.pl +++ b/bin/tests/system/start.pl @@ -236,7 +236,7 @@ sub construct_ns_command { $command .= "--tool=memcheck --track-origins=yes --leak-check=full "; } - $command .= "$NAMED -m none -M external "; + $command .= "$NAMED -m none "; } else { if ($taskset) { $command = "taskset $taskset $NAMED "; diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 2aee5fb00c..28e3a5d515 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -36,21 +36,6 @@ typedef void (*isc_mem_water_t)(void *, int); #define ISC_MEM_TRACKLINES 1 #endif /* ifndef ISC_MEM_TRACKLINES */ -/*% - * Define ISC_MEM_CHECKOVERRUN=1 to turn on checks for using memory outside - * the requested space. This will increase the size of each allocation. - * - * If we are performing a Coverity static analysis then ISC_MEM_CHECKOVERRUN - * can hide bugs that would otherwise discovered so force to zero. - */ -#ifdef __COVERITY__ -#undef ISC_MEM_CHECKOVERRUN -#define ISC_MEM_CHECKOVERRUN 0 -#endif /* ifdef __COVERITY__ */ -#ifndef ISC_MEM_CHECKOVERRUN -#define ISC_MEM_CHECKOVERRUN 1 -#endif /* ifndef ISC_MEM_CHECKOVERRUN */ - /*% * Define ISC_MEMPOOL_NAMES=1 to make memory pools store a symbolic * name so that the leaking pool can be more readily identified in @@ -106,31 +91,18 @@ LIBISC_EXTERNAL_DATA extern unsigned int isc_mem_defaultflags; #define _ISC_MEM_FLARG #endif /* if ISC_MEM_TRACKLINES */ -/*! - * Define ISC_MEM_USE_INTERNAL_MALLOC=1 to use the internal malloc() - * implementation in preference to the system one. The internal malloc() - * is very space-efficient, and quite fast on uniprocessor systems. It - * performs poorly on multiprocessor machines. - * JT: we can overcome the performance issue on multiprocessor machines - * by carefully separating memory contexts. - */ - -#ifndef ISC_MEM_USE_INTERNAL_MALLOC -#define ISC_MEM_USE_INTERNAL_MALLOC 1 -#endif /* ifndef ISC_MEM_USE_INTERNAL_MALLOC */ - /* * Flags for isc_mem_create() calls. */ -#define ISC_MEMFLAG_RESERVED 0x00000001 /* reserved, obsoleted, don't use */ -#define ISC_MEMFLAG_INTERNAL 0x00000002 /* use internal malloc */ +#define ISC_MEMFLAG_RESERVED1 0x00000001 /* reserved, obsoleted, don't use */ +#define ISC_MEMFLAG_RESERVED2 0x00000002 /* reserved, obsoleted, don't use */ #define ISC_MEMFLAG_FILL \ 0x00000004 /* fill with pattern after alloc and frees */ #if !ISC_MEM_USE_INTERNAL_MALLOC #define ISC_MEMFLAG_DEFAULT 0 #else /* if !ISC_MEM_USE_INTERNAL_MALLOC */ -#define ISC_MEMFLAG_DEFAULT ISC_MEMFLAG_INTERNAL | ISC_MEMFLAG_FILL +#define ISC_MEMFLAG_DEFAULT ISC_MEMFLAG_FILL #endif /* if !ISC_MEM_USE_INTERNAL_MALLOC */ /*% diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 879b6232f9..cd9084a716 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -56,10 +56,7 @@ LIBISC_EXTERNAL_DATA unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT; */ #define DEF_MAX_SIZE 1100 -#define DEF_MEM_TARGET 4096 #define ALIGNMENT_SIZE 8U /*%< must be a power of 2 */ -#define NUM_BASIC_BLOCKS 64 /*%< must be > 1 */ -#define TABLE_INCREMENT 1024 #define DEBUG_TABLE_COUNT 512U /* @@ -106,8 +103,6 @@ typedef struct { struct stats { unsigned long gets; unsigned long totalgets; - unsigned long blocks; - unsigned long freefrags; }; #define MEM_MAGIC ISC_MAGIC('M', 'e', 'm', 'C') @@ -160,16 +155,6 @@ struct isc__mem { ISC_LIST(isc__mempool_t) pools; unsigned int poolcnt; - /* ISC_MEMFLAG_INTERNAL */ - size_t mem_target; - element **freelists; - element *basic_blocks; - unsigned char **basic_table; - unsigned int basic_table_count; - unsigned int basic_table_size; - unsigned char *lowest; - unsigned char *highest; - #if ISC_MEM_TRACKLINES debuglist_t *debuglist; size_t debuglistcnt; @@ -349,268 +334,6 @@ delete_trace_entry(isc__mem_t *mctx, const void *ptr, size_t size, } #endif /* ISC_MEM_TRACKLINES */ -static inline size_t -rmsize(size_t size) { - /* - * round down to ALIGNMENT_SIZE - */ - return (size & (~(ALIGNMENT_SIZE - 1))); -} - -static inline size_t -quantize(size_t size) { - /*! - * Round up the result in order to get a size big - * enough to satisfy the request and be aligned on ALIGNMENT_SIZE - * byte boundaries. - */ - - if (size == 0U) { - return (ALIGNMENT_SIZE); - } - return ((size + ALIGNMENT_SIZE - 1) & (~(ALIGNMENT_SIZE - 1))); -} - -static inline void -more_basic_blocks(isc__mem_t *ctx) { - void *tmp; - unsigned char *curr, *next; - unsigned char *first, *last; - unsigned char **table; - unsigned int table_size; - - /* Require: we hold the context lock. */ - - INSIST(ctx->basic_table_count <= ctx->basic_table_size); - if (ctx->basic_table_count == ctx->basic_table_size) { - table_size = ctx->basic_table_size + TABLE_INCREMENT; - table = (ctx->memalloc)(table_size * sizeof(unsigned char *)); - ctx->malloced += table_size * sizeof(unsigned char *); - if (ctx->malloced > ctx->maxmalloced) { - ctx->maxmalloced = ctx->malloced; - } - if (ctx->basic_table_size != 0) { - memmove(table, ctx->basic_table, - ctx->basic_table_size * - sizeof(unsigned char *)); - (ctx->memfree)(ctx->basic_table); - ctx->malloced -= ctx->basic_table_size * - sizeof(unsigned char *); - } - ctx->basic_table = table; - ctx->basic_table_size = table_size; - } - - tmp = (ctx->memalloc)(NUM_BASIC_BLOCKS * ctx->mem_target); - ctx->total += NUM_BASIC_BLOCKS * ctx->mem_target; - ctx->basic_table[ctx->basic_table_count] = tmp; - ctx->basic_table_count++; - ctx->malloced += NUM_BASIC_BLOCKS * ctx->mem_target; - if (ctx->malloced > ctx->maxmalloced) { - ctx->maxmalloced = ctx->malloced; - } - - curr = tmp; - next = curr + ctx->mem_target; - for (int i = 0; i < (NUM_BASIC_BLOCKS - 1); i++) { - ((element *)curr)->next = (element *)next; - curr = next; - next += ctx->mem_target; - } - /* - * curr is now pointing at the last block in the - * array. - */ - ((element *)curr)->next = NULL; - first = tmp; - last = first + NUM_BASIC_BLOCKS * ctx->mem_target - 1; - if (first < ctx->lowest || ctx->lowest == NULL) { - ctx->lowest = first; - } - if (last > ctx->highest) { - ctx->highest = last; - } - ctx->basic_blocks = tmp; -} - -static inline void -more_frags(isc__mem_t *ctx, size_t new_size) { - int frags; - size_t total_size; - void *tmp; - unsigned char *curr, *next; - - /*! - * Try to get more fragments by chopping up a basic block. - */ - - if (ctx->basic_blocks == NULL) { - more_basic_blocks(ctx); - } - INSIST(ctx->basic_blocks != NULL); - - total_size = ctx->mem_target; - tmp = ctx->basic_blocks; - ctx->basic_blocks = ctx->basic_blocks->next; - frags = (int)(total_size / new_size); - ctx->stats[new_size].blocks++; - ctx->stats[new_size].freefrags += frags; - /* - * Set up a linked-list of blocks of size - * "new_size". - */ - curr = tmp; - next = curr + new_size; - total_size -= new_size; - for (int i = 0; i < (frags - 1); i++) { - ((element *)curr)->next = (element *)next; - curr = next; - next += new_size; - total_size -= new_size; - } - /* - * Add the remaining fragment of the basic block to a free list. - */ - total_size = rmsize(total_size); - if (total_size > 0U) { - ((element *)next)->next = ctx->freelists[total_size]; - ctx->freelists[total_size] = (element *)next; - ctx->stats[total_size].freefrags++; - } - /* - * curr is now pointing at the last block in the - * array. - */ - ((element *)curr)->next = NULL; - ctx->freelists[new_size] = tmp; -} - -static inline void * -mem_getunlocked(isc__mem_t *ctx, size_t size) { - size_t new_size = quantize(size); - void *ret; - - if (new_size >= ctx->max_size) { - /* - * memget() was called on something beyond our upper limit. - */ - ret = (ctx->memalloc)(size); - ctx->total += size; - ctx->inuse += size; - ctx->stats[ctx->max_size].gets++; - ctx->stats[ctx->max_size].totalgets++; - ctx->malloced += size; - if (ctx->malloced > ctx->maxmalloced) { - ctx->maxmalloced = ctx->malloced; - } - /* - * If we don't set new_size to size, then the - * ISC_MEMFLAG_FILL code might write over bytes we don't - * own. - */ - new_size = size; - goto done; - } - /* - * If there are no blocks in the free list for this size, get a chunk - * of memory and then break it up into "new_size"-sized blocks, adding - * them to the free list. - */ - if (ctx->freelists[new_size] == NULL) { - more_frags(ctx, new_size); - } - INSIST(ctx->freelists[new_size] != NULL); - - /* - * The free list uses the "rounded-up" size "new_size". - */ - - ret = ctx->freelists[new_size]; - ctx->freelists[new_size] = ctx->freelists[new_size]->next; - - /* - * The stats[] uses the _actual_ "size" requested by the - * caller, with the caveat (in the code above) that "size" >= the - * max. size (max_size) ends up getting recorded as a call to - * max_size. - */ - ctx->stats[size].gets++; - ctx->stats[size].totalgets++; - ctx->stats[new_size].freefrags--; - ctx->inuse += new_size; - -done: - if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0) && - ISC_LIKELY(ret != NULL)) - { - memset(ret, 0xbe, new_size); /* Mnemonic for "beef". */ - } - - return (ret); -} - -#if ISC_MEM_CHECKOVERRUN -static inline void -check_overrun(void *mem, size_t size, size_t new_size) { - unsigned char *cp; - - cp = (unsigned char *)mem; - cp += size; - while (size < new_size) { - INSIST(*cp == 0xbe); - cp++; - size++; - } -} -#endif /* if ISC_MEM_CHECKOVERRUN */ - -/* coverity[+free : arg-1] */ -static inline void -mem_putunlocked(isc__mem_t *ctx, void *mem, size_t size) { - size_t new_size = quantize(size); - - if (new_size >= ctx->max_size) { - /* - * memput() called on something beyond our upper limit. - */ - if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { - memset(mem, 0xde, size); /* Mnemonic for "dead". */ - } - - (ctx->memfree)(mem); - INSIST(ctx->stats[ctx->max_size].gets != 0U); - ctx->stats[ctx->max_size].gets--; - INSIST(size <= ctx->inuse); - ctx->inuse -= size; - ctx->malloced -= size; - return; - } - - if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { -#if ISC_MEM_CHECKOVERRUN - check_overrun(mem, size, new_size); -#endif /* if ISC_MEM_CHECKOVERRUN */ - memset(mem, 0xde, new_size); /* Mnemonic for "dead". */ - } - - /* - * The free list uses the "rounded-up" size "new_size". - */ - ((element *)mem)->next = ctx->freelists[new_size]; - ctx->freelists[new_size] = (element *)mem; - - /* - * The stats[] uses the _actual_ "size" requested by the - * caller, with the caveat (in the code above) that "size" >= the - * max. size (max_size) ends up getting recorded as a call to - * max_size. - */ - INSIST(ctx->stats[size].gets != 0U); - ctx->stats[size].gets--; - ctx->stats[new_size].freefrags++; - ctx->inuse -= new_size; -} - /*! * Perform a malloc, doing memory filling and overrun detection as necessary. */ @@ -795,13 +518,6 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { #endif /* if ISC_MEM_TRACKLINES */ ISC_LIST_INIT(ctx->pools); ctx->poolcnt = 0; - ctx->freelists = NULL; - ctx->basic_blocks = NULL; - ctx->basic_table = NULL; - ctx->basic_table_count = 0; - ctx->basic_table_size = 0; - ctx->lowest = NULL; - ctx->highest = NULL; ctx->stats = (ctx->memalloc)((ctx->max_size + 1) * sizeof(struct stats)); @@ -810,15 +526,6 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { ctx->malloced += (ctx->max_size + 1) * sizeof(struct stats); ctx->maxmalloced += (ctx->max_size + 1) * sizeof(struct stats); - if ((flags & ISC_MEMFLAG_INTERNAL) != 0) { - ctx->mem_target = DEF_MEM_TARGET; - ctx->freelists = - (ctx->memalloc)(ctx->max_size * sizeof(element *)); - memset(ctx->freelists, 0, ctx->max_size * sizeof(element *)); - ctx->malloced += ctx->max_size * sizeof(element *); - ctx->maxmalloced += ctx->max_size * sizeof(element *); - } - #if ISC_MEM_TRACKLINES if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGRECORD) != 0)) { unsigned int i; @@ -900,20 +607,6 @@ destroy(isc__mem_t *ctx) { (ctx->memfree)(ctx->stats); ctx->malloced -= (ctx->max_size + 1) * sizeof(struct stats); - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - for (i = 0; i < ctx->basic_table_count; i++) { - (ctx->memfree)(ctx->basic_table[i]); - ctx->malloced -= NUM_BASIC_BLOCKS * ctx->mem_target; - } - (ctx->memfree)(ctx->freelists); - ctx->malloced -= ctx->max_size * sizeof(element *); - if (ctx->basic_table != NULL) { - (ctx->memfree)(ctx->basic_table); - ctx->malloced -= ctx->basic_table_size * - sizeof(unsigned char *); - } - } - isc_mutex_destroy(&ctx->lock); ctx->malloced -= sizeof(*ctx); @@ -986,12 +679,8 @@ isc___mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { DELETE_TRACE(ctx, ptr, size, file, line); - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - mem_putunlocked(ctx, ptr, size); - } else { - mem_putstats(ctx, ptr, size); - mem_put(ctx, ptr, size); - } + mem_putstats(ctx, ptr, size); + mem_put(ctx, ptr, size); MCTXUNLOCK(ctx); destroy: @@ -1039,16 +728,9 @@ isc___mem_get(isc_mem_t *ctx0, size_t size FLARG) { return (isc__mem_allocate(ctx0, size FLARG_PASS)); } - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - MCTXLOCK(ctx); - ptr = mem_getunlocked(ctx, size); - } else { - ptr = mem_get(ctx, size); - MCTXLOCK(ctx); - if (ptr != NULL) { - mem_getstats(ctx, size); - } - } + ptr = mem_get(ctx, size); + MCTXLOCK(ctx); + mem_getstats(ctx, size); ADD_TRACE(ctx, ptr, size, file, line); @@ -1105,12 +787,8 @@ isc___mem_put(isc_mem_t *ctx0, void *ptr, size_t size FLARG) { DELETE_TRACE(ctx, ptr, size, file, line); - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - mem_putunlocked(ctx, ptr, size); - } else { - mem_putstats(ctx, ptr, size); - mem_put(ctx, ptr, size); - } + mem_putstats(ctx, ptr, size); + mem_put(ctx, ptr, size); /* * The check against ctx->lo_water == 0 is for the condition @@ -1205,12 +883,6 @@ isc_mem_stats(isc_mem_t *ctx0, FILE *out) { fprintf(out, "%s%5lu: %11lu gets, %11lu rem", (i == ctx->max_size) ? ">=" : " ", (unsigned long)i, s->totalgets, s->gets); - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0 && - (s->blocks != 0U || s->freefrags != 0U)) - { - fprintf(out, " (%lu bl, %lu ff)", s->blocks, - s->freefrags); - } fputc('\n', out); } @@ -1264,11 +936,7 @@ mem_allocateunlocked(isc_mem_t *ctx0, size_t size) { size += ALIGNMENT_SIZE; } - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - si = mem_getunlocked(ctx, size); - } else { - si = mem_get(ctx, size); - } + si = mem_get(ctx, size); if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0)) { si->u.ctx = ctx; @@ -1288,9 +956,7 @@ isc___mem_allocate(isc_mem_t *ctx0, size_t size FLARG) { MCTXLOCK(ctx); si = mem_allocateunlocked((isc_mem_t *)ctx, size); - if (((ctx->flags & ISC_MEMFLAG_INTERNAL) == 0)) { - mem_getstats(ctx, si[-1].u.size); - } + mem_getstats(ctx, si[-1].u.size); ADD_TRACE(ctx, si, si[-1].u.size, file, line); if (ctx->hi_water != 0U && ctx->inuse > ctx->hi_water && @@ -1385,12 +1051,8 @@ isc___mem_free(isc_mem_t *ctx0, void *ptr FLARG) { DELETE_TRACE(ctx, ptr, size, file, line); - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - mem_putunlocked(ctx, si, size); - } else { - mem_putstats(ctx, si, size); - mem_put(ctx, si, size); - } + mem_putstats(ctx, si, size); + mem_put(ctx, si, size); /* * The check against ctx->lo_water == 0 is for the condition @@ -1723,12 +1385,8 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { item = mpctx->items; mpctx->items = item->next; - if ((mctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - mem_putunlocked(mctx, item, mpctx->size); - } else { - mem_putstats(mctx, item, mpctx->size); - mem_put(mctx, item, mpctx->size); - } + mem_putstats(mctx, item, mpctx->size); + mem_put(mctx, item, mpctx->size); } MCTXUNLOCK(mctx); @@ -1794,17 +1452,8 @@ isc__mempool_get(isc_mempool_t *mpctx0 FLARG) { */ MCTXLOCK(mctx); for (i = 0; i < mpctx->fillcount; i++) { - if ((mctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - item = mem_getunlocked(mctx, mpctx->size); - } else { - item = mem_get(mctx, mpctx->size); - if (item != NULL) { - mem_getstats(mctx, mpctx->size); - } - } - if (ISC_UNLIKELY(item == NULL)) { - break; - } + item = mem_get(mctx, mpctx->size); + mem_getstats(mctx, mpctx->size); item->next = mpctx->items; mpctx->items = item; mpctx->freecount++; @@ -1873,12 +1522,8 @@ isc__mempool_put(isc_mempool_t *mpctx0, void *mem FLARG) { */ if (mpctx->freecount >= mpctx->freemax) { MCTXLOCK(mctx); - if ((mctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - mem_putunlocked(mctx, mem, mpctx->size); - } else { - mem_putstats(mctx, mem, mpctx->size); - mem_put(mctx, mem, mpctx->size); - } + mem_putstats(mctx, mem, mpctx->size); + mem_put(mctx, mem, mpctx->size); MCTXUNLOCK(mctx); if (mpctx->lock != NULL) { UNLOCK(mpctx->lock); @@ -2105,7 +1750,6 @@ typedef struct summarystat { uint64_t total; uint64_t inuse; uint64_t malloced; - uint64_t blocksize; uint64_t contextsize; } summarystat_t; @@ -2139,8 +1783,7 @@ xml_renderctx(isc__mem_t *ctx, summarystat_t *summary, summary->contextsize += sizeof(*ctx) + (ctx->max_size + 1) * sizeof(struct stats) + - ctx->max_size * sizeof(element *) + - ctx->basic_table_count * sizeof(char *); + ctx->max_size * sizeof(element *); #if ISC_MEM_TRACKLINES if (ctx->debuglist != NULL) { summary->contextsize += DEBUG_TABLE_COUNT * @@ -2182,19 +1825,6 @@ xml_renderctx(isc__mem_t *ctx, summarystat_t *summary, (uint64_t)ctx->maxmalloced)); TRY0(xmlTextWriterEndElement(writer)); /* maxmalloced */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "blocksize")); - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - summary->blocksize += ctx->basic_table_count * - NUM_BASIC_BLOCKS * ctx->mem_target; - TRY0(xmlTextWriterWriteFormatString( - writer, "%" PRIu64 "", - (uint64_t)ctx->basic_table_count * NUM_BASIC_BLOCKS * - ctx->mem_target)); - } else { - TRY0(xmlTextWriterWriteFormatString(writer, "%s", "-")); - } - TRY0(xmlTextWriterEndElement(writer)); /* blocksize */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "pools")); TRY0(xmlTextWriterWriteFormatString(writer, "%u", ctx->poolcnt)); TRY0(xmlTextWriterEndElement(writer)); /* pools */ @@ -2263,11 +1893,6 @@ isc_mem_renderxml(void *writer0) { summary.malloced)); TRY0(xmlTextWriterEndElement(writer)); /* InUse */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "BlockSize")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - summary.blocksize)); - TRY0(xmlTextWriterEndElement(writer)); /* BlockSize */ - TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "ContextSize")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", summary.contextsize)); @@ -2300,15 +1925,10 @@ json_renderctx(isc__mem_t *ctx, summarystat_t *summary, json_object *array) { summary->contextsize += sizeof(*ctx) + (ctx->max_size + 1) * sizeof(struct stats) + - ctx->max_size * sizeof(element *) + - ctx->basic_table_count * sizeof(char *); + ctx->max_size * sizeof(element *); summary->total += ctx->total; summary->inuse += ctx->inuse; summary->malloced += ctx->malloced; - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - summary->blocksize += ctx->basic_table_count * - NUM_BASIC_BLOCKS * ctx->mem_target; - } #if ISC_MEM_TRACKLINES if (ctx->debuglist != NULL) { summary->contextsize += DEBUG_TABLE_COUNT * @@ -2355,15 +1975,6 @@ json_renderctx(isc__mem_t *ctx, summarystat_t *summary, json_object *array) { CHECKMEM(obj); json_object_object_add(ctxobj, "maxmalloced", obj); - if ((ctx->flags & ISC_MEMFLAG_INTERNAL) != 0) { - uint64_t blocksize; - blocksize = ctx->basic_table_count * NUM_BASIC_BLOCKS * - ctx->mem_target; - obj = json_object_new_int64(blocksize); - CHECKMEM(obj); - json_object_object_add(ctxobj, "blocksize", obj); - } - obj = json_object_new_int64(ctx->poolcnt); CHECKMEM(obj); json_object_object_add(ctxobj, "pools", obj); @@ -2422,10 +2033,6 @@ isc_mem_renderjson(void *memobj0) { CHECKMEM(obj); json_object_object_add(memobj, "Malloced", obj); - obj = json_object_new_int64(summary.blocksize); - CHECKMEM(obj); - json_object_object_add(memobj, "BlockSize", obj); - obj = json_object_new_int64(summary.contextsize); CHECKMEM(obj); json_object_object_add(memobj, "ContextSize", obj); From 7de846977b2a4fd3b4cef6b0c7c90c7815030eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 4 Feb 2021 20:19:09 +0100 Subject: [PATCH 02/13] Remove the extra level of indirection via isc_memmethods_t Previously, the applications using libisc would be able to override the internal memory methods with own implementation. This was no longer possible, but the extra level of indirection was not removed. This commit removes the extra level of indirection for the memory methods and the default_memalloc() and default_memfree(). --- lib/isc/include/isc/mem.h | 46 ----- lib/isc/mem.c | 417 +++++++++++++------------------------- 2 files changed, 137 insertions(+), 326 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 28e3a5d515..3e789ef34d 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -134,52 +134,6 @@ LIBISC_EXTERNAL_DATA extern unsigned int isc_mem_defaultflags; * \endcode */ -/*% memory and memory pool methods */ -typedef struct isc_memmethods { - void *(*memget)(isc_mem_t *mctx, size_t size _ISC_MEM_FLARG); - void (*memput)(isc_mem_t *mctx, void *ptr, size_t size _ISC_MEM_FLARG); - void (*memputanddetach)(isc_mem_t **mctxp, void *ptr, - size_t size _ISC_MEM_FLARG); - void *(*memallocate)(isc_mem_t *mctx, size_t size _ISC_MEM_FLARG); - void *(*memreallocate)(isc_mem_t *mctx, void *ptr, - size_t size _ISC_MEM_FLARG); - char *(*memstrdup)(isc_mem_t *mctx, const char *s _ISC_MEM_FLARG); - char *(*memstrndup)(isc_mem_t *mctx, const char *s, - size_t size _ISC_MEM_FLARG); - void (*memfree)(isc_mem_t *mctx, void *ptr _ISC_MEM_FLARG); -} isc_memmethods_t; - -/*% - * This structure is actually just the common prefix of a memory context - * implementation's version of an isc_mem_t. - * \brief - * Direct use of this structure by clients is forbidden. mctx implementations - * may change the structure. 'magic' must be ISCAPI_MCTX_MAGIC for any of the - * isc_mem_ routines to work. mctx implementations must maintain all mctx - * invariants. - */ -struct isc_mem { - unsigned int impmagic; - unsigned int magic; - isc_memmethods_t *methods; -}; - -#define ISCAPI_MCTX_MAGIC ISC_MAGIC('A', 'm', 'c', 'x') -#define ISCAPI_MCTX_VALID(m) ((m) != NULL && (m)->magic == ISCAPI_MCTX_MAGIC) - -/*% - * This is the common prefix of a memory pool context. The same note as - * that for the mem structure applies. - */ -struct isc_mempool { - unsigned int impmagic; - unsigned int magic; -}; - -#define ISCAPI_MPOOL_MAGIC ISC_MAGIC('A', 'm', 'p', 'l') -#define ISCAPI_MPOOL_VALID(mp) \ - ((mp) != NULL && (mp)->magic == ISCAPI_MPOOL_MAGIC) - /*% * These functions are actually implemented in isc__mem_ * (two underscores). The single-underscore macros are used to pass diff --git a/lib/isc/mem.c b/lib/isc/mem.c index cd9084a716..e4c5a8ab04 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #ifdef HAVE_LIBXML2 @@ -62,9 +63,6 @@ LIBISC_EXTERNAL_DATA unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT; /* * Types. */ -typedef struct isc__mem isc__mem_t; -typedef struct isc__mempool isc__mempool_t; - #if ISC_MEM_TRACKLINES typedef struct debuglink debuglink_t; struct debuglink { @@ -95,7 +93,7 @@ typedef struct { */ union { size_t size; - isc__mem_t *ctx; + isc_mem_t *ctx; char bytes[ALIGNMENT_SIZE]; } u; } size_info; @@ -110,7 +108,7 @@ struct stats { /* List of all active memory contexts. */ -static ISC_LIST(isc__mem_t) contexts; +static ISC_LIST(isc_mem_t) contexts; static isc_once_t once = ISC_ONCE_INIT; static isc_mutex_t contextslock; @@ -121,20 +119,10 @@ static isc_mutex_t contextslock; */ static uint64_t totallost; -/*% - * Memory allocation and free function definitions. - * isc__memalloc_t must deal with memory allocation failure - * and must never return NULL. - */ -typedef void *(*isc__memalloc_t)(size_t); -typedef void (*isc__memfree_t)(void *); - -struct isc__mem { - isc_mem_t common; +struct isc_mem { + unsigned int magic; unsigned int flags; isc_mutex_t lock; - isc__memalloc_t memalloc; - isc__memfree_t memfree; size_t max_size; bool checkfree; struct stats *stats; @@ -152,7 +140,7 @@ struct isc__mem { bool is_overmem; isc_mem_water_t water; void *water_arg; - ISC_LIST(isc__mempool_t) pools; + ISC_LIST(isc_mempool_t) pools; unsigned int poolcnt; #if ISC_MEM_TRACKLINES @@ -160,19 +148,19 @@ struct isc__mem { size_t debuglistcnt; #endif /* if ISC_MEM_TRACKLINES */ - ISC_LINK(isc__mem_t) link; + ISC_LINK(isc_mem_t) link; }; #define MEMPOOL_MAGIC ISC_MAGIC('M', 'E', 'M', 'p') #define VALID_MEMPOOL(c) ISC_MAGIC_VALID(c, MEMPOOL_MAGIC) -struct isc__mempool { +struct isc_mempool { /* always unlocked */ - isc_mempool_t common; /*%< common header of mempool's */ - isc_mutex_t *lock; /*%< optional lock */ - isc__mem_t *mctx; /*%< our memory context */ + unsigned int magic; + isc_mutex_t *lock; /*%< optional lock */ + isc_mem_t *mctx; /*%< our memory context */ /*%< locked via the memory context's lock */ - ISC_LINK(isc__mempool_t) link; /*%< next pool in this mem 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 */ @@ -213,39 +201,16 @@ struct isc__mempool { } while (0) static void -print_active(isc__mem_t *ctx, FILE *out); +print_active(isc_mem_t *ctx, FILE *out); #endif /* ISC_MEM_TRACKLINES */ -static void * -isc___mem_get(isc_mem_t *ctx, size_t size FLARG); -static void -isc___mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG); -static void -isc___mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG); -static void * -isc___mem_allocate(isc_mem_t *ctx, size_t size FLARG); -static void * -isc___mem_reallocate(isc_mem_t *ctx, void *ptr, size_t size FLARG); -static char * -isc___mem_strdup(isc_mem_t *mctx, const char *s FLARG); -static char * -isc___mem_strndup(isc_mem_t *mctx, const char *s, size_t size FLARG); -static void -isc___mem_free(isc_mem_t *ctx, void *ptr FLARG); - -static isc_memmethods_t memmethods = { - isc___mem_get, isc___mem_put, isc___mem_putanddetach, - isc___mem_allocate, isc___mem_reallocate, isc___mem_strdup, - isc___mem_strndup, isc___mem_free, -}; - #if ISC_MEM_TRACKLINES /*! * mctx must be locked. */ static void -add_trace_entry(isc__mem_t *mctx, const void *ptr, size_t size FLARG) { +add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { debuglink_t *dl; uint32_t hash; uint32_t idx; @@ -288,7 +253,7 @@ add_trace_entry(isc__mem_t *mctx, const void *ptr, size_t size FLARG) { } static void -delete_trace_entry(isc__mem_t *mctx, const void *ptr, size_t size, +delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, const char *file, unsigned int line) { debuglink_t *dl; uint32_t hash; @@ -334,17 +299,22 @@ delete_trace_entry(isc__mem_t *mctx, const void *ptr, size_t size, } #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. */ static inline void * -mem_get(isc__mem_t *ctx, size_t size) { +mem_get(isc_mem_t *ctx, size_t size) { char *ret; #if ISC_MEM_CHECKOVERRUN size += 1; #endif /* if ISC_MEM_CHECKOVERRUN */ - ret = (ctx->memalloc)(size); + ret = default_memalloc(size); if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { if (ISC_LIKELY(ret != NULL)) { @@ -368,7 +338,7 @@ mem_get(isc__mem_t *ctx, size_t size) { */ /* coverity[+free : arg-1] */ static inline void -mem_put(isc__mem_t *ctx, void *mem, size_t size) { +mem_put(isc_mem_t *ctx, void *mem, size_t size) { #if ISC_MEM_CHECKOVERRUN INSIST(((unsigned char *)mem)[size] == 0xbe); size += 1; @@ -376,14 +346,14 @@ 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". */ } - (ctx->memfree)(mem); + default_memfree(mem); } /*! * Update internal counters after a memory get. */ static inline void -mem_getstats(isc__mem_t *ctx, size_t size) { +mem_getstats(isc_mem_t *ctx, size_t size) { ctx->total += size; ctx->inuse += size; @@ -408,7 +378,7 @@ mem_getstats(isc__mem_t *ctx, size_t size) { * Update internal counters after a memory put. */ static inline void -mem_putstats(isc__mem_t *ctx, void *ptr, size_t size) { +mem_putstats(isc_mem_t *ctx, void *ptr, size_t size) { UNUSED(ptr); INSIST(ctx->inuse >= size); @@ -478,7 +448,7 @@ static void mem_create(isc_mem_t **ctxp, unsigned int flags) { REQUIRE(ctxp != NULL && *ctxp == NULL); - isc__mem_t *ctx; + isc_mem_t *ctx; STATIC_ASSERT((ALIGNMENT_SIZE & (ALIGNMENT_SIZE - 1)) == 0, "wrong alignment size"); @@ -505,11 +475,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { ctx->is_overmem = false; ctx->water = NULL; ctx->water_arg = NULL; - ctx->common.impmagic = MEM_MAGIC; - ctx->common.magic = ISCAPI_MCTX_MAGIC; - ctx->common.methods = (isc_memmethods_t *)&memmethods; - ctx->memalloc = default_memalloc; - ctx->memfree = default_memfree; + ctx->magic = MEM_MAGIC; ctx->stats = NULL; ctx->checkfree = true; #if ISC_MEM_TRACKLINES @@ -520,7 +486,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { ctx->poolcnt = 0; ctx->stats = - (ctx->memalloc)((ctx->max_size + 1) * sizeof(struct stats)); + default_memalloc((ctx->max_size + 1) * sizeof(struct stats)); memset(ctx->stats, 0, (ctx->max_size + 1) * sizeof(struct stats)); ctx->malloced += (ctx->max_size + 1) * sizeof(struct stats); @@ -530,7 +496,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGRECORD) != 0)) { unsigned int i; - ctx->debuglist = (ctx->memalloc)( + ctx->debuglist = default_memalloc( (DEBUG_TABLE_COUNT * sizeof(debuglist_t))); for (i = 0; i < DEBUG_TABLE_COUNT; i++) { ISC_LIST_INIT(ctx->debuglist[i]); @@ -544,7 +510,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { ISC_LIST_INITANDAPPEND(contexts, ctx, link); UNLOCK(&contextslock); - *ctxp = (isc_mem_t *)ctx; + *ctxp = ctx; } /* @@ -552,7 +518,7 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { */ static void -destroy(isc__mem_t *ctx) { +destroy(isc_mem_t *ctx) { unsigned int i; LOCK(&contextslock); @@ -560,8 +526,7 @@ destroy(isc__mem_t *ctx) { totallost += ctx->inuse; UNLOCK(&contextslock); - ctx->common.impmagic = 0; - ctx->common.magic = 0; + ctx->magic = 0; INSIST(ISC_LIST_EMPTY(ctx->pools)); @@ -583,7 +548,7 @@ destroy(isc__mem_t *ctx) { } } - (ctx->memfree)(ctx->debuglist); + default_memfree(ctx->debuglist); ctx->malloced -= DEBUG_TABLE_COUNT * sizeof(debuglist_t); } #endif /* if ISC_MEM_TRACKLINES */ @@ -604,7 +569,7 @@ destroy(isc__mem_t *ctx) { } } - (ctx->memfree)(ctx->stats); + default_memfree(ctx->stats); ctx->malloced -= (ctx->max_size + 1) * sizeof(struct stats); isc_mutex_destroy(&ctx->lock); @@ -613,26 +578,24 @@ destroy(isc__mem_t *ctx) { if (ctx->checkfree) { INSIST(ctx->malloced == 0); } - (ctx->memfree)(ctx); + default_memfree(ctx); } void -isc_mem_attach(isc_mem_t *source0, isc_mem_t **targetp) { - REQUIRE(VALID_CONTEXT(source0)); +isc_mem_attach(isc_mem_t *source, isc_mem_t **targetp) { + REQUIRE(VALID_CONTEXT(source)); REQUIRE(targetp != NULL && *targetp == NULL); - isc__mem_t *source = (isc__mem_t *)source0; - isc_refcount_increment(&source->references); - *targetp = (isc_mem_t *)source; + *targetp = source; } void isc_mem_detach(isc_mem_t **ctxp) { REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); - isc__mem_t *ctx = (isc__mem_t *)*ctxp; + isc_mem_t *ctx = *ctxp; *ctxp = NULL; if (isc_refcount_decrement(&ctx->references) == 1) { @@ -652,11 +615,11 @@ isc_mem_detach(isc_mem_t **ctxp) { */ void -isc___mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { +isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); REQUIRE(ptr != NULL); - isc__mem_t *ctx = (isc__mem_t *)*ctxp; + isc_mem_t *ctx = *ctxp; *ctxp = NULL; if (ISC_UNLIKELY((isc_mem_debugging & @@ -670,7 +633,7 @@ isc___mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { } INSIST(oldsize == size); } - isc__mem_free((isc_mem_t *)ctx, ptr FLARG_PASS); + isc__mem_free(ctx, ptr FLARG_PASS); goto destroy; } @@ -699,7 +662,7 @@ isc_mem_destroy(isc_mem_t **ctxp) { REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); - isc__mem_t *ctx = (isc__mem_t *)*ctxp; + isc_mem_t *ctx = *ctxp; #if ISC_MEM_TRACKLINES if (isc_refcount_decrement(&ctx->references) > 1) { @@ -715,17 +678,16 @@ isc_mem_destroy(isc_mem_t **ctxp) { } void * -isc___mem_get(isc_mem_t *ctx0, size_t size FLARG) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { + REQUIRE(VALID_CONTEXT(ctx)); - isc__mem_t *ctx = (isc__mem_t *)ctx0; void *ptr; bool call_water = false; if (ISC_UNLIKELY((isc_mem_debugging & (ISC_MEM_DEBUGSIZE | ISC_MEM_DEBUGCTX)) != 0)) { - return (isc__mem_allocate(ctx0, size FLARG_PASS)); + return (isc__mem_allocate(ctx, size FLARG_PASS)); } ptr = mem_get(ctx, size); @@ -759,11 +721,10 @@ isc___mem_get(isc_mem_t *ctx0, size_t size FLARG) { } void -isc___mem_put(isc_mem_t *ctx0, void *ptr, size_t size FLARG) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) { + REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(ptr != NULL); - isc__mem_t *ctx = (isc__mem_t *)ctx0; bool call_water = false; size_info *si; size_t oldsize; @@ -779,7 +740,7 @@ isc___mem_put(isc_mem_t *ctx0, void *ptr, size_t size FLARG) { } INSIST(oldsize == size); } - isc__mem_free((isc_mem_t *)ctx, ptr FLARG_PASS); + isc__mem_free(ctx, ptr FLARG_PASS); return; } @@ -810,10 +771,8 @@ isc___mem_put(isc_mem_t *ctx0, void *ptr, size_t size FLARG) { } void -isc_mem_waterack(isc_mem_t *ctx0, int flag) { - REQUIRE(VALID_CONTEXT(ctx0)); - - isc__mem_t *ctx = (isc__mem_t *)ctx0; +isc_mem_waterack(isc_mem_t *ctx, int flag) { + REQUIRE(VALID_CONTEXT(ctx)); MCTXLOCK(ctx); if (flag == ISC_MEM_LOWATER) { @@ -826,7 +785,7 @@ isc_mem_waterack(isc_mem_t *ctx0, int flag) { #if ISC_MEM_TRACKLINES static void -print_active(isc__mem_t *mctx, FILE *out) { +print_active(isc_mem_t *mctx, FILE *out) { if (mctx->debuglist != NULL) { debuglink_t *dl; unsigned int i; @@ -864,13 +823,12 @@ print_active(isc__mem_t *mctx, FILE *out) { * Print the stats[] on the stream "out" with suitable formatting. */ void -isc_mem_stats(isc_mem_t *ctx0, FILE *out) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc_mem_stats(isc_mem_t *ctx, FILE *out) { + REQUIRE(VALID_CONTEXT(ctx)); - isc__mem_t *ctx = (isc__mem_t *)ctx0; size_t i; const struct stats *s; - const isc__mempool_t *pool; + const isc_mempool_t *pool; MCTXLOCK(ctx); @@ -927,8 +885,7 @@ isc_mem_stats(isc_mem_t *ctx0, FILE *out) { */ static void * -mem_allocateunlocked(isc_mem_t *ctx0, size_t size) { - isc__mem_t *ctx = (isc__mem_t *)ctx0; +mem_allocateunlocked(isc_mem_t *ctx, size_t size) { size_info *si; size += ALIGNMENT_SIZE; @@ -947,15 +904,14 @@ mem_allocateunlocked(isc_mem_t *ctx0, size_t size) { } void * -isc___mem_allocate(isc_mem_t *ctx0, size_t size FLARG) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { + REQUIRE(VALID_CONTEXT(ctx)); - isc__mem_t *ctx = (isc__mem_t *)ctx0; size_info *si; bool call_water = false; MCTXLOCK(ctx); - si = mem_allocateunlocked((isc_mem_t *)ctx, size); + si = mem_allocateunlocked(ctx, size); mem_getstats(ctx, si[-1].u.size); ADD_TRACE(ctx, si, si[-1].u.size, file, line); @@ -989,8 +945,8 @@ isc___mem_allocate(isc_mem_t *ctx0, size_t size FLARG) { } void * -isc___mem_reallocate(isc_mem_t *ctx0, void *ptr, size_t size FLARG) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc__mem_reallocate(isc_mem_t *ctx, void *ptr, size_t size FLARG) { + REQUIRE(VALID_CONTEXT(ctx)); void *new_ptr = NULL; size_t oldsize, copysize; @@ -1007,7 +963,7 @@ isc___mem_reallocate(isc_mem_t *ctx0, void *ptr, size_t size FLARG) { * NULL if allocation fails or doesn't happen. */ if (size > 0U) { - new_ptr = isc__mem_allocate(ctx0, size FLARG_PASS); + new_ptr = isc__mem_allocate(ctx, size FLARG_PASS); if (new_ptr != NULL && ptr != NULL) { oldsize = (((size_info *)ptr)[-1]).u.size; INSIST(oldsize >= ALIGNMENT_SIZE); @@ -1019,21 +975,20 @@ isc___mem_reallocate(isc_mem_t *ctx0, void *ptr, size_t size FLARG) { } copysize = (oldsize > size) ? size : oldsize; memmove(new_ptr, ptr, copysize); - isc__mem_free(ctx0, ptr FLARG_PASS); + isc__mem_free(ctx, ptr FLARG_PASS); } } else if (ptr != NULL) { - isc__mem_free(ctx0, ptr FLARG_PASS); + isc__mem_free(ctx, ptr FLARG_PASS); } return (new_ptr); } void -isc___mem_free(isc_mem_t *ctx0, void *ptr FLARG) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc__mem_free(isc_mem_t *ctx, void *ptr FLARG) { + REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(ptr != NULL); - isc__mem_t *ctx = (isc__mem_t *)ctx0; size_info *si; size_t size; bool call_water = false; @@ -1084,17 +1039,16 @@ isc___mem_free(isc_mem_t *ctx0, void *ptr FLARG) { */ char * -isc___mem_strdup(isc_mem_t *mctx0, const char *s FLARG) { - REQUIRE(VALID_CONTEXT(mctx0)); +isc__mem_strdup(isc_mem_t *mctx, const char *s FLARG) { + REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(s != NULL); - isc__mem_t *mctx = (isc__mem_t *)mctx0; size_t len; char *ns; len = strlen(s) + 1; - ns = isc__mem_allocate((isc_mem_t *)mctx, len FLARG_PASS); + ns = isc__mem_allocate(mctx, len FLARG_PASS); if (ns != NULL) { strlcpy(ns, s, len); @@ -1104,11 +1058,10 @@ isc___mem_strdup(isc_mem_t *mctx0, const char *s FLARG) { } char * -isc___mem_strndup(isc_mem_t *mctx0, const char *s, size_t size FLARG) { - REQUIRE(VALID_CONTEXT(mctx0)); +isc__mem_strndup(isc_mem_t *mctx, const char *s, size_t size FLARG) { + REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(s != NULL); - isc__mem_t *mctx = (isc__mem_t *)mctx0; size_t len; char *ns; @@ -1117,7 +1070,7 @@ isc___mem_strndup(isc_mem_t *mctx0, const char *s, size_t size FLARG) { len = size; } - ns = isc__mem_allocate((isc_mem_t *)mctx, len FLARG_PASS); + ns = isc__mem_allocate(mctx, len FLARG_PASS); if (ns != NULL) { strlcpy(ns, s, len); @@ -1127,10 +1080,8 @@ isc___mem_strndup(isc_mem_t *mctx0, const char *s, size_t size FLARG) { } void -isc_mem_setdestroycheck(isc_mem_t *ctx0, bool flag) { - REQUIRE(VALID_CONTEXT(ctx0)); - - isc__mem_t *ctx = (isc__mem_t *)ctx0; +isc_mem_setdestroycheck(isc_mem_t *ctx, bool flag) { + REQUIRE(VALID_CONTEXT(ctx)); MCTXLOCK(ctx); @@ -1140,10 +1091,9 @@ isc_mem_setdestroycheck(isc_mem_t *ctx0, bool flag) { } size_t -isc_mem_inuse(isc_mem_t *ctx0) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc_mem_inuse(isc_mem_t *ctx) { + REQUIRE(VALID_CONTEXT(ctx)); - isc__mem_t *ctx = (isc__mem_t *)ctx0; size_t inuse; MCTXLOCK(ctx); @@ -1156,10 +1106,9 @@ isc_mem_inuse(isc_mem_t *ctx0) { } size_t -isc_mem_maxinuse(isc_mem_t *ctx0) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc_mem_maxinuse(isc_mem_t *ctx) { + REQUIRE(VALID_CONTEXT(ctx)); - isc__mem_t *ctx = (isc__mem_t *)ctx0; size_t maxinuse; MCTXLOCK(ctx); @@ -1172,10 +1121,9 @@ isc_mem_maxinuse(isc_mem_t *ctx0) { } size_t -isc_mem_total(isc_mem_t *ctx0) { - REQUIRE(VALID_CONTEXT(ctx0)); +isc_mem_total(isc_mem_t *ctx) { + REQUIRE(VALID_CONTEXT(ctx)); - isc__mem_t *ctx = (isc__mem_t *)ctx0; size_t total; MCTXLOCK(ctx); @@ -1188,12 +1136,11 @@ isc_mem_total(isc_mem_t *ctx0) { } void -isc_mem_setwater(isc_mem_t *ctx0, isc_mem_water_t water, void *water_arg, +isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg, size_t hiwater, size_t lowater) { - REQUIRE(VALID_CONTEXT(ctx0)); + REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(hiwater >= lowater); - isc__mem_t *ctx = (isc__mem_t *)ctx0; bool callwater = false; isc_mem_water_t oldwater; void *oldwater_arg; @@ -1227,10 +1174,8 @@ isc_mem_setwater(isc_mem_t *ctx0, isc_mem_water_t water, void *water_arg, } ISC_NO_SANITIZE_THREAD bool -isc_mem_isovermem(isc_mem_t *ctx0) { - REQUIRE(VALID_CONTEXT(ctx0)); - - isc__mem_t *ctx = (isc__mem_t *)ctx0; +isc_mem_isovermem(isc_mem_t *ctx) { + REQUIRE(VALID_CONTEXT(ctx)); /* * We don't bother to lock the context because 100% accuracy isn't @@ -1241,10 +1186,8 @@ isc_mem_isovermem(isc_mem_t *ctx0) { } void -isc_mem_setname(isc_mem_t *ctx0, const char *name, void *tag) { - REQUIRE(VALID_CONTEXT(ctx0)); - - isc__mem_t *ctx = (isc__mem_t *)ctx0; +isc_mem_setname(isc_mem_t *ctx, const char *name, void *tag) { + REQUIRE(VALID_CONTEXT(ctx)); LOCK(&ctx->lock); strlcpy(ctx->name, name, sizeof(ctx->name)); @@ -1253,10 +1196,8 @@ isc_mem_setname(isc_mem_t *ctx0, const char *name, void *tag) { } const char * -isc_mem_getname(isc_mem_t *ctx0) { - REQUIRE(VALID_CONTEXT(ctx0)); - - isc__mem_t *ctx = (isc__mem_t *)ctx0; +isc_mem_getname(isc_mem_t *ctx) { + REQUIRE(VALID_CONTEXT(ctx)); if (ctx->name[0] == 0) { return (""); @@ -1266,10 +1207,8 @@ isc_mem_getname(isc_mem_t *ctx0) { } void * -isc_mem_gettag(isc_mem_t *ctx0) { - REQUIRE(VALID_CONTEXT(ctx0)); - - isc__mem_t *ctx = (isc__mem_t *)ctx0; +isc_mem_gettag(isc_mem_t *ctx) { + REQUIRE(VALID_CONTEXT(ctx)); return (ctx->tag); } @@ -1279,22 +1218,20 @@ isc_mem_gettag(isc_mem_t *ctx0) { */ void -isc_mempool_create(isc_mem_t *mctx0, size_t size, isc_mempool_t **mpctxp) { - REQUIRE(VALID_CONTEXT(mctx0)); +isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp) { + REQUIRE(VALID_CONTEXT(mctx)); REQUIRE(size > 0U); REQUIRE(mpctxp != NULL && *mpctxp == NULL); - isc__mem_t *mctx = (isc__mem_t *)mctx0; - isc__mempool_t *mpctx; + isc_mempool_t *mpctx; /* * Allocate space for this pool, initialize values, and if all works * well, attach to the memory context. */ - mpctx = isc_mem_get((isc_mem_t *)mctx, sizeof(isc__mempool_t)); + mpctx = isc_mem_get(mctx, sizeof(isc_mempool_t)); - mpctx->common.impmagic = MEMPOOL_MAGIC; - mpctx->common.magic = ISCAPI_MPOOL_MAGIC; + mpctx->magic = MEMPOOL_MAGIC; mpctx->lock = NULL; mpctx->mctx = mctx; /* @@ -1324,12 +1261,10 @@ isc_mempool_create(isc_mem_t *mctx0, size_t size, isc_mempool_t **mpctxp) { } void -isc_mempool_setname(isc_mempool_t *mpctx0, const char *name) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc_mempool_setname(isc_mempool_t *mpctx, const char *name) { + REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(name != NULL); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - #if ISC_MEMPOOL_NAMES if (mpctx->lock != NULL) { LOCK(mpctx->lock); @@ -1351,12 +1286,13 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { REQUIRE(mpctxp != NULL); REQUIRE(VALID_MEMPOOL(*mpctxp)); - isc__mempool_t *mpctx; - isc__mem_t *mctx; + isc_mempool_t *mpctx; + isc_mem_t *mctx; isc_mutex_t *lock; element *item; - mpctx = (isc__mempool_t *)*mpctxp; + mpctx = *mpctxp; + *mpctxp = NULL; #if ISC_MEMPOOL_NAMES if (mpctx->allocated > 0) { UNEXPECTED_ERROR(__FILE__, __LINE__, @@ -1398,37 +1334,31 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { mctx->poolcnt--; MCTXUNLOCK(mctx); - mpctx->common.impmagic = 0; - mpctx->common.magic = 0; + mpctx->magic = 0; - isc_mem_put((isc_mem_t *)mpctx->mctx, mpctx, sizeof(isc__mempool_t)); + isc_mem_put(mpctx->mctx, mpctx, sizeof(isc_mempool_t)); if (lock != NULL) { UNLOCK(lock); } - - *mpctxp = NULL; } void -isc_mempool_associatelock(isc_mempool_t *mpctx0, isc_mutex_t *lock) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc_mempool_associatelock(isc_mempool_t *mpctx, isc_mutex_t *lock) { + REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(lock != NULL); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - REQUIRE(mpctx->lock == NULL); mpctx->lock = lock; } void * -isc__mempool_get(isc_mempool_t *mpctx0 FLARG) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc__mempool_get(isc_mempool_t *mpctx FLARG) { + REQUIRE(VALID_MEMPOOL(mpctx)); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; element *item; - isc__mem_t *mctx; + isc_mem_t *mctx; unsigned int i; mctx = mpctx->mctx; @@ -1494,12 +1424,11 @@ out: /* coverity[+free : arg-1] */ void -isc__mempool_put(isc_mempool_t *mpctx0, void *mem FLARG) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { + REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(mem != NULL); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - isc__mem_t *mctx = mpctx->mctx; + isc_mem_t *mctx = mpctx->mctx; element *item; if (mpctx->lock != NULL) { @@ -1549,10 +1478,8 @@ isc__mempool_put(isc_mempool_t *mpctx0, void *mem FLARG) { */ void -isc_mempool_setfreemax(isc_mempool_t *mpctx0, unsigned int limit) { - REQUIRE(VALID_MEMPOOL(mpctx0)); - - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; +isc_mempool_setfreemax(isc_mempool_t *mpctx, unsigned int limit) { + REQUIRE(VALID_MEMPOOL(mpctx)); if (mpctx->lock != NULL) { LOCK(mpctx->lock); @@ -1566,10 +1493,9 @@ isc_mempool_setfreemax(isc_mempool_t *mpctx0, unsigned int limit) { } unsigned int -isc_mempool_getfreemax(isc_mempool_t *mpctx0) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc_mempool_getfreemax(isc_mempool_t *mpctx) { + REQUIRE(VALID_MEMPOOL(mpctx)); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; unsigned int freemax; if (mpctx->lock != NULL) { @@ -1586,10 +1512,9 @@ isc_mempool_getfreemax(isc_mempool_t *mpctx0) { } unsigned int -isc_mempool_getfreecount(isc_mempool_t *mpctx0) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc_mempool_getfreecount(isc_mempool_t *mpctx) { + REQUIRE(VALID_MEMPOOL(mpctx)); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; unsigned int freecount; if (mpctx->lock != NULL) { @@ -1606,12 +1531,10 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx0) { } void -isc_mempool_setmaxalloc(isc_mempool_t *mpctx0, unsigned int limit) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit) { + REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(limit > 0); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - if (mpctx->lock != NULL) { LOCK(mpctx->lock); } @@ -1624,10 +1547,9 @@ isc_mempool_setmaxalloc(isc_mempool_t *mpctx0, unsigned int limit) { } unsigned int -isc_mempool_getmaxalloc(isc_mempool_t *mpctx0) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc_mempool_getmaxalloc(isc_mempool_t *mpctx) { + REQUIRE(VALID_MEMPOOL(mpctx)); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; unsigned int maxalloc; if (mpctx->lock != NULL) { @@ -1644,10 +1566,9 @@ isc_mempool_getmaxalloc(isc_mempool_t *mpctx0) { } unsigned int -isc_mempool_getallocated(isc_mempool_t *mpctx0) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc_mempool_getallocated(isc_mempool_t *mpctx) { + REQUIRE(VALID_MEMPOOL(mpctx)); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; unsigned int allocated; if (mpctx->lock != NULL) { @@ -1664,12 +1585,10 @@ isc_mempool_getallocated(isc_mempool_t *mpctx0) { } void -isc_mempool_setfillcount(isc_mempool_t *mpctx0, unsigned int limit) { - REQUIRE(VALID_MEMPOOL(mpctx0)); +isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit) { + REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(limit > 0); - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - if (mpctx->lock != NULL) { LOCK(mpctx->lock); } @@ -1682,10 +1601,8 @@ isc_mempool_setfillcount(isc_mempool_t *mpctx0, unsigned int limit) { } unsigned int -isc_mempool_getfillcount(isc_mempool_t *mpctx0) { - REQUIRE(VALID_MEMPOOL(mpctx0)); - - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; +isc_mempool_getfillcount(isc_mempool_t *mpctx) { + REQUIRE(VALID_MEMPOOL(mpctx)); unsigned int fillcount; @@ -1707,7 +1624,7 @@ isc_mempool_getfillcount(isc_mempool_t *mpctx0) { */ static void print_contexts(FILE *file) { - isc__mem_t *ctx; + isc_mem_t *ctx; for (ctx = ISC_LIST_HEAD(contexts); ctx != NULL; ctx = ISC_LIST_NEXT(ctx, link)) { @@ -1741,8 +1658,7 @@ isc_mem_checkdestroyed(FILE *file) { } unsigned int -isc_mem_references(isc_mem_t *ctx0) { - isc__mem_t *ctx = (isc__mem_t *)ctx0; +isc_mem_references(isc_mem_t *ctx) { return (isc_refcount_current(&ctx->references)); } @@ -1761,8 +1677,7 @@ typedef struct summarystat { goto error; \ } while (0) static int -xml_renderctx(isc__mem_t *ctx, summarystat_t *summary, - xmlTextWriterPtr writer) { +xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { REQUIRE(VALID_CONTEXT(ctx)); int xmlrc; @@ -1850,7 +1765,7 @@ error: int isc_mem_renderxml(void *writer0) { - isc__mem_t *ctx; + isc_mem_t *ctx; summarystat_t summary; uint64_t lost; int xmlrc; @@ -1913,7 +1828,7 @@ error: #define CHECKMEM(m) RUNTIME_CHECK(m != NULL) static isc_result_t -json_renderctx(isc__mem_t *ctx, summarystat_t *summary, json_object *array) { +json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(summary != NULL); REQUIRE(array != NULL); @@ -1997,7 +1912,7 @@ json_renderctx(isc__mem_t *ctx, summarystat_t *summary, json_object *array) { isc_result_t isc_mem_renderjson(void *memobj0) { isc_result_t result = ISC_R_SUCCESS; - isc__mem_t *ctx; + isc_mem_t *ctx; summarystat_t summary; uint64_t lost; json_object *ctxarray, *obj; @@ -2057,73 +1972,15 @@ isc_mem_create(isc_mem_t **mctxp) { mem_create(mctxp, isc_mem_defaultflags); } -void * -isc__mem_get(isc_mem_t *mctx, size_t size FLARG) { - REQUIRE(ISCAPI_MCTX_VALID(mctx)); - - return (mctx->methods->memget(mctx, size FLARG_PASS)); -} - void -isc__mem_put(isc_mem_t *mctx, void *ptr, size_t size FLARG) { - REQUIRE(ISCAPI_MCTX_VALID(mctx)); - - mctx->methods->memput(mctx, ptr, size FLARG_PASS); -} - -void -isc__mem_putanddetach(isc_mem_t **mctxp, void *ptr, size_t size FLARG) { - REQUIRE(mctxp != NULL && ISCAPI_MCTX_VALID(*mctxp)); - - (*mctxp)->methods->memputanddetach(mctxp, ptr, size FLARG_PASS); -} - -void * -isc__mem_allocate(isc_mem_t *mctx, size_t size FLARG) { - REQUIRE(ISCAPI_MCTX_VALID(mctx)); - - return (mctx->methods->memallocate(mctx, size FLARG_PASS)); -} - -void * -isc__mem_reallocate(isc_mem_t *mctx, void *ptr, size_t size FLARG) { - REQUIRE(ISCAPI_MCTX_VALID(mctx)); - - return (mctx->methods->memreallocate(mctx, ptr, size FLARG_PASS)); -} - -char * -isc__mem_strdup(isc_mem_t *mctx, const char *s FLARG) { - REQUIRE(ISCAPI_MCTX_VALID(mctx)); - - return (mctx->methods->memstrdup(mctx, s FLARG_PASS)); -} - -char * -isc__mem_strndup(isc_mem_t *mctx, const char *s, size_t size FLARG) { - REQUIRE(ISCAPI_MCTX_VALID(mctx)); - - return (mctx->methods->memstrndup(mctx, s, size FLARG_PASS)); -} - -void -isc__mem_free(isc_mem_t *mctx, void *ptr FLARG) { - REQUIRE(ISCAPI_MCTX_VALID(mctx)); - - mctx->methods->memfree(mctx, ptr FLARG_PASS); -} - -void -isc__mem_printactive(isc_mem_t *ctx0, FILE *file) { +isc__mem_printactive(isc_mem_t *ctx, FILE *file) { #if ISC_MEM_TRACKLINES - REQUIRE(VALID_CONTEXT(ctx0)); + REQUIRE(VALID_CONTEXT(ctx)); REQUIRE(file != NULL); - isc__mem_t *ctx = (isc__mem_t *)ctx0; - print_active(ctx, file); #else /* if ISC_MEM_TRACKLINES */ - UNUSED(ctx0); + UNUSED(ctx); UNUSED(file); #endif /* if ISC_MEM_TRACKLINES */ } From 0f4413914595a7c27da56125d4c6e385ef199c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 14 Jul 2020 14:24:10 +0200 Subject: [PATCH 03/13] Bump the maximum number of hazard pointers in tests On 24-core machine, the tests would crash because we would run out of the hazard pointers. We now adjust the number of hazard pointers to be in the <128,256> interval based on the number of available cores. Note: This is just a band-aid and needs a proper fix. --- bin/dig/dighost.c | 2 +- bin/named/main.c | 2 +- bin/named/server.c | 4 ++-- lib/dns/resolver.c | 2 +- lib/dns/view.c | 2 +- lib/dns/zone.c | 2 +- lib/isc/include/isc/mem.h | 19 +------------------ lib/isc/mem.c | 12 +----------- lib/isc/tests/isctest.c | 3 +++ lib/isc/win32/libisc.def.in | 1 - lib/ns/client.c | 2 +- 11 files changed, 13 insertions(+), 38 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index a8df930b54..ff72aea4a5 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1401,7 +1401,7 @@ setup_libs(void) { } isc_mem_create(&mctx); - isc_mem_setname(mctx, "dig", NULL); + isc_mem_setname(mctx, "dig"); isc_log_create(mctx, &lctx, &logconfig); isc_log_setcontext(lctx); diff --git a/bin/named/main.c b/bin/named/main.c index 9b7074caa8..2201ce002d 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -1563,7 +1563,7 @@ main(int argc, char *argv[]) { } isc_mem_create(&named_g_mctx); - isc_mem_setname(named_g_mctx, "main", NULL); + isc_mem_setname(named_g_mctx, "main"); setup(); diff --git a/bin/named/server.c b/bin/named/server.c index 83ac8eed7a..8ae900676f 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4617,9 +4617,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * memory. */ isc_mem_create(&cmctx); - isc_mem_setname(cmctx, "cache", NULL); + isc_mem_setname(cmctx, "cache"); isc_mem_create(&hmctx); - isc_mem_setname(hmctx, "cache_heap", NULL); + isc_mem_setname(hmctx, "cache_heap"); CHECK(dns_cache_create(cmctx, hmctx, named_g_taskmgr, named_g_timermgr, view->rdclass, cachename, "rbt", 0, NULL, diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index a91a5d52ff..739f38fda0 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -10409,7 +10409,7 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr, * enabling threads because it will be require more memory. */ isc_mem_create(&res->buckets[i].mctx); - isc_mem_setname(res->buckets[i].mctx, name, NULL); + isc_mem_setname(res->buckets[i].mctx, name); isc_task_setname(res->buckets[i].task, name, res); ISC_LIST_INIT(res->buckets[i].fctxs); atomic_init(&res->buckets[i].exiting, false); diff --git a/lib/dns/view.c b/lib/dns/view.c index 87aaec269a..ae662f850e 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -828,7 +828,7 @@ dns_view_createresolver(dns_view_t *view, isc_taskmgr_t *taskmgr, isc_mem_create(&mctx); result = dns_adb_create(mctx, view, timermgr, taskmgr, &view->adb); - isc_mem_setname(mctx, "ADB", NULL); + isc_mem_setname(mctx, "ADB"); isc_mem_detach(&mctx); if (result != ISC_R_SUCCESS) { dns_resolver_shutdown(view->resolver); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 0269a9b42d..2fe91debe4 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -18358,7 +18358,7 @@ mctxinit(void **target, void *arg) { REQUIRE(target != NULL && *target == NULL); isc_mem_create(&mctx); - isc_mem_setname(mctx, "zonemgr-pool", NULL); + isc_mem_setname(mctx, "zonemgr-pool"); *target = mctx; return (ISC_R_SUCCESS); diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 3e789ef34d..01dac4c2e0 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -316,7 +316,7 @@ isc_mem_references(isc_mem_t *ctx); */ void -isc_mem_setname(isc_mem_t *ctx, const char *name, void *tag); +isc_mem_setname(isc_mem_t *ctx, const char *name); /*%< * Name 'ctx'. * @@ -324,8 +324,6 @@ isc_mem_setname(isc_mem_t *ctx, const char *name, void *tag); * *\li Only the first 15 characters of 'name' will be copied. * - *\li 'tag' is for debugging purposes only. - * * Requires: * *\li 'ctx' is a valid ctx. @@ -345,21 +343,6 @@ isc_mem_getname(isc_mem_t *ctx); * empty. */ -void * -isc_mem_gettag(isc_mem_t *ctx); -/*%< - * Get the tag value for 'task', as previously set using isc_mem_setname(). - * - * Requires: - *\li 'ctx' is a valid ctx. - * - * Notes: - *\li This function is for debugging purposes only. - * - * Requires: - *\li 'ctx' is a valid task. - */ - #ifdef HAVE_LIBXML2 int isc_mem_renderxml(void *writer0); diff --git a/lib/isc/mem.c b/lib/isc/mem.c index e4c5a8ab04..7a0c650247 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -128,7 +128,6 @@ struct isc_mem { struct stats *stats; isc_refcount_t references; char name[16]; - void *tag; size_t total; size_t inuse; size_t maxinuse; @@ -463,7 +462,6 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { ctx->flags = flags; isc_refcount_init(&ctx->references, 1); memset(ctx->name, 0, sizeof(ctx->name)); - ctx->tag = NULL; ctx->total = 0; ctx->inuse = 0; ctx->maxinuse = 0; @@ -1186,12 +1184,11 @@ isc_mem_isovermem(isc_mem_t *ctx) { } void -isc_mem_setname(isc_mem_t *ctx, const char *name, void *tag) { +isc_mem_setname(isc_mem_t *ctx, const char *name) { REQUIRE(VALID_CONTEXT(ctx)); LOCK(&ctx->lock); strlcpy(ctx->name, name, sizeof(ctx->name)); - ctx->tag = tag; UNLOCK(&ctx->lock); } @@ -1206,13 +1203,6 @@ isc_mem_getname(isc_mem_t *ctx) { return (ctx->name); } -void * -isc_mem_gettag(isc_mem_t *ctx) { - REQUIRE(VALID_CONTEXT(ctx)); - - return (ctx->tag); -} - /* * Memory pool stuff */ diff --git a/lib/isc/tests/isctest.c b/lib/isc/tests/isctest.c index 66c70e5004..bf33660f98 100644 --- a/lib/isc/tests/isctest.c +++ b/lib/isc/tests/isctest.c @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -85,6 +86,8 @@ create_managers(unsigned int workers) { workers = atoi(p); } + isc_hp_init(ISC_MAX(ISC_MIN(workers, 256), 128)); + netmgr = isc_nm_start(test_mctx, workers); CHECK(isc_taskmgr_create(test_mctx, workers, 0, netmgr, &taskmgr)); CHECK(isc_task_create(taskmgr, 0, &maintask)); diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index e460d5757e..7bc4926a96 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -368,7 +368,6 @@ isc_mem_create isc_mem_destroy isc_mem_detach isc_mem_getname -isc_mem_gettag isc_mem_inuse isc_mem_isovermem isc_mem_maxinuse diff --git a/lib/ns/client.c b/lib/ns/client.c index edd59197f2..f878247f55 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -2506,7 +2506,7 @@ ns_clientmgr_create(isc_mem_t *mctx, ns_server_t *sctx, isc_taskmgr_t *taskmgr, for (i = 0; i < npools; i++) { manager->mctxpool[i] = NULL; isc_mem_create(&manager->mctxpool[i]); - isc_mem_setname(manager->mctxpool[i], "client", NULL); + isc_mem_setname(manager->mctxpool[i], "client"); } manager->magic = MANAGER_MAGIC; From b09106e93aaa61d195f9bffe1bb493764dd8da1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 4 Feb 2021 21:56:49 +0100 Subject: [PATCH 04/13] Make the memory and mempool counters to be stdatomic types This is yet another step into unlocking some parts of the memory contexts. All the regularly updated variables has been turned into atomic types, so we can later remove the locks when updating various counters. Also unlock as much code as possible without breaking anything. --- lib/isc/include/isc/mem.h | 64 +-- lib/isc/mem.c | 836 +++++++++++++++--------------------- lib/isc/win32/libisc.def.in | 2 + 3 files changed, 394 insertions(+), 508 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 01dac4c2e0..2d4f70dedd 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -250,6 +250,19 @@ isc_mem_total(isc_mem_t *mctx); * not yet used. */ +size_t +isc_mem_malloced(isc_mem_t *ctx); +/*%< + * Get an estimate of the amount of memory allocated in 'mctx', in bytes. + */ + +size_t +isc_mem_maxmalloced(isc_mem_t *ctx); +/*%< + * Get an estimate of the largest amount of memory that has been + * allocated in 'mctx' at any time. + */ + bool isc_mem_isovermem(isc_mem_t *mctx); /*%< @@ -265,13 +278,13 @@ isc_mem_setwater(isc_mem_t *mctx, isc_mem_water_t water, void *water_arg, * Set high and low water marks for this memory context. * * When the memory usage of 'mctx' exceeds 'hiwater', - * '(water)(water_arg, #ISC_MEM_HIWATER)' will be called. 'water' needs to - * call isc_mem_waterack() with #ISC_MEM_HIWATER to acknowledge the state - * change. 'water' may be called multiple times. + * '(water)(water_arg, #ISC_MEM_HIWATER)' will be called. 'water' needs + *to call isc_mem_waterack() with #ISC_MEM_HIWATER to acknowledge the + *state change. 'water' may be called multiple times. * - * When the usage drops below 'lowater', 'water' will again be called, this - * time with #ISC_MEM_LOWATER. 'water' need to calls isc_mem_waterack() with - * #ISC_MEM_LOWATER to acknowledge the change. + * When the usage drops below 'lowater', 'water' will again be called, + *this time with #ISC_MEM_LOWATER. 'water' need to calls + *isc_mem_waterack() with #ISC_MEM_LOWATER to acknowledge the change. * * static void * water(void *arg, int mark) { @@ -396,7 +409,8 @@ isc_mempool_destroy(isc_mempool_t **mpctxp); void isc_mempool_setname(isc_mempool_t *mpctx, const char *name); /*%< - * Associate a name with a memory pool. At most 15 characters may be used. + * Associate a name with a memory pool. At most 15 characters may be + *used. * * Requires: *\li mpctx is a valid pool. @@ -408,15 +422,15 @@ 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. + * 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. + * 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: * @@ -426,19 +440,19 @@ isc_mempool_associatelock(isc_mempool_t *mpctx, isc_mutex_t *lock); * *\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. + *\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 unless - * the imposed externally provided locking protocols are followed. + * the unlocked nature of pools these are potentially random values + *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. + * 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. * * All functions require (in addition to other requirements): * mpctx is a valid memory pool @@ -486,8 +500,8 @@ isc_mempool_getallocated(isc_mempool_t *mpctx); unsigned int isc_mempool_getfillcount(isc_mempool_t *mpctx); /*%< - * Returns the number of items allocated as a block from the parent memory - * context when the free list is empty. + * Returns the number of items allocated as a block from the parent + * memory context when the free list is empty. */ void diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 7a0c650247..c3c4ac463a 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -45,6 +46,14 @@ #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 @@ -56,9 +65,11 @@ LIBISC_EXTERNAL_DATA unsigned int isc_mem_defaultflags = ISC_MEMFLAG_DEFAULT; * Constants. */ -#define DEF_MAX_SIZE 1100 -#define ALIGNMENT_SIZE 8U /*%< must be a power of 2 */ +#define ALIGNMENT 8U /*%< must be a power of 2 */ +#define ALIGNMENT_SIZE sizeof(size_info) #define DEBUG_TABLE_COUNT 512U +#define STATS_BUCKETS 512U +#define STATS_BUCKET_SIZE 32U /* * Types. @@ -88,19 +99,15 @@ struct element { }; typedef struct { - /*! - * This structure must be ALIGNMENT_SIZE bytes. - */ - union { + alignas(ALIGNMENT) union { size_t size; isc_mem_t *ctx; - char bytes[ALIGNMENT_SIZE]; - } u; + }; } size_info; struct stats { - unsigned long gets; - unsigned long totalgets; + atomic_size_t gets; + atomic_size_t totalgets; }; #define MEM_MAGIC ISC_MAGIC('M', 'e', 'm', 'C') @@ -123,20 +130,19 @@ struct isc_mem { unsigned int magic; unsigned int flags; isc_mutex_t lock; - size_t max_size; bool checkfree; - struct stats *stats; + struct stats stats[STATS_BUCKETS + 1]; isc_refcount_t references; char name[16]; - size_t total; - size_t inuse; - size_t maxinuse; - size_t malloced; - size_t maxmalloced; - size_t hi_water; - size_t lo_water; - bool hi_called; - bool is_overmem; + atomic_size_t total; + atomic_size_t inuse; + 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; ISC_LIST(isc_mempool_t) pools; @@ -161,16 +167,16 @@ struct isc_mempool { /*%< locked via the memory context's lock */ 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 */ - unsigned int maxalloc; /*%< max number of items allowed */ - unsigned int allocated; /*%< # of items currently given out */ - unsigned int freecount; /*%< # of items on reserved list */ - unsigned int freemax; /*%< # of items allowed on free list */ - unsigned int 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. */ - unsigned int gets; /*%< # of requests to this pool */ - /*%< Debugging only. */ + atomic_size_t gets; /*%< # of requests to this pool */ + /*%< Debugging only. */ #if ISC_MEMPOOL_NAMES char name[16]; /*%< printed name in stats reports */ #endif /* if ISC_MEMPOOL_NAMES */ @@ -186,27 +192,48 @@ struct isc_mempool { #define ISC_MEMFUNC_SCOPE #else /* if !ISC_MEM_TRACKLINES */ #define TRACE_OR_RECORD (ISC_MEM_DEBUGTRACE | ISC_MEM_DEBUGRECORD) -#define ADD_TRACE(a, b, c, d, e) \ - do { \ - if (ISC_UNLIKELY((isc_mem_debugging & TRACE_OR_RECORD) != 0 && \ - b != NULL)) \ - add_trace_entry(a, b, c, d, e); \ - } while (0) -#define DELETE_TRACE(a, b, c, d, e) \ - do { \ - if (ISC_UNLIKELY((isc_mem_debugging & TRACE_OR_RECORD) != 0 && \ - b != NULL)) \ - delete_trace_entry(a, b, c, d, e); \ - } while (0) + +#define SHOULD_TRACE_OR_RECORD(ptr) \ + (ISC_UNLIKELY((isc_mem_debugging & TRACE_OR_RECORD) != 0) && \ + ptr != NULL) + +#define ADD_TRACE(a, b, c, d, e) \ + if (SHOULD_TRACE_OR_RECORD(b)) { \ + add_trace_entry(a, b, c, d, e); \ + } + +#define DELETE_TRACE(a, b, c, d, e) \ + if (SHOULD_TRACE_OR_RECORD(b)) { \ + delete_trace_entry(a, b, c, d, e); \ + } static void print_active(isc_mem_t *ctx, FILE *out); - #endif /* ISC_MEM_TRACKLINES */ +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); + } + + return (malloced); +} + +static inline size_t +decrement_malloced(isc_mem_t *ctx, size_t size) { + size_t malloced = atomic_fetch_sub_release(&ctx->malloced, size) - size; + INSIST(size >= 0); + + return (malloced); +} + #if ISC_MEM_TRACKLINES /*! - * mctx must be locked. + * mctx must not be locked. */ static void add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { @@ -214,13 +241,15 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { uint32_t hash; uint32_t idx; + MCTXLOCK(mctx); + if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { fprintf(stderr, "add %p size %zu file %s line %u mctx %p\n", ptr, size, file, line, mctx); } if (mctx->debuglist == NULL) { - return; + goto unlock; } #ifdef __COVERITY__ @@ -236,10 +265,7 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { dl = malloc(sizeof(debuglink_t)); INSIST(dl != NULL); - mctx->malloced += sizeof(debuglink_t); - if (mctx->malloced > mctx->maxmalloced) { - mctx->maxmalloced = mctx->malloced; - } + increment_malloced(mctx, sizeof(debuglink_t)); ISC_LINK_INIT(dl, link); dl->ptr = ptr; @@ -249,6 +275,8 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { ISC_LIST_PREPEND(mctx->debuglist[idx], dl, link); mctx->debuglistcnt++; +unlock: + MCTXUNLOCK(mctx); } static void @@ -258,13 +286,15 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, uint32_t hash; uint32_t idx; + MCTXLOCK(mctx); + if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { fprintf(stderr, "del %p size %zu file %s line %u mctx %p\n", ptr, size, file, line, mctx); } if (mctx->debuglist == NULL) { - return; + goto unlock; } #ifdef __COVERITY__ @@ -282,9 +312,9 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, while (ISC_LIKELY(dl != NULL)) { if (ISC_UNLIKELY(dl->ptr == ptr)) { ISC_LIST_UNLINK(mctx->debuglist[idx], dl, link); - mctx->malloced -= sizeof(*dl); + decrement_malloced(mctx, sizeof(*dl)); free(dl); - return; + goto unlock; } dl = ISC_LIST_NEXT(dl, link); } @@ -295,6 +325,8 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, */ INSIST(0); ISC_UNREACHABLE(); +unlock: + MCTXUNLOCK(mctx); } #endif /* ISC_MEM_TRACKLINES */ @@ -348,29 +380,25 @@ mem_put(isc_mem_t *ctx, void *mem, size_t size) { default_memfree(mem); } +#define stats_bucket(ctx, size) \ + ((size / STATS_BUCKET_SIZE) >= STATS_BUCKETS \ + ? &ctx->stats[STATS_BUCKETS] \ + : &ctx->stats[size / STATS_BUCKET_SIZE]) + /*! * Update internal counters after a memory get. */ static inline void mem_getstats(isc_mem_t *ctx, size_t size) { - ctx->total += size; - ctx->inuse += size; + struct stats *stats = stats_bucket(ctx, size); - if (size > ctx->max_size) { - ctx->stats[ctx->max_size].gets++; - ctx->stats[ctx->max_size].totalgets++; - } else { - ctx->stats[size].gets++; - ctx->stats[size].totalgets++; - } + atomic_fetch_add_relaxed(&ctx->total, size); + atomic_fetch_add_release(&ctx->inuse, size); -#if ISC_MEM_CHECKOVERRUN - size += 1; -#endif /* if ISC_MEM_CHECKOVERRUN */ - ctx->malloced += size; - if (ctx->malloced > ctx->maxmalloced) { - ctx->maxmalloced = ctx->malloced; - } + atomic_fetch_add_relaxed(&stats->gets, 1); + atomic_fetch_add_relaxed(&stats->totalgets, 1); + + increment_malloced(ctx, size); } /*! @@ -378,22 +406,15 @@ mem_getstats(isc_mem_t *ctx, size_t size) { */ static inline void mem_putstats(isc_mem_t *ctx, void *ptr, size_t size) { + struct stats *stats = stats_bucket(ctx, size); + UNUSED(ptr); - INSIST(ctx->inuse >= size); - ctx->inuse -= size; + INSIST(atomic_fetch_sub_release(&ctx->inuse, size) >= size); - if (size > ctx->max_size) { - INSIST(ctx->stats[ctx->max_size].gets > 0U); - ctx->stats[ctx->max_size].gets--; - } else { - INSIST(ctx->stats[size].gets > 0U); - ctx->stats[size].gets--; - } -#if ISC_MEM_CHECKOVERRUN - size += 1; -#endif /* if ISC_MEM_CHECKOVERRUN */ - ctx->malloced -= size; + INSIST(atomic_fetch_sub_release(&stats->gets, 1) >= 1); + + decrement_malloced(ctx, size); } /* @@ -450,45 +471,30 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { isc_mem_t *ctx; STATIC_ASSERT((ALIGNMENT_SIZE & (ALIGNMENT_SIZE - 1)) == 0, - "wrong alignment size"); + "alignment size not power of 2"); + STATIC_ASSERT(ALIGNMENT_SIZE >= sizeof(size_info), + "alignment size too small"); RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS); - ctx = (default_memalloc)(sizeof(*ctx)); + ctx = default_memalloc(sizeof(*ctx)); + + *ctx = (isc_mem_t){ + .magic = MEM_MAGIC, + .flags = flags, + .checkfree = true, + }; isc_mutex_init(&ctx->lock); - - ctx->max_size = DEF_MAX_SIZE; - ctx->flags = flags; isc_refcount_init(&ctx->references, 1); - memset(ctx->name, 0, sizeof(ctx->name)); - ctx->total = 0; - ctx->inuse = 0; - ctx->maxinuse = 0; - ctx->malloced = sizeof(*ctx); - ctx->maxmalloced = sizeof(*ctx); - ctx->hi_water = 0; - ctx->lo_water = 0; - ctx->hi_called = false; - ctx->is_overmem = false; - ctx->water = NULL; - ctx->water_arg = NULL; - ctx->magic = MEM_MAGIC; - ctx->stats = NULL; - ctx->checkfree = true; -#if ISC_MEM_TRACKLINES - ctx->debuglist = NULL; - ctx->debuglistcnt = 0; -#endif /* if ISC_MEM_TRACKLINES */ + + atomic_init(&ctx->total, 0); + atomic_init(&ctx->inuse, 0); + atomic_init(&ctx->maxinuse, 0); + atomic_init(&ctx->malloced, sizeof(*ctx)); + atomic_init(&ctx->maxmalloced, sizeof(*ctx)); + ISC_LIST_INIT(ctx->pools); - ctx->poolcnt = 0; - - ctx->stats = - default_memalloc((ctx->max_size + 1) * sizeof(struct stats)); - - memset(ctx->stats, 0, (ctx->max_size + 1) * sizeof(struct stats)); - ctx->malloced += (ctx->max_size + 1) * sizeof(struct stats); - ctx->maxmalloced += (ctx->max_size + 1) * sizeof(struct stats); #if ISC_MEM_TRACKLINES if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGRECORD) != 0)) { @@ -499,8 +505,8 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { for (i = 0; i < DEBUG_TABLE_COUNT; i++) { ISC_LIST_INIT(ctx->debuglist[i]); } - ctx->malloced += DEBUG_TABLE_COUNT * sizeof(debuglist_t); - ctx->maxmalloced += DEBUG_TABLE_COUNT * sizeof(debuglist_t); + increment_malloced(ctx, + DEBUG_TABLE_COUNT * sizeof(debuglist_t)); } #endif /* if ISC_MEM_TRACKLINES */ @@ -518,10 +524,11 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { static void destroy(isc_mem_t *ctx) { unsigned int i; + size_t malloced; LOCK(&contextslock); ISC_LIST_UNLINK(contexts, ctx, link); - totallost += ctx->inuse; + totallost += isc_mem_inuse(ctx); UNLOCK(&contextslock); ctx->magic = 0; @@ -542,39 +549,40 @@ destroy(isc_mem_t *ctx) { ISC_LIST_UNLINK(ctx->debuglist[i], dl, link); free(dl); - ctx->malloced -= sizeof(*dl); + decrement_malloced(ctx, sizeof(*dl)); } } default_memfree(ctx->debuglist); - ctx->malloced -= DEBUG_TABLE_COUNT * sizeof(debuglist_t); + decrement_malloced(ctx, + DEBUG_TABLE_COUNT * sizeof(debuglist_t)); } #endif /* if ISC_MEM_TRACKLINES */ if (ctx->checkfree) { - for (i = 0; i <= ctx->max_size; i++) { - if (ctx->stats[i].gets != 0U) { + for (i = 0; i <= STATS_BUCKETS; i++) { + struct stats *stats = &ctx->stats[i]; + size_t gets = atomic_load_acquire(&stats->gets); + if (gets != 0U) { fprintf(stderr, "Failing assertion due to probable " "leaked memory in context %p (\"%s\") " - "(stats[%u].gets == %lu).\n", - ctx, ctx->name, i, ctx->stats[i].gets); + "(stats[%u].gets == %zu).\n", + ctx, ctx->name, i, gets); #if ISC_MEM_TRACKLINES print_active(ctx, stderr); #endif /* if ISC_MEM_TRACKLINES */ - INSIST(ctx->stats[i].gets == 0U); + INSIST(gets == 0U); } } } - default_memfree(ctx->stats); - ctx->malloced -= (ctx->max_size + 1) * sizeof(struct stats); - isc_mutex_destroy(&ctx->lock); - ctx->malloced -= sizeof(*ctx); + malloced = decrement_malloced(ctx, sizeof(*ctx)); + if (ctx->checkfree) { - INSIST(ctx->malloced == 0); + INSIST(malloced == 0); } default_memfree(ctx); } @@ -625,7 +633,7 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { { if ((isc_mem_debugging & ISC_MEM_DEBUGSIZE) != 0) { size_info *si = &(((size_info *)ptr)[-1]); - size_t oldsize = si->u.size - ALIGNMENT_SIZE; + size_t oldsize = si->size - ALIGNMENT_SIZE; if ((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0) { oldsize -= ALIGNMENT_SIZE; } @@ -636,13 +644,10 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) { goto destroy; } - MCTXLOCK(ctx); - DELETE_TRACE(ctx, ptr, size, file, line); mem_putstats(ctx, ptr, size); mem_put(ctx, ptr, size); - MCTXUNLOCK(ctx); destroy: if (isc_refcount_decrement(&ctx->references) == 1) { @@ -675,6 +680,55 @@ isc_mem_destroy(isc_mem_t **ctxp) { *ctxp = NULL; } +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); + + if (hi_water != 0U && inuse > hi_water) { + atomic_store(&ctx->is_overmem, true); + if (!atomic_load_acquire(&ctx->hi_called)) { + call_water = true; + } + } + 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) + { + 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; + } + } + + return (call_water); +} + void * isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); @@ -689,27 +743,11 @@ isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { } ptr = mem_get(ctx, size); - MCTXLOCK(ctx); mem_getstats(ctx, size); ADD_TRACE(ctx, ptr, size, file, line); - if (ctx->hi_water != 0U && ctx->inuse > ctx->hi_water) { - ctx->is_overmem = true; - if (!ctx->hi_called) { - call_water = true; - } - } - if (ctx->inuse > ctx->maxinuse) { - ctx->maxinuse = ctx->inuse; - if (ctx->hi_water != 0U && ctx->inuse > ctx->hi_water && - (isc_mem_debugging & ISC_MEM_DEBUGUSAGE) != 0) - { - fprintf(stderr, "maxinuse = %lu\n", - (unsigned long)ctx->inuse); - } - } - MCTXUNLOCK(ctx); + call_water = hi_water(ctx); if (call_water && (ctx->water != NULL)) { (ctx->water)(ctx->water_arg, ISC_MEM_HIWATER); @@ -725,14 +763,14 @@ isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) { bool call_water = false; size_info *si; - size_t oldsize; 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->u.size - ALIGNMENT_SIZE; + oldsize = si->size - ALIGNMENT_SIZE; if ((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0) { oldsize -= ALIGNMENT_SIZE; } @@ -742,26 +780,12 @@ isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) { return; } - MCTXLOCK(ctx); - DELETE_TRACE(ctx, ptr, size, file, line); mem_putstats(ctx, ptr, size); mem_put(ctx, ptr, size); - /* - * 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. - */ - if ((ctx->inuse < ctx->lo_water) || (ctx->lo_water == 0U)) { - ctx->is_overmem = false; - if (ctx->hi_called) { - call_water = true; - } - } - - MCTXUNLOCK(ctx); + call_water = lo_water(ctx); if (call_water && (ctx->water != NULL)) { (ctx->water)(ctx->water_arg, ISC_MEM_LOWATER); @@ -772,13 +796,11 @@ void isc_mem_waterack(isc_mem_t *ctx, int flag) { REQUIRE(VALID_CONTEXT(ctx)); - MCTXLOCK(ctx); if (flag == ISC_MEM_LOWATER) { - ctx->hi_called = false; + atomic_store(&ctx->hi_called, false); } else if (flag == ISC_MEM_HIWATER) { - ctx->hi_called = true; + atomic_store(&ctx->hi_called, true); } - MCTXUNLOCK(ctx); } #if ISC_MEM_TRACKLINES @@ -789,7 +811,8 @@ print_active(isc_mem_t *mctx, FILE *out) { unsigned int i; bool found; - fprintf(out, "Dump of all outstanding memory allocations:\n"); + fprintf(out, "Dump of all outstanding memory " + "allocations:\n"); found = false; for (i = 0; i < DEBUG_TABLE_COUNT; i++) { dl = ISC_LIST_HEAD(mctx->debuglist[i]); @@ -801,7 +824,8 @@ print_active(isc_mem_t *mctx, FILE *out) { while (dl != NULL) { if (dl->ptr != NULL) { fprintf(out, - "\tptr %p size %zu file %s " + "\tptr %p size %zu " + "file %s " "line %u\n", dl->ptr, dl->size, dl->file, dl->line); @@ -824,30 +848,32 @@ void isc_mem_stats(isc_mem_t *ctx, FILE *out) { REQUIRE(VALID_CONTEXT(ctx)); - size_t i; - const struct stats *s; - const isc_mempool_t *pool; + isc_mempool_t *pool; MCTXLOCK(ctx); - for (i = 0; i <= ctx->max_size; i++) { - s = &ctx->stats[i]; + for (size_t i = 0; i <= STATS_BUCKETS; i++) { + size_t totalgets; + size_t gets; + struct stats *stats = &ctx->stats[i]; - if (s->totalgets == 0U && s->gets == 0U) { - continue; + totalgets = atomic_load_acquire(&stats->totalgets); + gets = atomic_load_acquire(&stats->gets); + + if (totalgets != 0U && gets != 0U) { + fprintf(out, "%s%5zu: %11zu gets, %11zu rem", + (i == STATS_BUCKETS) ? ">=" : " ", i, + totalgets, gets); + fputc('\n', out); } - fprintf(out, "%s%5lu: %11lu gets, %11lu rem", - (i == ctx->max_size) ? ">=" : " ", (unsigned long)i, - s->totalgets, s->gets); - fputc('\n', out); } /* - * Note that since a pool can be locked now, these stats might be - * somewhat off if the pool is in active use at the time the stats - * are dumped. The link fields are protected by the isc_mem_t's - * lock, however, so walking this list and extracting integers from - * stats fields is always safe. + * Note that since a pool can be locked now, these stats might + * be somewhat off if the pool is in active use at the time the + * stats are dumped. The link fields are protected by the + * isc_mem_t's lock, however, so walking this list and + * extracting integers from stats fields is always safe. */ pool = ISC_LIST_HEAD(ctx->pools); if (pool != NULL) { @@ -857,15 +883,19 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { "freemax", "fillcount", "gets", "L"); } while (pool != NULL) { - fprintf(out, "%15s %10lu %10u %10u %10u %10u %10u %10u %s\n", + fprintf(out, + "%15s %10zu %10zu %10zu %10zu %10zu %10zu %10zu %s\n", #if ISC_MEMPOOL_NAMES pool->name, #else /* if ISC_MEMPOOL_NAMES */ "(not tracked)", #endif /* if ISC_MEMPOOL_NAMES */ - (unsigned long)pool->size, pool->maxalloc, - pool->allocated, pool->freecount, pool->freemax, - pool->fillcount, pool->gets, + pool->size, atomic_load_relaxed(&pool->maxalloc), + 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), (pool->lock == NULL ? "N" : "Y")); pool = ISC_LIST_NEXT(pool, link); } @@ -894,10 +924,10 @@ mem_allocateunlocked(isc_mem_t *ctx, size_t size) { si = mem_get(ctx, size); if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0)) { - si->u.ctx = ctx; + si->ctx = ctx; si++; } - si->u.size = size; + si->size = size; return (&si[1]); } @@ -908,34 +938,14 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size FLARG) { size_info *si; bool call_water = false; - MCTXLOCK(ctx); si = mem_allocateunlocked(ctx, size); - mem_getstats(ctx, si[-1].u.size); + mem_getstats(ctx, si[-1].size); - ADD_TRACE(ctx, si, si[-1].u.size, file, line); - if (ctx->hi_water != 0U && ctx->inuse > ctx->hi_water && - !ctx->is_overmem) { - ctx->is_overmem = true; - } + ADD_TRACE(ctx, si, si[-1].size, file, line); - if (ctx->hi_water != 0U && !ctx->hi_called && - ctx->inuse > ctx->hi_water) { - ctx->hi_called = true; - call_water = true; - } - if (ctx->inuse > ctx->maxinuse) { - ctx->maxinuse = ctx->inuse; - if (ISC_UNLIKELY(ctx->hi_water != 0U && - ctx->inuse > ctx->hi_water && - (isc_mem_debugging & ISC_MEM_DEBUGUSAGE) != 0)) - { - fprintf(stderr, "maxinuse = %lu\n", - (unsigned long)ctx->inuse); - } - } - MCTXUNLOCK(ctx); + call_water = hi_water(ctx); - if (call_water) { + if (call_water && (ctx->water != NULL)) { (ctx->water)(ctx->water_arg, ISC_MEM_HIWATER); } @@ -947,15 +957,16 @@ isc__mem_reallocate(isc_mem_t *ctx, void *ptr, size_t size FLARG) { REQUIRE(VALID_CONTEXT(ctx)); void *new_ptr = NULL; - size_t oldsize, copysize; /* - * 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 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. @@ -963,7 +974,8 @@ isc__mem_reallocate(isc_mem_t *ctx, void *ptr, size_t size FLARG) { if (size > 0U) { new_ptr = isc__mem_allocate(ctx, size FLARG_PASS); if (new_ptr != NULL && ptr != NULL) { - oldsize = (((size_info *)ptr)[-1]).u.size; + size_t oldsize, copysize; + oldsize = (((size_info *)ptr)[-1]).size; INSIST(oldsize >= ALIGNMENT_SIZE); oldsize -= ALIGNMENT_SIZE; if (ISC_UNLIKELY((isc_mem_debugging & @@ -993,41 +1005,21 @@ isc__mem_free(isc_mem_t *ctx, void *ptr FLARG) { if (ISC_UNLIKELY((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0)) { si = &(((size_info *)ptr)[-2]); - REQUIRE(si->u.ctx == ctx); - size = si[1].u.size; + REQUIRE(si->ctx == ctx); + size = si[1].size; } else { si = &(((size_info *)ptr)[-1]); - size = si->u.size; + size = si->size; } - MCTXLOCK(ctx); - DELETE_TRACE(ctx, ptr, size, file, line); mem_putstats(ctx, si, size); mem_put(ctx, si, size); - /* - * 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. - */ - if (ctx->is_overmem && - (ctx->inuse < ctx->lo_water || ctx->lo_water == 0U)) { - ctx->is_overmem = false; - } + call_water = lo_water(ctx); - if (ctx->hi_called && - (ctx->inuse < ctx->lo_water || ctx->lo_water == 0U)) { - ctx->hi_called = false; - - if (ctx->water != NULL) { - call_water = true; - } - } - MCTXUNLOCK(ctx); - - if (call_water) { + if (call_water && (ctx->water != NULL)) { (ctx->water)(ctx->water_arg, ISC_MEM_LOWATER); } } @@ -1092,45 +1084,35 @@ size_t isc_mem_inuse(isc_mem_t *ctx) { REQUIRE(VALID_CONTEXT(ctx)); - size_t inuse; - - MCTXLOCK(ctx); - - inuse = ctx->inuse; - - MCTXUNLOCK(ctx); - - return (inuse); + return (atomic_load_acquire(&ctx->inuse)); } size_t isc_mem_maxinuse(isc_mem_t *ctx) { REQUIRE(VALID_CONTEXT(ctx)); - size_t maxinuse; - - MCTXLOCK(ctx); - - maxinuse = ctx->maxinuse; - - MCTXUNLOCK(ctx); - - return (maxinuse); + return (atomic_load_acquire(&ctx->maxinuse)); } size_t isc_mem_total(isc_mem_t *ctx) { REQUIRE(VALID_CONTEXT(ctx)); - size_t total; + return (atomic_load_acquire(&ctx->total)); +} - MCTXLOCK(ctx); +size_t +isc_mem_malloced(isc_mem_t *ctx) { + REQUIRE(VALID_CONTEXT(ctx)); - total = ctx->total; + return (atomic_load_acquire(&ctx->malloced)); +} - MCTXUNLOCK(ctx); +size_t +isc_mem_maxmalloced(isc_mem_t *ctx) { + REQUIRE(VALID_CONTEXT(ctx)); - return (total); + return (atomic_load_acquire(&ctx->maxmalloced)); } void @@ -1147,22 +1129,23 @@ isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg, oldwater = ctx->water; oldwater_arg = ctx->water_arg; if (water == NULL) { - callwater = ctx->hi_called; + callwater = atomic_load(&ctx->hi_called); ctx->water = NULL; ctx->water_arg = NULL; - ctx->hi_water = 0; - ctx->lo_water = 0; + atomic_store_release(&ctx->hi_water, 0); + atomic_store_release(&ctx->lo_water, 0); } else { - if (ctx->hi_called && + if (atomic_load_acquire(&ctx->hi_called) && (ctx->water != water || ctx->water_arg != water_arg || - ctx->inuse < lowater || lowater == 0U)) + atomic_load_acquire(&ctx->inuse) < lowater || + lowater == 0U)) { callwater = true; } ctx->water = water; ctx->water_arg = water_arg; - ctx->hi_water = hiwater; - ctx->lo_water = lowater; + atomic_store_release(&ctx->hi_water, hiwater); + atomic_store_release(&ctx->lo_water, lowater); } MCTXUNLOCK(ctx); @@ -1171,16 +1154,11 @@ isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg, } } -ISC_NO_SANITIZE_THREAD bool +bool isc_mem_isovermem(isc_mem_t *ctx) { REQUIRE(VALID_CONTEXT(ctx)); - /* - * We don't bother to lock the context because 100% accuracy isn't - * necessary (and even if we locked the context the returned value - * could be different from the actual state when it's used anyway) - */ - return (ctx->is_overmem); + return (atomic_load_relaxed(&ctx->is_overmem)); } void @@ -1215,32 +1193,30 @@ isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp) { isc_mempool_t *mpctx; - /* - * Allocate space for this pool, initialize values, and if all works - * well, attach to the memory context. - */ - mpctx = isc_mem_get(mctx, sizeof(isc_mempool_t)); - - mpctx->magic = MEMPOOL_MAGIC; - mpctx->lock = NULL; - mpctx->mctx = mctx; /* * Mempools are stored as a linked list of element. */ if (size < sizeof(element)) { size = sizeof(element); } - mpctx->size = size; - mpctx->maxalloc = UINT_MAX; - mpctx->allocated = 0; - mpctx->freecount = 0; - mpctx->freemax = 1; - mpctx->fillcount = 1; - mpctx->gets = 0; -#if ISC_MEMPOOL_NAMES - mpctx->name[0] = 0; -#endif /* if ISC_MEMPOOL_NAMES */ - mpctx->items = NULL; + + /* + * Allocate space for this pool, initialize values, and if all + * works well, attach to the memory context. + */ + mpctx = isc_mem_get(mctx, sizeof(isc_mempool_t)); + + *mpctx = (isc_mempool_t){ + .magic = MEMPOOL_MAGIC, + .mctx = mctx, + .size = size, + }; + + atomic_init(&mpctx->maxalloc, SIZE_MAX); + atomic_init(&mpctx->allocated, 0); + atomic_init(&mpctx->freecount, 0); + atomic_init(&mpctx->freemax, 1); + atomic_init(&mpctx->fillcount, 1); *mpctxp = (isc_mempool_t *)mpctx; @@ -1256,15 +1232,12 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name) { REQUIRE(name != NULL); #if ISC_MEMPOOL_NAMES - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } + MPCTXLOCK(mpctx); strlcpy(mpctx->name, name, sizeof(mpctx->name)); - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } + MPCTXUNLOCK(mpctx); + #else /* if ISC_MEMPOOL_NAMES */ UNUSED(mpctx); UNUSED(name); @@ -1284,14 +1257,14 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { mpctx = *mpctxp; *mpctxp = NULL; #if ISC_MEMPOOL_NAMES - if (mpctx->allocated > 0) { + if (atomic_load_acquire(&mpctx->allocated) > 0) { UNEXPECTED_ERROR(__FILE__, __LINE__, "isc_mempool_destroy(): mempool %s " "leaked memory", mpctx->name); } #endif /* if ISC_MEMPOOL_NAMES */ - REQUIRE(mpctx->allocated == 0); + REQUIRE(atomic_load_acquire(&mpctx->allocated) == 0); mctx = mpctx->mctx; @@ -1304,17 +1277,15 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { /* * Return any items on the free list */ - MCTXLOCK(mctx); while (mpctx->items != NULL) { - INSIST(mpctx->freecount > 0); - mpctx->freecount--; + INSIST(atomic_fetch_sub_release(&mpctx->freecount, 1) > 0); + item = mpctx->items; mpctx->items = item->next; mem_putstats(mctx, item, mpctx->size); mem_put(mctx, item, mpctx->size); } - MCTXUNLOCK(mctx); /* * Remove our linked list entry from the memory context. @@ -1348,37 +1319,33 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { REQUIRE(VALID_MEMPOOL(mpctx)); element *item; - isc_mem_t *mctx; unsigned int i; - - mctx = mpctx->mctx; - - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } + size_t allocated = atomic_load_acquire(&mpctx->allocated); + size_t maxalloc = atomic_load_acquire(&mpctx->maxalloc); /* * Don't let the caller go over quota */ - if (ISC_UNLIKELY(mpctx->allocated >= mpctx->maxalloc)) { + if (ISC_UNLIKELY(allocated >= maxalloc)) { item = NULL; goto out; } + MPCTXLOCK(mpctx); if (ISC_UNLIKELY(mpctx->items == NULL)) { + isc_mem_t *mctx = mpctx->mctx; + size_t fillcount = atomic_load_acquire(&mpctx->fillcount); /* - * We need to dip into the well. Lock the memory context - * here and fill up our free list. + * We need to dip into the well. Lock the memory + * context here and fill up our free list. */ - MCTXLOCK(mctx); - for (i = 0; i < mpctx->fillcount; i++) { + for (i = 0; i < fillcount; i++) { item = mem_get(mctx, mpctx->size); mem_getstats(mctx, mpctx->size); item->next = mpctx->items; mpctx->items = item; - mpctx->freecount++; + atomic_fetch_add_relaxed(&mpctx->freecount, 1); } - MCTXUNLOCK(mctx); } /* @@ -1390,24 +1357,15 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) { } mpctx->items = item->next; - INSIST(mpctx->freecount > 0); - mpctx->freecount--; - mpctx->gets++; - mpctx->allocated++; + + INSIST(atomic_fetch_sub_release(&mpctx->freecount, 1) > 0); + atomic_fetch_add_relaxed(&mpctx->gets, 1); + atomic_fetch_add_relaxed(&mpctx->allocated, 1); out: - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } + MPCTXUNLOCK(mpctx); -#if ISC_MEM_TRACKLINES - if (ISC_UNLIKELY(((isc_mem_debugging & TRACE_OR_RECORD) != 0) && - item != NULL)) { - MCTXLOCK(mctx); - ADD_TRACE(mctx, item, mpctx->size, file, line); - MCTXUNLOCK(mctx); - } -#endif /* ISC_MEM_TRACKLINES */ + ADD_TRACE(mpctx->mctx, item, mpctx->size, file, line); return (item); } @@ -1420,47 +1378,33 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) { isc_mem_t *mctx = mpctx->mctx; element *item; + size_t freecount = atomic_load_acquire(&mpctx->freecount); + size_t freemax = atomic_load_acquire(&mpctx->freemax); - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } + INSIST(atomic_fetch_sub_release(&mpctx->allocated, 1) > 0); - INSIST(mpctx->allocated > 0); - mpctx->allocated--; - -#if ISC_MEM_TRACKLINES - if (ISC_UNLIKELY((isc_mem_debugging & TRACE_OR_RECORD) != 0)) { - MCTXLOCK(mctx); - DELETE_TRACE(mctx, mem, mpctx->size, file, line); - MCTXUNLOCK(mctx); - } -#endif /* ISC_MEM_TRACKLINES */ + DELETE_TRACE(mctx, mem, mpctx->size, file, line); /* * If our free list is full, return this to the mctx directly. */ - if (mpctx->freecount >= mpctx->freemax) { - MCTXLOCK(mctx); + if (freecount >= freemax) { mem_putstats(mctx, mem, mpctx->size); mem_put(mctx, mem, mpctx->size); - MCTXUNLOCK(mctx); - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } return; } /* * Otherwise, attach it to our free list and bump the counter. */ - mpctx->freecount++; + MPCTXLOCK(mpctx); + item = (element *)mem; item->next = mpctx->items; mpctx->items = item; + atomic_fetch_add_relaxed(&mpctx->freecount, 1); - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } + MPCTXUNLOCK(mpctx); } /* @@ -1471,15 +1415,11 @@ void isc_mempool_setfreemax(isc_mempool_t *mpctx, unsigned int limit) { REQUIRE(VALID_MEMPOOL(mpctx)); - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } + MPCTXLOCK(mpctx); mpctx->freemax = limit; - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } + MPCTXUNLOCK(mpctx); } unsigned int @@ -1488,15 +1428,11 @@ isc_mempool_getfreemax(isc_mempool_t *mpctx) { unsigned int freemax; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } + MPCTXLOCK(mpctx); freemax = mpctx->freemax; - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } + MPCTXUNLOCK(mpctx); return (freemax); } @@ -1505,19 +1441,7 @@ unsigned int isc_mempool_getfreecount(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - unsigned int freecount; - - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - freecount = mpctx->freecount; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (freecount); + return (atomic_load_relaxed(&mpctx->freecount)); } void @@ -1525,53 +1449,21 @@ isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(limit > 0); - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - mpctx->maxalloc = limit; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } + atomic_store_release(&mpctx->maxalloc, limit); } unsigned int isc_mempool_getmaxalloc(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - unsigned int maxalloc; - - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - maxalloc = mpctx->maxalloc; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (maxalloc); + return (atomic_load_relaxed(&mpctx->maxalloc)); } unsigned int isc_mempool_getallocated(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - unsigned int allocated; - - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - allocated = mpctx->allocated; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (allocated); + return (atomic_load_relaxed(&mpctx->allocated)); } void @@ -1579,34 +1471,14 @@ isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(limit > 0); - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - mpctx->fillcount = limit; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } + atomic_store_release(&mpctx->fillcount, limit); } unsigned int isc_mempool_getfillcount(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - unsigned int fillcount; - - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - fillcount = mpctx->fillcount; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (fillcount); + return (atomic_load_relaxed(&mpctx->fillcount)); } /* @@ -1686,9 +1558,7 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { TRY0(xmlTextWriterEndElement(writer)); /* name */ } - summary->contextsize += sizeof(*ctx) + - (ctx->max_size + 1) * sizeof(struct stats) + - ctx->max_size * sizeof(element *); + summary->contextsize += sizeof(*ctx); #if ISC_MEM_TRACKLINES if (ctx->debuglist != NULL) { summary->contextsize += DEBUG_TABLE_COUNT * @@ -1702,32 +1572,32 @@ xml_renderctx(isc_mem_t *ctx, summarystat_t *summary, xmlTextWriterPtr writer) { isc_refcount_current(&ctx->references))); TRY0(xmlTextWriterEndElement(writer)); /* references */ - summary->total += ctx->total; + summary->total += isc_mem_total(ctx); TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "total")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)ctx->total)); + (uint64_t)isc_mem_total(ctx))); TRY0(xmlTextWriterEndElement(writer)); /* total */ - summary->inuse += ctx->inuse; + summary->inuse += isc_mem_inuse(ctx); TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "inuse")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)ctx->inuse)); + (uint64_t)isc_mem_inuse(ctx))); TRY0(xmlTextWriterEndElement(writer)); /* inuse */ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "maxinuse")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)ctx->maxinuse)); + (uint64_t)isc_mem_maxinuse(ctx))); TRY0(xmlTextWriterEndElement(writer)); /* maxinuse */ - summary->malloced += ctx->malloced; + summary->malloced += isc_mem_malloced(ctx); TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "malloced")); TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)ctx->malloced)); + (uint64_t)isc_mem_malloced(ctx))); TRY0(xmlTextWriterEndElement(writer)); /* malloced */ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "maxmalloced")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)ctx->maxmalloced)); + TRY0(xmlTextWriterWriteFormatString( + writer, "%" PRIu64 "", (uint64_t)isc_mem_maxmalloced(ctx))); TRY0(xmlTextWriterEndElement(writer)); /* maxmalloced */ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "pools")); @@ -1736,13 +1606,15 @@ 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")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)ctx->hi_water)); + TRY0(xmlTextWriterWriteFormatString( + writer, "%" PRIu64 "", + (uint64_t)atomic_load_relaxed(&ctx->hi_water))); TRY0(xmlTextWriterEndElement(writer)); /* hiwater */ TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "lowater")); - TRY0(xmlTextWriterWriteFormatString(writer, "%" PRIu64 "", - (uint64_t)ctx->lo_water)); + TRY0(xmlTextWriterWriteFormatString( + writer, "%" PRIu64 "", + (uint64_t)atomic_load_relaxed(&ctx->lo_water))); TRY0(xmlTextWriterEndElement(writer)); /* lowater */ TRY0(xmlTextWriterEndElement(writer)); /* context */ @@ -1828,12 +1700,10 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { MCTXLOCK(ctx); - summary->contextsize += sizeof(*ctx) + - (ctx->max_size + 1) * sizeof(struct stats) + - ctx->max_size * sizeof(element *); - summary->total += ctx->total; - summary->inuse += ctx->inuse; - summary->malloced += ctx->malloced; + summary->contextsize += sizeof(*ctx); + summary->total += isc_mem_total(ctx); + summary->inuse += isc_mem_inuse(ctx); + summary->malloced += isc_mem_malloced(ctx); #if ISC_MEM_TRACKLINES if (ctx->debuglist != NULL) { summary->contextsize += DEBUG_TABLE_COUNT * @@ -1860,23 +1730,23 @@ json_renderctx(isc_mem_t *ctx, summarystat_t *summary, json_object *array) { CHECKMEM(obj); json_object_object_add(ctxobj, "references", obj); - obj = json_object_new_int64(ctx->total); + obj = json_object_new_int64(isc_mem_total(ctx)); CHECKMEM(obj); json_object_object_add(ctxobj, "total", obj); - obj = json_object_new_int64(ctx->inuse); + obj = json_object_new_int64(isc_mem_inuse(ctx)); CHECKMEM(obj); json_object_object_add(ctxobj, "inuse", obj); - obj = json_object_new_int64(ctx->maxinuse); + obj = json_object_new_int64(isc_mem_maxinuse(ctx)); CHECKMEM(obj); json_object_object_add(ctxobj, "maxinuse", obj); - obj = json_object_new_int64(ctx->malloced); + obj = json_object_new_int64(isc_mem_malloced(ctx)); CHECKMEM(obj); json_object_object_add(ctxobj, "malloced", obj); - obj = json_object_new_int64(ctx->maxmalloced); + obj = json_object_new_int64(isc_mem_maxmalloced(ctx)); CHECKMEM(obj); json_object_object_add(ctxobj, "maxmalloced", obj); @@ -1886,11 +1756,11 @@ 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(ctx->hi_water); + obj = json_object_new_int64(atomic_load_relaxed(&ctx->hi_water)); CHECKMEM(obj); json_object_object_add(ctxobj, "hiwater", obj); - obj = json_object_new_int64(ctx->lo_water); + obj = json_object_new_int64(atomic_load_relaxed(&ctx->lo_water)); CHECKMEM(obj); json_object_object_add(ctxobj, "lowater", obj); diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index 7bc4926a96..b02fb22bbb 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -370,7 +370,9 @@ isc_mem_detach isc_mem_getname isc_mem_inuse isc_mem_isovermem +isc_mem_malloced isc_mem_maxinuse +isc_mem_maxmalloced isc_mem_references @IF NOTYET isc_mem_renderjson From c9fe12443f1a7c840ce79c7a7c910e9ef9190e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 4 Feb 2021 23:10:39 +0100 Subject: [PATCH 05/13] Make the mempool names unconditional The named memory pools were default and always compiled-in. Remove the extra complexity by removing the #define and #ifdefs around the code. --- lib/isc/include/isc/mem.h | 9 --------- lib/isc/mem.c | 41 ++++++++------------------------------- 2 files changed, 8 insertions(+), 42 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 2d4f70dedd..7d63ffef23 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -36,15 +36,6 @@ typedef void (*isc_mem_water_t)(void *, int); #define ISC_MEM_TRACKLINES 1 #endif /* ifndef ISC_MEM_TRACKLINES */ -/*% - * Define ISC_MEMPOOL_NAMES=1 to make memory pools store a symbolic - * name so that the leaking pool can be more readily identified in - * case of a memory leak. - */ -#ifndef ISC_MEMPOOL_NAMES -#define ISC_MEMPOOL_NAMES 1 -#endif /* ifndef ISC_MEMPOOL_NAMES */ - LIBISC_EXTERNAL_DATA extern unsigned int isc_mem_debugging; LIBISC_EXTERNAL_DATA extern unsigned int isc_mem_defaultflags; diff --git a/lib/isc/mem.c b/lib/isc/mem.c index c3c4ac463a..103667a087 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -177,9 +177,7 @@ struct isc_mempool { /*%< Stats only. */ atomic_size_t gets; /*%< # of requests to this pool */ /*%< Debugging only. */ -#if ISC_MEMPOOL_NAMES - char name[16]; /*%< printed name in stats reports */ -#endif /* if ISC_MEMPOOL_NAMES */ + char name[16]; /*%< printed name in stats reports */ }; /* @@ -226,7 +224,6 @@ 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; - INSIST(size >= 0); return (malloced); } @@ -885,12 +882,8 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) { while (pool != NULL) { fprintf(out, "%15s %10zu %10zu %10zu %10zu %10zu %10zu %10zu %s\n", -#if ISC_MEMPOOL_NAMES - pool->name, -#else /* if ISC_MEMPOOL_NAMES */ - "(not tracked)", -#endif /* if ISC_MEMPOOL_NAMES */ - pool->size, atomic_load_relaxed(&pool->maxalloc), + pool->name, pool->size, + atomic_load_relaxed(&pool->maxalloc), atomic_load_relaxed(&pool->allocated), atomic_load_relaxed(&pool->freecount), atomic_load_relaxed(&pool->freemax), @@ -1129,7 +1122,7 @@ isc_mem_setwater(isc_mem_t *ctx, isc_mem_water_t water, void *water_arg, oldwater = ctx->water; oldwater_arg = ctx->water_arg; if (water == NULL) { - callwater = atomic_load(&ctx->hi_called); + callwater = atomic_load_acquire(&ctx->hi_called); ctx->water = NULL; ctx->water_arg = NULL; atomic_store_release(&ctx->hi_water, 0); @@ -1231,17 +1224,11 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name) { REQUIRE(VALID_MEMPOOL(mpctx)); REQUIRE(name != NULL); -#if ISC_MEMPOOL_NAMES MPCTXLOCK(mpctx); strlcpy(mpctx->name, name, sizeof(mpctx->name)); MPCTXUNLOCK(mpctx); - -#else /* if ISC_MEMPOOL_NAMES */ - UNUSED(mpctx); - UNUSED(name); -#endif /* if ISC_MEMPOOL_NAMES */ } void @@ -1256,14 +1243,12 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { mpctx = *mpctxp; *mpctxp = NULL; -#if ISC_MEMPOOL_NAMES if (atomic_load_acquire(&mpctx->allocated) > 0) { UNEXPECTED_ERROR(__FILE__, __LINE__, "isc_mempool_destroy(): mempool %s " "leaked memory", mpctx->name); } -#endif /* if ISC_MEMPOOL_NAMES */ REQUIRE(atomic_load_acquire(&mpctx->allocated) == 0); mctx = mpctx->mctx; @@ -1415,26 +1400,14 @@ void isc_mempool_setfreemax(isc_mempool_t *mpctx, unsigned int limit) { REQUIRE(VALID_MEMPOOL(mpctx)); - MPCTXLOCK(mpctx); - - mpctx->freemax = limit; - - MPCTXUNLOCK(mpctx); + atomic_store_release(&mpctx->freemax, limit); } unsigned int isc_mempool_getfreemax(isc_mempool_t *mpctx) { REQUIRE(VALID_MEMPOOL(mpctx)); - unsigned int freemax; - - MPCTXLOCK(mpctx); - - freemax = mpctx->freemax; - - MPCTXUNLOCK(mpctx); - - return (freemax); + return (atomic_load_acquire(&mpctx->freemax)); } unsigned int @@ -1484,6 +1457,7 @@ isc_mempool_getfillcount(isc_mempool_t *mpctx) { /* * Requires contextslock to be held by caller. */ +#if ISC_MEM_TRACKLINES static void print_contexts(FILE *file) { isc_mem_t *ctx; @@ -1497,6 +1471,7 @@ print_contexts(FILE *file) { } fflush(file); } +#endif void isc_mem_checkdestroyed(FILE *file) { From f34f943b167573c0829dead547e4faafc5f6e6fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 5 Feb 2021 10:25:07 +0100 Subject: [PATCH 06/13] Disable memory debugging features in non-developer build The two memory debugging features: ISC_MEM_DEFAULTFILL (ISC_MEMFLAG_FILL) and ISC_MEM_TRACKLINES were always enabled in all builds and the former was only disabled in `named`. This commits disables those two features in non-developer build to make the memory allocator significantly faster. --- bin/named/main.c | 9 --------- configure.ac | 2 +- lib/isc/include/isc/mem.h | 12 ++++++++---- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/bin/named/main.c b/bin/named/main.c index 2201ce002d..c66425e18f 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -1524,15 +1524,6 @@ main(int argc, char *argv[]) { pk11_result_register(); #endif /* if USE_PKCS11 */ -#if !ISC_MEM_DEFAULTFILL - /* - * Update the default flags to remove ISC_MEMFLAG_FILL - * before we parse the command line. If disabled here, - * it can be turned back on with -M fill. - */ - isc_mem_defaultflags &= ~ISC_MEMFLAG_FILL; -#endif /* if !ISC_MEM_DEFAULTFILL */ - parse_command_line(argc, argv); #ifdef ENABLE_AFL diff --git a/configure.ac b/configure.ac index c8d75b2bf7..1d1d1c7fa6 100644 --- a/configure.ac +++ b/configure.ac @@ -158,7 +158,7 @@ AC_ARG_ENABLE([developer], AS_IF([test "$enable_developer" = "yes"], [DEVELOPER_MODE=yes - STD_CPPFLAGS="$STD_CPPFLAGS -DISC_MEM_DEFAULTFILL=1 -DISC_LIST_CHECKINIT=1" + STD_CPPFLAGS="$STD_CPPFLAGS -DISC_MEM_DEFAULTFILL=1 -DISC_MEM_TRACKLINES=1 -DISC_LIST_CHECKINIT=1" test "${enable_fixed_rrset+set}" = set || enable_fixed_rrset=yes test "${enable_querytrace+set}" = set || enable_querytrace=yes test "${with_cmocka+set}" = set || with_cmocka=yes diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 7d63ffef23..1d274316dd 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -33,7 +33,7 @@ typedef void (*isc_mem_water_t)(void *, int); * allocation and freeing by file and line number. */ #ifndef ISC_MEM_TRACKLINES -#define ISC_MEM_TRACKLINES 1 +#define ISC_MEM_TRACKLINES 0 #endif /* ifndef ISC_MEM_TRACKLINES */ LIBISC_EXTERNAL_DATA extern unsigned int isc_mem_debugging; @@ -90,10 +90,14 @@ LIBISC_EXTERNAL_DATA extern unsigned int isc_mem_defaultflags; #define ISC_MEMFLAG_FILL \ 0x00000004 /* fill with pattern after alloc and frees */ -#if !ISC_MEM_USE_INTERNAL_MALLOC -#define ISC_MEMFLAG_DEFAULT 0 -#else /* if !ISC_MEM_USE_INTERNAL_MALLOC */ +/*% + * Define ISC_MEM_DEFAULTFILL=1 to turn filling the memory with pattern + * after alloc and free. + */ +#if ISC_MEM_DEFAULTFILL #define ISC_MEMFLAG_DEFAULT ISC_MEMFLAG_FILL +#else /* if !ISC_MEM_USE_INTERNAL_MALLOC */ +#define ISC_MEMFLAG_DEFAULT 0 #endif /* if !ISC_MEM_USE_INTERNAL_MALLOC */ /*% From 549e5b693a58c243bf779d37726c5d8fc6b981ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 5 Feb 2021 10:25:07 +0100 Subject: [PATCH 07/13] Modify the way we benchmark mem_{get,put} Previously, the mem_{get,put} benchmark would pass the allocation size as thread_create argument. This has been now changed, so the allocation size is stored and decremented (divided) in atomic variable and the thread create routing is given a memory context. This will allow to write tests where each thread is given different memory context and do the same for mempool benchmarking. --- lib/isc/tests/mem_test.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index 3b7c3e9ee1..a39b59c19b 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -22,6 +22,7 @@ #define UNIT_TESTING #include +#include #include #include #include @@ -372,17 +373,22 @@ isc_mem_traceflag_test(void **state) { #define NUM_ITEMS 1024 /* 768 */ #define ITEM_SIZE 65534 +static atomic_size_t mem_size; + static isc_threadresult_t mem_thread(isc_threadarg_t arg) { + isc_mem_t *mctx = (isc_mem_t *)arg; void *items[NUM_ITEMS]; - size_t size = *((size_t *)arg); + size_t size = atomic_load(&mem_size); + while (!atomic_compare_exchange_weak(&mem_size, &size, size / 2)) + ; for (int i = 0; i < ITERS; i++) { for (int j = 0; j < NUM_ITEMS; j++) { - items[j] = isc_mem_get(test_mctx, size); + items[j] = isc_mem_get(mctx, size); } for (int j = 0; j < NUM_ITEMS; j++) { - isc_mem_put(test_mctx, items[j], size); + isc_mem_put(mctx, items[j], size); } } @@ -396,16 +402,16 @@ isc_mem_benchmark(void **state) { isc_time_t ts1, ts2; double t; isc_result_t result; - size_t size = ITEM_SIZE; UNUSED(state); + atomic_init(&mem_size, ITEM_SIZE); + result = isc_time_now(&ts1); assert_int_equal(result, ISC_R_SUCCESS); for (int i = 0; i < nthreads; i++) { - isc_thread_create(mem_thread, &size, &threads[i]); - size = size / 2; + isc_thread_create(mem_thread, test_mctx, &threads[i]); } for (int i = 0; i < nthreads; i++) { isc_thread_join(threads[i], NULL); @@ -446,7 +452,6 @@ isc_mempool_benchmark(void **state) { isc_time_t ts1, ts2; double t; isc_result_t result; - size_t size = ITEM_SIZE; isc_mempool_t *mp = NULL; isc_mutex_t mplock; @@ -466,7 +471,6 @@ isc_mempool_benchmark(void **state) { for (int i = 0; i < nthreads; i++) { isc_thread_create(mempool_thread, mp, &threads[i]); - size = size / 2; } for (int i = 0; i < nthreads; i++) { isc_thread_join(threads[i], NULL); From ff47b47f1a32e7e5c6380bf199e146f03a81edc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 5 Feb 2021 17:18:28 +0100 Subject: [PATCH 08/13] Remove overrun checking code from memory allocator The ISC_MEM_CHECKOVERRUN would add canary byte at the end of every allocations and check whether the canary byte hasn't been changed at the free time. The AddressSanitizer and valgrind memory checks surpases simple checks like this, so there's no need to actually keep the code inside the allocator. --- lib/isc/mem.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 103667a087..9f6db54823 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -339,9 +339,6 @@ static inline void * mem_get(isc_mem_t *ctx, size_t size) { char *ret; -#if ISC_MEM_CHECKOVERRUN - size += 1; -#endif /* if ISC_MEM_CHECKOVERRUN */ ret = default_memalloc(size); if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { @@ -349,14 +346,6 @@ mem_get(isc_mem_t *ctx, size_t size) { memset(ret, 0xbe, size); /* Mnemonic for "beef". */ } } -#if ISC_MEM_CHECKOVERRUN - else - { - if (ISC_LIKELY(ret != NULL)) { - ret[size - 1] = 0xbe; - } - } -#endif /* if ISC_MEM_CHECKOVERRUN */ return (ret); } @@ -367,10 +356,6 @@ mem_get(isc_mem_t *ctx, size_t size) { /* coverity[+free : arg-1] */ static inline void mem_put(isc_mem_t *ctx, void *mem, size_t size) { -#if ISC_MEM_CHECKOVERRUN - INSIST(((unsigned char *)mem)[size] == 0xbe); - size += 1; -#endif /* if ISC_MEM_CHECKOVERRUN */ if (ISC_UNLIKELY((ctx->flags & ISC_MEMFLAG_FILL) != 0)) { memset(mem, 0xde, size); /* Mnemonic for "dead". */ } From 4775e9f256025026e9a9d7f246b66148375c9d5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 9 Feb 2021 13:25:46 +0100 Subject: [PATCH 09/13] Move most of the OpenSSL initialization to isc_tls Since we now require both libcrypto and libssl to be initialized for netmgr, we move all the OpenSSL initialization code except the engine initialization to isc_tls API. The isc_tls_initialize() and isc_tls_destroy() has been made idempotent, so they could be called multiple time. However when isc_tls_destroy() has been called, the isc_tls_initialize() could not be called again. --- lib/dns/dst_api.c | 2 +- lib/dns/dst_internal.h | 2 +- lib/dns/openssl_link.c | 63 +++-------------------- lib/isc/tls.c | 111 ++++++++++++++++++++++++++++++++++++----- 4 files changed, 107 insertions(+), 71 deletions(-) diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 7e89a72a2b..9692ac66be 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -201,7 +201,7 @@ dst_lib_init(isc_mem_t *mctx, const char *engine) { RETERR(dst__hmacsha256_init(&dst_t_func[DST_ALG_HMACSHA256])); RETERR(dst__hmacsha384_init(&dst_t_func[DST_ALG_HMACSHA384])); RETERR(dst__hmacsha512_init(&dst_t_func[DST_ALG_HMACSHA512])); - RETERR(dst__openssl_init(mctx, engine)); + RETERR(dst__openssl_init(engine)); RETERR(dst__openssldh_init(&dst_t_func[DST_ALG_DH])); #if USE_OPENSSL RETERR(dst__opensslrsa_init(&dst_t_func[DST_ALG_RSASHA1], diff --git a/lib/dns/dst_internal.h b/lib/dns/dst_internal.h index 776638fdc4..793606e013 100644 --- a/lib/dns/dst_internal.h +++ b/lib/dns/dst_internal.h @@ -201,7 +201,7 @@ struct dst_func { * Initializers */ isc_result_t -dst__openssl_init(isc_mem_t *, const char *engine); +dst__openssl_init(const char *engine); #define dst__pkcs11_init pk11_initialize isc_result_t diff --git a/lib/dns/openssl_link.c b/lib/dns/openssl_link.c index ff4760b917..bb26adcf26 100644 --- a/lib/dns/openssl_link.c +++ b/lib/dns/openssl_link.c @@ -39,8 +39,6 @@ #include "dst_internal.h" #include "dst_openssl.h" -static isc_mem_t *dst__mctx = NULL; - #if !defined(OPENSSL_NO_ENGINE) #include #endif /* if !defined(OPENSSL_NO_ENGINE) */ @@ -67,36 +65,14 @@ enable_fips_mode(void) { } isc_result_t -dst__openssl_init(isc_mem_t *mctx, const char *engine) { - isc_result_t result; - - REQUIRE(dst__mctx == NULL); - isc_mem_attach(mctx, &dst__mctx); - -#if defined(OPENSSL_NO_ENGINE) - UNUSED(engine); -#endif /* if defined(OPENSSL_NO_ENGINE) */ - - enable_fips_mode(); +dst__openssl_init(const char *engine) { + isc_result_t result = ISC_R_SUCCESS; isc_tls_initialize(); -#if !defined(OPENSSL_NO_ENGINE) -#if !defined(CONF_MFLAGS_DEFAULT_SECTION) - OPENSSL_config(NULL); -#else /* if !defined(CONF_MFLAGS_DEFAULT_SECTION) */ - /* - * OPENSSL_config() can only be called a single time as of - * 1.0.2e so do the steps individually. - */ - OPENSSL_load_builtin_modules(); - ENGINE_load_builtin_engines(); - ERR_clear_error(); - CONF_modules_load_file(NULL, NULL, - CONF_MFLAGS_DEFAULT_SECTION | - CONF_MFLAGS_IGNORE_MISSING_FILE); -#endif /* if !defined(CONF_MFLAGS_DEFAULT_SECTION) */ + enable_fips_mode(); +#if !defined(OPENSSL_NO_ENGINE) if (engine != NULL && *engine == '\0') { engine = NULL; } @@ -114,54 +90,27 @@ dst__openssl_init(isc_mem_t *mctx, const char *engine) { } } -#endif /* !defined(OPENSSL_NO_ENGINE) */ - - /* Protect ourselves against unseeded PRNG */ - if (RAND_status() != 1) { - FATAL_ERROR(__FILE__, __LINE__, - "OpenSSL pseudorandom number generator " - "cannot be initialized (see the `PRNG not " - "seeded' message in the OpenSSL FAQ)"); - } - return (ISC_R_SUCCESS); - -#if !defined(OPENSSL_NO_ENGINE) cleanup_rm: if (e != NULL) { ENGINE_free(e); } e = NULL; +#else + UNUSED(engine); #endif /* if !defined(OPENSSL_NO_ENGINE) */ return (result); } void dst__openssl_destroy(void) { -#if (OPENSSL_VERSION_NUMBER < 0x10100000L) || defined(LIBRESSL_VERSION_NUMBER) - /* - * Sequence taken from apps_shutdown() in . - */ - CONF_modules_free(); - OBJ_cleanup(); - EVP_cleanup(); #if !defined(OPENSSL_NO_ENGINE) if (e != NULL) { ENGINE_free(e); } e = NULL; - ENGINE_cleanup(); #endif /* if !defined(OPENSSL_NO_ENGINE) */ - CRYPTO_cleanup_all_ex_data(); - ERR_clear_error(); - -#ifdef DNS_CRYPTO_LEAKS - CRYPTO_mem_leaks_fp(stderr); -#endif /* ifdef DNS_CRYPTO_LEAKS */ - -#endif isc_tls_destroy(); - isc_mem_detach(&dst__mctx); } static isc_result_t diff --git a/lib/isc/tls.c b/lib/isc/tls.c index ea5de5dbf9..57066a4121 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -9,8 +9,10 @@ * information regarding copyright ownership. */ +#include #include #include +#include #include #include @@ -24,10 +26,12 @@ #include "openssl_shim.h" static isc_once_t init_once = ISC_ONCE_INIT; +static isc_once_t shut_once = ISC_ONCE_INIT; static atomic_bool init_done = ATOMIC_VAR_INIT(false); +static atomic_bool shut_done = ATOMIC_VAR_INIT(false); +static isc_mem_t *isc__tls_mctx = NULL; #if OPENSSL_VERSION_NUMBER < 0x10100000L -static isc_mem_t *isc__tls_mctx = NULL; static isc_mutex_t *locks = NULL; static int nlocks; @@ -48,22 +52,82 @@ isc__tls_set_thread_id(CRYPTO_THREADID *id) { } #endif +#if 0 +static void * +isc__tls_malloc(size_t size, const char *file, int line) { + UNUSED(file); + UNUSED(line); + + return (isc_mem_allocate(isc__tls_mctx, size)); +} + +static void * +isc__tls_realloc(void *ptr, size_t size, const char *file, int line) { + UNUSED(file); + UNUSED(line); + + return (isc__mem_reallocate(isc__tls_mctx, ptr, size)); +} + +static void +isc__tls_free(void *ptr, const char *file, int line) { + UNUSED(file); + UNUSED(line); + + if (ptr == NULL) { + return; + } + + isc__mem_free(isc__tls_mctx, ptr); +} +#endif + static void isc__tls_initialize(void) { REQUIRE(!atomic_load(&init_done)); - RUNTIME_CHECK(OPENSSL_init_ssl(0, NULL) == 1); -#if OPENSSL_VERSION_NUMBER < 0x10100000L isc_mem_create(&isc__tls_mctx); + /* isc_mem_setdestroycheck(isc__tls_mctx, false); */ + /* REQUIRE(CRYPTO_set_mem_functions(isc__tls_malloc, isc__tls_realloc, + * isc__tls_free) == 1); */ + +#if OPENSSL_VERSION_NUMBER >= 0x10100000L + RUNTIME_CHECK(OPENSSL_init_ssl(OPENSSL_INIT_ENGINE_ALL_BUILTIN | + OPENSSL_INIT_LOAD_CONFIG, + NULL) == 1); +#else nlocks = CRYPTO_num_locks(); locks = isc_mem_get(isc__tls_mctx, nlocks * sizeof(locks[0])); isc_mutexblock_init(locks, nlocks); CRYPTO_set_locking_callback(isc__tls_lock_callback); CRYPTO_THREADID_set_callback(isc__tls_set_thread_id); + + CRYPTO_malloc_init(); ERR_load_crypto_strings(); + SSL_load_error_strings(); + SSL_library_init(); + +#if !defined(OPENSSL_NO_ENGINE) + ENGINE_load_builtin_engines(); #endif - atomic_store(&init_done, true); + OpenSSL_add_all_algorithms(); + OPENSSL_load_builtin_modules(); + + CONF_modules_load_file(NULL, NULL, + CONF_MFLAGS_DEFAULT_SECTION | + CONF_MFLAGS_IGNORE_MISSING_FILE); +#endif + + /* Protect ourselves against unseeded PRNG */ + if (RAND_status() != 1) { + FATAL_ERROR(__FILE__, __LINE__, + "OpenSSL pseudorandom number generator " + "cannot be initialized (see the `PRNG not " + "seeded' message in the OpenSSL FAQ)"); + } + + atomic_compare_exchange_strong(&init_done, &(bool){ false }, true); } void @@ -73,26 +137,45 @@ isc_tls_initialize(void) { REQUIRE(atomic_load(&init_done)); } -void -isc_tls_destroy(void) { +static void +isc__tls_destroy(void) { REQUIRE(atomic_load(&init_done)); -#if (OPENSSL_VERSION_NUMBER < 0x10100000L) + REQUIRE(!atomic_load(&shut_done)); +#if OPENSSL_VERSION_NUMBER < 0x10100000L + + CONF_modules_unload(1); + OBJ_cleanup(); + EVP_cleanup(); +#if !defined(OPENSSL_NO_ENGINE) + ENGINE_cleanup(); +#endif + CRYPTO_cleanup_all_ex_data(); + ERR_remove_thread_state(NULL); + RAND_cleanup(); ERR_free_strings(); - ERR_remove_thread_state(NULL); CRYPTO_set_locking_callback(NULL); + /* REQUIRE(CRYPTO_set_mem_functions(OPENSSL_malloc, OPENSSL_realloc, + * OPENSSL_free) == 1); */ + if (locks != NULL) { INSIST(isc__tls_mctx != NULL); isc_mutexblock_destroy(locks, nlocks); isc_mem_put(isc__tls_mctx, locks, nlocks * sizeof(locks[0])); locks = NULL; } - - if (isc__tls_mctx != NULL) { - isc_mem_detach(&isc__tls_mctx); - } #endif + isc_mem_detach(&isc__tls_mctx); + + atomic_compare_exchange_strong(&shut_done, &(bool){ false }, true); +} + +void +isc_tls_destroy(void) { + isc_result_t result = isc_once_do(&shut_once, isc__tls_destroy); + REQUIRE(result == ISC_R_SUCCESS); + REQUIRE(atomic_load(&shut_done)); } void @@ -115,6 +198,8 @@ isc_tlsctx_createclient(isc_tlsctx_t **ctxp) { REQUIRE(ctxp != NULL && *ctxp == NULL); + isc_tls_initialize(); + method = TLS_client_method(); if (method == NULL) { goto ssl_error; @@ -163,6 +248,8 @@ isc_tlsctx_createserver(const char *keyfile, const char *certfile, REQUIRE(ctxp != NULL && *ctxp == NULL); + isc_tls_initialize(); + if (ephemeral) { INSIST(keyfile == NULL); INSIST(certfile == NULL); From f2254620556d3d29822475e94c69959c2787c24d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 9 Feb 2021 13:25:52 +0100 Subject: [PATCH 10/13] Fix the invalid condition variable Although harmless, the memmove() in tlsdns and tcpdns was guarded by a current message length variable that was always bigger than 0 instead of correct current buffer length remainder variable. --- lib/isc/netmgr/tcpdns.c | 2 +- lib/isc/netmgr/tlsdns.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index cbc2cab0ed..d7b8bbde42 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -999,7 +999,7 @@ processbuffer(isc_nmsocket_t *sock) { len += 2; sock->buf_len -= len; - if (len > 0) { + if (sock->buf_len > 0) { memmove(sock->buf, sock->buf + len, sock->buf_len); } diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index d518dc97ec..90aa605232 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -1147,7 +1147,7 @@ processbuffer(isc_nmsocket_t *sock) { len += 2; sock->buf_len -= len; - if (len > 0) { + if (sock->buf_len > 0) { memmove(sock->buf, sock->buf + len, sock->buf_len); } From 4bde4f050be7365b972f8a5511138888c3367b99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 14 Jul 2020 10:52:02 +0200 Subject: [PATCH 11/13] Disable calling DllMain() on thread creation/destruction Disables the DLL_THREAD_ATTACH and DLL_THREAD_DETACH notifications for the specified dynamic-link library (DLL). This can reduce the size of the working set for some applications. --- lib/isc/win32/DLLMain.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/isc/win32/DLLMain.c b/lib/isc/win32/DLLMain.c index 91f14e7b00..3acfee4f46 100644 --- a/lib/isc/win32/DLLMain.c +++ b/lib/isc/win32/DLLMain.c @@ -19,27 +19,32 @@ __declspec(dllexport) BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) { switch (fdwReason) { /* - * The DLL is loading due to process - * initialization or a call to LoadLibrary. + * The DLL is loading due to process initialization or a call to + * LoadLibrary. */ case DLL_PROCESS_ATTACH: - break; - - /* The attached process creates a new thread. */ - case DLL_THREAD_ATTACH: - break; - - /* The thread of the attached process terminates. */ - case DLL_THREAD_DETACH: + /* + * Disable DllMain() invocation on Thread creation/destruction + */ + DisableThreadLibraryCalls(hinstDLL); break; /* - * The DLL is unloading from a process due to - * process termination or a call to FreeLibrary. + * The DLL is unloading from a process due to process + * termination or a call to FreeLibrary. */ case DLL_PROCESS_DETACH: break; + case DLL_THREAD_ATTACH: + case DLL_THREAD_DETACH: + /* + * Calling DllMain when attaching/detaching process has been + * disabled. + */ + INSIST(0); + break; + default: break; } From 494d0da522101fe7ddfb46db6b8c0eb9ac6a690a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 9 Feb 2021 17:44:40 +0100 Subject: [PATCH 12/13] Use library constructor/destructor to initialize OpenSSL Instead of calling isc_tls_initialize()/isc_tls_destroy() explicitly use gcc/clang attributes on POSIX and DLLMain on Windows to initialize and shutdown OpenSSL library. This resolves the issue when isc_nm_create() / isc_nm_destroy() was called multiple times and it would call OpenSSL library destructors from isc_nm_destroy(). At the same time, since we now have introduced the ctor/dtor for libisc, this commit moves the isc_mem API initialization (the list of the contexts) and changes the isc_mem_checkdestroyed() to schedule the checking of memory context on library unload instead of executing the code immediately. --- configure.ac | 6 +++ lib/dns/openssl_link.c | 3 -- lib/isc/include/isc/tls.h | 6 --- lib/isc/include/isc/util.h | 10 +++++ lib/isc/lib.c | 23 ++++++++++ lib/isc/mem.c | 42 ++++++++++++++----- lib/isc/mem_p.h | 16 +++++-- lib/isc/netmgr/netmgr.c | 4 -- lib/isc/netmgr/tlsdns.c | 11 +++++ lib/isc/tests/tlsdns_test.c | 9 +++- lib/isc/tls.c | 84 +++++++++++++------------------------ lib/isc/tls_p.h | 18 ++++++++ lib/isc/win32/DLLMain.c | 12 ++++++ lib/isc/win32/libisc.def.in | 7 +++- util/copyrights | 1 + 15 files changed, 165 insertions(+), 87 deletions(-) create mode 100644 lib/isc/tls_p.h diff --git a/configure.ac b/configure.ac index 1d1d1c7fa6..a85bc0a3c7 100644 --- a/configure.ac +++ b/configure.ac @@ -1580,6 +1580,12 @@ test -z "$with_dlz_stub" && with_dlz_stub=no AC_CHECK_HEADERS([glob.h]) +# +# Support for constructor and destructor attributes +# +AX_GCC_FUNC_ATTRIBUTE([constructor]) +AX_GCC_FUNC_ATTRIBUTE([destructor]) + # # Files to configure. These are listed here because we used to # specify them as arguments to AC_OUTPUT. diff --git a/lib/dns/openssl_link.c b/lib/dns/openssl_link.c index bb26adcf26..1060fcc4ff 100644 --- a/lib/dns/openssl_link.c +++ b/lib/dns/openssl_link.c @@ -68,8 +68,6 @@ isc_result_t dst__openssl_init(const char *engine) { isc_result_t result = ISC_R_SUCCESS; - isc_tls_initialize(); - enable_fips_mode(); #if !defined(OPENSSL_NO_ENGINE) @@ -110,7 +108,6 @@ dst__openssl_destroy(void) { } e = NULL; #endif /* if !defined(OPENSSL_NO_ENGINE) */ - isc_tls_destroy(); } static isc_result_t diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h index d0318262e5..3741f19f5e 100644 --- a/lib/isc/include/isc/tls.h +++ b/lib/isc/include/isc/tls.h @@ -19,12 +19,6 @@ typedef struct ssl_ctx_st isc_tlsctx_t; -void -isc_tls_initialize(void); - -void -isc_tls_destroy(void); - void isc_tlsctx_free(isc_tlsctx_t **ctpx); /*% diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h index 8941ec125e..3c8c40b679 100644 --- a/lib/isc/include/isc/util.h +++ b/lib/isc/include/isc/util.h @@ -48,6 +48,16 @@ #define ISC_NONSTRING #endif /* __GNUC__ */ +#if HAVE_FUNC_ATTRIBUTE_CONSTRUCTOR && HAVE_FUNC_ATTRIBUTE_DESTRUCTOR +#define ISC_CONSTRUCTOR(priority) __attribute__((constructor(priority))) +#define ISC_DESTRUCTOR(priority) __attribute__((destructor(priority))) +#elif WIN32 +#define ISC_CONSTRUCTOR(priority) +#define ISC_DESTRUCTOR(priority) +#else +#error Either __attribute__((constructor|destructor))__ or DllMain support needed to compile BIND 9. +#endif + /*% * The opposite: silent warnings about stored values which are never read. */ diff --git a/lib/isc/lib.c b/lib/isc/lib.c index 8bee4c21a4..dad744903d 100644 --- a/lib/isc/lib.c +++ b/lib/isc/lib.c @@ -13,6 +13,12 @@ #include #include +#include +#include +#include + +#include "mem_p.h" +#include "tls_p.h" /*** *** Functions @@ -22,3 +28,20 @@ void isc_lib_register(void) { isc_bind9 = false; } + +void +isc__initialize(void) ISC_CONSTRUCTOR(101); +void +isc__shutdown(void) ISC_DESTRUCTOR(101); + +void +isc__initialize(void) { + isc__mem_initialize(); + isc__tls_initialize(); +} + +void +isc__shutdown(void) { + isc__tls_shutdown(); + isc__mem_shutdown(); +} diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 9f6db54823..53ea7e6e3a 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -117,7 +117,8 @@ struct stats { static ISC_LIST(isc_mem_t) contexts; -static isc_once_t once = ISC_ONCE_INIT; +static isc_once_t init_once = ISC_ONCE_INIT; +static isc_once_t shut_once = ISC_ONCE_INIT; static isc_mutex_t contextslock; /*% @@ -440,12 +441,29 @@ default_memfree(void *ptr) { } static void -initialize_action(void) { +mem_initialize(void) { isc_mutex_init(&contextslock); ISC_LIST_INIT(contexts); totallost = 0; } +void +isc__mem_initialize(void) { + RUNTIME_CHECK(isc_once_do(&init_once, mem_initialize) == ISC_R_SUCCESS); +} + +static void +mem_shutdown(void) { + isc__mem_checkdestroyed(); + + isc_mutex_destroy(&contextslock); +} + +void +isc__mem_shutdown(void) { + RUNTIME_CHECK(isc_once_do(&shut_once, mem_shutdown) == ISC_R_SUCCESS); +} + static void mem_create(isc_mem_t **ctxp, unsigned int flags) { REQUIRE(ctxp != NULL && *ctxp == NULL); @@ -457,8 +475,6 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) { STATIC_ASSERT(ALIGNMENT_SIZE >= sizeof(size_info), "alignment size too small"); - RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS); - ctx = default_memalloc(sizeof(*ctx)); *ctx = (isc_mem_t){ @@ -1458,13 +1474,20 @@ print_contexts(FILE *file) { } #endif +static atomic_uintptr_t checkdestroyed = ATOMIC_VAR_INIT(0); + void isc_mem_checkdestroyed(FILE *file) { -#if !ISC_MEM_TRACKLINES - UNUSED(file); -#endif /* if !ISC_MEM_TRACKLINES */ + atomic_store_release(&checkdestroyed, (uintptr_t)file); +} - RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS); +void +isc__mem_checkdestroyed(void) { + FILE *file = (FILE *)atomic_load_acquire(&checkdestroyed); + + if (file == NULL) { + return; + } LOCK(&contextslock); if (!ISC_LIST_EMPTY(contexts)) { @@ -1597,8 +1620,6 @@ isc_mem_renderxml(void *writer0) { TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "contexts")); - RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS); - LOCK(&contextslock); lost = totallost; for (ctx = ISC_LIST_HEAD(contexts); ctx != NULL; @@ -1739,7 +1760,6 @@ isc_mem_renderjson(void *memobj0) { json_object *memobj = (json_object *)memobj0; memset(&summary, 0, sizeof(summary)); - RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS); ctxarray = json_object_new_array(); CHECKMEM(ctxarray); diff --git a/lib/isc/mem_p.h b/lib/isc/mem_p.h index f1889c668b..dc17e06420 100644 --- a/lib/isc/mem_p.h +++ b/lib/isc/mem_p.h @@ -9,8 +9,11 @@ * information regarding copyright ownership. */ -#ifndef ISC_MEM_P_H -#define ISC_MEM_P_H +#pragma once + +#include + +#include /*! \file */ @@ -21,4 +24,11 @@ isc__mem_printactive(isc_mem_t *mctx, FILE *file); * a single memory context. */ -#endif /* ISC_MEM_P_H */ +void +isc__mem_checkdestroyed(void); + +void +isc__mem_initialize(void); + +void +isc__mem_shutdown(void); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 5ec2ea0707..eb7a23e3a4 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -216,8 +216,6 @@ isc_nm_start(isc_mem_t *mctx, uint32_t workers) { isc__nm_winsock_initialize(); #endif /* WIN32 */ - isc_tls_initialize(); - mgr = isc_mem_get(mctx, sizeof(*mgr)); *mgr = (isc_nm_t){ .nworkers = workers }; @@ -374,8 +372,6 @@ nm_destroy(isc_nm_t **mgr0) { mgr->nworkers * sizeof(isc__networker_t)); isc_mem_putanddetach(&mgr->mctx, mgr, sizeof(*mgr)); - isc_tls_destroy(); - #ifdef WIN32 isc__nm_winsock_destroy(); #endif /* WIN32 */ diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index 90aa605232..eaa8921b15 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -398,6 +398,7 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer, REQUIRE(VALID_NM(mgr)); REQUIRE(local != NULL); REQUIRE(peer != NULL); + REQUIRE(sslctx != NULL); sa_family = peer->addr.type.sa.sa_family; @@ -1656,6 +1657,16 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { csock->tls.state = TLS_STATE_NONE; csock->tls.ssl = SSL_new(ssock->tls.ctx); + + if (csock->tls.ssl == NULL) { + char errbuf[256]; + unsigned long err = ERR_get_error(); + + ERR_error_string_n(err, errbuf, sizeof(errbuf)); + fprintf(stderr, "%s:SSL_new(%p) -> %s\n", __func__, + ssock->tls.ctx, errbuf); + } + RUNTIME_CHECK(csock->tls.ssl != NULL); r = BIO_new_bio_pair(&csock->tls.ssl_wbio, TLS_BUF_SIZE, diff --git a/lib/isc/tests/tlsdns_test.c b/lib/isc/tests/tlsdns_test.c index 76b8974417..b7bef12cca 100644 --- a/lib/isc/tests/tlsdns_test.c +++ b/lib/isc/tests/tlsdns_test.c @@ -217,8 +217,13 @@ nm_setup(void **state) { int tlsdns_listen_sock = -1; isc_nm_t **nm = NULL; - isc_tlsctx_createserver(NULL, NULL, &tlsdns_listen_ctx); - isc_tlsctx_createclient(&tlsdns_connect_ctx); + if (isc_tlsctx_createserver(NULL, NULL, &tlsdns_listen_ctx) != + ISC_R_SUCCESS) { + return (-1); + } + if (isc_tlsctx_createclient(&tlsdns_connect_ctx) != ISC_R_SUCCESS) { + return (-1); + } tlsdns_listen_addr = (isc_sockaddr_t){ .length = 0 }; tlsdns_listen_sock = setup_ephemeral_port(&tlsdns_listen_addr, diff --git a/lib/isc/tls.c b/lib/isc/tls.c index 57066a4121..cec759d924 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -24,12 +24,12 @@ #include #include "openssl_shim.h" +#include "tls_p.h" static isc_once_t init_once = ISC_ONCE_INIT; static isc_once_t shut_once = ISC_ONCE_INIT; static atomic_bool init_done = ATOMIC_VAR_INIT(false); static atomic_bool shut_done = ATOMIC_VAR_INIT(false); -static isc_mem_t *isc__tls_mctx = NULL; #if OPENSSL_VERSION_NUMBER < 0x10100000L static isc_mutex_t *locks = NULL; @@ -52,53 +52,30 @@ isc__tls_set_thread_id(CRYPTO_THREADID *id) { } #endif -#if 0 -static void * -isc__tls_malloc(size_t size, const char *file, int line) { - UNUSED(file); - UNUSED(line); - - return (isc_mem_allocate(isc__tls_mctx, size)); -} - -static void * -isc__tls_realloc(void *ptr, size_t size, const char *file, int line) { - UNUSED(file); - UNUSED(line); - - return (isc__mem_reallocate(isc__tls_mctx, ptr, size)); -} - static void -isc__tls_free(void *ptr, const char *file, int line) { - UNUSED(file); - UNUSED(line); - - if (ptr == NULL) { - return; - } - - isc__mem_free(isc__tls_mctx, ptr); -} -#endif - -static void -isc__tls_initialize(void) { +tls_initialize(void) { REQUIRE(!atomic_load(&init_done)); - isc_mem_create(&isc__tls_mctx); - /* isc_mem_setdestroycheck(isc__tls_mctx, false); */ - - /* REQUIRE(CRYPTO_set_mem_functions(isc__tls_malloc, isc__tls_realloc, - * isc__tls_free) == 1); */ - #if OPENSSL_VERSION_NUMBER >= 0x10100000L RUNTIME_CHECK(OPENSSL_init_ssl(OPENSSL_INIT_ENGINE_ALL_BUILTIN | OPENSSL_INIT_LOAD_CONFIG, NULL) == 1); #else nlocks = CRYPTO_num_locks(); - locks = isc_mem_get(isc__tls_mctx, nlocks * sizeof(locks[0])); + /* + * 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. + * + * 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. + */ + locks = calloc(nlocks, sizeof(locks[0])); isc_mutexblock_init(locks, nlocks); CRYPTO_set_locking_callback(isc__tls_lock_callback); CRYPTO_THREADID_set_callback(isc__tls_set_thread_id); @@ -127,20 +104,22 @@ isc__tls_initialize(void) { "seeded' message in the OpenSSL FAQ)"); } - atomic_compare_exchange_strong(&init_done, &(bool){ false }, true); + REQUIRE(atomic_compare_exchange_strong(&init_done, &(bool){ false }, + true)); } void -isc_tls_initialize(void) { - isc_result_t result = isc_once_do(&init_once, isc__tls_initialize); +isc__tls_initialize(void) { + isc_result_t result = isc_once_do(&init_once, tls_initialize); REQUIRE(result == ISC_R_SUCCESS); REQUIRE(atomic_load(&init_done)); } static void -isc__tls_destroy(void) { +tls_shutdown(void) { REQUIRE(atomic_load(&init_done)); REQUIRE(!atomic_load(&shut_done)); + #if OPENSSL_VERSION_NUMBER < 0x10100000L CONF_modules_unload(1); @@ -156,24 +135,20 @@ isc__tls_destroy(void) { CRYPTO_set_locking_callback(NULL); - /* REQUIRE(CRYPTO_set_mem_functions(OPENSSL_malloc, OPENSSL_realloc, - * OPENSSL_free) == 1); */ - if (locks != NULL) { - INSIST(isc__tls_mctx != NULL); isc_mutexblock_destroy(locks, nlocks); - isc_mem_put(isc__tls_mctx, locks, nlocks * sizeof(locks[0])); + free(locks); locks = NULL; } #endif - isc_mem_detach(&isc__tls_mctx); - atomic_compare_exchange_strong(&shut_done, &(bool){ false }, true); + REQUIRE(atomic_compare_exchange_strong(&shut_done, &(bool){ false }, + true)); } void -isc_tls_destroy(void) { - isc_result_t result = isc_once_do(&shut_once, isc__tls_destroy); +isc__tls_shutdown(void) { + isc_result_t result = isc_once_do(&shut_once, tls_shutdown); REQUIRE(result == ISC_R_SUCCESS); REQUIRE(atomic_load(&shut_done)); } @@ -198,8 +173,6 @@ isc_tlsctx_createclient(isc_tlsctx_t **ctxp) { REQUIRE(ctxp != NULL && *ctxp == NULL); - isc_tls_initialize(); - method = TLS_client_method(); if (method == NULL) { goto ssl_error; @@ -248,8 +221,6 @@ isc_tlsctx_createserver(const char *keyfile, const char *certfile, REQUIRE(ctxp != NULL && *ctxp == NULL); - isc_tls_initialize(); - if (ephemeral) { INSIST(keyfile == NULL); INSIST(certfile == NULL); @@ -365,6 +336,7 @@ ssl_error: isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_NETMGR, ISC_LOG_ERROR, "Error initializing TLS context: %s", errbuf); + if (ctx != NULL) { SSL_CTX_free(ctx); } diff --git a/lib/isc/tls_p.h b/lib/isc/tls_p.h new file mode 100644 index 0000000000..5d44c64b93 --- /dev/null +++ b/lib/isc/tls_p.h @@ -0,0 +1,18 @@ +/* + * 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 + +void +isc__tls_initialize(void); + +void +isc__tls_shutdown(void); diff --git a/lib/isc/win32/DLLMain.c b/lib/isc/win32/DLLMain.c index 3acfee4f46..c649effbc2 100644 --- a/lib/isc/win32/DLLMain.c +++ b/lib/isc/win32/DLLMain.c @@ -12,6 +12,13 @@ #include #include +#include +#include +#include + +#include "mem_p.h" +#include "tls_p.h" + /* * Called when we enter the DLL */ @@ -27,6 +34,8 @@ __declspec(dllexport) BOOL WINAPI * Disable DllMain() invocation on Thread creation/destruction */ DisableThreadLibraryCalls(hinstDLL); + isc__mem_initialize(); + isc__tls_initialize(); break; /* @@ -34,6 +43,8 @@ __declspec(dllexport) BOOL WINAPI * termination or a call to FreeLibrary. */ case DLL_PROCESS_DETACH: + isc__tls_shutdown(); + isc__mem_shutdown(); break; case DLL_THREAD_ATTACH: @@ -43,6 +54,7 @@ __declspec(dllexport) BOOL WINAPI * disabled. */ INSIST(0); + ISC_UNREACHABLE(); break; default: diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index b02fb22bbb..5797632b87 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -362,6 +362,9 @@ isc_md_get_block_size isc_md_type_get_size isc_md_type_get_block_size isc_md +isc__mem_checkdestroyed +isc__mem_initialize +isc__mem_shutdown isc_mem_attach isc_mem_checkdestroyed isc_mem_create @@ -707,8 +710,8 @@ isc_timermgr_create isc_timermgr_createinctx isc_timermgr_destroy isc_timermgr_poke -isc_tls_initialize -isc_tls_destroy +isc__tls_initialize +isc__tls_shutdown isc_tlsctx_createclient isc_tlsctx_createserver isc_tlsctx_free diff --git a/util/copyrights b/util/copyrights index 5c0d643f4b..ee79ea4c98 100644 --- a/util/copyrights +++ b/util/copyrights @@ -1994,6 +1994,7 @@ ./lib/isc/tests/uv_wrap.h C 2020,2021 ./lib/isc/timer.c C 1998,1999,2000,2001,2002,2004,2005,2007,2008,2009,2011,2012,2013,2014,2015,2016,2017,2018,2019,2020,2021 ./lib/isc/tls.c C 2021 +./lib/isc/tls_p.h C 2021 ./lib/isc/tm.c C 2014,2016,2018,2019,2020,2021 ./lib/isc/unix/dir.c C 1999,2000,2001,2004,2005,2007,2008,2009,2011,2012,2016,2017,2018,2019,2020,2021 ./lib/isc/unix/errno.c C 2016,2018,2019,2020,2021 From 0302e548924b8fc4c0830c30762c4758267c9da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 5 Feb 2021 13:39:38 +0100 Subject: [PATCH 13/13] Add CHANGES note for GL #2433 --- CHANGES | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index 59e62a15d6..60ba9d5311 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,11 @@ +5585. [func] Implementations of memory contexts and memory pools were + refactored to reduce lock contention for shared memory + contexts by replacing mutexes with atomic operations. + The internal memory allocator was simplified so that it + is only a thin wrapper around the system allocator. + Since this change makes the "-M external" named option + redundant, the latter was removed. [GL #2433] + 5584. [bug] Rollback setting IP_DONTFRAG option on the UDP sockets. [GL #2487]