From 96c95a46577d6ba82338549421889b42a4f5932f Mon Sep 17 00:00:00 2001 From: Maurizio Lombardi Date: Fri, 1 Oct 2021 12:11:39 +0200 Subject: [PATCH] target: iscsi: simplify the connection closing mechanism When the connection reinstatement is performed, the target driver executes a complex scheme of complete()/wait_for_completion() that is not really needed. Considering that: 1) The callers of iscsit_connection_reinstatement_rcfr() and iscsit_cause_connection_reinstatement() hold a reference to the conn structure. 2) iscsit_close_connection() will sleep when calling iscsit_check_conn_usage_count() until the conn structure's refcount reaches zero. we can optimize the driver the following way: * The threads that must sleep until the connection is closed will all wait for the "conn_wait_comp" completion, iscsit_close_connection() will then call complete_all() to wake them up. No need to have multiple completion structures. * The conn_post_wait_comp completion is not necessary and can be removed because iscsit_close_connection() sleeps until all the other threads release the conn structure. (see the iscsit_check_conn_usage_count() function) V2: do not set connection_reinstatement to 1 in iscsit_close_connection() otherwise we could race with iscsit_cause_connection_reinstatement() and the SIGINT signal would not be sent to the tx and rx threads. I also removed the iscsit_connection_reinstatement_rcfr() function because it does the same things as iscsit_cause_connection_reinstatement() with sleep = 1. Signed-off-by: Maurizio Lombardi --- drivers/target/iscsi/iscsi_target.c | 26 ++-------------------- drivers/target/iscsi/iscsi_target_erl0.c | 27 ----------------------- drivers/target/iscsi/iscsi_target_erl0.h | 1 - drivers/target/iscsi/iscsi_target_login.c | 4 +--- drivers/target/iscsi/iscsi_target_util.c | 3 --- include/target/iscsi/iscsi_target_core.h | 4 ---- 6 files changed, 3 insertions(+), 62 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 2c54c5d8412d..52aa088f128e 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -2934,7 +2934,7 @@ iscsit_build_logout_rsp(struct iscsi_cmd *cmd, struct iscsi_conn *conn, logout_conn = iscsit_get_conn_from_cid_rcfr(sess, cmd->logout_cid); if (logout_conn) { - iscsit_connection_reinstatement_rcfr(logout_conn); + iscsit_cause_connection_reinstatement(logout_conn, 1); iscsit_dec_conn_usage_count(logout_conn); } @@ -4228,29 +4228,7 @@ int iscsit_close_connection( * up the connection reinstatement semaphore that is being blocked on * in iscsit_cause_connection_reinstatement(). */ - spin_lock_bh(&conn->state_lock); - if (atomic_read(&conn->sleep_on_conn_wait_comp)) { - spin_unlock_bh(&conn->state_lock); - complete(&conn->conn_wait_comp); - wait_for_completion(&conn->conn_post_wait_comp); - spin_lock_bh(&conn->state_lock); - } - - /* - * If connection reinstatement is being performed on this connection - * by receiving a REMOVECONNFORRECOVERY logout request, up the - * connection wait rcfr semaphore that is being blocked on - * an iscsit_connection_reinstatement_rcfr(). - */ - if (atomic_read(&conn->connection_wait_rcfr)) { - spin_unlock_bh(&conn->state_lock); - complete(&conn->conn_wait_rcfr_comp); - wait_for_completion(&conn->conn_post_wait_comp); - spin_lock_bh(&conn->state_lock); - } - atomic_set(&conn->connection_reinstatement, 1); - spin_unlock_bh(&conn->state_lock); - + complete_all(&conn->conn_wait_comp); /* * If any other processes are accessing this connection pointer we * must wait until they have completed. diff --git a/drivers/target/iscsi/iscsi_target_erl0.c b/drivers/target/iscsi/iscsi_target_erl0.c index 102c9cbf59f3..0eae641d4d08 100644 --- a/drivers/target/iscsi/iscsi_target_erl0.c +++ b/drivers/target/iscsi/iscsi_target_erl0.c @@ -819,30 +819,6 @@ int iscsit_stop_time2retain_timer(struct iscsi_session *sess) return 0; } -void iscsit_connection_reinstatement_rcfr(struct iscsi_conn *conn) -{ - spin_lock_bh(&conn->state_lock); - if (atomic_read(&conn->connection_exit)) { - spin_unlock_bh(&conn->state_lock); - goto sleep; - } - - if (atomic_read(&conn->transport_failed)) { - spin_unlock_bh(&conn->state_lock); - goto sleep; - } - spin_unlock_bh(&conn->state_lock); - - if (conn->tx_thread && conn->tx_thread_active) - send_sig(SIGINT, conn->tx_thread, 1); - if (conn->rx_thread && conn->rx_thread_active) - send_sig(SIGINT, conn->rx_thread, 1); - -sleep: - wait_for_completion(&conn->conn_wait_rcfr_comp); - complete(&conn->conn_post_wait_comp); -} - void iscsit_cause_connection_reinstatement(struct iscsi_conn *conn, int sleep) { spin_lock_bh(&conn->state_lock); @@ -871,12 +847,9 @@ void iscsit_cause_connection_reinstatement(struct iscsi_conn *conn, int sleep) spin_unlock_bh(&conn->state_lock); return; } - - atomic_set(&conn->sleep_on_conn_wait_comp, 1); spin_unlock_bh(&conn->state_lock); wait_for_completion(&conn->conn_wait_comp); - complete(&conn->conn_post_wait_comp); } EXPORT_SYMBOL(iscsit_cause_connection_reinstatement); diff --git a/drivers/target/iscsi/iscsi_target_erl0.h b/drivers/target/iscsi/iscsi_target_erl0.h index 883ebf6d36cf..7db4300924e8 100644 --- a/drivers/target/iscsi/iscsi_target_erl0.h +++ b/drivers/target/iscsi/iscsi_target_erl0.h @@ -14,7 +14,6 @@ extern int iscsit_check_post_dataout(struct iscsi_cmd *, unsigned char *, u8); extern void iscsit_start_time2retain_handler(struct iscsi_session *); extern void iscsit_handle_time2retain_timeout(struct timer_list *t); extern int iscsit_stop_time2retain_timer(struct iscsi_session *); -extern void iscsit_connection_reinstatement_rcfr(struct iscsi_conn *); extern void iscsit_cause_connection_reinstatement(struct iscsi_conn *, int); extern void iscsit_fall_back_to_erl0(struct iscsi_session *); extern void iscsit_take_action_for_connection_exit(struct iscsi_conn *, bool *); diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index 1a9c50401bdb..419f01b936f2 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -567,7 +567,7 @@ int iscsi_login_post_auth_non_zero_tsih( " performing connection reinstatement.\n", conn_ptr->cid, sess->sess_ops->InitiatorName); - iscsit_connection_reinstatement_rcfr(conn_ptr); + iscsit_cause_connection_reinstatement(conn_ptr, 1); iscsit_dec_conn_usage_count(conn_ptr); } @@ -1096,9 +1096,7 @@ static struct iscsi_conn *iscsit_alloc_conn(struct iscsi_np *np) INIT_LIST_HEAD(&conn->conn_cmd_list); INIT_LIST_HEAD(&conn->immed_queue_list); INIT_LIST_HEAD(&conn->response_queue_list); - init_completion(&conn->conn_post_wait_comp); init_completion(&conn->conn_wait_comp); - init_completion(&conn->conn_wait_rcfr_comp); init_completion(&conn->conn_waiting_on_uc_comp); init_completion(&conn->conn_logout_comp); init_completion(&conn->rx_half_close_comp); diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 6dd5810e2af1..d7b1f9110d49 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -824,9 +824,6 @@ struct iscsi_conn *iscsit_get_conn_from_cid_rcfr(struct iscsi_session *sess, u16 list_for_each_entry(conn, &sess->sess_conn_list, conn_list) { if (conn->cid == cid) { iscsit_inc_conn_usage_count(conn); - spin_lock(&conn->state_lock); - atomic_set(&conn->connection_wait_rcfr, 1); - spin_unlock(&conn->state_lock); spin_unlock_bh(&sess->conn_lock); return conn; } diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h index 1eccb2ac7d02..aeb8932507c2 100644 --- a/include/target/iscsi/iscsi_target_core.h +++ b/include/target/iscsi/iscsi_target_core.h @@ -542,12 +542,8 @@ struct iscsi_conn { atomic_t connection_exit; atomic_t connection_recovery; atomic_t connection_reinstatement; - atomic_t connection_wait_rcfr; - atomic_t sleep_on_conn_wait_comp; atomic_t transport_failed; - struct completion conn_post_wait_comp; struct completion conn_wait_comp; - struct completion conn_wait_rcfr_comp; struct completion conn_waiting_on_uc_comp; struct completion conn_logout_comp; struct completion tx_half_close_comp; -- 2.27.0