Remove the maximum allocation limit from the isc_mempool

The only place where the limits on the maximum number of allocated items
from the pool was the dns_dispatch where we already have different
limits in place.

As an example the maximum number of buffers is guarded by:

    if (disp->mgr->buffers >= DNS_DISPATCH_MAXBUFFERS) {
            UNLOCK(&disp->mgr->buffer_lock);
            return (NULL);
    }

but then at the same time we were limiting the maximum number of items
we can get from the disp->bpool.

By removing the maximum allocation limit from the isc_mempool API, we
can simplify the logic in many places as the isc_mempool_get() would
never fail now and it would always return a chunk of memory.

diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c
index f226d156bf3a24e2c98708d94bc16f433ee56418..d2f6e2c826e2b986af90093fb19fe2b4e36d7580 100644 (file)
--- a/lib/dns/dispatch.c
+++ b/lib/dns/dispatch.c
@@ -1817,19 +1817,16 @@ dns_dispatchmgr_create(isc_mem_t *mctx, dns_dispatchmgr_t **mgrp) {
        isc_mempool_create(mgr->mctx, sizeof(dns_dispatch_t), &mgr->dpool);
 
        isc_mempool_setname(mgr->depool, "dispmgr_depool");
-       isc_mempool_setmaxalloc(mgr->depool, 32768);
        isc_mempool_setfreemax(mgr->depool, 32768);
        isc_mempool_associatelock(mgr->depool, &mgr->depool_lock);
        isc_mempool_setfillcount(mgr->depool, 32);
 
        isc_mempool_setname(mgr->rpool, "dispmgr_rpool");
-       isc_mempool_setmaxalloc(mgr->rpool, 32768);
        isc_mempool_setfreemax(mgr->rpool, 32768);
        isc_mempool_associatelock(mgr->rpool, &mgr->rpool_lock);
        isc_mempool_setfillcount(mgr->rpool, 32);
 
        isc_mempool_setname(mgr->dpool, "dispmgr_dpool");
-       isc_mempool_setmaxalloc(mgr->dpool, 32768);
        isc_mempool_setfreemax(mgr->dpool, 32768);
        isc_mempool_associatelock(mgr->dpool, &mgr->dpool_lock);
        isc_mempool_setfillcount(mgr->dpool, 32);
@@ -1996,14 +1993,12 @@ dns_dispatchmgr_setudp(dns_dispatchmgr_t *mgr, unsigned int buffersize,
                 * complexity.
                 */
                if (maxbuffers > mgr->maxbuffers) {
-                       isc_mempool_setmaxalloc(mgr->bpool, maxbuffers);
                        isc_mempool_setfreemax(mgr->bpool, maxbuffers);
                        mgr->maxbuffers = maxbuffers;
                }
        } else {
                isc_mempool_create(mgr->mctx, buffersize, &mgr->bpool);
                isc_mempool_setname(mgr->bpool, "dispmgr_bpool");
-               isc_mempool_setmaxalloc(mgr->bpool, maxbuffers);
                isc_mempool_setfreemax(mgr->bpool, maxbuffers);
                isc_mempool_associatelock(mgr->bpool, &mgr->bpool_lock);
                isc_mempool_setfillcount(mgr->bpool, 32);
@@ -2012,8 +2007,6 @@ dns_dispatchmgr_setudp(dns_dispatchmgr_t *mgr, unsigned int buffersize,
        /* Create or adjust socket pool */
        if (mgr->spool != NULL) {
                if (maxrequests < DNS_DISPATCH_POOLSOCKS * 2) {
-                       isc_mempool_setmaxalloc(mgr->spool,
-                                               DNS_DISPATCH_POOLSOCKS * 2);
                        isc_mempool_setfreemax(mgr->spool,
                                               DNS_DISPATCH_POOLSOCKS * 2);
                }
@@ -2023,7 +2016,6 @@ dns_dispatchmgr_setudp(dns_dispatchmgr_t *mgr, unsigned int buffersize,
        isc_mempool_create(mgr->mctx, sizeof(dispsocket_t), &mgr->spool);
 
        isc_mempool_setname(mgr->spool, "dispmgr_spool");
-       isc_mempool_setmaxalloc(mgr->spool, maxrequests);
        isc_mempool_setfreemax(mgr->spool, maxrequests);
        isc_mempool_associatelock(mgr->spool, &mgr->spool_lock);
        isc_mempool_setfillcount(mgr->spool, 32);
@@ -2923,7 +2915,6 @@ dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr,
        isc_mutex_init(&disp->sepool_lock);
 
        isc_mempool_setname(disp->sepool, "disp_sepool");
-       isc_mempool_setmaxalloc(disp->sepool, 32768);
        isc_mempool_setfreemax(disp->sepool, 32768);
        isc_mempool_associatelock(disp->sepool, &disp->sepool_lock);
        isc_mempool_setfillcount(disp->sepool, 16);

diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h
index 1d274316dd625aa73aeec4ca6d1330d090d387d3..73c12fe60f21c9d3a4d24d712826504dd139da54 100644 (file)
--- a/lib/isc/include/isc/mem.h
+++ b/lib/isc/include/isc/mem.h
@@ -382,7 +382,6 @@ isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp);
  *\li  mpctxp != NULL and *mpctxp == NULL
  *
  * Defaults:
- *\li  maxalloc = UINT_MAX
  *\li  freemax = 1
  *\li  fillcount = 1
  *
@@ -442,12 +441,10 @@ isc_mempool_associatelock(isc_mempool_t *mpctx, isc_mutex_t *lock);
 /*
  * 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.
+ * unless the imposed externally provided locking protocols are followed.
  *
  * Also note that the quota limits will not always take immediate
- *effect. For instance, setting "maxalloc" to a number smaller than the
- *currently allocated count is permitted.  New allocations will be
- *refused until the count drops below this threshold.
+ * effect.
  *
  * All functions require (in addition to other requirements):
  *     mpctx is a valid memory pool
@@ -471,21 +468,6 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx);
  * Returns current size of the free list.
  */
 
-unsigned int
-isc_mempool_getmaxalloc(isc_mempool_t *mpctx);
-/*!<
- * Returns the maximum allowed number of allocations.
- */
-
-void
-isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit);
-/*%<
- * Sets the maximum allowed number of allocations.
- *
- * Additional requirements:
- *\li  limit > 0
- */
-
 unsigned int
 isc_mempool_getallocated(isc_mempool_t *mpctx);
 /*%<

diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index a2579fbaa6ded79af6f7eb51f8f2651296d4213c..db839740ddf9c3d9808010190590b0f906c86418 100644 (file)
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -170,7 +170,6 @@ struct isc_mempool {
        /*%< optionally locked from here down */
        element *items;          /*%< low water item list */
        size_t size;             /*%< size of each item on this pool */
-       atomic_size_t maxalloc;  /*%< max number of items allowed */
        atomic_size_t allocated; /*%< # of items currently given out */
        atomic_size_t freecount; /*%< # of items on reserved list */
        atomic_size_t freemax;   /*%< # of items allowed on free list */
@@ -876,15 +875,13 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) {
        pool = ISC_LIST_HEAD(ctx->pools);
        if (pool != NULL) {
                fprintf(out, "[Pool statistics]\n");
-               fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %10s %1s\n",
-                       "name", "size", "maxalloc", "allocated", "freecount",
-                       "freemax", "fillcount", "gets", "L");
+               fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %1s\n", "name",
+                       "size", "allocated", "freecount", "freemax",
+                       "fillcount", "gets", "L");
        }
        while (pool != NULL) {
-               fprintf(out,
-                       "%15s %10zu %10zu %10zu %10zu %10zu %10zu %10zu %s\n",
+               fprintf(out, "%15s %10zu %10zu %10zu %10zu %10zu %10zu %s\n",
                        pool->name, pool->size,
-                       atomic_load_relaxed(&pool->maxalloc),
                        atomic_load_relaxed(&pool->allocated),
                        atomic_load_relaxed(&pool->freecount),
                        atomic_load_relaxed(&pool->freemax),
@@ -1206,7 +1203,6 @@ isc_mempool_create(isc_mem_t *mctx, size_t size, isc_mempool_t **mpctxp) {
                .size = size,
        };
 
-       atomic_init(&mpctx->maxalloc, SIZE_MAX);
        atomic_init(&mpctx->allocated, 0);
        atomic_init(&mpctx->freecount, 0);
        atomic_init(&mpctx->freemax, 1);
@@ -1304,18 +1300,8 @@ void *
 isc__mempool_get(isc_mempool_t *mpctx FLARG) {
        REQUIRE(VALID_MEMPOOL(mpctx));
 
-       size_t allocated = atomic_fetch_add_release(&mpctx->allocated, 1);
-       size_t maxalloc = atomic_load_acquire(&mpctx->maxalloc);
-
-       /*
-        * Don't let the caller go over quota.
-        */
-       if (ISC_UNLIKELY(allocated >= maxalloc)) {
-               atomic_fetch_sub_release(&mpctx->allocated, 1);
-               return (NULL);
-       }
-
-       atomic_fetch_add_relaxed(&mpctx->gets, 1);
+       (void)atomic_fetch_add_release(&mpctx->allocated, 1);
+       (void)atomic_fetch_add_relaxed(&mpctx->gets, 1);
 
        return (isc__mem_get(mpctx->mctx, mpctx->size FLARG_PASS));
 }
@@ -1338,16 +1324,8 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) {
 
        REQUIRE(VALID_MEMPOOL(mpctx));
 
-       size_t allocated = atomic_fetch_add_release(&mpctx->allocated, 1);
-       size_t maxalloc = atomic_load_acquire(&mpctx->maxalloc);
-
-       /*
-        * Don't let the caller go over quota
-        */
-       if (ISC_UNLIKELY(allocated >= maxalloc)) {
-               atomic_fetch_sub_release(&mpctx->allocated, 1);
-               return (NULL);
-       }
+       element *item;
+       unsigned int i;
 
        MPCTXLOCK(mpctx);
        if (ISC_UNLIKELY(mpctx->items == NULL)) {
@@ -1371,7 +1349,6 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) {
         */
        item = mpctx->items;
        if (ISC_UNLIKELY(item == NULL)) {
-               atomic_fetch_sub_release(&mpctx->allocated, 1);
                goto out;
        }
 
@@ -1379,6 +1356,7 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) {
 
        INSIST(atomic_fetch_sub_release(&mpctx->freecount, 1) > 0);
        atomic_fetch_add_relaxed(&mpctx->gets, 1);
+       atomic_fetch_add_release(&mpctx->allocated, 1);
 
        ADD_TRACE(mpctx->mctx, item, mpctx->size, file, line);
 
@@ -1453,21 +1431,6 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx) {
        return (atomic_load_relaxed(&mpctx->freecount));
 }
 
-void
-isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit) {
-       REQUIRE(VALID_MEMPOOL(mpctx));
-       REQUIRE(limit > 0);
-
-       atomic_store_release(&mpctx->maxalloc, limit);
-}
-
-unsigned int
-isc_mempool_getmaxalloc(isc_mempool_t *mpctx) {
-       REQUIRE(VALID_MEMPOOL(mpctx));
-
-       return (atomic_load_relaxed(&mpctx->maxalloc));
-}
-
 unsigned int
 isc_mempool_getallocated(isc_mempool_t *mpctx) {
        REQUIRE(VALID_MEMPOOL(mpctx));

diff --git a/lib/isc/tests/mem_test.c b/lib/isc/tests/mem_test.c
index 4b42b39de08a337d53b8738a5a8e9cd1d0b56135..857951c5bc925cfd6cec0be9116b473e51001218 100644 (file)
--- a/lib/isc/tests/mem_test.c
+++ b/lib/isc/tests/mem_test.c
@@ -82,7 +82,6 @@ isc_mem_test(void **state) {
 
        isc_mempool_setfreemax(mp1, MP1_FREEMAX);
        isc_mempool_setfillcount(mp1, MP1_FILLCNT);
-       isc_mempool_setmaxalloc(mp1, MP1_MAXALLOC);
 
        /*
         * Allocate MP1_MAXALLOC items from the pool.  This is our max.
@@ -92,12 +91,6 @@ isc_mem_test(void **state) {
                assert_non_null(items1[i]);
        }
 
-       /*
-        * Try to allocate one more.  This should fail.
-        */
-       tmp = isc_mempool_get(mp1);
-       assert_null(tmp);
-
        /*
         * Free the first 11 items.  Verify that there are 10 free items on
         * the free list (which is our max).

diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in
index 3662fd57ab3d35493030a7b9dde5915ae7091fef..f20ac37c7b8cc9eaf6641c27cf8f629167d0fdff 100644 (file)
--- a/lib/isc/win32/libisc.def.in
+++ b/lib/isc/win32/libisc.def.in
@@ -397,10 +397,8 @@ isc_mempool_getallocated
 isc_mempool_getfillcount
 isc_mempool_getfreecount
 isc_mempool_getfreemax
-isc_mempool_getmaxalloc
 isc_mempool_setfillcount
 isc_mempool_setfreemax
-isc_mempool_setmaxalloc
 isc_mempool_setname
 isc_mutexblock_destroy
 isc_mutexblock_init