[6.0] 82297b9cb complement the cleanup started in the previous commit

Reza Naghibi reza at naghibi.com
Wed Aug 19 13:17:06 UTC 2020


commit 82297b9cbcc85052c1157c9f40713ff83d101705
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Feb 25 19:12:07 2019 +0100

    complement the cleanup started in the previous commit
    
    Thank you, @Dridi
    
    - yes, allocations outside the lock were motivated by minimizing the
      critical section, but n_backend could actually change, so this was
      wrong. As we use an RW lock, doing more work under it should only have
      marginal impact.
    
    - n_backend == 0 is probably best handled as a special case

diff --git a/lib/libvmod_directors/shard_dir.c b/lib/libvmod_directors/shard_dir.c
index be240eb70..d17cb6e4c 100644
--- a/lib/libvmod_directors/shard_dir.c
+++ b/lib/libvmod_directors/shard_dir.c
@@ -347,11 +347,7 @@ sharddir_pick_be_locked(VRT_CTX, struct sharddir *shardd,
 	CHECK_OBJ_NOTNULL(shardd, SHARDDIR_MAGIC);
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 	AN(ctx->vsl);
-
-	if (shardd->n_backend == 0) {
-		shard_err0(ctx, shardd, "no backends");
-		return (NULL);
-	}
+	assert(shardd->n_backend > 0);
 
 	assert(shardd->hashcircle);
 
@@ -444,18 +440,20 @@ sharddir_pick_be(VRT_CTX, struct sharddir *shardd,
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(shardd, SHARDDIR_MAGIC);
 
-	/* NB: Allocate bitmap on the stack
-	 *
-	 * XXX: Should we read n_backend under the lock and check whether
-	 * n_backend is zero before allocating the state?
-	 */
+	sharddir_rdlock(shardd);
+
+	if (shardd->n_backend == 0) {
+		shard_err0(ctx, shardd, "no backends");
+		sharddir_unlock(shardd);
+		return (NULL);
+	}
+
 	picklist_sz = VBITMAP_SZ(shardd->n_backend);
 	char picklist_spc[picklist_sz];
 
 	memset(state, 0, sizeof(state));
 	init_state(state, ctx, shardd, vbit_init(picklist_spc, picklist_sz));
 
-	sharddir_rdlock(shardd);
 	be = sharddir_pick_be_locked(ctx, shardd, key, alt, warmup, rampup,
 	    healthy, state);
 	sharddir_unlock(shardd);


More information about the varnish-commit mailing list