MEDIUM: muxes: mark idle conns tasklets with TASK_F_USR1

The muxes are touching the idle_conns_lock all the time now because
they need to be careful that no other thread has stolen their tasklet's
context.

This patch changes this a little bit by setting the TASK_F_USR1 flag on
the tasklet before marking a connection idle, and removing it once it's
not idle anymore. Thanks to this we have the guarantee that a tasklet
without this flag cannot be present in an idle list and does not need
to go through this costly lock. This is especially true for front
connections.

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index d5302a004451f5ca582aa9c2da2927ac9e95bead..be063cf403b3987acad4d1a9d1b8b27e3864a96e 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -2975,36 +2975,42 @@ schedule:
 }
 
 /* this is the tasklet referenced in fconn->wait_event.tasklet */
-struct task *fcgi_io_cb(struct task *t, void *ctx, unsigned int status)
+struct task *fcgi_io_cb(struct task *t, void *ctx, unsigned int state)
 {
        struct connection *conn;
-       struct fcgi_conn *fconn;
+       struct fcgi_conn *fconn = ctx;
        struct tasklet *tl = (struct tasklet *)t;
        int conn_in_list;
        int ret = 0;
 
-
-       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-       if (tl->context == NULL) {
-               /* The connection has been taken over by another thread,
-                * we're no longer responsible for it, so just free the
-                * tasklet, and do nothing.
+       if (state & TASK_F_USR1) {
+               /* the tasklet was idling on an idle connection, it might have
+                * been stolen, let's be careful!
                 */
-               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-               tasklet_free(tl);
-               return NULL;
-
-       }
-       fconn = ctx;
-       conn = fconn->conn;
-
-       TRACE_POINT(FCGI_EV_FCONN_WAKE, conn);
+               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               if (tl->context == NULL) {
+                       /* The connection has been taken over by another thread,
+                        * we're no longer responsible for it, so just free the
+                        * tasklet, and do nothing.
+                        */
+                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       tasklet_free(tl);
+                       return NULL;
+               }
+               conn = fconn->conn;
+               TRACE_POINT(FCGI_EV_FCONN_WAKE, conn);
 
-       conn_in_list = conn->flags & CO_FL_LIST_MASK;
-       if (conn_in_list)
-               conn_delete_from_tree(&conn->hash_node->node);
+               conn_in_list = conn->flags & CO_FL_LIST_MASK;
+               if (conn_in_list)
+                       conn_delete_from_tree(&conn->hash_node->node);
 
-       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+       } else {
+               /* we're certain the connection was not in an idle list */
+               conn = fconn->conn;
+               TRACE_ENTER(FCGI_EV_FCONN_WAKE, conn);
+               conn_in_list = 0;
+       }
 
        if (!(fconn->wait_event.events & SUB_RETRY_SEND))
                ret = fcgi_send(fconn);
@@ -3480,6 +3486,10 @@ static struct conn_stream *fcgi_attach(struct connection *conn, struct session *
                cs_free(cs);
                goto err;
        }
+
+       /* the connection is not idle anymore, let's mark this */
+       HA_ATOMIC_AND(&fconn->wait_event.tasklet->state, ~TASK_F_USR1);
+
        TRACE_LEAVE(FCGI_EV_FSTRM_NEW, conn, fstrm);
        return cs;
 
@@ -3606,6 +3616,10 @@ static void fcgi_detach(struct conn_stream *cs)
                                        fconn->conn->owner = NULL;
                                }
 
+                               /* mark that the tasklet may lose its context to another thread and
+                                * that the handler needs to check it under the idle conns lock.
+                                */
+                               HA_ATOMIC_OR(&fconn->wait_event.tasklet->state, TASK_F_USR1);
                                if (!srv_add_to_idle_list(objt_server(fconn->conn->target), fconn->conn, 1)) {
                                        /* The server doesn't want it, let's kill the connection right away */
                                        fconn->conn->mux->destroy(fconn);

diff --git a/src/mux_h1.c b/src/mux_h1.c
index d5b9e9944b565d89b19cd7fc55d20c5efc6a12d5..7d1bed48dd95d08627110c04dcc4b4ac2f530694 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -2799,38 +2799,45 @@ static int h1_process(struct h1c * h1c)
        return -1;
 }
 
