Move all the ban counters into a "ban-mtx" group, update them directly

in VSC_C_main and protect them with the ban-mtx.

bin/varnishd/VSC_main.vsc

@@ -595,6 +595,7 @@
 
 .. varnish_vsc:: bans
 	:type:	gauge
+	:group: ban_mtx
 	:oneliner:	Count of bans
 
 	Number of all bans in system, including bans superseded by newer
@@ -603,6 +604,7 @@
 .. varnish_vsc:: bans_completed
 	:type:	gauge
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Number of bans marked 'completed'
 
 	Number of bans which are no longer active, either because they got
@@ -611,6 +613,7 @@
 .. varnish_vsc:: bans_obj
 	:type:	gauge
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Number of bans using obj.*
 
 	Number of bans which use obj.* variables.  These bans can possibly
@@ -619,6 +622,7 @@
 .. varnish_vsc:: bans_req
 	:type:	gauge
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Number of bans using req.*
 
 	Number of bans which use req.* variables.  These bans can not be
@@ -626,18 +630,21 @@
 
 .. varnish_vsc:: bans_added
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Bans added
 
 	Counter of bans added to ban list.
 
 .. varnish_vsc:: bans_deleted
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Bans deleted
 
 	Counter of bans deleted from ban list.
 
 .. varnish_vsc:: bans_tested
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Bans tested against objects (lookup)
 
 	Count of how many bans and objects have been tested against each
@@ -645,12 +652,14 @@
 
 .. varnish_vsc:: bans_obj_killed
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Objects killed by bans (lookup)
 
 	Number of objects killed by bans during object lookup.
 
 .. varnish_vsc:: bans_lurker_tested
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Bans tested against objects (lurker)
 
 	Count of how many bans and objects have been tested against each
@@ -658,6 +667,7 @@
 
 .. varnish_vsc:: bans_tests_tested
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Ban tests tested against objects (lookup)
 
 	Count of how many tests and objects have been tested against each
@@ -666,6 +676,7 @@
 
 .. varnish_vsc:: bans_lurker_tests_tested
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Ban tests tested against objects (lurker)
 
 	Count of how many tests and objects have been tested against each
@@ -674,12 +685,14 @@
 
 .. varnish_vsc:: bans_lurker_obj_killed
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Objects killed by bans (lurker)
 
 	Number of objects killed by the ban-lurker.
 
 .. varnish_vsc:: bans_lurker_obj_killed_cutoff
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Objects killed by bans for cutoff (lurker)
 
 	Number of objects killed by the ban-lurker to keep the number of
@@ -687,12 +700,14 @@
 
 .. varnish_vsc:: bans_dups
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Bans superseded by other bans
 
 	Count of bans replaced by later identical bans.
 
 .. varnish_vsc:: bans_lurker_contention
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Lurker gave way for lookup
 
 	Number of times the ban-lurker had to wait for lookups.
@@ -701,6 +716,7 @@
 	:type:	gauge
 	:format:	bytes
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Bytes used by the persisted ban lists
 
 	Number of bytes used by the persisted ban lists.
@@ -709,6 +725,7 @@
 	:type:	gauge
 	:format:	bytes
 	:level:	diag
+	:group: ban_mtx
 	:oneliner:	Extra bytes in persisted ban lists due to fragmentation
 
 	Number of extra bytes accumulated through dropped and completed

bin/varnishd/cache/cache.h

@@ -578,7 +578,7 @@ struct sess {
 struct ban_proto *BAN_Build(void);
 const char *BAN_AddTest(struct ban_proto *,
     const char *, const char *, const char *);
-const char *BAN_Commit(struct ban_proto *b, struct VSC_main *stats);
+const char *BAN_Commit(struct ban_proto *b);
 void BAN_Abandon(struct ban_proto *b);
 
 /* cache_cli.c [CLI] */

bin/varnishd/cache/cache_ban.c

@@ -152,7 +152,7 @@ ban_equal(const uint8_t *bs1, const uint8_t *bs2)
 }
 
 void
