summaryrefslogtreecommitdiff
path: root/net/rxrpc/conn_client.c
diff options
context:
space:
mode:
authorDavid Howells <dhowells@redhat.com>2016-06-30 10:45:22 +0100
committerDavid Howells <dhowells@redhat.com>2016-07-06 10:50:04 +0100
commit001c11224910b25e59a65ce1b49cfecdb4c631c0 (patch)
treec51c38af1e4d79b14debf5ee780c0c277c4c7c45 /net/rxrpc/conn_client.c
parentd991b4a32f65076efaf78739c4a46406ca8c7e79 (diff)
rxrpc: Maintain an extra ref on a conn for the cache list
Overhaul the usage count accounting for the rxrpc_connection struct to make it easier to implement RCU access from the data_ready handler. The problem is that currently we're using a lock to prevent the garbage collector from trying to clean up a connection that we're contemplating unidling. We could just stick incoming packets on the connection we find, but we've then got a problem that we may race when dispatching a work item to process it as we need to give that a ref to prevent the rxrpc_connection struct from disappearing in the meantime. Further, incoming packets may get discarded if attached to an rxrpc_connection struct that is going away. Whilst this is not a total disaster - the client will presumably resend - it would delay processing of the call. This would affect the AFS client filesystem's service manager operation. To this end: (1) We now maintain an extra count on the connection usage count whilst it is on the connection list. This mean it is not in use when its refcount is 1. (2) When trying to reuse an old connection, we only increment the refcount if it is greater than 0. If it is 0, we replace it in the tree with a new candidate connection. (3) Two connection flags are added to indicate whether or not a connection is in the local's client connection tree (used by sendmsg) or the peer's service connection tree (used by data_ready). This makes sure that we don't try and remove a connection if it got replaced. The flags are tested under lock with the removal operation to prevent the reaper from killing the rxrpc_connection struct whilst someone else is trying to effect a replacement. This could probably be alleviated by using memory barriers between the flag set/test and the rb_tree ops. The rb_tree op would still need to be under the lock, however. (4) When trying to reap an old connection, we try to flip the usage count from 1 to 0. If it's not 1 at that point, then it must've come back to life temporarily and we ignore it. Signed-off-by: David Howells <dhowells@redhat.com>
Diffstat (limited to 'net/rxrpc/conn_client.c')
-rw-r--r--net/rxrpc/conn_client.c42
1 files changed, 34 insertions, 8 deletions
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index 9180164a51aa..aa21462f3236 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -84,7 +84,7 @@ error:
/*
* Release a connection ID for a client connection from the global pool.
*/
-void rxrpc_put_client_connection_id(struct rxrpc_connection *conn)
+static void rxrpc_put_client_connection_id(struct rxrpc_connection *conn)
{
if (test_bit(RXRPC_CONN_HAS_IDR, &conn->flags)) {
spin_lock(&rxrpc_conn_id_lock);
@@ -278,12 +278,13 @@ int rxrpc_connect_call(struct rxrpc_call *call,
* lock before dropping the client conn lock.
*/
_debug("new conn");
+ set_bit(RXRPC_CONN_IN_CLIENT_CONNS, &candidate->flags);
+ rb_link_node(&candidate->client_node, parent, pp);
+ rb_insert_color(&candidate->client_node, &local->client_conns);
+attached:
conn = candidate;
candidate = NULL;
- rb_link_node(&conn->client_node, parent, pp);
- rb_insert_color(&conn->client_node, &local->client_conns);
-
atomic_set(&conn->avail_chans, RXRPC_MAXCALLS - 1);
spin_lock(&conn->channel_lock);
spin_unlock(&local->client_conns_lock);
@@ -307,13 +308,22 @@ found_channel:
_leave(" = %p {u=%d}", conn, atomic_read(&conn->usage));
return 0;
- /* We found a suitable connection already in existence. Discard any
- * candidate we may have allocated, and try to get a channel on this
- * one.
+ /* We found a potentially suitable connection already in existence. If
+ * we can reuse it (ie. its usage count hasn't been reduced to 0 by the
+ * reaper), discard any candidate we may have allocated, and try to get
+ * a channel on this one, otherwise we have to replace it.
*/
found_extant_conn:
_debug("found conn");
- rxrpc_get_connection(conn);
+ if (!rxrpc_get_connection_maybe(conn)) {
+ set_bit(RXRPC_CONN_IN_CLIENT_CONNS, &candidate->flags);
+ rb_replace_node(&conn->client_node,
+ &candidate->client_node,
+ &local->client_conns);
+ clear_bit(RXRPC_CONN_IN_CLIENT_CONNS, &conn->flags);
+ goto attached;
+ }
+
spin_unlock(&local->client_conns_lock);
rxrpc_put_connection(candidate);
@@ -357,3 +367,19 @@ interrupted:
_leave(" = -ERESTARTSYS");
return -ERESTARTSYS;
}
+
+/*
+ * Remove a client connection from the local endpoint's tree, thereby removing
+ * it as a target for reuse for new client calls.
+ */
+void rxrpc_unpublish_client_conn(struct rxrpc_connection *conn)
+{
+ struct rxrpc_local *local = conn->params.local;
+
+ spin_lock(&local->client_conns_lock);
+ if (test_and_clear_bit(RXRPC_CONN_IN_CLIENT_CONNS, &conn->flags))
+ rb_erase(&conn->client_node, &local->client_conns);
+ spin_unlock(&local->client_conns_lock);
+
+ rxrpc_put_client_connection_id(conn);
+}