-struct task *h1_io_cb(struct task *t, void *ctx, unsigned int status)
+struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state)
 {
        struct connection *conn;
        struct tasklet *tl = (struct tasklet *)t;
        int conn_in_list;
-       struct h1c *h1c;
+       struct h1c *h1c = ctx;
        int ret = 0;
 
+       if (state & TASK_F_USR1) {
+               /* the tasklet was idling on an idle connection, it might have
+                * been stolen, let's be careful!
+                */
+               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               if (tl->context == NULL) {
+                       /* The connection has been taken over by another thread,
+                        * we're no longer responsible for it, so just free the
+                        * tasklet, and do nothing.
+                        */
+                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       tasklet_free(tl);
+                       return NULL;
+               }
+               conn = h1c->conn;
+               TRACE_POINT(H1_EV_H1C_WAKE, conn);
 
-       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-       if (tl->context == NULL) {
-               /* The connection has been taken over by another thread,
-                * we're no longer responsible for it, so just free the
-                * tasklet, and do nothing.
+               /* Remove the connection from the list, to be sure nobody attempts
+                * to use it while we handle the I/O events
                 */
+               conn_in_list = conn->flags & CO_FL_LIST_MASK;
+               if (conn_in_list)
+                       conn_delete_from_tree(&conn->hash_node->node);
+
                HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-               tasklet_free(tl);
-               return NULL;
+       } else {
+               /* we're certain the connection was not in an idle list */
+               conn = h1c->conn;
+               TRACE_ENTER(H1_EV_H1C_WAKE, conn);
+               conn_in_list = 0;
        }
-       h1c = ctx;
-       conn = h1c->conn;
-
-       TRACE_POINT(H1_EV_H1C_WAKE, conn);
-
-       /* Remove the connection from the list, to be sure nobody attempts
-        * to use it while we handle the I/O events
-        */
-       conn_in_list = conn->flags & CO_FL_LIST_MASK;
-       if (conn_in_list)
-               conn_delete_from_tree(&conn->hash_node->node);
-
-       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
 
        if (!(h1c->wait_event.events & SUB_RETRY_SEND))
                ret = h1_send(h1c);
@@ -2998,6 +3005,9 @@ static struct conn_stream *h1_attach(struct connection *conn, struct session *se
                goto err;
        }
 
+       /* the connection is not idle anymore, let's mark this */
+       HA_ATOMIC_AND(&h1c->wait_event.tasklet->state, ~TASK_F_USR1);
+
        TRACE_LEAVE(H1_EV_STRM_NEW, conn, h1s);
        return cs;
   err:
@@ -3090,6 +3100,11 @@ static void h1_detach(struct conn_stream *cs)
                else {
                        if (h1c->conn->owner == sess)
                                h1c->conn->owner = NULL;
+
+                       /* mark that the tasklet may lose its context to another thread and
+                        * that the handler needs to check it under the idle conns lock.
+                        */
+                       HA_ATOMIC_OR(&h1c->wait_event.tasklet->state, TASK_F_USR1);
                        h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
                        if (!srv_add_to_idle_list(objt_server(h1c->conn->target), h1c->conn, is_not_first)) {
                                /* The server doesn't want it, let's kill the connection right away */

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 4fcd2d6d9036fb9cb199012bc88e8e7495c46a6f..2ac61f7ac4dcda67d5c4e716e85bcdd2a238b445 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -3773,39 +3773,46 @@ schedule:
 }
 
 /* this is the tasklet referenced in h2c->wait_event.tasklet */
-struct task *h2_io_cb(struct task *t, void *ctx, unsigned int status)
+struct task *h2_io_cb(struct task *t, void *ctx, unsigned int state)
 {
        struct connection *conn;
        struct tasklet *tl = (struct tasklet *)t;
        int conn_in_list;
-       struct h2c *h2c;
+       struct h2c *h2c = ctx;
        int ret = 0;
 
-
-       HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-       if (t->context == NULL) {
-               /* The connection has been taken over by another thread,
-                * we're no longer responsible for it, so just free the
-                * tasklet, and do nothing.
+       if (state & TASK_F_USR1) {
+               /* the tasklet was idling on an idle connection, it might have
+                * been stolen, let's be careful!
                 */
-               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
-               tasklet_free(tl);
-               goto leave;
-       }
-       h2c = ctx;
-       conn = h2c->conn;
-
-       TRACE_ENTER(H2_EV_H2C_WAKE, conn);
+               HA_SPIN_LOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               if (t->context == NULL) {
+                       /* The connection has been taken over by another thread,
+                        * we're no longer responsible for it, so just free the
+                        * tasklet, and do nothing.
+                        */
+                       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+                       tasklet_free(tl);
+                       goto leave;
+               }
+               conn = h2c->conn;
+               TRACE_ENTER(H2_EV_H2C_WAKE, conn);
 
-       conn_in_list = conn->flags & CO_FL_LIST_MASK;
+               conn_in_list = conn->flags & CO_FL_LIST_MASK;
 
-       /* Remove the connection from the list, to be sure nobody attempts
-        * to use it while we handle the I/O events
-        */
-       if (conn_in_list)
-               conn_delete_from_tree(&conn->hash_node->node);
+               /* Remove the connection from the list, to be sure nobody attempts
+                * to use it while we handle the I/O events
+                */
+               if (conn_in_list)
+                       conn_delete_from_tree(&conn->hash_node->node);
 
-       HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+               HA_SPIN_UNLOCK(IDLE_CONNS_LOCK, &idle_conns[tid].idle_conns_lock);
+       } else {
+               /* we're certain the connection was not in an idle list */
+               conn = h2c->conn;
+               TRACE_ENTER(H2_EV_H2C_WAKE, conn);
+               conn_in_list = 0;
+       }
 
        if (!(h2c->wait_event.events & SUB_RETRY_SEND))
                ret = h2_send(h2c);
@@ -4096,6 +4103,10 @@ static struct conn_stream *h2_attach(struct connection *conn, struct session *se
                cs_free(cs);
                return NULL;
        }
+
+       /* the connection is not idle anymore, let's mark this */
+       HA_ATOMIC_AND(&h2c->wait_event.tasklet->state, ~TASK_F_USR1);
+
        TRACE_LEAVE(H2_EV_H2S_NEW, conn, h2s);
        return cs;
 }
@@ -4239,6 +4250,10 @@ static void h2_detach(struct conn_stream *cs)
                                                h2c->conn->owner = NULL;
                                        }
 
+                                       /* mark that the tasklet may lose its context to another thread and
+                                        * that the handler needs to check it under the idle conns lock.
+                                        */
+                                       HA_ATOMIC_OR(&h2c->wait_event.tasklet->state, TASK_F_USR1);
                                        if (!srv_add_to_idle_list(objt_server(h2c->conn->target), h2c->conn, 1)) {
                                                /* The server doesn't want it, let's kill the connection right away */
                                                h2c->conn->mux->destroy(h2c);