-ban_mark_completed(struct ban *b, struct VSC_main *stats)
+ban_mark_completed(struct ban *b)
 {
 	unsigned ln;
 
@@ -166,7 +166,7 @@ ban_mark_completed(struct ban *b, struct VSC_main *stats)
 		b->spec[BANS_FLAGS] |= BANS_FLAG_COMPLETED;
 		VWMB();
 		vbe32enc(b->spec + BANS_LENGTH, BANS_HEAD_LEN);
-		stats->bans_completed++;
+		VSC_C_main->bans_completed++;
 		bans_persisted_fragmentation += ln - ban_len(b->spec);
 		/*
 		 * XXX absolute update of gauges - may be inaccurate for
@@ -367,7 +367,6 @@ ban_reload(const uint8_t *ban, unsigned len)
 	struct ban *b, *b2;
 	int duplicate = 0;
 	double t0, t1, t2 = 9e99;
-	struct VSC_main *stats = VSC_C_main;	// XXX accurate?
 	ASSERT_CLI();
 	Lck_AssertHeld(&ban_mtx);
 
@@ -386,8 +385,8 @@ ban_reload(const uint8_t *ban, unsigned len)
 			duplicate = 1;
 	}
 
-	stats->bans++;
-	stats->bans_added++;
+	VSC_C_main->bans++;
+	VSC_C_main->bans_added++;
 
 	b2 = ban_alloc();
 	AN(b2);
@@ -395,13 +394,13 @@ ban_reload(const uint8_t *ban, unsigned len)
 	AN(b2->spec);
 	memcpy(b2->spec, ban, len);
 	if (ban[BANS_FLAGS] & BANS_FLAG_REQ) {
-		stats->bans_req++;
+		VSC_C_main->bans_req++;
 		b2->flags |= BANS_FLAG_REQ;
 	}
 	if (duplicate)
-		stats->bans_dups++;
+		VSC_C_main->bans_dups++;
 	if (duplicate || (ban[BANS_FLAGS] & BANS_FLAG_COMPLETED))
-		ban_mark_completed(b2, stats);
+		ban_mark_completed(b2);
 	if (b == NULL)
 		VTAILQ_INSERT_TAIL(&ban_head, b2, list);
 	else
@@ -418,8 +417,8 @@ ban_reload(const uint8_t *ban, unsigned len)
 		if (b->flags & BANS_FLAG_COMPLETED)
 			continue;
 		if (ban_equal(b->spec, ban)) {
-			ban_mark_completed(b, stats);
-			stats->bans_dups++;
+			ban_mark_completed(b);
+			VSC_C_main->bans_dups++;
 		}
 	}
 }
@@ -548,7 +547,6 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 	struct vsl_log *vsl;
 	struct ban *b0, *bn;
 	unsigned tests;
-	struct VSC_main *stats;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
@@ -557,7 +555,6 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 	assert(oc->refcnt > 0);
 
 	vsl = req->vsl;
-	stats = wrk->stats;
 
 	CHECK_OBJ_NOTNULL(oc->ban, BAN_MAGIC);
 
@@ -600,8 +597,8 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 
 	Lck_Lock(&ban_mtx);
 	bn->refcount--;
-	stats->bans_tested++;
-	stats->bans_tests_tested += tests;
+	VSC_C_main->bans_tested++;
+	VSC_C_main->bans_tests_tested += tests;
 
 	if (b == bn) {
 		/* not banned */
@@ -612,6 +609,8 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 		oc->ban = b0;
 		b = NULL;
 	}
+	if (b != NULL)
+		VSC_C_main->bans_obj_killed++;
 
 	if (VTAILQ_LAST(&ban_head, banhead_s)->refcount == 0)
 		ban_kick_lurker();
@@ -624,7 +623,6 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 		return (0);
 	} else {
 		VSLb(vsl, SLT_ExpBan, "%u banned lookup", ObjGetXID(wrk, oc));
-		stats->bans_obj_killed++;
 		return (1);
 	}
 }
@@ -672,7 +670,7 @@ ccf_ban(struct cli *cli, const char * const *av, void *priv)
 
 	if (err == NULL) {
 		// XXX racy - grab wstat lock?
-		err = BAN_Commit(bp, VSC_C_main);
+		err = BAN_Commit(bp);
 	}
 
 	if (err != NULL) {
@@ -821,9 +819,9 @@ BAN_Init(void)
 	bp = BAN_Build();
 	AN(bp);
 	AZ(pthread_cond_init(&ban_lurker_cond, NULL));
-	AZ(BAN_Commit(bp, VSC_C_main));
+	AZ(BAN_Commit(bp));
 	Lck_Lock(&ban_mtx);
-	ban_mark_completed(VTAILQ_FIRST(&ban_head), VSC_C_main);
+	ban_mark_completed(VTAILQ_FIRST(&ban_head));
 	Lck_Unlock(&ban_mtx);
 }
 

bin/varnishd/cache/cache_ban.h

@@ -109,7 +109,7 @@ extern pthread_cond_t	ban_lurker_cond;
 extern uint64_t bans_persisted_bytes;
 extern uint64_t bans_persisted_fragmentation;
 
-void ban_mark_completed(struct ban *b, struct VSC_main *stats);
+void ban_mark_completed(struct ban *);
 unsigned ban_len(const uint8_t *banspec);
 void ban_info_new(const uint8_t *ban, unsigned len);
 void ban_info_drop(const uint8_t *ban, unsigned len);

