From: Nicholas Bellinger <nab@xxxxxxxxxxxxx>
To address a bug where se_cmd fabric driver level shutdown
occurs while TMR CMD_T_ABORTED is happening, resulting in
potential -1 ->cmd_kref, this patch adds target_tmr_put_cmd()
wrapper and CMD_T_FABRIC_STOP bit used to determine when
TMR + front-end driver I_T nexus shutdown is occuring.
It changes target_sess_cmd_list_set_waiting() to obtain
se_cmd->cmd_kref + set CMD_T_FABRIC_STOP, and drop local
reference in target_wait_for_sess_cmds() + invoke extra
target_put_sess_cmd() during Task Aborted Status (TAS)
when necessary.
Also, allow transport_wait_for_tasks() callers to set
CMD_T_FABRIC_STOP, and aware of CMD_T_ABORTED + CMD_T_TAS
status bits.
Note transport_generic_free_cmd() is expected to block on
cmd->cmd_wait_comp in order to follow what iscsi-target
expects during iscsi_conn context se_cmd shutdown.
Cc: Quinn Tran <quinn.tran@xxxxxxxxxx>
Cc: Himanshu Madhani <himanshu.madhani@xxxxxxxxxx>
Cc: Sagi Grimberg <sagig@xxxxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Hannes Reinecke <hare@xxxxxxx>
Cc: Andy Grover <agrover@xxxxxxxxxx>
Cc: Mike Christie <mchristi@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx # 3.10+
Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxx>
---
drivers/target/iscsi/iscsi_target_erl2.c | 3 +-
drivers/target/target_core_tmr.c | 49 +++++++++++++++++++----
drivers/target/target_core_transport.c | 69 +++++++++++++++++++++++++++-----
include/target/target_core_base.h | 2 +
include/target/target_core_fabric.h | 2 +-
5 files changed, 106 insertions(+), 19 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target_erl2.c b/drivers/target/iscsi/iscsi_target_erl2.c
index e24f1c7..1e79acd 100644
--- a/drivers/target/iscsi/iscsi_target_erl2.c
+++ b/drivers/target/iscsi/iscsi_target_erl2.c
@@ -316,6 +316,7 @@ int iscsit_prepare_cmds_for_realligance(struct iscsi_conn *conn)
u32 cmd_count = 0;
struct iscsi_cmd *cmd, *cmd_tmp;
struct iscsi_conn_recovery *cr;
+ bool aborted = false, tas = false;
/*
* Allocate an struct iscsi_conn_recovery for this connection.
@@ -398,7 +399,7 @@ int iscsit_prepare_cmds_for_realligance(struct iscsi_conn *conn)
iscsit_free_all_datain_reqs(cmd);
- transport_wait_for_tasks(&cmd->se_cmd);
+ transport_wait_for_tasks(&cmd->se_cmd, false, &aborted, &tas);
/*
* Add the struct iscsi_cmd to the connection recovery cmd list
*/
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 9d67d16..3f86f22 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -110,6 +110,7 @@ static int target_check_cdb_and_preempt(struct list_head *list,
static bool __target_check_io_state(struct se_cmd *se_cmd)
{
struct se_session *sess = se_cmd->se_sess;
+ bool ret;
assert_spin_locked(&sess->sess_cmd_lock);
WARN_ON_ONCE(!irqs_disabled());
@@ -129,10 +130,36 @@ static bool __target_check_io_state(struct se_cmd *se_cmd)
spin_unlock(&se_cmd->t_state_lock);
return false;
}
+ if (sess->sess_tearing_down || se_cmd->cmd_wait_set) {
+ pr_debug("Attempted to abort io tag: %llu already shutdown,"
+ " skipping\n", se_cmd->tag);
+ spin_unlock(&se_cmd->t_state_lock);
+ return false;
+ }
se_cmd->transport_state |= CMD_T_ABORTED;
spin_unlock(&se_cmd->t_state_lock);
- return kref_get_unless_zero(&se_cmd->cmd_kref);
+ ret = kref_get_unless_zero(&se_cmd->cmd_kref);
+ if (ret)
+ se_cmd->cmd_wait_set = 1;
+ return ret;
+}
+
+static void target_tmr_put_cmd(struct se_cmd *se_cmd)
+{
+ unsigned long flags;
+ bool fabric_stop;
+
+ spin_lock_irqsave(&se_cmd->t_state_lock, flags);
+ fabric_stop = (se_cmd->transport_state & CMD_T_FABRIC_STOP);
+ spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
+
+ target_put_sess_cmd(se_cmd);
+
+ if (!fabric_stop) {
+ wait_for_completion(&se_cmd->cmd_wait_comp);
+ se_cmd->se_tfo->release_cmd(se_cmd);
+ }
}
void core_tmr_abort_task(
@@ -143,6 +170,7 @@ void core_tmr_abort_task(
struct se_cmd *se_cmd;
unsigned long flags;
u64 ref_tag;
+ bool aborted = false, tas = false;
spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
list_for_each_entry(se_cmd, &se_sess->sess_cmd_list, se_cmd_list) {
@@ -175,10 +203,10 @@ void core_tmr_abort_task(
spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
cancel_work_sync(&se_cmd->work);
- transport_wait_for_tasks(se_cmd);
+ transport_wait_for_tasks(se_cmd, false, &aborted, &tas);
transport_cmd_finish_abort(se_cmd, true);
- target_put_sess_cmd(se_cmd);
+ target_tmr_put_cmd(se_cmd);
printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for"
" ref_tag: %llu\n", ref_tag);
@@ -203,7 +231,7 @@ static void core_tmr_drain_tmr_list(
struct se_tmr_req *tmr_p, *tmr_pp;
struct se_cmd *cmd;
unsigned long flags;
- bool rc;
+ bool rc, aborted = false, tas = false;
/*
* Release all pending and outgoing TMRs aside from the received
* LUN_RESET tmr..
@@ -252,8 +280,12 @@ static void core_tmr_drain_tmr_list(
spin_unlock(&sess->sess_cmd_lock);
if (!rc) {
printk("LUN_RESET TMR: non-zero kref_get_unless_zero\n");
+ spin_unlock(&sess->sess_cmd_lock);
continue;
}
+ cmd->cmd_wait_set = true;
+ spin_unlock(&sess->sess_cmd_lock);
+
list_move_tail(&tmr_p->tmr_list, &drain_tmr_list);
}
spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
@@ -268,10 +300,10 @@ static void core_tmr_drain_tmr_list(
tmr_p->function, tmr_p->response, cmd->t_state);
cancel_work_sync(&cmd->work);
- transport_wait_for_tasks(cmd);
+ transport_wait_for_tasks(cmd, false, &aborted, &tas);
transport_cmd_finish_abort(cmd, 1);
- target_put_sess_cmd(cmd);
+ target_tmr_put_cmd(cmd);
}
}
@@ -287,6 +319,7 @@ static void core_tmr_drain_state_list(
struct se_cmd *cmd, *next;
unsigned long flags;
int rc;
+ bool aborted = false, tas_set = false;
/*
* Complete outstanding commands with TASK_ABORTED SAM status.
@@ -367,10 +400,10 @@ static void core_tmr_drain_state_list(
* cancel_work_sync may block.
*/
cancel_work_sync(&cmd->work);
- transport_wait_for_tasks(cmd);
+ transport_wait_for_tasks(cmd, false, &aborted, &tas_set);
core_tmr_handle_tas_abort(tmr_sess, cmd, tas);
- target_put_sess_cmd(cmd);
+ target_tmr_put_cmd(cmd);
}
}
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 94e372a..36a70e0 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2434,15 +2434,17 @@ static void transport_write_pending_qf(struct se_cmd *cmd)
int transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks)
{
int ret = 0;
+ bool aborted = false, tas = false;
if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD)) {
if (wait_for_tasks && (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB))
- transport_wait_for_tasks(cmd);
+ transport_wait_for_tasks(cmd, true, &aborted, &tas);
- ret = transport_put_cmd(cmd);
+ if (!aborted || tas)
+ ret = transport_put_cmd(cmd);
} else {
if (wait_for_tasks)
- transport_wait_for_tasks(cmd);
+ transport_wait_for_tasks(cmd, true, &aborted, &tas);
/*
* Handle WRITE failure case where transport_generic_new_cmd()
* has already added se_cmd to state_list, but fabric has
@@ -2454,9 +2456,21 @@ int transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks)
if (cmd->se_lun)
transport_lun_remove_cmd(cmd);
- ret = transport_put_cmd(cmd);
+ if (!aborted || tas)
+ ret = transport_put_cmd(cmd);
}
- return ret;
+ /*
+ * If the task has been internally aborted due to TMR ABORT_TASK
+ * or LUN_RESET, target_core_tmr.c is responsible for performing
+ * the remaining calls to target_put_sess_cmd(), and not the
+ * callers of this function.
+ */
+ if (aborted) {
+ pr_debug("Detected CMD_T_ABORTED for ITT: %llu\n", cmd->tag);
+ wait_for_completion(&cmd->cmd_wait_comp);
+ cmd->se_tfo->release_cmd(cmd);
+ }
+ return (aborted) ? 1 : ret;
}
EXPORT_SYMBOL(transport_generic_free_cmd);
@@ -2517,7 +2531,7 @@ static void target_release_cmd_kref(struct kref *kref)
se_cmd->se_tfo->release_cmd(se_cmd);
return;
}
- if (se_sess->sess_tearing_down && se_cmd->cmd_wait_set) {
+ if (se_cmd->cmd_wait_set) {
spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
target_free_cmd_mem(se_cmd);
complete(&se_cmd->cmd_wait_comp);
@@ -2555,6 +2569,7 @@ void target_sess_cmd_list_set_waiting(struct se_session *se_sess)
{
struct se_cmd *se_cmd;
unsigned long flags;
+ int rc;
spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
if (se_sess->sess_tearing_down) {
@@ -2564,8 +2579,15 @@ void target_sess_cmd_list_set_waiting(struct se_session *se_sess)
se_sess->sess_tearing_down = 1;
list_splice_init(&se_sess->sess_cmd_list, &se_sess->sess_wait_list);
- list_for_each_entry(se_cmd, &se_sess->sess_wait_list, se_cmd_list)
- se_cmd->cmd_wait_set = 1;
+ list_for_each_entry(se_cmd, &se_sess->sess_wait_list, se_cmd_list) {
+ rc = kref_get_unless_zero(&se_cmd->cmd_kref);
+ if (rc) {
+ se_cmd->cmd_wait_set = 1;
+ spin_lock(&se_cmd->t_state_lock);
+ se_cmd->transport_state |= CMD_T_FABRIC_STOP;
+ spin_unlock(&se_cmd->t_state_lock);
+ }
+ }
spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
}
@@ -2578,6 +2600,7 @@ void target_wait_for_sess_cmds(struct se_session *se_sess)
{
struct se_cmd *se_cmd, *tmp_cmd;
unsigned long flags;
+ bool tas;
list_for_each_entry_safe(se_cmd, tmp_cmd,
&se_sess->sess_wait_list, se_cmd_list) {
@@ -2587,6 +2610,15 @@ void target_wait_for_sess_cmds(struct se_session *se_sess)
" %d\n", se_cmd, se_cmd->t_state,
se_cmd->se_tfo->get_cmd_state(se_cmd));
+ spin_lock_irqsave(&se_cmd->t_state_lock, flags);
+ tas = (se_cmd->transport_state & CMD_T_TAS);
+ spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
+
+ if (!target_put_sess_cmd(se_cmd)) {
+ if (tas)
+ target_put_sess_cmd(se_cmd);
+ }
+
wait_for_completion(&se_cmd->cmd_wait_comp);
pr_debug("After cmd_wait_comp: se_cmd: %p t_state: %d"
" fabric state: %d\n", se_cmd, se_cmd->t_state,
@@ -2611,15 +2643,33 @@ void transport_clear_lun_ref(struct se_lun *lun)
/**
* transport_wait_for_tasks - wait for completion to occur
* @cmd: command to wait
+ * @fabric_stop: set if command is being stopped + shutdown from
+ * fabric driver
+ * @aborted: set if command has been internally aborted
+ * @tas: set if command is has task aborted status
*
* Called from frontend fabric context to wait for storage engine
* to pause and/or release frontend generated struct se_cmd.
*/
-bool transport_wait_for_tasks(struct se_cmd *cmd)
+bool transport_wait_for_tasks(struct se_cmd *cmd, bool fabric_stop,
+ bool *aborted, bool *tas)
{
unsigned long flags;
spin_lock_irqsave(&cmd->t_state_lock, flags);
+ if (fabric_stop)
+ cmd->transport_state |= CMD_T_FABRIC_STOP;
+
+ if (cmd->transport_state & CMD_T_ABORTED)
+ *aborted = true;
+ else
+ *aborted = false;
+
+ if (cmd->transport_state & CMD_T_TAS)
+ *tas = true;
+ else
+ *tas = false;
+
if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD) &&
!(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)) {
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
@@ -2869,6 +2919,7 @@ void transport_send_task_abort(struct se_cmd *cmd)
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
return;
}
+ cmd->transport_state |= CMD_T_TAS;
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
/*
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 1a76726..1579539e 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -493,6 +493,8 @@ struct se_cmd {
#define CMD_T_DEV_ACTIVE (1 << 7)
#define CMD_T_REQUEST_STOP (1 << 8)
#define CMD_T_BUSY (1 << 9)
+#define CMD_T_TAS (1 << 10)
+#define CMD_T_FABRIC_STOP (1 << 11)
spinlock_t t_state_lock;
struct kref cmd_kref;
struct completion t_transport_stop_comp;
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 5665340..240ea10 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -148,7 +148,7 @@ void target_execute_cmd(struct se_cmd *cmd);
int transport_generic_free_cmd(struct se_cmd *, int);
-bool transport_wait_for_tasks(struct se_cmd *);
+bool transport_wait_for_tasks(struct se_cmd *, bool, bool *, bool *);
int transport_check_aborted_status(struct se_cmd *, int);
int transport_send_check_condition_and_sense(struct se_cmd *,
sense_reason_t, int);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
