Kick the ban_lurker into action whenever we do something that

makes work for it.  Have it sleep as long as possible, rather
than hot-polling for work to do.

bin/varnishd/cache/cache_ban.c

@@ -552,6 +552,9 @@ BAN_CheckObject(struct worker *wrk, struct objcore *oc, struct req *req)
 		b0->refcount++;
 	}
 
+	if (oc->ban->refcount == 0 && VTAILQ_NEXT(oc->ban, list) == NULL)
+		ban_kick_lurker();
+
 	Lck_Unlock(&ban_mtx);
 
 	if (b == oc->ban) {	/* not banned */
@@ -703,6 +706,7 @@ ccf_ban_list(struct cli *cli, const char * const *av, void *priv)
 
 	Lck_Lock(&ban_mtx);
 	bl->refcount--;
+	ban_kick_lurker();	// XXX: Mostly for testcase b00009.vtc
 	Lck_Unlock(&ban_mtx);
 }
 
@@ -775,8 +779,10 @@ BAN_Shutdown(void)
 {
 	void *status;
 
+	Lck_Lock(&ban_mtx);
 	ban_shutdown = 1;
 	ban_kick_lurker();
+	Lck_Unlock(&ban_mtx);
 
 	AZ(pthread_join(ban_thread, &status));
 	AZ(status);

bin/varnishd/cache/cache_ban_build.c

@@ -319,6 +319,8 @@ BAN_Commit(struct ban_proto *bp)
 			}
 		}
 	}
+	if (!(b->flags & BANS_FLAG_REQ))
+		ban_kick_lurker();
 	Lck_Unlock(&ban_mtx);
 
 	BAN_Abandon(bp);

bin/varnishd/cache/cache_ban_lurker.c

@@ -45,10 +45,10 @@ pthread_cond_t	ban_lurker_cond;
 void
 ban_kick_lurker(void)
 {
-	Lck_Lock(&ban_mtx);
+
+	Lck_AssertHeld(&ban_mtx);
 	ban_generation++;
 	AZ(pthread_cond_signal(&ban_lurker_cond));
-	Lck_Unlock(&ban_mtx);
 }
 
 static void
@@ -191,14 +191,17 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt,
  * Ban lurker thread
  */
 