bin/varnishd/cache/cache_ban_build.c

@@ -246,7 +246,7 @@ BAN_AddTest(struct ban_proto *bp,
  */
 
 const char *
-BAN_Commit(struct ban_proto *bp, struct VSC_main *stats)
+BAN_Commit(struct ban_proto *bp)
 {
 	struct ban  *b, *bi;
 	ssize_t ln;
@@ -294,8 +294,8 @@ BAN_Commit(struct ban_proto *bp, struct VSC_main *stats)
 	VTAILQ_INSERT_HEAD(&ban_head, b, list);
 	ban_start = b;
 
-	stats->bans++;
-	stats->bans_added++;
+	VSC_C_main->bans++;
+	VSC_C_main->bans_added++;
 	bans_persisted_bytes += ln;
 	/*
 	 * XXX absolute update of gauges - may be inaccurate for Pool_Sumstat
@@ -304,9 +304,9 @@ BAN_Commit(struct ban_proto *bp, struct VSC_main *stats)
 	VSC_C_main->bans_persisted_bytes = bans_persisted_bytes;
 
 	if (b->flags & BANS_FLAG_OBJ)
-		stats->bans_obj++;
+		VSC_C_main->bans_obj++;
 	if (b->flags & BANS_FLAG_REQ)
-		stats->bans_req++;
+		VSC_C_main->bans_req++;
 
 	if (bi != NULL)
 		ban_info_new(b->spec, ln);	/* Notify stevedores */
@@ -317,8 +317,8 @@ BAN_Commit(struct ban_proto *bp, struct VSC_main *stats)
 		    bi = VTAILQ_NEXT(bi, list)) {
 			if (!(bi->flags & BANS_FLAG_COMPLETED) &&
 			    ban_equal(b->spec, bi->spec)) {
-				ban_mark_completed(bi, stats);
-				stats->bans_dups++;
+				ban_mark_completed(bi);
+				VSC_C_main->bans_dups++;
 			}
 		}
 	}

bin/varnishd/cache/cache_ban_lurker.c

@@ -62,7 +62,7 @@ ban_kick_lurker(void)
  */
 
 static int
