From 346fb2a550ae038894a492bb1dd736146fd5d977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 13 Dec 2021 09:45:06 +0100 Subject: [PATCH] Remove locking mechanism from the isc_mempool Now, that all the locked mempools have been replaced with simple isc_mem context, remove now unused optional locking from isc_mempool. --- bin/tests/optional/mempool_test.c | 8 -- lib/isc/include/isc/mem.h | 27 ----- lib/isc/mem.c | 165 +++--------------------------- lib/isc/tests/mem_test.c | 66 ------------ lib/isc/win32/libisc.def.in | 1 - 5 files changed, 15 insertions(+), 252 deletions(-) diff --git a/bin/tests/optional/mempool_test.c b/bin/tests/optional/mempool_test.c index e6cca4d260..c3380ff921 100644 --- a/bin/tests/optional/mempool_test.c +++ b/bin/tests/optional/mempool_test.c @@ -21,15 +21,12 @@ main(int argc, char *argv[]) { void *tmp; isc_mempool_t *mp1, *mp2; unsigned int i, j; - isc_mutex_t lock; UNUSED(argc); UNUSED(argv); isc_mem_debugging = ISC_MEM_DEBUGRECORD; - isc_mutex_init(&lock); - mctx = NULL; isc_mem_create(&mctx); @@ -39,9 +36,6 @@ main(int argc, char *argv[]) { mp2 = NULL; isc_mempool_create(mctx, 31, &mp2); - isc_mempool_associatelock(mp1, &lock); - isc_mempool_associatelock(mp2, &lock); - isc_mem_stats(mctx, stderr); isc_mempool_setfreemax(mp1, 10); @@ -112,7 +106,5 @@ main(int argc, char *argv[]) { isc_mem_destroy(&mctx); - isc_mutex_destroy(&lock); - return (0); } diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index ed8b13398c..5d2ac73da3 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -495,33 +495,6 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name); *\li name != NULL; */ -void -isc_mempool_associatelock(isc_mempool_t *mpctx, isc_mutex_t *lock); -/*%< - * Associate a lock with this memory pool. - * - * This lock is used when getting or putting items using this memory pool, - * and it is also used to set or get internal state via the isc_mempool_get*() - * and isc_mempool_set*() set of functions. - * - * Multiple pools can each share a single lock. For instance, if "manager" - * type object contained pools for various sizes of events, and each of - * these pools used a common lock. Note that this lock must NEVER be used - * by other than mempool routines once it is given to a pool, since that can - * easily cause double locking. - * - * Requires: - * - *\li mpctpx is a valid pool. - * - *\li lock != NULL. - * - *\li No previous lock is assigned to this pool. - * - *\li The lock is initialized before calling this function via the normal - * means of doing that. - */ - /* * The following functions get/set various parameters. Note that due to * the unlocked nature of pools these are potentially random values unless diff --git a/lib/isc/mem.c b/lib/isc/mem.c index f84d3005be..6c5e86222b 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -185,19 +185,16 @@ struct isc__mem { 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 */ - /*%< locked via the memory context's lock */ + isc_mempool_t common; /*%< common header of mempool's */ + isc__mem_t *mctx; /*%< our memory context */ ISC_LINK(isc__mempool_t) link; /*%< next pool in this mem context */ - /*%< optionally locked from here down */ - element *items; /*%< low water item list */ - size_t size; /*%< size of each item on this pool */ - 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 */ + 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 */ /*%< Stats only. */ unsigned int gets; /*%< # of requests to this pool */ /*%< Debugging only. */ @@ -1259,8 +1256,7 @@ isc_mem_stats(isc_mem_t *ctx0, FILE *out) { #endif /* if ISC_MEMPOOL_NAMES */ (unsigned long)pool->size, pool->maxalloc, pool->allocated, pool->freecount, pool->freemax, - pool->fillcount, pool->gets, - (pool->lock == NULL ? "N" : "Y")); + pool->fillcount, pool->gets, "N"); pool = ISC_LIST_NEXT(pool, link); } @@ -1655,7 +1651,6 @@ isc_mempool_create(isc_mem_t *mctx0, size_t size, isc_mempool_t **mpctxp) { mpctx->common.impmagic = MEMPOOL_MAGIC; mpctx->common.magic = ISCAPI_MPOOL_MAGIC; - mpctx->lock = NULL; mpctx->mctx = mctx; /* * Mempools are stored as a linked list of element. @@ -1691,15 +1686,7 @@ isc_mempool_setname(isc_mempool_t *mpctx0, const char *name) { isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; #if ISC_MEMPOOL_NAMES - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - strlcpy(mpctx->name, name, sizeof(mpctx->name)); - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } #else /* if ISC_MEMPOOL_NAMES */ UNUSED(mpctx); UNUSED(name); @@ -1713,7 +1700,6 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { isc__mempool_t *mpctx; isc__mem_t *mctx; - isc_mutex_t *lock; element *item; mpctx = (isc__mempool_t *)*mpctxp; @@ -1729,12 +1715,6 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { mctx = mpctx->mctx; - lock = mpctx->lock; - - if (lock != NULL) { - LOCK(lock); - } - /* * Return any items on the free list */ @@ -1767,25 +1747,9 @@ isc_mempool_destroy(isc_mempool_t **mpctxp) { isc_mem_put((isc_mem_t *)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)); - REQUIRE(lock != NULL); - - isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - - REQUIRE(mpctx->lock == NULL); - - mpctx->lock = lock; -} - #if __SANITIZE_ADDRESS__ void * isc__mempool_get(isc_mempool_t *mpctx0 FLARG) { @@ -1854,10 +1818,6 @@ isc__mempool_get(isc_mempool_t *mpctx0 FLARG) { mctx = mpctx->mctx; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - /* * Don't let the caller go over quota */ @@ -1906,10 +1866,6 @@ isc__mempool_get(isc_mempool_t *mpctx0 FLARG) { mpctx->allocated++; out: - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - #if ISC_MEM_TRACKLINES if (ISC_UNLIKELY(((isc_mem_debugging & TRACE_OR_RECORD) != 0) && item != NULL)) { @@ -1932,10 +1888,6 @@ isc__mempool_put(isc_mempool_t *mpctx0, void *mem FLARG) { isc__mem_t *mctx = mpctx->mctx; element *item; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - INSIST(mpctx->allocated > 0); mpctx->allocated--; @@ -1959,9 +1911,6 @@ isc__mempool_put(isc_mempool_t *mpctx0, void *mem FLARG) { mem_put(mctx, mem, mpctx->size); } MCTXUNLOCK(mctx); - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } return; } @@ -1972,10 +1921,6 @@ isc__mempool_put(isc_mempool_t *mpctx0, void *mem FLARG) { item = (element *)mem; item->next = mpctx->items; mpctx->items = item; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } } #endif /* __SANITIZE_ADDRESS__ */ @@ -1990,15 +1935,7 @@ isc_mempool_setfreemax(isc_mempool_t *mpctx0, unsigned int limit) { isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - mpctx->freemax = limit; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } } unsigned int @@ -2006,19 +1943,8 @@ isc_mempool_getfreemax(isc_mempool_t *mpctx0) { REQUIRE(VALID_MEMPOOL(mpctx0)); isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - unsigned int freemax; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - freemax = mpctx->freemax; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (freemax); + return (mpctx->freemax); } unsigned int @@ -2026,19 +1952,8 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx0) { REQUIRE(VALID_MEMPOOL(mpctx0)); isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - unsigned int freecount; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - freecount = mpctx->freecount; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (freecount); + return (mpctx->freecount); } void @@ -2048,15 +1963,7 @@ isc_mempool_setmaxalloc(isc_mempool_t *mpctx0, unsigned int limit) { isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - mpctx->maxalloc = limit; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } } unsigned int @@ -2064,19 +1971,8 @@ isc_mempool_getmaxalloc(isc_mempool_t *mpctx0) { REQUIRE(VALID_MEMPOOL(mpctx0)); isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - unsigned int maxalloc; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - maxalloc = mpctx->maxalloc; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (maxalloc); + return (mpctx->maxalloc); } unsigned int @@ -2084,19 +1980,8 @@ isc_mempool_getallocated(isc_mempool_t *mpctx0) { REQUIRE(VALID_MEMPOOL(mpctx0)); isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - unsigned int allocated; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - allocated = mpctx->allocated; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (allocated); + return (mpctx->allocated); } void @@ -2106,15 +1991,7 @@ isc_mempool_setfillcount(isc_mempool_t *mpctx0, unsigned int limit) { isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - mpctx->fillcount = limit; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } } unsigned int @@ -2123,19 +2000,7 @@ isc_mempool_getfillcount(isc_mempool_t *mpctx0) { isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0; - unsigned int fillcount; - - if (mpctx->lock != NULL) { - LOCK(mpctx->lock); - } - - fillcount = mpctx->fillcount; - - if (mpctx->lock != NULL) { - UNLOCK(mpctx->lock); - } - - return (fillcount); + return (mpctx->fillcount); } /* diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c index 2fcc8b2688..e9ad41d74c 100644 --- a/lib/isc/tests/mem_test.c +++ b/lib/isc/tests/mem_test.c @@ -424,70 +424,6 @@ isc_mem_benchmark(void **state) { (nthreads * ITERS * NUM_ITEMS) / (t / 1000000.0)); } -static isc_threadresult_t -mempool_thread(isc_threadarg_t arg) { - isc_mempool_t *mp = (isc_mempool_t *)arg; - void *items[NUM_ITEMS]; - - for (int i = 0; i < ITERS; i++) { - for (int j = 0; j < NUM_ITEMS; j++) { - items[j] = isc_mempool_get(mp); - } - for (int j = 0; j < NUM_ITEMS; j++) { - isc_mempool_put(mp, items[j]); - } - } - - return ((isc_threadresult_t)0); -} - -static void -isc_mempool_benchmark(void **state) { - int nthreads = ISC_MAX(ISC_MIN(isc_os_ncpus(), 32), 1); - isc_thread_t threads[32]; - isc_time_t ts1, ts2; - double t; - isc_result_t result; - size_t size = ITEM_SIZE; - isc_mempool_t *mp = NULL; - isc_mutex_t mplock; - - isc_mutex_init(&mplock); - - isc_mempool_create(test_mctx, ITEM_SIZE, &mp); - - isc_mempool_associatelock(mp, &mplock); - - isc_mempool_setfreemax(mp, 32768); - isc_mempool_setfillcount(mp, ISC_MAX(NUM_ITEMS / nthreads, 1)); - - UNUSED(state); - - result = isc_time_now(&ts1); - assert_int_equal(result, ISC_R_SUCCESS); - - for (int i = 0; i < nthreads; i++) { - isc_thread_create(mempool_thread, mp, &threads[i]); - size = size / 2; - } - for (int i = 0; i < nthreads; i++) { - isc_thread_join(threads[i], NULL); - } - - result = isc_time_now(&ts2); - assert_int_equal(result, ISC_R_SUCCESS); - - t = isc_time_microdiff(&ts2, &ts1); - - printf("[ TIME ] isc_mempool_benchmark: " - "%d isc_mempool_{get,put} calls, %f seconds, %f calls/second\n", - nthreads * ITERS * NUM_ITEMS, t / 1000000.0, - (nthreads * ITERS * NUM_ITEMS) / (t / 1000000.0)); - - isc_mempool_destroy(&mp); - isc_mutex_destroy(&mplock); -} - #endif /* __SANITIZE_THREAD */ /* @@ -507,8 +443,6 @@ main(void) { #if !defined(__SANITIZE_THREAD__) cmocka_unit_test_setup_teardown(isc_mem_benchmark, _setup, _teardown), - cmocka_unit_test_setup_teardown(isc_mempool_benchmark, _setup, - _teardown), #endif /* __SANITIZE_THREAD__ */ #if ISC_MEM_TRACKLINES cmocka_unit_test_setup_teardown(isc_mem_noflags_test, _setup, diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index 3858c4320d..de65c63b7d 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -398,7 +398,6 @@ isc_mem_stats isc_mem_total isc_mem_waterack isc_meminfo_totalphys -isc_mempool_associatelock isc_mempool_create isc_mempool_destroy isc_mempool_getallocated