mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-22 06:11:32 +02:00
Code Issues Projects Releases Wiki Activity

MINOR: listener/api: add lli hint to listener functions

Browse Source 
Add listener lock hint (AKA lli) to (stop/resume/pause)_listener() functions.
All these functions implicitely take the listener lock when they are called:
It could be useful to be able to call them while already holding the lock, so
we're adding lli hint to make them take the lock only when it is missing.

This should only be backported if explicitly required by another commit
--

-> 2.4 and 2.5 common backport notes:

These 2 commits need to be backported first:
 - 187396e34 "CLEANUP: listener: function comment typo in stop_listener()"
 - a57786e87 "BUG/MINOR: listener: null pointer dereference suspected by
   coverity"

-> 2.4 special backport notes:

In addition to the previously mentionned dependencies, the patch needs to be
slightly adapted to match the corresponding contextual lines:

Replace this:

    |@@ -471,7 +474,8 @@ int pause_listener(struct listener *l, int lpx)
    |        if (!lpx && px)
    |                HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
    |
    |-       HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |+       if (!lli)
    |+               HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |
    |        if (l->state <= LI_PAUSED)
    |                goto end;

By this:

    |@@ -471,7 +474,8 @@ int pause_listener(struct listener *l, int lpx)
    |        if (!lpx && px)
    |                HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
    |
    |-       HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |+       if (!lli)
    |+               HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
    |
    |        if ((global.mode & (MODE_DAEMON | MODE_MWORKER)) &&
    |            !(proc_mask(l->rx.settings->bind_proc) & pid_bit))

Replace this:

    |@@ -169,7 +169,7 @@ void protocol_stop_now(void)
    |        HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
    |        list_for_each_entry(proto, &protocols, list) {
    |                list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
    |-                       stop_listener(listener, 0, 1);
    |+                       stop_listener(listener, 0, 1, 0);
    |        }
    |        HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
    | }

By this:

    |@@ -169,7 +169,7 @@ void protocol_stop_now(void)
    |        HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
    |        list_for_each_entry(proto, &protocols, list) {
    |                list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
    |                        if (!listener->bind_conf->frontend->grace)
    |-                               stop_listener(listener, 0, 1);
    |+                               stop_listener(listener, 0, 1, 0);
    |        }
    |        HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);

