[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