From 2dc393573430f853e56e25bf4b41c34ba2aa8fd6 Mon Sep 17 00:00:00 2001 From: Andy Grover Date: Fri, 11 Jun 2010 13:49:13 -0700 Subject: RDS: move rds_shutdown_worker impl. to rds_conn_shutdown This fits better in connection.c, rather than threads.c. Signed-off-by: Andy Grover --- net/rds/threads.c | 61 ++++++++----------------------------------------------- 1 file changed, 8 insertions(+), 53 deletions(-) (limited to 'net/rds/threads.c') diff --git a/net/rds/threads.c b/net/rds/threads.c index 786c20eaaf5e..6e2e43d5f576 100644 --- a/net/rds/threads.c +++ b/net/rds/threads.c @@ -110,7 +110,7 @@ EXPORT_SYMBOL_GPL(rds_connect_complete); * We should *always* start with a random backoff; otherwise a broken connection * will always take several iterations to be re-established. */ -static void rds_queue_reconnect(struct rds_connection *conn) +void rds_queue_reconnect(struct rds_connection *conn) { unsigned long rand; @@ -156,58 +156,6 @@ void rds_connect_worker(struct work_struct *work) } } -void rds_shutdown_worker(struct work_struct *work) -{ - struct rds_connection *conn = container_of(work, struct rds_connection, c_down_w); - - /* shut it down unless it's down already */ - if (!rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_DOWN)) { - /* - * Quiesce the connection mgmt handlers before we start tearing - * things down. We don't hold the mutex for the entire - * duration of the shutdown operation, else we may be - * deadlocking with the CM handler. Instead, the CM event - * handler is supposed to check for state DISCONNECTING - */ - mutex_lock(&conn->c_cm_lock); - if (!rds_conn_transition(conn, RDS_CONN_UP, RDS_CONN_DISCONNECTING) && - !rds_conn_transition(conn, RDS_CONN_ERROR, RDS_CONN_DISCONNECTING)) { - rds_conn_error(conn, "shutdown called in state %d\n", - atomic_read(&conn->c_state)); - mutex_unlock(&conn->c_cm_lock); - return; - } - mutex_unlock(&conn->c_cm_lock); - - mutex_lock(&conn->c_send_lock); - conn->c_trans->conn_shutdown(conn); - rds_conn_reset(conn); - mutex_unlock(&conn->c_send_lock); - - if (!rds_conn_transition(conn, RDS_CONN_DISCONNECTING, RDS_CONN_DOWN)) { - /* This can happen - eg when we're in the middle of tearing - * down the connection, and someone unloads the rds module. - * Quite reproduceable with loopback connections. - * Mostly harmless. - */ - rds_conn_error(conn, - "%s: failed to transition to state DOWN, " - "current state is %d\n", - __func__, - atomic_read(&conn->c_state)); - return; - } - } - - /* Then reconnect if it's still live. - * The passive side of an IB loopback connection is never added - * to the conn hash, so we never trigger a reconnect on this - * conn - the reconnect is always triggered by the active peer. */ - cancel_delayed_work(&conn->c_conn_w); - if (!hlist_unhashed(&conn->c_hash_node)) - rds_queue_reconnect(conn); -} - void rds_send_worker(struct work_struct *work) { struct rds_connection *conn = container_of(work, struct rds_connection, c_send_w.work); @@ -252,6 +200,13 @@ void rds_recv_worker(struct work_struct *work) } } +void rds_shutdown_worker(struct work_struct *work) +{ + struct rds_connection *conn = container_of(work, struct rds_connection, c_down_w); + + rds_conn_shutdown(conn); +} + void rds_threads_exit(void) { destroy_workqueue(rds_wq); -- cgit v1.2.2 From 8690bfa17aea4c42da1bcf90a7af93d161eca624 Mon Sep 17 00:00:00 2001 From: Andy Grover Date: Tue, 12 Jan 2010 11:56:44 -0800 Subject: RDS: cleanup: remove "== NULL"s and "!= NULL"s in ptr comparisons Favor "if (foo)" style over "if (foo != NULL)". Signed-off-by: Andy Grover --- net/rds/threads.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/rds/threads.c') diff --git a/net/rds/threads.c b/net/rds/threads.c index 6e2e43d5f576..7a8ca7a1d983 100644 --- a/net/rds/threads.c +++ b/net/rds/threads.c @@ -215,7 +215,7 @@ void rds_threads_exit(void) int __init rds_threads_init(void) { rds_wq = create_workqueue("krdsd"); - if (rds_wq == NULL) + if (!rds_wq) return -ENOMEM; return 0; -- cgit v1.2.2 From 0f4b1c7e89e699f588807a914ec6e6396c851a72 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Fri, 4 Jun 2010 14:41:41 -0700 Subject: rds: fix rds_send_xmit() serialization rds_send_xmit() was changed to hold an interrupt masking spinlock instead of a mutex so that it could be called from the IB receive tasklet path. This broke the TCP transport because its xmit method can block and masks and unmasks interrupts. This patch serializes callers to rds_send_xmit() with a simple bit instead of the current spinlock or previous mutex. This enables rds_send_xmit() to be called from any context and to call functions which block. Getting rid of the c_send_lock exposes the bare c_lock acquisitions which are changed to block interrupts. A waitqueue is added so that rds_conn_shutdown() can wait for callers to leave rds_send_xmit() before tearing down partial send state. This lets us get rid of c_senders. rds_send_xmit() is changed to check the conn state after acquiring the RDS_IN_XMIT bit to resolve races with the shutdown path. Previously both worked with the conn state and then the lock in the same order, allowing them to race and execute the paths concurrently. rds_send_reset() isn't racing with rds_send_xmit() now that rds_conn_shutdown() properly ensures that rds_send_xmit() can't start once the conn state has been changed. We can remove its previous use of the spinlock. Finally, c_send_generation is redundant. Callers can race to test the c_flags bit by simply retrying instead of racing to test the c_send_generation atomic. Signed-off-by: Zach Brown --- net/rds/threads.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/rds/threads.c') diff --git a/net/rds/threads.c b/net/rds/threads.c index 7a8ca7a1d983..2bab9bf07b91 100644 --- a/net/rds/threads.c +++ b/net/rds/threads.c @@ -61,7 +61,7 @@ * * Transition to state DISCONNECTING/DOWN: * - Inside the shutdown worker; synchronizes with xmit path - * through c_send_lock, and with connection management callbacks + * through RDS_IN_XMIT, and with connection management callbacks * via c_cm_lock. * * For receive callbacks, we rely on the underlying transport -- cgit v1.2.2 From 80c51be56ffa257d3177f0d750d90be65d30c22f Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Tue, 6 Jul 2010 15:08:48 -0700 Subject: RDS: return to a single-threaded krdsd We were seeing very nasty bugs due to fundamental assumption the current code makes about concurrent work struct processing. The code simpy isn't able to handle concurrent connection shutdown work function execution today, for example, which is very much possible once a multi-threaded krdsd was introduced. The problem compounds as additional work structs are added to the mix. krdsd is no longer perforance critical now that send and receive posting and FMR flushing are done elsewhere, so the safest fix is to move back to the single threaded krdsd that the current code was built around. Signed-off-by: Zach Brown --- net/rds/threads.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/rds/threads.c') diff --git a/net/rds/threads.c b/net/rds/threads.c index 2bab9bf07b91..c08c220efac5 100644 --- a/net/rds/threads.c +++ b/net/rds/threads.c @@ -214,7 +214,7 @@ void rds_threads_exit(void) int __init rds_threads_init(void) { - rds_wq = create_workqueue("krdsd"); + rds_wq = create_singlethread_workqueue("krdsd"); if (!rds_wq) return -ENOMEM; -- cgit v1.2.2 From ef87b7ea39a91906218a262686bcb8bad8b6b46e Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Fri, 9 Jul 2010 12:26:20 -0700 Subject: RDS: remove __init and __exit annotation The trivial amount of memory saved isn't worth the cost of dealing with section mismatches. Signed-off-by: Zach Brown --- net/rds/threads.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/rds/threads.c') diff --git a/net/rds/threads.c b/net/rds/threads.c index c08c220efac5..0fd90f8c5f59 100644 --- a/net/rds/threads.c +++ b/net/rds/threads.c @@ -212,7 +212,7 @@ void rds_threads_exit(void) destroy_workqueue(rds_wq); } -int __init rds_threads_init(void) +int rds_threads_init(void) { rds_wq = create_singlethread_workqueue("krdsd"); if (!rds_wq) -- cgit v1.2.2