Replace this:

    |@@ -2315,7 +2315,7 @@ void stop_proxy(struct proxy *p)
    |        HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
    |
    |        list_for_each_entry(l, &p->conf.listeners, by_fe)
    |-               stop_listener(l, 1, 0);
    |+               stop_listener(l, 1, 0, 0);
    |
    |        if (!(p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) && !p->li_ready) {
    |                /* might be just a backend */

By this:

    |@@ -2315,7 +2315,7 @@ void stop_proxy(struct proxy *p)
    |        HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
    |
    |        list_for_each_entry(l, &p->conf.listeners, by_fe)
    |-               stop_listener(l, 1, 0);
    |+               stop_listener(l, 1, 0, 0);
    |
    |        if (!p->disabled && !p->li_ready) {
    |                /* might be just a backend */
This commit is contained in:
Aurelien DARRAGON 2023-02-06 17:06:03 +01:00 committed by Christopher Faulet
parent 9c3214c7b4
commit 4059e094db
4 changed files with 52 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
include/haproxy/listener.h
View File

@ -42,29 +42,31 @@ void listener_set_state(struct listener *l, enum li_state st);
* closes upon SHUT_WR and refuses to rebind. So a common validation path * closes upon SHUT_WR and refuses to rebind. So a common validation path
* involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling * involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling
* is disabled. It normally returns non-zero, unless an error is reported. * is disabled. It normally returns non-zero, unless an error is reported.
* It will need to operate under the proxy's lock. The caller is * It will need to operate under the proxy's lock and the listener's lock.
* responsible for indicating in lpx whether the proxy locks is * The caller is responsible for indicating in lpx, lli whether the respective
* already held (non-zero) or not (zero) so that the function picks it. * locks are already held (non-zero) or not (zero) so that the function pick
* the missing ones, in this order.
*/ */
int pause_listener(struct listener *l, int lpx); int pause_listener(struct listener *l, int lpx, int lli);
/* This function tries to resume a temporarily disabled listener. /* This function tries to resume a temporarily disabled listener.
* The resulting state will either be LI_READY or LI_FULL. 0 is returned * The resulting state will either be LI_READY or LI_FULL. 0 is returned
* in case of failure to resume (eg: dead socket). * in case of failure to resume (eg: dead socket).
* It will need to operate under the proxy's lock. The caller is * It will need to operate under the proxy's lock and the listener's lock.
* responsible for indicating in lpx whether the proxy locks is * The caller is responsible for indicating in lpx, lli whether the respective
* already held (non-zero) or not (zero) so that the function picks it. * locks are already held (non-zero) or not (zero) so that the function pick
* the missing ones, in this order.
*/ */
int resume_listener(struct listener *l, int lpx); int resume_listener(struct listener *l, int lpx, int lli);
/* /*
* This function completely stops a listener. It will need to operate under the * This function completely stops a listener. It will need to operate under the
* proxy's lock and the protocol's lock. The caller is * proxy's lock, the protocol's and the listener's lock. The caller is
* responsible for indicating in lpx, lpr whether the respective locks are * responsible for indicating in lpx, lpr, lli whether the respective locks are
* already held (non-zero) or not (zero) so that the function picks the missing * already held (non-zero) or not (zero) so that the function picks the missing
* ones, in this order. * ones, in this order.
*/ */
void stop_listener(struct listener *l, int lpx, int lpr); void stop_listener(struct listener *l, int lpx, int lpr, int lli);
/* This function adds the specified listener's file descriptor to the polling /* This function adds the specified listener's file descriptor to the polling
* lists if it is in the LI_LISTEN state. The listener enters LI_READY or * lists if it is in the LI_LISTEN state. The listener enters LI_READY or

56
src/listener.c
View File

@ -336,12 +336,12 @@ void enable_listener(struct listener *listener)
* This function completely stops a listener. * This function completely stops a listener.
* The proxy's listeners count is updated and the proxy is * The proxy's listeners count is updated and the proxy is
* disabled and woken up after the last one is gone. * disabled and woken up after the last one is gone.
* It will need to operate under the proxy's lock and the protocol's lock. * It will need to operate under the proxy's lock, the protocol's lock and
* The caller is responsible for indicating in lpx, lpr whether the * the listener's lock. The caller is responsible for indicating in lpx,
* respective locks are already held (non-zero) or not (zero) so that the * lpr, lli whether the respective locks are already held (non-zero) or
* function picks the missing ones, in this order. * not (zero) so that the function picks the missing ones, in this order.
*/ */
void stop_listener(struct listener *l, int lpx, int lpr) void stop_listener(struct listener *l, int lpx, int lpr, int lli)
{ {
struct proxy *px = l->bind_conf->frontend; struct proxy *px = l->bind_conf->frontend;
@ -358,7 +358,8 @@ void stop_listener(struct listener *l, int lpx, int lpr)
if (!lpr) if (!lpr)
HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); if (!lli)
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
if (l->state > LI_INIT) { if (l->state > LI_INIT) {
do_unbind_listener(l); do_unbind_listener(l);
@ -370,7 +371,8 @@ void stop_listener(struct listener *l, int lpx, int lpr)
proxy_cond_disable(px); proxy_cond_disable(px);
} }
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock); if (!lli)
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
if (!lpr) if (!lpr)
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
@ -459,11 +461,12 @@ int default_resume_listener(struct listener *l)
* closes upon SHUT_WR and refuses to rebind. So a common validation path * closes upon SHUT_WR and refuses to rebind. So a common validation path
* involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling * involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling
* is disabled. It normally returns non-zero, unless an error is reported. * is disabled. It normally returns non-zero, unless an error is reported.
* It will need to operate under the proxy's lock. The caller is * It will need to operate under the proxy's lock and the listener's lock.
* responsible for indicating in lpx whether the proxy locks is * The caller is responsible for indicating in lpx, lli whether the respective
* already held (non-zero) or not (zero) so that the function picks it. * locks are already held (non-zero) or not (zero) so that the function pick
* the missing ones, in this order.
*/ */
int pause_listener(struct listener *l, int lpx) int pause_listener(struct listener *l, int lpx, int lli)
{ {
struct proxy *px = l->bind_conf->frontend; struct proxy *px = l->bind_conf->frontend;
int ret = 1; int ret = 1;
@ -471,7 +474,8 @@ int pause_listener(struct listener *l, int lpx)
if (!lpx && px) if (!lpx && px)
HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock); HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); if (!lli)
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
if (l->state <= LI_PAUSED) if (l->state <= LI_PAUSED)
goto end; goto end;
@ -490,7 +494,8 @@ int pause_listener(struct listener *l, int lpx)
send_log(px, LOG_WARNING, "Paused %s %s.\n", proxy_cap_str(px->cap), px->id); send_log(px, LOG_WARNING, "Paused %s %s.\n", proxy_cap_str(px->cap), px->id);
} }
end: end:
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock); if (!lli)
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
if (!lpx && px) if (!lpx && px)
HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock); HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
@ -507,11 +512,12 @@ int pause_listener(struct listener *l, int lpx)
* state, it's totally rebound. This can happen if a pause() has completely * state, it's totally rebound. This can happen if a pause() has completely
* stopped it. If the resume fails, 0 is returned and an error might be * stopped it. If the resume fails, 0 is returned and an error might be
* displayed. * displayed.
* It will need to operate under the proxy's lock. The caller is * It will need to operate under the proxy's lock and the listener's lock.
* responsible for indicating in lpx whether the proxy locks is * The caller is responsible for indicating in lpx, lli whether the respective
* already held (non-zero) or not (zero) so that the function picks it. * locks are already held (non-zero) or not (zero) so that the function pick
* the missing ones, in this order.
*/ */
int resume_listener(struct listener *l, int lpx) int resume_listener(struct listener *l, int lpx, int lli)
{ {
struct proxy *px = l->bind_conf->frontend; struct proxy *px = l->bind_conf->frontend;
int was_paused = px && px->li_paused; int was_paused = px && px->li_paused;
@ -520,7 +526,8 @@ int resume_listener(struct listener *l, int lpx)
if (!lpx && px) if (!lpx && px)
HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock); HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock);
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); if (!lli)
HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock);
/* check that another thread didn't to the job in parallel (e.g. at the /* check that another thread didn't to the job in parallel (e.g. at the
* end of listen_accept() while we'd come from dequeue_all_listeners(). * end of listen_accept() while we'd come from dequeue_all_listeners().
@ -555,7 +562,8 @@ int resume_listener(struct listener *l, int lpx)
send_log(px, LOG_WARNING, "Resumed %s %s.\n", proxy_cap_str(px->cap), px->id); send_log(px, LOG_WARNING, "Resumed %s %s.\n", proxy_cap_str(px->cap), px->id);
} }
end: end:
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock); if (!lli)
HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
if (!lpx && px) if (!lpx && px)
HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock); HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock);
@ -602,7 +610,7 @@ void dequeue_all_listeners()
/* This cannot fail because the listeners are by definition in /* This cannot fail because the listeners are by definition in
* the LI_LIMITED state. * the LI_LIMITED state.
*/ */
resume_listener(listener, 0); resume_listener(listener, 0, 0);
} }
} }
@ -615,7 +623,7 @@ void dequeue_proxy_listeners(struct proxy *px)
/* This cannot fail because the listeners are by definition in /* This cannot fail because the listeners are by definition in
* the LI_LIMITED state. * the LI_LIMITED state.
*/ */
resume_listener(listener, 0); resume_listener(listener, 0, 0);
} }
} }
@ -1186,7 +1194,7 @@ void listener_accept(struct listener *l)
(!tick_isset(global_listener_queue_task->expire) || (!tick_isset(global_listener_queue_task->expire) ||
tick_is_expired(global_listener_queue_task->expire, now_ms))))) { tick_is_expired(global_listener_queue_task->expire, now_ms))))) {
/* at least one thread has to this when quitting */ /* at least one thread has to this when quitting */
resume_listener(l, 0); resume_listener(l, 0, 0);
/* Dequeues all of the listeners waiting for a resource */ /* Dequeues all of the listeners waiting for a resource */
dequeue_all_listeners(); dequeue_all_listeners();
@ -1205,7 +1213,7 @@ void listener_accept(struct listener *l)
* Let's put it to pause in this case. * Let's put it to pause in this case.
*/ */
if (l->rx.proto && l->rx.proto->rx_listening(&l->rx) == 0) { if (l->rx.proto && l->rx.proto->rx_listening(&l->rx) == 0) {
pause_listener(l, 0); pause_listener(l, 0, 0);
goto end; goto end;
} }
@ -1245,7 +1253,7 @@ void listener_release(struct listener *l)
_HA_ATOMIC_DEC(&l->thr_conn[tid]); _HA_ATOMIC_DEC(&l->thr_conn[tid]);
if (l->state == LI_FULL || l->state == LI_LIMITED) if (l->state == LI_FULL || l->state == LI_LIMITED)
resume_listener(l, 0); resume_listener(l, 0, 0);
/* Dequeues all of the listeners waiting for a resource */ /* Dequeues all of the listeners waiting for a resource */
dequeue_all_listeners(); dequeue_all_listeners();

6
src/protocol.c
View File

@ -169,7 +169,7 @@ void protocol_stop_now(void)
HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
list_for_each_entry(proto, &protocols, list) { list_for_each_entry(proto, &protocols, list) {
list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list) list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list)
stop_listener(listener, 0, 1); stop_listener(listener, 0, 1, 0);
} }
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
} }
@ -189,7 +189,7 @@ int protocol_pause_all(void)
HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
list_for_each_entry(proto, &protocols, list) { list_for_each_entry(proto, &protocols, list) {
list_for_each_entry(listener, &proto->receivers, rx.proto_list) list_for_each_entry(listener, &proto->receivers, rx.proto_list)
if (!pause_listener(listener, 0)) if (!pause_listener(listener, 0, 0))
err |= ERR_FATAL; err |= ERR_FATAL;
} }
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
@ -211,7 +211,7 @@ int protocol_resume_all(void)
HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
list_for_each_entry(proto, &protocols, list) { list_for_each_entry(proto, &protocols, list) {
list_for_each_entry(listener, &proto->receivers, rx.proto_list) list_for_each_entry(listener, &proto->receivers, rx.proto_list)
if (!resume_listener(listener, 0)) if (!resume_listener(listener, 0, 0))
err |= ERR_FATAL; err |= ERR_FATAL;
} }
HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);

