simplify the isc_stat structure to take avantage of atomics

diff --git a/lib/isc/stats.c b/lib/isc/stats.c
index e36ea447da0068ebf846d6d31fb635d3a8347d6b..883c1c373b19d9cf8d2b0d245175a56c10346ca7 100644 (file)
--- a/lib/isc/stats.c
+++ b/lib/isc/stats.c
@@ -21,7 +21,7 @@
 #include <isc/mem.h>
 #include <isc/platform.h>
 #include <isc/print.h>
-#include <isc/rwlock.h>
+#include <isc/refcount.h>
 #include <isc/stats.h>
 #include <isc/util.h>
 
@@ -31,78 +31,30 @@
 typedef atomic_int_fast64_t isc_stat_t;
 
 struct isc_stats {
-       /*% Unlocked */
-       unsigned int    magic;
-       isc_mem_t       *mctx;
-       int             ncounters;
-
-       isc_mutex_t     lock;
-       unsigned int    references; /* locked by lock */
-
-       /*%
-        * Locked by counterlock or unlocked if efficient rwlock is not
-        * available.
-        */
-       isc_stat_t      *counters;
-
-       /*%
-        * We don't want to lock the counters while we are dumping, so we first
-        * copy the current counter values into a local array.  This buffer
-        * will be used as the copy destination.  It's allocated on creation
-        * of the stats structure so that the dump operation won't fail due
-        * to memory allocation failure.
-        * XXX: this approach is weird for non-threaded build because the
-        * additional memory and the copy overhead could be avoided.  We prefer
-        * simplicity here, however, under the assumption that this function
-        * should be only rarely called.
-        */
-       uint64_t        *copiedcounters;
+       unsigned int            magic;
+       isc_mem_t               *mctx;
+       isc_refcount_t          refs;
+       int                     ncounters;
+       isc_stat_t              *counters;
 };
 
 static isc_result_t
 create_stats(isc_mem_t *mctx, int ncounters, isc_stats_t **statsp) {
        isc_stats_t *stats;
-       isc_result_t result = ISC_R_SUCCESS;
 
        REQUIRE(statsp != NULL && *statsp == NULL);
 
        stats = isc_mem_get(mctx, sizeof(*stats));
-       if (stats == NULL)
-               return (ISC_R_NOMEMORY);
-
-       isc_mutex_init(&stats->lock);
-
        stats->counters = isc_mem_get(mctx, sizeof(isc_stat_t) * ncounters);
-       if (stats->counters == NULL) {
-               result = ISC_R_NOMEMORY;
-               goto clean_mutex;
-       }
-       stats->copiedcounters = isc_mem_get(mctx,
-                                           sizeof(uint64_t) * ncounters);
-       if (stats->copiedcounters == NULL) {
-               result = ISC_R_NOMEMORY;
-               goto clean_counters;
-       }
-
-       stats->references = 1;
+       isc_refcount_init(&stats->refs, 1);
        memset(stats->counters, 0, sizeof(isc_stat_t) * ncounters);
        stats->mctx = NULL;
        isc_mem_attach(mctx, &stats->mctx);
        stats->ncounters = ncounters;
        stats->magic = ISC_STATS_MAGIC;
-
        *statsp = stats;
 
-       return (result);
-
-clean_counters:
-       isc_mem_put(mctx, stats->counters, sizeof(isc_stat_t) * ncounters);
-
-clean_mutex:
-       isc_mutex_destroy(&stats->lock);
-       isc_mem_put(mctx, stats, sizeof(*stats));
-
-       return (result);
+       return (ISC_R_SUCCESS);
 }
 
 void
@@ -110,10 +62,7 @@ isc_stats_attach(isc_stats_t *stats, isc_stats_t **statsp) {
        REQUIRE(ISC_STATS_VALID(stats));
        REQUIRE(statsp != NULL && *statsp == NULL);
 
-       LOCK(&stats->lock);
-       stats->references++;
-       UNLOCK(&stats->lock);
-
+       isc_refcount_increment(&stats->refs);
        *statsp = stats;
 }
 
@@ -126,21 +75,11 @@ isc_stats_detach(isc_stats_t **statsp) {
        stats = *statsp;
        *statsp = NULL;
 
-       LOCK(&stats->lock);
-       stats->references--;
-
-       if (stats->references == 0) {
-               isc_mem_put(stats->mctx, stats->copiedcounters,
-                           sizeof(isc_stat_t) * stats->ncounters);
+       if (isc_refcount_decrement(&stats->refs) == 1) {
                isc_mem_put(stats->mctx, stats->counters,
                            sizeof(isc_stat_t) * stats->ncounters);
-               UNLOCK(&stats->lock);
-               isc_mutex_destroy(&stats->lock);
                isc_mem_putanddetach(&stats->mctx, stats, sizeof(*stats));
-               return;
        }
-
-       UNLOCK(&stats->lock);
 }
 
 int
@@ -184,16 +123,12 @@ isc_stats_dump(isc_stats_t *stats, isc_stats_dumper_t dump_fn,
        REQUIRE(ISC_STATS_VALID(stats));
 
        for (i = 0; i < stats->ncounters; i++) {
-               stats->copiedcounters[i] =
-                       atomic_load_explicit(&stats->counters[i],
-                                            memory_order_relaxed);
-       }
-
-       for (i = 0; i < stats->ncounters; i++) {
-               if ((options & ISC_STATSDUMP_VERBOSE) == 0 &&
-                   stats->copiedcounters[i] == 0)
-                               continue;
-               dump_fn((isc_statscounter_t)i, stats->copiedcounters[i], arg);
+               uint32_t counter = atomic_load_explicit(&stats->counters[i],
+                                                       memory_order_relaxed);
+               if ((options & ISC_STATSDUMP_VERBOSE) == 0 && counter == 0) {
+                       continue;
+               }
+               dump_fn((isc_statscounter_t)i, counter, arg);
        }
 }