]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Clean up isc_mempool API
authorOndřej Surý <ondrej@sury.org>
Wed, 12 May 2021 22:29:11 +0000 (00:29 +0200)
committerOndřej Surý <ondrej@sury.org>
Fri, 9 Jul 2021 13:58:02 +0000 (15:58 +0200)
- isc_mempool_get() can no longer fail; when there are no more objects
  in the pool, more are always allocated. checking for NULL return is
  no longer necessary.
- the isc_mempool_setmaxalloc() and isc_mempool_getmaxalloc() functions
  are no longer used and have been removed.

cocci/isc_mempool_get_never_fail.spatch [new file with mode: 0644]
lib/dns/message.c
lib/isc/include/isc/mem.h
lib/isc/mem.c
lib/isc/tests/mem_test.c

diff --git a/cocci/isc_mempool_get_never_fail.spatch b/cocci/isc_mempool_get_never_fail.spatch
new file mode 100644 (file)
index 0000000..232ff9b
--- /dev/null
@@ -0,0 +1,41 @@
+@@
+statement S;
+expression V;
+@@
+
+V = isc_mempool_get(...);
+- if (V == NULL) S
+
+@@
+type T;
+statement S;
+expression V;
+@@
+
+V = (T *)isc_mempool_get(...);
+- if (V == NULL) S
+
+@@
+statement S;
+expression V;
+@@
+
+if (V == NULL) V = isc_mempool_get(...);
+- if (V == NULL) S
+
+@@
+statement S1, S2;
+expression V;
+@@
+
+V = isc_mempool_get(...);
+- if (V == NULL) S1 else { S2 }
++ S2
+
+@@
+type T;
+expression V, E1, E2;
+@@
+
+- V = (T)isc_mempool_get(E1, E2);
++ V = isc_mempool_get(E1, E2);
index ad21bb17b99a798ce9eda1a2c67f2aa2011bf578..976f3e78efe5c3cb7fbd0e5e5254d7e891bbd137 100644 (file)
@@ -1076,10 +1076,6 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                        goto cleanup;
                }
                rdataset = isc_mempool_get(msg->rdspool);
-               if (rdataset == NULL) {
-                       result = ISC_R_NOMEMORY;
-                       goto cleanup;
-               }
 
                /*
                 * Convert rdatalist to rdataset, and attach the latter to
@@ -1516,10 +1512,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
 
                if (result == ISC_R_NOTFOUND) {
                        rdataset = isc_mempool_get(msg->rdspool);
-                       if (rdataset == NULL) {
-                               result = ISC_R_NOMEMORY;
-                               goto cleanup;
-                       }
                        free_rdataset = true;
 
                        rdatalist = newrdatalist(msg);
@@ -2558,9 +2550,6 @@ dns_message_gettemprdataset(dns_message_t *msg, dns_rdataset_t **item) {
        REQUIRE(item != NULL && *item == NULL);
 
        *item = isc_mempool_get(msg->rdspool);
-       if (*item == NULL) {
-               return (ISC_R_NOMEMORY);
-       }
 
        dns_rdataset_init(*item);
        return (ISC_R_SUCCESS);
index f129285f90e4ae1cea1615b3ac48f6da00d463bf..fde46200a5ff533ee30602946a834ff10f5f751f 100644 (file)
@@ -375,7 +375,6 @@ isc__mempool_create(isc_mem_t *mctx, size_t size,
  *\li  mpctxp != NULL and *mpctxp == NULL
  *
  * Defaults:
- *\li  maxalloc = UINT_MAX
  *\li  freemax = 1
  *\li  fillcount = 1
  *
@@ -412,9 +411,7 @@ isc_mempool_setname(isc_mempool_t *mpctx, const char *name);
  *unless the imposed externally provided locking protocols are followed.
  *
  * Also note that the quota limits will not always take immediate
- *effect. For instance, setting "maxalloc" to a number smaller than the
- *currently allocated count is permitted.  New allocations will be
- *refused until the count drops below this threshold.
+ * effect.
  *
  * All functions require (in addition to other requirements):
  *     mpctx is a valid memory pool
@@ -438,21 +435,6 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx);
  * Returns current size of the free list.
  */
 
-unsigned int
-isc_mempool_getmaxalloc(isc_mempool_t *mpctx);
-/*!<
- * Returns the maximum allowed number of allocations.
- */
-
-void
-isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit);
-/*%<
- * Sets the maximum allowed number of allocations.
- *
- * Additional requirements:
- *\li  limit > 0
- */
-
 unsigned int
 isc_mempool_getallocated(isc_mempool_t *mpctx);
 /*%<
index 9f3a6143d9f5af2de6f1db5f47fbff5b5cbfd67b..52e2b998eae0997e1df328c24021f96dbb8c7efe 100644 (file)
@@ -154,7 +154,6 @@ struct isc_mempool {
        ISC_LINK(isc_mempool_t) link; /*%< next pool in this mem context */
        element *items;               /*%< low water item list */
        size_t size;                  /*%< size of each item on this pool */
