[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