From: Ondřej Surý <ondrej@sury.org>
Date: Sat, 17 Mar 2018 20:51:45 +0000 (+0000)
Subject: Reintroduce the rwlock needed for dumping stats consistently
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=41b34f74162e39e8801be5ff8537f4647d935886;p=thirdparty%2Fbind9.git

Reintroduce the rwlock needed for dumping stats consistently
---

diff --git a/lib/isc/stats.c b/lib/isc/stats.c
index 8c4fd62e83e..36c8f646a7c 100644
--- a/lib/isc/stats.c
+++ b/lib/isc/stats.c
@@ -44,6 +44,7 @@ struct isc_stats {
 	 * Locked by counterlock or unlocked if efficient rwlock is not
 	 * available.
 	 */
+	isc_rwlock_t	counterlock;
 	isc_stat_t	*counters;
 
 	/*%
@@ -80,13 +81,19 @@ create_stats(isc_mem_t *mctx, int ncounters, isc_stats_t **statsp) {
 		result = ISC_R_NOMEMORY;
 		goto clean_mutex;
 	}
+
 	stats->copiedcounters = isc_mem_get(mctx,
 					    sizeof(isc_uint64_t) * ncounters);
 	if (stats->copiedcounters == NULL) {
 		result = ISC_R_NOMEMORY;
 		goto clean_counters;
 	}
-
+	
+	result = isc_rwlock_init(&stats->counterlock, 0, 0);
+	if (result != ISC_R_SUCCESS) {
+		goto clean_copiedcounters;
+	}
+	
 	stats->references = 1;
 	memset(stats->counters, 0, sizeof(isc_stat_t) * ncounters);
 	stats->mctx = NULL;
@@ -98,6 +105,10 @@ create_stats(isc_mem_t *mctx, int ncounters, isc_stats_t **statsp) {
 
 	return (result);
 
+clean_copiedcounters:
+	isc_mem_put(mctx, stats->copiedcounters,
+		    sizeof(isc_stat_t) * ncounters);
+	
 clean_counters:
 	isc_mem_put(mctx, stats->counters, sizeof(isc_stat_t) * ncounters);
 
@@ -141,6 +152,7 @@ isc_stats_detach(isc_stats_t **statsp) {
 			    sizeof(isc_stat_t) * stats->ncounters);
 		UNLOCK(&stats->lock);
 		DESTROYLOCK(&stats->lock);
+		isc_rwlock_destroy(&stats->counterlock);
 		isc_mem_putanddetach(&stats->mctx, stats, sizeof(*stats));
 		return;
 	}
@@ -155,17 +167,6 @@ isc_stats_ncounters(isc_stats_t *stats) {
 	return (stats->ncounters);
 }
 
-static void
-copy_counters(isc_stats_t *stats) {
-	int i;
-
-	for (i = 0; i < stats->ncounters; i++) {
-		stats->copiedcounters[i] =
-			atomic_load_explicit(&stats->counters[i],
-					     memory_order_relaxed);
-	}
-}
-
 isc_result_t
 isc_stats_create(isc_mem_t *mctx, isc_stats_t **statsp, int ncounters) {
 	REQUIRE(statsp != NULL && *statsp == NULL);
@@ -177,9 +178,15 @@ void
 isc_stats_increment(isc_stats_t *stats, isc_statscounter_t counter) {
 	REQUIRE(ISC_STATS_VALID(stats));
 	REQUIRE(counter < stats->ncounters);
-
+	/*
+	 * We use a "read" lock to prevent other threads from reading the
+	 * counter while we "writing" a counter field.  The write access itself
+	 * is protected by the atomic operation.
+	 */
+	isc_rwlock_lock(&stats->counterlock, isc_rwlocktype_read);
 	atomic_fetch_add_explicit(&stats->counters[counter], 1,
 				  memory_order_relaxed);
+	isc_rwlock_unlock(&stats->counterlock, isc_rwlocktype_read);
 }
 
 void
@@ -187,8 +194,10 @@ isc_stats_decrement(isc_stats_t *stats, isc_statscounter_t counter) {
 	REQUIRE(ISC_STATS_VALID(stats));
 	REQUIRE(counter < stats->ncounters);
 
+	isc_rwlock_lock(&stats->counterlock, isc_rwlocktype_read);
 	atomic_fetch_sub_explicit(&stats->counters[counter], 1,
 				  memory_order_relaxed);
+	isc_rwlock_unlock(&stats->counterlock, isc_rwlocktype_read);
 }
 
 void
@@ -199,7 +208,17 @@ isc_stats_dump(isc_stats_t *stats, isc_stats_dumper_t dump_fn,
 
 	REQUIRE(ISC_STATS_VALID(stats));
 
-	copy_counters(stats);
+	/*
+	 * We use a "write" lock before "reading" the statistics counters as
+	 * an exclusive lock.
+	 */
+	isc_rwlock_lock(&stats->counterlock, isc_rwlocktype_write);
+	for (i = 0; i < stats->ncounters; i++) {
+		stats->copiedcounters[i] =
+			atomic_load_explicit(&stats->counters[i],
+					     memory_order_relaxed);
+	}
+	isc_rwlock_unlock(&stats->counterlock, isc_rwlocktype_write);
 
 	for (i = 0; i < stats->ncounters; i++) {
 		if ((options & ISC_STATSDUMP_VERBOSE) == 0 &&