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

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 */

diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h
index e5f8236794efdfb9bc12299da1fc7221bf90f773..2b6c3dc27dd72d118de4d8de0ab5013e303709ec 100644 (file)
--- a/include/haproxy/listener.h
+++ b/include/haproxy/listener.h
@@ -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
  * 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.
- * It will need to operate under the proxy's lock. The caller is
- * responsible for indicating in lpx whether the proxy locks is
- * already held (non-zero) or not (zero) so that the function picks it.
+ * It will need to operate under the proxy's lock and the listener's lock.
+ * The caller is responsible for indicating in lpx, lli whether the respective
+ * 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.
  * The resulting state will either be LI_READY or LI_FULL. 0 is returned
  * in case of failure to resume (eg: dead socket).
- * It will need to operate under the proxy's lock. The caller is
- * responsible for indicating in lpx whether the proxy locks is
- * already held (non-zero) or not (zero) so that the function picks it.
+ * It will need to operate under the proxy's lock and the listener's lock.
+ * The caller is responsible for indicating in lpx, lli whether the respective
+ * 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
- * proxy's lock and the protocol's lock. The caller is
- * responsible for indicating in lpx, lpr whether the respective locks are
+ * proxy's lock, the protocol's and the listener's lock. The caller is
+ * 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
  * 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
  * lists if it is in the LI_LISTEN state. The listener enters LI_READY or

diff --git a/src/listener.c b/src/listener.c
index 81fdc2f6bb483ec3ba10a3672792cd468a320cda..2956722337cc9f7a52c23a91ebe761cca87f4794 100644 (file)
--- a/src/listener.c
+++ b/src/listener.c
@@ -336,12 +336,12 @@ void enable_listener(struct listener *listener)
  * This function completely stops a listener.
  * The proxy's listeners count is updated and the proxy is
  * 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.
- * The caller is responsible for indicating in lpx, lpr whether the
- * respective locks are already held (non-zero) or not (zero) so that the
- * function picks the missing ones, in this order.
+ * It will need to operate under the proxy's lock, the protocol's lock and
+ * the listener's lock. The caller is 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 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;
 
@@ -358,7 +358,8 @@ void stop_listener(struct listener *l, int lpx, int lpr)
        if (!lpr)
                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) {
                do_unbind_listener(l);
@@ -370,7 +371,8 @@ void stop_listener(struct listener *l, int lpx, int lpr)
                        proxy_cond_disable(px);
        }
 
-       HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
+       if (!lli)
+               HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
 
        if (!lpr)
                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
  * 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.
- * It will need to operate under the proxy's lock. The caller is
- * responsible for indicating in lpx whether the proxy locks is
- * already held (non-zero) or not (zero) so that the function picks it.
+ * It will need to operate under the proxy's lock and the listener's lock.
+ * The caller is responsible for indicating in lpx, lli whether the respective
+ * 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;
        int ret = 1;
@@ -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;
@@ -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);
        }
   end:
-       HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
+       if (!lli)
+               HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
 
        if (!lpx && px)
                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
  * stopped it. If the resume fails, 0 is returned and an error might be
  * displayed.
- * It will need to operate under the proxy's lock. The caller is
- * responsible for indicating in lpx whether the proxy locks is
- * already held (non-zero) or not (zero) so that the function picks it.
+ * It will need to operate under the proxy's lock and the listener's lock.
+ * The caller is responsible for indicating in lpx, lli whether the respective
+ * 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;
        int was_paused = px && px->li_paused;
@@ -520,7 +526,8 @@ int resume_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);
 
        /* 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().
@@ -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);
        }
   end:
-       HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
+       if (!lli)
+               HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock);
 
        if (!lpx && px)
                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
                 * 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
                 * 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_is_expired(global_listener_queue_task->expire, now_ms))))) {
                /* 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 */
                dequeue_all_listeners();
@@ -1205,7 +1213,7 @@ void listener_accept(struct listener *l)
         * Let's put it to pause in this case.
         */
        if (l->rx.proto && l->rx.proto->rx_listening(&l->rx) == 0) {
-               pause_listener(l, 0);
+               pause_listener(l, 0, 0);
                goto end;
        }
 
@@ -1245,7 +1253,7 @@ void listener_release(struct listener *l)
        _HA_ATOMIC_DEC(&l->thr_conn[tid]);
 
        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 */
        dequeue_all_listeners();

diff --git a/src/protocol.c b/src/protocol.c
index 62091a51b014e69bcb3851e4d071975f854de416..71bfa83f5418f4e82268020d0e3b9032121f503d 100644 (file)
--- a/src/protocol.c
+++ b/src/protocol.c
@@ -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);
 }
@@ -189,7 +189,7 @@ int protocol_pause_all(void)
        HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
        list_for_each_entry(proto, &protocols, 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;
        }
        HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
@@ -211,7 +211,7 @@ int protocol_resume_all(void)
        HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
        list_for_each_entry(proto, &protocols, 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;
        }
        HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);

diff --git a/src/proxy.c b/src/proxy.c
index 404037d80bbb4f558175097859a8a0c034e5a2f1..98d902862e56b8a55ca3d6c12271a94dc83af4a6 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2286,7 +2286,7 @@ int pause_proxy(struct proxy *p)
                goto end;
 
        list_for_each_entry(l, &p->conf.listeners, by_fe)
-               pause_listener(l, 1);
+               pause_listener(l, 1, 0);
 
        if (p->li_ready) {
                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);
 
        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 */
@@ -2344,7 +2344,7 @@ int resume_proxy(struct proxy *p)
 
        fail = 0;
        list_for_each_entry(l, &p->conf.listeners, by_fe) {
-               if (!resume_listener(l, 1)) {
+               if (!resume_listener(l, 1, 0)) {
                        int port;
 
                        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;
        list_for_each_entry(l, &px->conf.listeners, by_fe) {
                if (l->state == LI_FULL)
-                       resume_listener(l, 1);
+                       resume_listener(l, 1, 0);
        }
 
        if (px->maxconn > px->feconn)