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.
This commit is contained in:
Frdric Lcaille 2023-08-21 18:44:24 +02:00 committed by Willy Tarreau
parent 5fea59754b
commit 81815a9a83
5 changed files with 46 additions and 45 deletions

View File

@ -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

View File

@ -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;
}

View File

@ -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:

View File

@ -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 #<id> or <file>.\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 #<id> or <file>.\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");

View File

@ -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;