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 e5f8236..2b6c3dc 100644
--- a/include/haproxy/listener.h
+++ b/include/haproxy/listener.h
@@ -42,29 +42,31 @@
* 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 81fdc2f..2956722 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -336,12 +336,12 @@
* 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 @@
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 @@
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 @@
* 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 @@
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 @@
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 @@
* 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 @@
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 @@
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 @@
/* 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 @@
/* 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 @@
(!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 @@
* 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 @@
_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 62091a5..71bfa83 100644
--- a/src/protocol.c
+++ b/src/protocol.c
@@ -169,7 +169,7 @@
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 @@
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 @@
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 404037d..98d9028 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2286,7 +2286,7 @@
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 @@
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 @@
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 @@
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)