From 433675486af4635416f8ece70b92b020a5b91005 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Sun, 20 Dec 2020 21:36:33 +0100 Subject: scsi: target: iscsi: Avoid in_interrupt() usage in iscsit_close_session() iscsit_close_session() uses in_interrupt() to decide if it needs to check the return value of iscsit_check_session_usage_count() if it was not able to sleep. The usage of in_interrupt() in drivers is phased out and Linus clearly requested that code which changes behaviour depending on context should either be separated or the context be conveyed in an argument passed by the caller, which usually knows the context. iscsit_close_session() has two callers: - iscsit_handle_time2retain_timeout() A timer_list callback. - iscsit_close_connection() Runs in preemptible context, acquires a mutex. Add an argument to iscsit_close_session() indicating if sleeping is possible. Link: https://lore.kernel.org/r/20201220203638.43615-2-bigeasy@linutronix.de Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target.c | 10 +++++----- drivers/target/iscsi/iscsi_target.h | 2 +- drivers/target/iscsi/iscsi_target_erl0.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 518fac4864cf..5abf03125388 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -4327,7 +4327,7 @@ int iscsit_close_connection( atomic_read(&sess->session_fall_back_to_erl0)) { spin_unlock_bh(&sess->conn_lock); complete_all(&sess->session_wait_comp); - iscsit_close_session(sess); + iscsit_close_session(sess, true); return 0; } else if (atomic_read(&sess->session_logout)) { @@ -4337,7 +4337,7 @@ int iscsit_close_connection( if (atomic_read(&sess->session_close)) { spin_unlock_bh(&sess->conn_lock); complete_all(&sess->session_wait_comp); - iscsit_close_session(sess); + iscsit_close_session(sess, true); } else { spin_unlock_bh(&sess->conn_lock); } @@ -4353,7 +4353,7 @@ int iscsit_close_connection( if (atomic_read(&sess->session_close)) { spin_unlock_bh(&sess->conn_lock); complete_all(&sess->session_wait_comp); - iscsit_close_session(sess); + iscsit_close_session(sess, true); } else { spin_unlock_bh(&sess->conn_lock); } @@ -4366,7 +4366,7 @@ int iscsit_close_connection( * If the iSCSI Session for the iSCSI Initiator Node exists, * forcefully shutdown the iSCSI NEXUS. */ -int iscsit_close_session(struct iscsi_session *sess) +int iscsit_close_session(struct iscsi_session *sess, bool can_sleep) { struct iscsi_portal_group *tpg = sess->tpg; struct se_portal_group *se_tpg = &tpg->tpg_se_tpg; @@ -4399,7 +4399,7 @@ int iscsit_close_session(struct iscsi_session *sess) * time2retain handler) and contain and active session usage count we * restart the timer and exit. */ - if (!in_interrupt()) { + if (can_sleep) { iscsit_check_session_usage_count(sess); } else { if (iscsit_check_session_usage_count(sess) == 2) { diff --git a/drivers/target/iscsi/iscsi_target.h b/drivers/target/iscsi/iscsi_target.h index 7409ce2a6607..b35a96ded9c1 100644 --- a/drivers/target/iscsi/iscsi_target.h +++ b/drivers/target/iscsi/iscsi_target.h @@ -41,7 +41,7 @@ extern void iscsit_thread_get_cpumask(struct iscsi_conn *); extern int iscsi_target_tx_thread(void *); extern int iscsi_target_rx_thread(void *); extern int iscsit_close_connection(struct iscsi_conn *); -extern int iscsit_close_session(struct iscsi_session *); +extern int iscsit_close_session(struct iscsi_session *, bool can_sleep); extern void iscsit_fail_session(struct iscsi_session *); extern void iscsit_stop_session(struct iscsi_session *, int, int); extern int iscsit_release_sessions_for_tpg(struct iscsi_portal_group *, int); diff --git a/drivers/target/iscsi/iscsi_target_erl0.c b/drivers/target/iscsi/iscsi_target_erl0.c index b4abd7b68e6d..102c9cbf59f3 100644 --- a/drivers/target/iscsi/iscsi_target_erl0.c +++ b/drivers/target/iscsi/iscsi_target_erl0.c @@ -765,7 +765,7 @@ void iscsit_handle_time2retain_timeout(struct timer_list *t) iscsit_fill_cxn_timeout_err_stats(sess); spin_unlock_bh(&se_tpg->session_lock); - iscsit_close_session(sess); + iscsit_close_session(sess, false); } void iscsit_start_time2retain_handler(struct iscsi_session *sess) -- cgit v1.2.3-59-g8ed1b From efc9d73063c15f1aba8920b9f9ceaba4f3fb8ed9 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Sun, 20 Dec 2020 21:36:34 +0100 Subject: scsi: target: iscsi: Avoid in_interrupt() usage in iscsit_check_session_usage_count() iscsit_check_session_usage_count() uses in_interrupt() to find out if it is safe to invoke wait_for_completion(). The usage of in_interrupt() in drivers is phased out and Linus clearly requested that code which changes behaviour depending on context should either be separated or the context be conveyed in an argument passed by the caller, which usually knows the context. There is only one caller of iscsit_check_session_usage_count() which already has an argument indicating if it is safe to sleep. Extend iscsit_check_session_usage_count() by an argument indicating if it may sleep. Link: https://lore.kernel.org/r/20201220203638.43615-3-bigeasy@linutronix.de Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target.c | 4 ++-- drivers/target/iscsi/iscsi_target_util.c | 4 ++-- drivers/target/iscsi/iscsi_target_util.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 5abf03125388..13de37ff9dc9 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -4400,9 +4400,9 @@ int iscsit_close_session(struct iscsi_session *sess, bool can_sleep) * restart the timer and exit. */ if (can_sleep) { - iscsit_check_session_usage_count(sess); + iscsit_check_session_usage_count(sess, can_sleep); } else { - if (iscsit_check_session_usage_count(sess) == 2) { + if (iscsit_check_session_usage_count(sess, can_sleep) == 2) { atomic_set(&sess->session_logout, 0); iscsit_start_time2retain_handler(sess); return 0; diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 45ba07c6ec27..50028b1d3b84 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -779,13 +779,13 @@ void iscsit_free_cmd(struct iscsi_cmd *cmd, bool shutdown) } EXPORT_SYMBOL(iscsit_free_cmd); -int iscsit_check_session_usage_count(struct iscsi_session *sess) +int iscsit_check_session_usage_count(struct iscsi_session *sess, bool can_sleep) { spin_lock_bh(&sess->session_usage_lock); if (sess->session_usage_count != 0) { sess->session_waiting_on_uc = 1; spin_unlock_bh(&sess->session_usage_lock); - if (in_interrupt()) + if (!can_sleep) return 2; wait_for_completion(&sess->session_waiting_on_uc_comp); diff --git a/drivers/target/iscsi/iscsi_target_util.h b/drivers/target/iscsi/iscsi_target_util.h index 68e84803b0a1..a2c2401f2945 100644 --- a/drivers/target/iscsi/iscsi_target_util.h +++ b/drivers/target/iscsi/iscsi_target_util.h @@ -40,7 +40,7 @@ extern void iscsit_free_queue_reqs_for_conn(struct iscsi_conn *); extern void iscsit_release_cmd(struct iscsi_cmd *); extern void __iscsit_free_cmd(struct iscsi_cmd *, bool); extern void iscsit_free_cmd(struct iscsi_cmd *, bool); -extern int iscsit_check_session_usage_count(struct iscsi_session *); +extern int iscsit_check_session_usage_count(struct iscsi_session *sess, bool can_sleep); extern void iscsit_dec_session_usage_count(struct iscsi_session *); extern void iscsit_inc_session_usage_count(struct iscsi_session *); extern struct iscsi_conn *iscsit_get_conn_from_cid(struct iscsi_session *, u16); -- cgit v1.2.3-59-g8ed1b From f88a10f80da9ed1ab1ba7496b70e9a0cdd8f7cf8 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Sun, 20 Dec 2020 21:36:35 +0100 Subject: scsi: target: iscsi: Redo iscsit_check_session_usage_count() return code The return value of iscsit_check_session_usage_count() is only checked if it was not allowed to sleep. If it returns `2' then a timer is prepared. If it returns something else or if it was allowed to sleep then it is ignored. Let iscsit_check_session_usage_count() return true if it needs to arm the timer - otherwise false. This simplifies the code flow of the only caller. Link: https://lore.kernel.org/r/20201220203638.43615-4-bigeasy@linutronix.de Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target.c | 12 ++++-------- drivers/target/iscsi/iscsi_target_util.c | 9 +++++---- drivers/target/iscsi/iscsi_target_util.h | 2 +- 3 files changed, 10 insertions(+), 13 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 13de37ff9dc9..d0e7ed8f28cc 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -4399,14 +4399,10 @@ int iscsit_close_session(struct iscsi_session *sess, bool can_sleep) * time2retain handler) and contain and active session usage count we * restart the timer and exit. */ - if (can_sleep) { - iscsit_check_session_usage_count(sess, can_sleep); - } else { - if (iscsit_check_session_usage_count(sess, can_sleep) == 2) { - atomic_set(&sess->session_logout, 0); - iscsit_start_time2retain_handler(sess); - return 0; - } + if (iscsit_check_session_usage_count(sess, can_sleep)) { + atomic_set(&sess->session_logout, 0); + iscsit_start_time2retain_handler(sess); + return 0; } transport_deregister_session(sess->se_sess); diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 50028b1d3b84..9468b017b4a7 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -779,21 +779,22 @@ void iscsit_free_cmd(struct iscsi_cmd *cmd, bool shutdown) } EXPORT_SYMBOL(iscsit_free_cmd); -int iscsit_check_session_usage_count(struct iscsi_session *sess, bool can_sleep) +bool iscsit_check_session_usage_count(struct iscsi_session *sess, + bool can_sleep) { spin_lock_bh(&sess->session_usage_lock); if (sess->session_usage_count != 0) { sess->session_waiting_on_uc = 1; spin_unlock_bh(&sess->session_usage_lock); if (!can_sleep) - return 2; + return true; wait_for_completion(&sess->session_waiting_on_uc_comp); - return 1; + return false; } spin_unlock_bh(&sess->session_usage_lock); - return 0; + return false; } void iscsit_dec_session_usage_count(struct iscsi_session *sess) diff --git a/drivers/target/iscsi/iscsi_target_util.h b/drivers/target/iscsi/iscsi_target_util.h index a2c2401f2945..8ee1c133a9b7 100644 --- a/drivers/target/iscsi/iscsi_target_util.h +++ b/drivers/target/iscsi/iscsi_target_util.h @@ -40,7 +40,7 @@ extern void iscsit_free_queue_reqs_for_conn(struct iscsi_conn *); extern void iscsit_release_cmd(struct iscsi_cmd *); extern void __iscsit_free_cmd(struct iscsi_cmd *, bool); extern void iscsit_free_cmd(struct iscsi_cmd *, bool); -extern int iscsit_check_session_usage_count(struct iscsi_session *sess, bool can_sleep); +extern bool iscsit_check_session_usage_count(struct iscsi_session *sess, bool can_sleep); extern void iscsit_dec_session_usage_count(struct iscsi_session *); extern void iscsit_inc_session_usage_count(struct iscsi_session *); extern struct iscsi_conn *iscsit_get_conn_from_cid(struct iscsi_session *, u16); -- cgit v1.2.3-59-g8ed1b