8
src/proxy.c
View File

@ -2286,7 +2286,7 @@ int pause_proxy(struct proxy *p)
goto end; goto end;
list_for_each_entry(l, &p->conf.listeners, by_fe) list_for_each_entry(l, &p->conf.listeners, by_fe)
pause_listener(l, 1); pause_listener(l, 1, 0);
if (p->li_ready) { if (p->li_ready) {
ha_warning("%s %s failed to enter pause mode.\n", proxy_cap_str(p->cap), p->id); ha_warning("%s %s failed to enter pause mode.\n", proxy_cap_str(p->cap), p->id);
@ -2315,7 +2315,7 @@ void stop_proxy(struct proxy *p)
HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock); HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
list_for_each_entry(l, &p->conf.listeners, by_fe) list_for_each_entry(l, &p->conf.listeners, by_fe)
stop_listener(l, 1, 0); stop_listener(l, 1, 0, 0);
if (!(p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) && !p->li_ready) { if (!(p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) && !p->li_ready) {
/* might be just a backend */ /* might be just a backend */
@ -2344,7 +2344,7 @@ int resume_proxy(struct proxy *p)
fail = 0; fail = 0;
list_for_each_entry(l, &p->conf.listeners, by_fe) { list_for_each_entry(l, &p->conf.listeners, by_fe) {
if (!resume_listener(l, 1)) { if (!resume_listener(l, 1, 0)) {
int port; int port;
port = get_host_port(&l->rx.addr); port = get_host_port(&l->rx.addr);
@ -3035,7 +3035,7 @@ static int cli_parse_set_maxconn_frontend(char **args, char *payload, struct app
px->maxconn = v; px->maxconn = v;
list_for_each_entry(l, &px->conf.listeners, by_fe) { list_for_each_entry(l, &px->conf.listeners, by_fe) {
if (l->state == LI_FULL) if (l->state == LI_FULL)
resume_listener(l, 1); resume_listener(l, 1, 0);
} }
if (px->maxconn > px->feconn) if (px->maxconn > px->feconn)