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 0947e8b7f5196ebdeebd6c2fe39cd366a0b04535..7d0699bf8dbea9fe575788739311de516c480936 100644 (file)
--- 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 30a871315278229950a595cb7442f9e095c6afab..85bd31610494b2d20279a85794a889bac90c6e8b 100644 (file)
--- 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 e815095876fcb4aa63d09bc546fa6a515bbb8f34..d168cf5e034c433857d8518c23c00f3203182733 100644 (file)
--- 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 f4e74fb99fde6ac25be93bf87dae66585b9eaa9a..2bed88a0c92b60d4eb7c3f6a504248060cd57f81 100644 (file)
--- 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 #<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");

diff --git a/src/pattern.c b/src/pattern.c
index b6c666f15c7ab77892c8970da0483a695572aca7..b393112d3060ab8fb4e4910e4b2f37d26947c720 100644 (file)
--- 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;