Remove locking mechanism from the isc_mempool

Now, that all the locked mempools have been replaced with simple isc_mem
context, remove unused optional locking from isc_mempool API.

diff --git a/bin/tests/optional/mempool_test.c b/bin/tests/optional/mempool_test.c
index e6cca4d260fb54030851cee292db21b0fffd4006..c3380ff9219acebc7fdb2309921ea56c44a14856 100644 (file)
--- 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 58e1d0ee0088a3967282093f52bf0b1d9d4f0500..0248b03f135fc706ac0932de1d71739b4a18bf70 100644 (file)
--- 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 f84d3005be9f939bfbe99d20375790a1f559470d..e51b281458c311337b6bc471bd7f027305db0d48 100644 (file)
--- 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) {
@@ -1796,10 +1760,6 @@ isc__mempool_get(isc_mempool_t *mpctx0 FLARG) {
        isc__mempool_t *mpctx = (isc__mempool_t *)mpctx0;
        isc_mem_t *mctx = (isc_mem_t *)mpctx->mctx;
 
-       if (mpctx->lock != NULL) {
-               LOCK(mpctx->lock);
-       }
-
        /*
         * Don't let the caller go over quota
         */
@@ -1812,10 +1772,6 @@ isc__mempool_get(isc_mempool_t *mpctx0 FLARG) {
        mpctx->allocated++;
 
 out:
-       if (mpctx->lock != NULL) {
-               UNLOCK(mpctx->lock);
-       }
-
        return (item);
 }
 
@@ -1828,18 +1784,10 @@ isc__mempool_put(isc_mempool_t *mpctx0, void *mem FLARG) {
 
        REQUIRE(mem != NULL);
 
-       if (mpctx->lock != NULL) {
-               LOCK(mpctx->lock);
-       }
-
        INSIST(mpctx->allocated > 0);
        mpctx->allocated--;
 
        isc__mem_put(mctx, mem, mpctx->size FLARG_PASS);
-
-       if (mpctx->lock != NULL) {
-               UNLOCK(mpctx->lock);
-       }
 }
 
 #else /* __SANITIZE_ADDRESS__ */
@@ -1854,10 +1802,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 +1850,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 +1872,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 +1895,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 +1905,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 +1919,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 +1927,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 +1936,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 +1947,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 +1955,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 +1964,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 +1975,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 +1984,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 2fcc8b2688f95a988e68bec734b6e12a6e4552fe..e9ad41d74c63d604c00d676277c2fdc426b383c0 100644 (file)
--- 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 3858c4320db7b011e4b5c5b0931f066dbd1d5c49..de65c63b7d52689669295aca5dd72b0296ffa1b0 100644 (file)
--- 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