Hi, As discussed yesterday on IRC, please find attached a really simple patch to add locking in VBE operations. It was surprisingly easy, so I'm rather suspicious. I have also removed CLI assertions in (soon to be) dynamic operations.
Best Regards, Dridi
From fd3d2ab96e5b5fd4a3f06fcd3bc0ac3e5c45fb67 Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune <[email protected]> Date: Tue, 19 May 2015 06:12:25 +0200 Subject: [PATCH] Add locking around the backends list operations It's really just a new lock called `vbe` and VTAILQ manipulations turned into critical sections. CLI assertions have also been removed from `add` and `remove` operations, those will be allowed at any time for dynamic backends. --- bin/varnishd/cache/cache_backend_cfg.c | 14 ++++++++------ include/tbl/locks.h | 1 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/bin/varnishd/cache/cache_backend_cfg.c b/bin/varnishd/cache/cache_backend_cfg.c index 08b58d3..0410a9a 100644 --- a/bin/varnishd/cache/cache_backend_cfg.c +++ b/bin/varnishd/cache/cache_backend_cfg.c @@ -46,11 +46,8 @@ #include "vrt.h" #include "vtim.h" -/* - * The list of backends is not locked, it is only ever accessed from - * the CLI thread, so there is no need. - */ static VTAILQ_HEAD(, backend) backends = VTAILQ_HEAD_INITIALIZER(backends); +static struct lock backends_mtx; /*-------------------------------------------------------------------- */ @@ -59,9 +56,9 @@ void VBE_DeleteBackend(struct backend *b) { - ASSERT_CLI(); CHECK_OBJ_NOTNULL(b, BACKEND_MAGIC); + Lck_Lock(&backends_mtx); VTAILQ_REMOVE(&backends, b, list); free(b->ipv4); free(b->ipv6); @@ -71,6 +68,7 @@ VBE_DeleteBackend(struct backend *b) Lck_Delete(&b->mtx); FREE_OBJ(b); VSC_C_main->n_backend--; + Lck_Unlock(&backends_mtx); } /*-------------------------------------------------------------------- @@ -85,7 +83,6 @@ VBE_AddBackend(const char *vcl, const struct vrt_backend *vb) struct backend *b; char buf[128]; - ASSERT_CLI(); AN(vb->vcl_name); assert(vb->ipv4_suckaddr != NULL || vb->ipv6_suckaddr != NULL); @@ -118,8 +115,10 @@ VBE_AddBackend(const char *vcl, const struct vrt_backend *vb) b->health_changed = VTIM_real(); b->admin_health = ah_probe; + Lck_Lock(&backends_mtx); VTAILQ_INSERT_TAIL(&backends, b, list); VSC_C_main->n_backend++; + Lck_Unlock(&backends_mtx); return (b); } @@ -176,6 +175,7 @@ backend_find(struct cli *cli, const char *matcher, bf_func *func, void *priv) VSB_printf(vsb, "%s.%s", vcc->loaded_name, matcher); } AZ(VSB_finish(vsb)); + Lck_Lock(&backends_mtx); VTAILQ_FOREACH(b, &backends, list) { if (fnmatch(VSB_data(vsb), b->display_name, 0)) continue; @@ -186,6 +186,7 @@ backend_find(struct cli *cli, const char *matcher, bf_func *func, void *priv) break; } } + Lck_Unlock(&backends_mtx); VSB_delete(vsb); return (found); } @@ -313,4 +314,5 @@ VBE_InitCfg(void) { CLI_AddFuncs(backend_cmds); + Lck_New(&backends_mtx, lck_vbe); } diff --git a/include/tbl/locks.h b/include/tbl/locks.h index e4b4914..010de43 100644 --- a/include/tbl/locks.h +++ b/include/tbl/locks.h @@ -46,6 +46,7 @@ LOCK(exp) LOCK(lru) LOCK(cli) LOCK(ban) +LOCK(vbe) LOCK(vbp) LOCK(backend) LOCK(vcapace) -- 2.1.0
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
