From: Frédéric Lécaille Date: Mon, 21 Aug 2023 16:44:24 +0000 (+0200) Subject: MEDIUM: map/acl: Replace map/acl spin lock by a read/write lock. X-Git-Tag: v2.9-dev4~6 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=81815a9a83517e0b4f2ec0e33f5de02e3927d86c;p=thirdparty%2Fhaproxy.git MEDIUM: map/acl: Replace map/acl spin lock by a read/write lock. Replace ->lock type of pat_ref struct by HA_RWLOCK_T. Replace all calls to HA_SPIN_LOCK() (resp. HA_SPIN_UNLOCK()) by HA_RWLOCK_WRLOCK() (resp. HA_RWLOCK_WRUNLOCK()) when a write access is required. There is only one read access which is needed. This is in the "show map" command callback, cli_io_handler_map_lookup() where a HA_SPIN_LOCK() call is replaced by HA_RWLOCK_RDLOCK() (resp. HA_SPIN_UNLOCK() by HA_RWLOCK_RDUNLOCK). Replace HA_SPIN_INIT() calls by HA_RWLOCK_INIT() calls. --- diff --git a/include/haproxy/pattern-t.h b/include/haproxy/pattern-t.h index 0947e8b7f5..7d0699bf8d 100644 --- a/include/haproxy/pattern-t.h +++ b/include/haproxy/pattern-t.h @@ -112,7 +112,8 @@ struct pat_ref { int unique_id; /* Each pattern reference have unique id. */ unsigned long long revision; /* updated for each update */ unsigned long long entry_cnt; /* the total number of entries */ - __decl_thread(HA_SPINLOCK_T lock); /* Lock used to protect pat ref elements */ + THREAD_ALIGN(64); + __decl_thread(HA_RWLOCK_T lock); /* Lock used to protect pat ref elements */ }; /* This is a part of struct pat_ref. Each entry contains one pattern and one diff --git a/src/hlua.c b/src/hlua.c index 30a8713152..85bd316104 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -1878,9 +1878,9 @@ __LJMP static int hlua_del_acl(lua_State *L) if (!ref) WILL_LJMP(luaL_error(L, "'del_acl': unknown acl file '%s'", name)); - HA_SPIN_LOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ref->lock); pat_ref_delete(ref, key); - HA_SPIN_UNLOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ref->lock); return 0; } @@ -1902,9 +1902,9 @@ static int hlua_del_map(lua_State *L) if (!ref) WILL_LJMP(luaL_error(L, "'del_map': unknown acl file '%s'", name)); - HA_SPIN_LOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ref->lock); pat_ref_delete(ref, key); - HA_SPIN_UNLOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ref->lock); return 0; } @@ -1926,10 +1926,10 @@ static int hlua_add_acl(lua_State *L) if (!ref) WILL_LJMP(luaL_error(L, "'add_acl': unknown acl file '%s'", name)); - HA_SPIN_LOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ref->lock); if (pat_ref_find_elt(ref, key) == NULL) pat_ref_add(ref, key, NULL, NULL); - HA_SPIN_UNLOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ref->lock); return 0; } @@ -1954,12 +1954,12 @@ static int hlua_set_map(lua_State *L) if (!ref) WILL_LJMP(luaL_error(L, "'set_map': unknown map file '%s'", name)); - HA_SPIN_LOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ref->lock); if (pat_ref_find_elt(ref, key) != NULL) pat_ref_set(ref, key, value, NULL, NULL); else pat_ref_add(ref, key, value, NULL); - HA_SPIN_UNLOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ref->lock); return 0; } diff --git a/src/http_act.c b/src/http_act.c index e815095876..d168cf5e03 100644 --- a/src/http_act.c +++ b/src/http_act.c @@ -1840,10 +1840,10 @@ static enum act_return http_action_set_map(struct act_rule *rule, struct proxy * switch (rule->action) { case 0: // add-acl /* add entry only if it does not already exist */ - HA_SPIN_LOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ref->lock); if (pat_ref_find_elt(ref, key->area) == NULL) pat_ref_add(ref, key->area, NULL, NULL); - HA_SPIN_UNLOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ref->lock); break; case 1: // set-map @@ -1859,7 +1859,7 @@ static enum act_return http_action_set_map(struct act_rule *rule, struct proxy * value->data = build_logline(s, value->area, value->size, &rule->arg.map.value); value->area[value->data] = '\0'; - HA_SPIN_LOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ref->lock); elt = pat_ref_find_elt(ref, key->area); if (elt) { /* update entry if it exists */ @@ -1869,16 +1869,16 @@ static enum act_return http_action_set_map(struct act_rule *rule, struct proxy * /* insert a new entry */ pat_ref_add(ref, key->area, value->area, NULL); } - HA_SPIN_UNLOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ref->lock); break; } case 2: // del-acl case 3: // del-map /* returned code: 1=ok, 0=ko */ - HA_SPIN_LOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ref->lock); pat_ref_delete(ref, key->area); - HA_SPIN_UNLOCK(PATREF_LOCK, &ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ref->lock); break; default: diff --git a/src/map.c b/src/map.c index f4e74fb99f..2bed88a0c9 100644 --- a/src/map.c +++ b/src/map.c @@ -354,9 +354,9 @@ static int cli_io_handler_pat_list(struct appctx *appctx) * reference to the last ref_elt being dumped. */ if (!LIST_ISEMPTY(&ctx->bref.users)) { - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); LIST_DEL_INIT(&ctx->bref.users); - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); } return 1; } @@ -367,7 +367,7 @@ static int cli_io_handler_pat_list(struct appctx *appctx) __fallthrough; case STATE_LIST: - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); if (!LIST_ISEMPTY(&ctx->bref.users)) { LIST_DELETE(&ctx->bref.users); @@ -398,14 +398,14 @@ static int cli_io_handler_pat_list(struct appctx *appctx) * this stream's users so that it can remove us upon termination. */ LIST_APPEND(&elt->back_refs, &ctx->bref.users); - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); return 0; } skip: /* get next list entry and check the end of the list */ ctx->bref.ref = elt->list.n; } - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); __fallthrough; default: @@ -489,7 +489,7 @@ static int cli_io_handler_map_lookup(struct appctx *appctx) __fallthrough; case STATE_LIST: - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_RDLOCK(PATREF_LOCK, &ctx->ref->lock); /* for each lookup type */ while (ctx->expr) { /* initialise chunk to build new message */ @@ -575,7 +575,7 @@ static int cli_io_handler_map_lookup(struct appctx *appctx) /* let's try again later from this stream. We add ourselves into * this stream's users so that it can remove us upon termination. */ - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_RDUNLOCK(PATREF_LOCK, &ctx->ref->lock); return 0; } @@ -583,7 +583,7 @@ static int cli_io_handler_map_lookup(struct appctx *appctx) ctx->expr = pat_expr_get_next(ctx->expr, &ctx->ref->pat); } - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_RDUNLOCK(PATREF_LOCK, &ctx->ref->lock); __fallthrough; default: @@ -679,9 +679,9 @@ static void cli_release_show_map(struct appctx *appctx) struct show_map_ctx *ctx = appctx->svcctx; if (!LIST_ISEMPTY(&ctx->bref.users)) { - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); LIST_DEL_INIT(&ctx->bref.users); - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); } } @@ -778,30 +778,30 @@ static int cli_parse_set_map(char **args, char *payload, struct appctx *appctx, /* Try to modify the entry. */ err = NULL; - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); if (!pat_ref_set_by_id(ctx->ref, ref, args[4], &err)) { - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); if (err) return cli_dynerr(appctx, memprintf(&err, "%s.\n", err)); else return cli_err(appctx, "Failed to update an entry.\n"); } - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); } else { /* Else, use the entry identifier as pattern * string, and update the value. */ err = NULL; - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); if (!pat_ref_set(ctx->ref, args[3], args[4], &err, NULL)) { - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); if (err) return cli_dynerr(appctx, memprintf(&err, "%s.\n", err)); else return cli_err(appctx, "Failed to update an entry.\n"); } - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); } /* The set is done, send message. */ @@ -921,9 +921,9 @@ static int cli_parse_add_map(char **args, char *payload, struct appctx *appctx, if (ctx->display_flags != PAT_REF_MAP) value = NULL; - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); ret = !!pat_ref_load(ctx->ref, gen ? genid : ctx->ref->curr_gen, key, value, -1, &err); - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); if (!ret) { if (err) @@ -983,25 +983,25 @@ static int cli_parse_del_map(char **args, char *payload, struct appctx *appctx, return cli_err(appctx, "Malformed identifier. Please use # or .\n"); /* Try to delete the entry. */ - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); if (!pat_ref_delete_by_id(ctx->ref, ref)) { - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); /* The entry is not found, send message. */ return cli_err(appctx, "Key not found.\n"); } - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); } else { /* Else, use the entry identifier as pattern * string and try to delete the entry. */ - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); if (!pat_ref_delete(ctx->ref, args[3])) { - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); /* The entry is not found, send message. */ return cli_err(appctx, "Key not found.\n"); } - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); } /* The deletion is done, send message. */ @@ -1018,9 +1018,9 @@ static int cli_io_handler_clear_map(struct appctx *appctx) struct show_map_ctx *ctx = appctx->svcctx; int finished; - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); finished = pat_ref_purge_range(ctx->ref, ctx->curr_gen, ctx->prev_gen, 100); - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); if (!finished) { /* let's come back later */ @@ -1135,13 +1135,13 @@ static int cli_parse_commit_map(char **args, char *payload, struct appctx *appct return cli_err(appctx, "Unknown ACL identifier. Please use # or .\n"); } - HA_SPIN_LOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRLOCK(PATREF_LOCK, &ctx->ref->lock); if (genid - (ctx->ref->curr_gen + 1) < ctx->ref->next_gen - ctx->ref->curr_gen) ret = pat_ref_commit(ctx->ref, genid); else ret = 1; - HA_SPIN_UNLOCK(PATREF_LOCK, &ctx->ref->lock); + HA_RWLOCK_WRUNLOCK(PATREF_LOCK, &ctx->ref->lock); if (ret != 0) return cli_err(appctx, "Version number out of range.\n"); diff --git a/src/pattern.c b/src/pattern.c index b6c666f15c..b393112d30 100644 --- a/src/pattern.c +++ b/src/pattern.c @@ -1875,7 +1875,7 @@ struct pat_ref *pat_ref_new(const char *reference, const char *display, unsigned LIST_INIT(&ref->head); ref->ebpt_root = EB_ROOT; LIST_INIT(&ref->pat); - HA_SPIN_INIT(&ref->lock); + HA_RWLOCK_INIT(&ref->lock); LIST_APPEND(&pattern_reference, &ref->list); return ref; @@ -1912,7 +1912,7 @@ struct pat_ref *pat_ref_newid(int unique_id, const char *display, unsigned int f LIST_INIT(&ref->head); ref->ebpt_root = EB_ROOT; LIST_INIT(&ref->pat); - HA_SPIN_INIT(&ref->lock); + HA_RWLOCK_INIT(&ref->lock); LIST_APPEND(&pattern_reference, &ref->list); return ref;