[master] 5666ed7a6 complement the cleanup started in the previous commit

Nils Goroll nils.goroll at uplex.de
Mon Feb 25 18:17:08 UTC 2019


commit 5666ed7a613f0d2fcf22d1f480eccd9bdddc685a
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 7a96565e3..e40bb67bb 100644
--- a/lib/libvmod_directors/shard_dir.c
+++ b/lib/libvmod_directors/shard_dir.c
@@ -354,11 +354,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);
 
@@ -451,18 +447,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