-static int
+static double
 ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
 {
 	struct ban *b, *bt;
 	struct banhead_s obans;
-	double d;
+	double d, dt, n;
 	int i;
 
+	dt = 49.62;		// Random, non-magic
+	if (cache_param->ban_lurker_sleep == 0)
+		return (dt);
 	/* Make a list of the bans we can do something about */
 	VTAILQ_INIT(&obans);
 	Lck_Lock(&ban_mtx);
@@ -206,26 +209,27 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
 	Lck_Unlock(&ban_mtx);
 	i = 0;
 	d = VTIM_real() - cache_param->ban_lurker_age;
-	while (b != NULL) {
-		if (b->flags & BANS_FLAG_COMPLETED) {
-			;
-		} else if (b->flags & BANS_FLAG_REQ) {
-			;
-		} else if (b == VTAILQ_LAST(&ban_head, banhead_s)) {
-			;
-		} else if (ban_time(b->spec) > d) {
-			;
-		} else {
+	for (; b != NULL; b = VTAILQ_NEXT(b, list)) {
+		if (b->flags & BANS_FLAG_COMPLETED)
+			continue;
+		if (b->flags & BANS_FLAG_REQ)
+			continue;
+		if (b == VTAILQ_LAST(&ban_head, banhead_s))
+			continue;	// XXX: why ?
+		n = ban_time(b->spec) - d;
+		if (n < 0) {
 			VTAILQ_INSERT_TAIL(&obans, b, l_list);
 			i++;
+		} else if (n < dt) {
+			dt = n;
 		}
-		b = VTAILQ_NEXT(b, list);
 	}
 	if (DO_DEBUG(DBG_LURKER))
-		VSLb(vsl, SLT_Debug, "lurker: %d actionable bans", i);
+		VSLb(vsl, SLT_Debug, "lurker: %d actionable bans, dt = %lf", i, dt);
 	if (i == 0)
-		return (0);
+		return (dt);
 
+	dt = cache_param->ban_lurker_sleep;
 	/* Go though all the bans to test the objects */
 	VTAILQ_FOREACH_REVERSE(bt, &ban_head, banhead_s, list) {
 		if (bt == VTAILQ_LAST(&obans, banhead_s)) {
@@ -247,7 +251,7 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl)
 		if (VTAILQ_EMPTY(&obans))
 			break;
 	}
-	return (1);
+	return (dt);
 }
 
 void * __match_proto__(bgthread_t)
@@ -255,6 +259,7 @@ ban_lurker(struct worker *wrk, void *priv)
 {
 	struct vsl_log vsl;
 	volatile double d;
+	unsigned gen = ban_generation + 1;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	AZ(priv);
@@ -262,13 +267,17 @@ ban_lurker(struct worker *wrk, void *priv)
 	VSL_Setup(&vsl, NULL, 0);
 
 	while (!ban_shutdown) {
-		d = cache_param->ban_lurker_sleep;
-		if (d <= 0.0 || !ban_lurker_work(wrk, &vsl))
-			d = 0.609;	// Random, non-magic
+		d = ban_lurker_work(wrk, &vsl);
+		if (DO_DEBUG(DBG_LURKER))
+			VSLb(&vsl, SLT_Debug, "lurker: sleep = %lf", d);
 		ban_cleantail();
 		d += VTIM_real();
 		Lck_Lock(&ban_mtx);
-		(void)Lck_CondWait(&ban_lurker_cond, &ban_mtx, d);
+		if (gen == ban_generation) {
+			(void)Lck_CondWait(&ban_lurker_cond, &ban_mtx, d);
+			ban_batch = 0;
+		}
+		gen = ban_generation;
 		Lck_Unlock(&ban_mtx);
 	}
 	pthread_exit(0);

bin/varnishtest/tests/c00049.vtc

@@ -40,6 +40,7 @@ varnish v1 -vcl+backend {} -start
 varnish v1 -cliok "param.set ban_lurker_age 0"
 varnish v1 -cliok "param.set ban_lurker_sleep 0"
 varnish v1 -cliok "param.set debug +lurker"
+varnish v1 -cliok "param.set debug +syncvsl"
 
 
 client c1 {
@@ -114,6 +115,8 @@ varnish v1 -expect bans_dups == 0
 
 varnish v1 -cliok "param.set ban_lurker_sleep .01"
 
+varnish v1 -cliok "ban.list"
+
 delay 2
 
 varnish v1 -cliok "ban.list"

bin/varnishtest/tests/p00009.vtc

@@ -53,5 +53,4 @@ client c1 {
 
 # Expect our duplicate
 varnish v1 -expect bans_dups == 1
-# One more than before restart due to the new ban_magic
-varnish v1 -expect bans_completed == 3
+varnish v1 -expect bans_completed == 1

bin/varnishtest/tests/r01030.vtc

@@ -40,9 +40,9 @@ client c1 {
 } -run
 
 #delay 0.1
-varnish v1 -expect bans_lurker_tests_tested == 0
+#varnish v1 -expect bans_lurker_tests_tested == 0
 
-delay 1.5
+#delay 1.5
 varnish v1 -expect bans_lurker_tests_tested >= 1
 
 varnish v1 -cliok "param.set ban_lurker_sleep 5.01"
@@ -58,7 +58,7 @@ client c2 {
 } -run
 
 #delay 0.1
-varnish v1 -expect bans_lurker_tests_tested == 1
+#varnish v1 -expect bans_lurker_tests_tested == 1
 
-delay 1.1
+#delay 1.1
 varnish v1 -expect bans_lurker_tests_tested == 2

include/tbl/params.h

@@ -135,10 +135,10 @@ PARAM(
 	/* units */	"seconds",
 	/* flags */	0,
 	/* s-text */
-	"The ban lurker only process bans when they are this old.  "
-	"When a ban is added, the most frequently hit objects will "
-	"get tested against it as part of object lookup.  This parameter "
-	"prevents the ban-lurker from kicking in, until the rush is over.",
+	"The ban lurker will ignore bans until they are this old.  "
+	"When a ban is added, the active traffic will be tested against it "
+	"as part of object lookup.  This parameter "
+	"holds the ban-lurker off, until the rush is over.",
 	/* l-text */	"",
 	/* func */	NULL
 )
@@ -152,9 +152,9 @@ PARAM(
 	/* units */	NULL,
 	/* flags */	0,
 	/* s-text */
-	"The ban lurker slees ${ban_lurker_sleep} after examining this "
-	"many objects.  Use this to pace the ban-lurker if it eats too "
-	"many resources.",
+	"The ban lurker sleeps ${ban_lurker_sleep} after examining this "
+	"many objects."
+	"  Use this to pace the ban-lurker if it eats too many resources.",
 	/* l-text */	"",
 	/* func */	NULL
 )
@@ -169,7 +169,8 @@ PARAM(
 	/* flags */	0,
 	/* s-text */
 	"How long the ban lurker sleeps after examining ${ban_lurker_batch} "
-	"objects.\n"
+	"objects."
+	"  Use this to pace the ban-lurker if it eats too many resources.\n"
 	"A value of zero will disable the ban lurker entirely.",
 	/* l-text */	"",
 	/* func */	NULL