-ban_cleantail(const struct ban *victim, struct VSC_main *stats)
+ban_cleantail(const struct ban *victim)
 {
 	struct ban *b, *bt;
 	struct banhead_s freelist = VTAILQ_HEAD_INITIALIZER(freelist);
@@ -78,13 +78,13 @@ ban_cleantail(const struct ban *victim, struct VSC_main *stats)
 		if (b != VTAILQ_FIRST(&ban_head) && b->refcount == 0) {
 			assert(VTAILQ_EMPTY(&b->objcore));
 			if (b->flags & BANS_FLAG_COMPLETED)
-				stats->bans_completed--;
+				VSC_C_main->bans_completed--;
 			if (b->flags & BANS_FLAG_OBJ)
-				stats->bans_obj--;
+				VSC_C_main->bans_obj--;
 			if (b->flags & BANS_FLAG_REQ)
-				stats->bans_req--;
-			stats->bans--;
-			stats->bans_deleted++;
+				VSC_C_main->bans_req--;
+			VSC_C_main->bans--;
+			VSC_C_main->bans_deleted++;
 			VTAILQ_REMOVE(&ban_head, b, list);
 			VTAILQ_INSERT_TAIL(&freelist, b, list);
 			bans_persisted_fragmentation +=
@@ -125,7 +125,7 @@ ban_cleantail(const struct ban *victim, struct VSC_main *stats)
  */
 
 static struct objcore *
-ban_lurker_getfirst(struct vsl_log *vsl, struct ban *bt, struct VSC_main *stats)
+ban_lurker_getfirst(struct vsl_log *vsl, struct ban *bt)
 {
 	struct objhead *oh;
 	struct objcore *oc, *noc;
@@ -154,8 +154,8 @@ ban_lurker_getfirst(struct vsl_log *vsl, struct ban *bt, struct VSC_main *stats)
 			assert(move_oc == 0);
 
 			/* hold off to give lookup a chance and reiterate */
+			VSC_C_main->bans_lurker_contention++;
 			Lck_Unlock(&ban_mtx);
-			stats->bans_lurker_contention++;
 			VSL_Flush(vsl, 0);
 			VTIM_sleep(cache_param->ban_lurker_holdoff);
 			Lck_Lock(&ban_mtx);
@@ -209,10 +209,9 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt,
 	struct objcore *oc;
 	unsigned tests;
 	int i;
-	struct VSC_main *stats;
+	uint64_t tested = 0, tested_tests = 0, lok = 0, lokc = 0;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
-	stats = wrk->stats;
 
 	/*
 	 * First see if there is anything to do, and if so, insert markers
@@ -232,9 +231,16 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt,
 			VTIM_sleep(cache_param->ban_lurker_sleep);
 			ban_batch = 0;
 		}
-		oc = ban_lurker_getfirst(vsl, bt, stats);
-		if (oc == NULL)
+		oc = ban_lurker_getfirst(vsl, bt);
+		if (oc == NULL) {
+			Lck_Lock(&ban_mtx);
+			VSC_C_main->bans_lurker_tested += tested;
+			VSC_C_main->bans_lurker_tests_tested += tested_tests;
+			VSC_C_main->bans_lurker_obj_killed += lok;
+			VSC_C_main->bans_lurker_obj_killed_cutoff += lokc;
+			Lck_Unlock(&ban_mtx);
 			return;
+		}
 		i = 0;
 		VTAILQ_FOREACH_REVERSE_SAFE(bl, obans, banhead_s, l_list, bln) {
 			if (oc->ban != bt) {
@@ -256,21 +262,20 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt,
 				tests = 0;
 				i = ban_evaluate(wrk, bl->spec, oc, NULL,
 				    &tests);
-				stats->bans_lurker_tested++;
-				stats->bans_lurker_tests_tested += tests;
+				tested++;
+				tested_tests += tests;
 			}
 			if (i) {
 				if (kill) {
 					VSLb(vsl, SLT_ExpBan,
 					    "%u killed for lurker cutoff",
 					    ObjGetXID(wrk, oc));
-					stats->
-					    bans_lurker_obj_killed_cutoff++;
+					lokc++;
 				} else {
 					VSLb(vsl, SLT_ExpBan,
 					    "%u banned by lurker",
 					    ObjGetXID(wrk, oc));
-					stats->bans_lurker_obj_killed++;
+					lok++;
 				}
 				HSH_Kill(oc);
 				break;
@@ -278,6 +283,11 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt,
 		}
 		if (i == 0 && oc->ban == bt) {
 			Lck_Lock(&ban_mtx);
+			VSC_C_main->bans_lurker_tested += tested;
+			VSC_C_main->bans_lurker_tests_tested += tested_tests;
+			VSC_C_main->bans_lurker_obj_killed += lok;
+			VSC_C_main->bans_lurker_obj_killed_cutoff += lokc;
+			tested = tested_tests = lok = lokc = 0;
 			if (oc->ban == bt) {
 				bt->refcount--;
 				VTAILQ_REMOVE(&bt->objcore, oc, ban_list);
@@ -311,14 +321,12 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
 	struct banhead_s obans;
 	double d, dt, n;
 	unsigned count = 0, cutoff = UINT_MAX;
-	struct VSC_main *stats;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
-	stats = wrk->stats;
 
 	dt = 49.62;		// Random, non-magic
 	if (cache_param->ban_lurker_sleep == 0) {
-		(void)ban_cleantail(NULL, stats);
+		(void)ban_cleantail(NULL);
 		return (dt);
 	}
 	if (cache_param->ban_cutoff > 0)
@@ -358,7 +366,7 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
 	 * containted the first oban, all obans were on the tail and we're
 	 * done.
 	 */
-	if (ban_cleantail(VTAILQ_FIRST(&obans), stats))
+	if (ban_cleantail(VTAILQ_FIRST(&obans)))
 		return (dt);
 
 	if (VTAILQ_FIRST(&obans) == NULL)
@@ -386,7 +394,7 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
 
 	Lck_Lock(&ban_mtx);
 	VTAILQ_FOREACH(b, &obans, l_list) {
-		ban_mark_completed(b, stats);
+		ban_mark_completed(b);
 		if (b == bd)
 			break;
 	}

bin/varnishd/cache/cache_vrt.c

@@ -559,26 +559,6 @@ VRT_synth_page(VRT_CTX, const char *str, ...)
 
 /*--------------------------------------------------------------------*/
 
-static struct VSC_main *
-vrt_stats(VRT_CTX)
-{
-	struct worker *wrk;
-
-	if (ctx->req) {
-		CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
-		wrk = ctx->req->wrk;
-	} else if (ctx->bo) {
-		CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
-		wrk = ctx->bo->wrk;
-	} else {
-		// XXX
-		return (VSC_C_main);
-	}
-
-	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
-	return (wrk->stats);
-}
-
 VCL_VOID
 VRT_ban_string(VRT_CTX, VCL_STRING str)
 {
@@ -634,7 +614,7 @@ VRT_ban_string(VRT_CTX, VCL_STRING str)
 			break;
 		}
 		if (av[++i] == NULL) {
-			err = BAN_Commit(bp, vrt_stats(ctx));
+			err = BAN_Commit(bp);
 			if (err == NULL)
 				bp = NULL;
 			else