-       atomic_size_t maxalloc;       /*%< max number of items allowed */
        atomic_size_t allocated;      /*%< # of items currently given out */
        atomic_size_t freecount;      /*%< # of items on reserved list */
        atomic_size_t freemax;        /*%< # of items allowed on free list */
@@ -805,15 +804,14 @@ isc_mem_stats(isc_mem_t *ctx, FILE *out) {
        pool = ISC_LIST_HEAD(ctx->pools);
        if (pool != NULL) {
                fprintf(out, "[Pool statistics]\n");
-               fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %10s %1s\n",
-                       "name", "size", "maxalloc", "allocated", "freecount",
-                       "freemax", "fillcount", "gets", "L");
+               fprintf(out, "%15s %10s %10s %10s %10s %10s %10s %1s\n", "name",
+                       "size", "allocated", "freecount", "freemax",
+                       "fillcount", "gets", "L");
        }
        while (pool != NULL) {
                fprintf(out,
                        "%15s %10zu %10zu %10zu %10zu %10zu %10zu %10zu %s\n",
-                       pool->name, pool->size,
-                       atomic_load_relaxed(&pool->maxalloc),
+                       pool->name, pool->size, (size_t)0,
                        atomic_load_relaxed(&pool->allocated),
                        atomic_load_relaxed(&pool->freecount),
                        atomic_load_relaxed(&pool->freemax),
@@ -1109,7 +1107,6 @@ isc__mempool_create(isc_mem_t *mctx, size_t size,
                .size = size,
        };
 
-       atomic_init(&mpctx->maxalloc, SIZE_MAX);
        atomic_init(&mpctx->allocated, 0);
        atomic_init(&mpctx->freecount, 0);
        atomic_init(&mpctx->freemax, 1);
@@ -1199,17 +1196,7 @@ void *
 isc__mempool_get(isc_mempool_t *mpctx FLARG) {
        REQUIRE(VALID_MEMPOOL(mpctx));
 
-       allocated = atomic_fetch_add_release(&mpctx->allocated, 1);
-       maxalloc = atomic_load_acquire(&mpctx->maxalloc);
-
-       /*
-        * Don't let the caller go over quota.
-        */
-       if (ISC_UNLIKELY(allocated >= maxalloc)) {
-               atomic_fetch_sub_release(&mpctx->allocated, 1);
-               return (NULL);
-       }
-
+       (void)atomic_fetch_add_relaxed(&mpctx->allocated, 1);
        atomic_fetch_add_relaxed(&mpctx->gets, 1);
 
        return (isc__mem_get(mpctx->mctx, mpctx->size FLARG_PASS));
@@ -1220,8 +1207,7 @@ isc__mempool_put(isc_mempool_t *mpctx, void *mem FLARG) {
        REQUIRE(VALID_MEMPOOL(mpctx));
        REQUIRE(mem != NULL);
 
-       INSIST(atomic_fetch_sub_release(&mpctx->allocated, 1) > 0);
-
+       atomic_fetch_sub_relaxed(&mpctx->allocated, 1);
        isc__mem_put(mpctx->mctx, mem, mpctx->size FLARG_PASS);
 }
 
@@ -1234,16 +1220,7 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) {
 
        REQUIRE(VALID_MEMPOOL(mpctx));
 
-       allocated = atomic_fetch_add_release(&mpctx->allocated, 1);
-       maxalloc = atomic_load_acquire(&mpctx->maxalloc);
-
-       /*
-        * Don't let the caller go over quota
-        */
-       if (ISC_UNLIKELY(allocated >= maxalloc)) {
-               atomic_fetch_sub_release(&mpctx->allocated, 1);
-               return (NULL);
-       }
+       (void)atomic_fetch_add_release(&mpctx->allocated, 1);
 
        if (ISC_UNLIKELY(mpctx->items == NULL)) {
                isc_mem_t *mctx = mpctx->mctx;
@@ -1333,21 +1310,6 @@ isc_mempool_getfreecount(isc_mempool_t *mpctx) {
        return (atomic_load_relaxed(&mpctx->freecount));
 }
 
-void
-isc_mempool_setmaxalloc(isc_mempool_t *mpctx, unsigned int limit) {
-       REQUIRE(VALID_MEMPOOL(mpctx));
-       REQUIRE(limit > 0);
-
-       atomic_store_release(&mpctx->maxalloc, limit);
-}
-
-unsigned int
-isc_mempool_getmaxalloc(isc_mempool_t *mpctx) {
-       REQUIRE(VALID_MEMPOOL(mpctx));
-
-       return (atomic_load_relaxed(&mpctx->maxalloc));
-}
-
 unsigned int
 isc_mempool_getallocated(isc_mempool_t *mpctx) {
        REQUIRE(VALID_MEMPOOL(mpctx));
index d930971c73742f838533240a9e5470a02b45bc1b..c9602927f9177e5616508c5f4faa119588cc04e5 100644 (file)
@@ -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).