From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: target-devel <target-devel@vger.kernel.org>,
linux-scsi <linux-scsi@vger.kernel.org>
Cc: Roland Dreier <roland@purestorage.com>,
Nicholas Bellinger <nab@linux-iscsi.org>,
Christoph Hellwig <hch@lst.de>
Subject: [PATCH 2/4] target: Re-org of core_tmr_lun_reset + FREE_CMD_INTR bugfix
Date: Fri, 30 Sep 2011 04:55:09 +0000 [thread overview]
Message-ID: <1317358511-10664-3-git-send-email-nab@linux-iscsi.org> (raw)
In-Reply-To: <1317358511-10664-1-git-send-email-nab@linux-iscsi.org>
From: Nicholas Bellinger <nab@linux-iscsi.org>
This patch is a re-orginzation of core_tmr_lun_reset() logic to properly
scan the active tmr_list, dev->state_task_list and qobj->qobj_list w/ the
relivent locks held, and performing a list_move_tail onto seperate local
scope lists before performing the full drain.
This involves breaking out the code into three seperate list specific
functions: core_tmr_drain_tmr_list(), core_tmr_drain_task_list() and
core_tmr_drain_cmd_list().
This patch also includes a bugfix related to TRANSPORT_FREE_CMD_INTR
operation, where core_tmr_drain_cmd_list will skip this cause in order
to prevent an ABORT_TASK status from being returned for descriptors that
are already queued up to be related by processing thread context.
Reported-by: Roland Dreier <roland@purestorage.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@risingtidesystems.com>
---
drivers/target/target_core_tmr.c | 226 ++++++++++++++++++++++++--------------
1 files changed, 141 insertions(+), 85 deletions(-)
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 7bce92f..a536f8a 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -67,15 +67,16 @@ void core_tmr_release_req(
struct se_tmr_req *tmr)
{
struct se_device *dev = tmr->tmr_dev;
+ unsigned long flags;
if (!dev) {
kmem_cache_free(se_tmr_req_cache, tmr);
return;
}
- spin_lock_irq(&dev->se_tmr_lock);
+ spin_lock_irqsave(&dev->se_tmr_lock, flags);
list_del(&tmr->tmr_list);
- spin_unlock_irq(&dev->se_tmr_lock);
+ spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
kmem_cache_free(se_tmr_req_cache, tmr);
}
@@ -100,54 +101,20 @@ static void core_tmr_handle_tas_abort(
transport_cmd_finish_abort(cmd, 0);
}
-int core_tmr_lun_reset(
+static void core_tmr_drain_tmr_list(
struct se_device *dev,
struct se_tmr_req *tmr,
- struct list_head *preempt_and_abort_list,
- struct se_cmd *prout_cmd)
+ struct list_head *preempt_and_abort_list)
{
- struct se_cmd *cmd, *tcmd;
- struct se_node_acl *tmr_nacl = NULL;
- struct se_portal_group *tmr_tpg = NULL;
- struct se_queue_obj *qobj = &dev->dev_queue_obj;
+ LIST_HEAD(drain_tmr_list);
struct se_tmr_req *tmr_p, *tmr_pp;
- struct se_task *task, *task_tmp;
+ struct se_cmd *cmd;
unsigned long flags;
- int fe_count, tas;
- /*
- * TASK_ABORTED status bit, this is configurable via ConfigFS
- * struct se_device attributes. spc4r17 section 7.4.6 Control mode page
- *
- * A task aborted status (TAS) bit set to zero specifies that aborted
- * tasks shall be terminated by the device server without any response
- * to the application client. A TAS bit set to one specifies that tasks
- * aborted by the actions of an I_T nexus other than the I_T nexus on
- * which the command was received shall be completed with TASK ABORTED
- * status (see SAM-4).
- */
- tas = dev->se_sub_dev->se_dev_attrib.emulate_tas;
- /*
- * Determine if this se_tmr is coming from a $FABRIC_MOD
- * or struct se_device passthrough..
- */
- if (tmr && tmr->task_cmd && tmr->task_cmd->se_sess) {
- tmr_nacl = tmr->task_cmd->se_sess->se_node_acl;
- tmr_tpg = tmr->task_cmd->se_sess->se_tpg;
- if (tmr_nacl && tmr_tpg) {
- pr_debug("LUN_RESET: TMR caller fabric: %s"
- " initiator port %s\n",
- tmr_tpg->se_tpg_tfo->get_fabric_name(),
- tmr_nacl->initiatorname);
- }
- }
- pr_debug("LUN_RESET: %s starting for [%s], tas: %d\n",
- (preempt_and_abort_list) ? "Preempt" : "TMR",
- dev->transport->name, tas);
/*
* Release all pending and outgoing TMRs aside from the received
* LUN_RESET tmr..
*/
- spin_lock_irq(&dev->se_tmr_lock);
+ spin_lock_irqsave(&dev->se_tmr_lock, flags);
list_for_each_entry_safe(tmr_p, tmr_pp, &dev->dev_tmr_list, tmr_list) {
/*
* Allow the received TMR to return with FUNCTION_COMPLETE.
@@ -169,29 +136,48 @@ int core_tmr_lun_reset(
(core_scsi3_check_cdb_abort_and_preempt(
preempt_and_abort_list, cmd) != 0))
continue;
- spin_unlock_irq(&dev->se_tmr_lock);
-
- spin_lock_irqsave(&cmd->t_state_lock, flags);
+
+ spin_lock(&cmd->t_state_lock);
if (!atomic_read(&cmd->t_transport_active)) {
- spin_unlock_irqrestore(&cmd->t_state_lock, flags);
- spin_lock_irq(&dev->se_tmr_lock);
+ spin_unlock(&cmd->t_state_lock);
continue;
}
if (cmd->t_state == TRANSPORT_ISTATE_PROCESSING) {
- spin_unlock_irqrestore(&cmd->t_state_lock, flags);
- spin_lock_irq(&dev->se_tmr_lock);
+ spin_unlock(&cmd->t_state_lock);
continue;
}
- pr_debug("LUN_RESET: %s releasing TMR %p Function: 0x%02x,"
- " Response: 0x%02x, t_state: %d\n",
- (preempt_and_abort_list) ? "Preempt" : "", tmr_p,
- tmr_p->function, tmr_p->response, cmd->t_state);
- spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+ spin_unlock(&cmd->t_state_lock);
+
+ list_move_tail(&tmr->tmr_list, &drain_tmr_list);
+ }
+ spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
+ while (!list_empty(&drain_tmr_list)) {
+ tmr = list_entry(drain_tmr_list.next, struct se_tmr_req, tmr_list);
+ list_del(&tmr->tmr_list);
+ cmd = tmr_p->task_cmd;
+
+ printk("LUN_RESET: %s releasing TMR %p Function: 0x%02x,"
+ " Response: 0x%02x, t_state: %d\n",
+ (preempt_and_abort_list) ? "Preempt" : "", tmr,
+ tmr->function, tmr->response, cmd->t_state);
+
transport_cmd_finish_abort_tmr(cmd);
- spin_lock_irq(&dev->se_tmr_lock);
}
- spin_unlock_irq(&dev->se_tmr_lock);
+}
+
+static void core_tmr_drain_task_list(
+ struct se_device *dev,
+ struct se_cmd *prout_cmd,
+ struct se_node_acl *tmr_nacl,
+ int tas,
+ struct list_head *preempt_and_abort_list)
+{
+ LIST_HEAD(drain_task_list);
+ struct se_cmd *cmd;
+ struct se_task *task, *task_tmp;
+ unsigned long flags;
+ int fe_count;
/*
* Complete outstanding struct se_task CDBs with TASK_ABORTED SAM status.
* This is following sam4r17, section 5.6 Aborting commands, Table 38
@@ -236,19 +222,26 @@ int core_tmr_lun_reset(
if (prout_cmd == cmd)
continue;
- list_del(&task->t_state_list);
+ list_move_tail(&task->t_state_list, &drain_task_list);
atomic_set(&task->task_state_active, 0);
- spin_unlock_irqrestore(&dev->execute_task_lock, flags);
+ atomic_set(&task->task_execute_queue, 0);
+ }
+ spin_unlock_irqrestore(&dev->execute_task_lock, flags);
+
+ while (!list_empty(&drain_task_list)) {
+ task = list_entry(drain_task_list.next, struct se_task, t_state_list);
+ list_del(&task->t_state_list);
+ cmd = task->task_se_cmd;
spin_lock_irqsave(&cmd->t_state_lock, flags);
- pr_debug("LUN_RESET: %s cmd: %p task: %p"
+ printk("LUN_RESET: %s cmd: %p task: %p"
" ITT/CmdSN: 0x%08x/0x%08x, i_state: %d, t_state/"
"def_t_state: %d/%d cdb: 0x%02x\n",
(preempt_and_abort_list) ? "Preempt" : "", cmd, task,
cmd->se_tfo->get_task_tag(cmd), 0,
cmd->se_tfo->get_cmd_state(cmd), cmd->t_state,
cmd->deferred_t_state, cmd->t_task_cdb[0]);
- pr_debug("LUN_RESET: ITT[0x%08x] - pr_res_key: 0x%016Lx"
+ printk("LUN_RESET: ITT[0x%08x] - pr_res_key: 0x%016Lx"
" t_task_cdbs: %d t_task_cdbs_left: %d"
" t_task_cdbs_sent: %d -- t_transport_active: %d"
" t_transport_stop: %d t_transport_sent: %d\n",
@@ -265,55 +258,58 @@ int core_tmr_lun_reset(
spin_unlock_irqrestore(
&cmd->t_state_lock, flags);
- pr_debug("LUN_RESET: Waiting for task: %p to shutdown"
+ printk("LUN_RESET: Waiting for task: %p to shutdown"
" for dev: %p\n", task, dev);
wait_for_completion(&task->task_stop_comp);
- pr_debug("LUN_RESET Completed task: %p shutdown for"
+ printk("LUN_RESET Completed task: %p shutdown for"
" dev: %p\n", task, dev);
spin_lock_irqsave(&cmd->t_state_lock, flags);
atomic_dec(&cmd->t_task_cdbs_left);
atomic_set(&task->task_active, 0);
atomic_set(&task->task_stop, 0);
- } else {
- if (atomic_read(&task->task_execute_queue) != 0)
- transport_remove_task_from_execute_queue(task, dev);
}
__transport_stop_task_timer(task, &flags);
if (!atomic_dec_and_test(&cmd->t_task_cdbs_ex_left)) {
- spin_unlock_irqrestore(
- &cmd->t_state_lock, flags);
- pr_debug("LUN_RESET: Skipping task: %p, dev: %p for"
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+ printk("LUN_RESET: Skipping task: %p, dev: %p for"
" t_task_cdbs_ex_left: %d\n", task, dev,
atomic_read(&cmd->t_task_cdbs_ex_left));
-
- spin_lock_irqsave(&dev->execute_task_lock, flags);
continue;
}
fe_count = atomic_read(&cmd->t_fe_count);
if (atomic_read(&cmd->t_transport_active)) {
- pr_debug("LUN_RESET: got t_transport_active = 1 for"
+ printk("LUN_RESET: got t_transport_active = 1 for"
" task: %p, t_fe_count: %d dev: %p\n", task,
fe_count, dev);
atomic_set(&cmd->t_transport_aborted, 1);
- spin_unlock_irqrestore(&cmd->t_state_lock,
- flags);
- core_tmr_handle_tas_abort(tmr_nacl, cmd, tas, fe_count);
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
- spin_lock_irqsave(&dev->execute_task_lock, flags);
+ core_tmr_handle_tas_abort(tmr_nacl, cmd, tas, fe_count);
continue;
}
- pr_debug("LUN_RESET: Got t_transport_active = 0 for task: %p,"
+ printk("LUN_RESET: Got t_transport_active = 0 for task: %p,"
" t_fe_count: %d dev: %p\n", task, fe_count, dev);
atomic_set(&cmd->t_transport_aborted, 1);
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
- core_tmr_handle_tas_abort(tmr_nacl, cmd, tas, fe_count);
- spin_lock_irqsave(&dev->execute_task_lock, flags);
+ core_tmr_handle_tas_abort(tmr_nacl, cmd, tas, fe_count);
}
- spin_unlock_irqrestore(&dev->execute_task_lock, flags);
+}
+
+static void core_tmr_drain_cmd_list(
+ struct se_device *dev,
+ struct se_cmd *prout_cmd,
+ struct se_node_acl *tmr_nacl,
+ int tas,
+ struct list_head *preempt_and_abort_list)
+{
+ LIST_HEAD(drain_cmd_list);
+ struct se_queue_obj *qobj = &dev->dev_queue_obj;
+ struct se_cmd *cmd, *tcmd;
+ unsigned long flags;
/*
* Release all commands remaining in the struct se_device cmd queue.
*
@@ -337,13 +333,28 @@ int core_tmr_lun_reset(
*/
if (prout_cmd == cmd)
continue;
+ /*
+ * Skip direct processing of TRANSPORT_FREE_CMD_INTR for
+ * HW target mode fabrics.
+ */
+ spin_lock(&cmd->t_state_lock);
+ if (cmd->t_state == TRANSPORT_FREE_CMD_INTR) {
+ spin_unlock(&cmd->t_state_lock);
+ continue;
+ }
+ spin_unlock(&cmd->t_state_lock);
- atomic_dec(&cmd->t_transport_queue_active);
+ atomic_set(&cmd->t_transport_queue_active, 0);
atomic_dec(&qobj->queue_cnt);
+ list_move_tail(&cmd->se_queue_node, &drain_cmd_list);
+ }
+ spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
+
+ while (!list_empty(&drain_cmd_list)) {
+ cmd = list_entry(drain_cmd_list.next, struct se_cmd, se_queue_node);
list_del_init(&cmd->se_queue_node);
- spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
- pr_debug("LUN_RESET: %s from Device Queue: cmd: %p t_state:"
+ printk("LUN_RESET: %s from Device Queue: cmd: %p t_state:"
" %d t_fe_count: %d\n", (preempt_and_abort_list) ?
"Preempt" : "", cmd, cmd->t_state,
atomic_read(&cmd->t_fe_count));
@@ -354,9 +365,53 @@ int core_tmr_lun_reset(
core_tmr_handle_tas_abort(tmr_nacl, cmd, tas,
atomic_read(&cmd->t_fe_count));
- spin_lock_irqsave(&qobj->cmd_queue_lock, flags);
}
- spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags);
+}
+
+int core_tmr_lun_reset(
+ struct se_device *dev,
+ struct se_tmr_req *tmr,
+ struct list_head *preempt_and_abort_list,
+ struct se_cmd *prout_cmd)
+{
+ struct se_node_acl *tmr_nacl = NULL;
+ struct se_portal_group *tmr_tpg = NULL;
+ int tas;
+ /*
+ * TASK_ABORTED status bit, this is configurable via ConfigFS
+ * struct se_device attributes. spc4r17 section 7.4.6 Control mode page
+ *
+ * A task aborted status (TAS) bit set to zero specifies that aborted
+ * tasks shall be terminated by the device server without any response
+ * to the application client. A TAS bit set to one specifies that tasks
+ * aborted by the actions of an I_T nexus other than the I_T nexus on
+ * which the command was received shall be completed with TASK ABORTED
+ * status (see SAM-4).
+ */
+ tas = dev->se_sub_dev->se_dev_attrib.emulate_tas;
+ /*
+ * Determine if this se_tmr is coming from a $FABRIC_MOD
+ * or struct se_device passthrough..
+ */
+ if (tmr && tmr->task_cmd && tmr->task_cmd->se_sess) {
+ tmr_nacl = tmr->task_cmd->se_sess->se_node_acl;
+ tmr_tpg = tmr->task_cmd->se_sess->se_tpg;
+ if (tmr_nacl && tmr_tpg) {
+ printk("LUN_RESET: TMR caller fabric: %s"
+ " initiator port %s\n",
+ tmr_tpg->se_tpg_tfo->get_fabric_name(),
+ tmr_nacl->initiatorname);
+ }
+ }
+ printk("LUN_RESET: %s starting for [%s], tas: %d\n",
+ (preempt_and_abort_list) ? "Preempt" : "TMR",
+ dev->transport->name, tas);
+
+ core_tmr_drain_tmr_list(dev, tmr, preempt_and_abort_list);
+ core_tmr_drain_task_list(dev, prout_cmd, tmr_nacl, tas,
+ preempt_and_abort_list);
+ core_tmr_drain_cmd_list(dev, prout_cmd, tmr_nacl, tas,
+ preempt_and_abort_list);
/*
* Clear any legacy SPC-2 reservation when called during
* LOGICAL UNIT RESET
@@ -367,15 +422,16 @@ int core_tmr_lun_reset(
dev->dev_reserved_node_acl = NULL;
dev->dev_flags &= ~DF_SPC2_RESERVATIONS;
spin_unlock(&dev->dev_reservation_lock);
- pr_debug("LUN_RESET: SCSI-2 Released reservation\n");
+ printk("LUN_RESET: SCSI-2 Released reservation\n");
}
spin_lock_irq(&dev->stats_lock);
dev->num_resets++;
spin_unlock_irq(&dev->stats_lock);
- pr_debug("LUN_RESET: %s for [%s] Complete\n",
+ printk("LUN_RESET: %s for [%s] Complete\n",
(preempt_and_abort_list) ? "Preempt" : "TMR",
dev->transport->name);
return 0;
}
+
--
1.7.2.5
next prev parent reply other threads:[~2011-09-30 4:55 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-30 4:55 [PATCH 0/4] target: LUN_RESET bugfixes for HW target mode Nicholas A. Bellinger
2011-09-30 4:55 ` [PATCH 1/4] target: Prevent cmd->se_queue_node double add Nicholas A. Bellinger
2011-10-09 1:24 ` Christoph Hellwig
2011-09-30 4:55 ` Nicholas A. Bellinger [this message]
2011-10-09 1:30 ` [PATCH 2/4] target: Re-org of core_tmr_lun_reset + FREE_CMD_INTR bugfix Christoph Hellwig
2011-10-09 2:00 ` Nicholas A. Bellinger
2011-09-30 4:55 ` [PATCH 3/4] target: Fix transport_cmd_finish_abort queue removal bug Nicholas A. Bellinger
2011-10-09 1:33 ` Christoph Hellwig
2011-10-09 2:03 ` Nicholas A. Bellinger
2011-10-09 2:27 ` Christoph Hellwig
2011-10-09 9:06 ` Nicholas A. Bellinger
2011-09-30 4:55 ` [PATCH 4/4] target: Prevent transport_send_task_abort when CHECK_CONDITION status Nicholas A. Bellinger
2011-09-30 5:25 ` [PATCH 0/4] target: LUN_RESET bugfixes for HW target mode Nicholas A. Bellinger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1317358511-10664-3-git-send-email-nab@linux-iscsi.org \
--to=nab@linux-iscsi.org \
--cc=hch@lst.de \
--cc=linux-scsi@vger.kernel.org \
--cc=roland@purestorage.com \
--cc=target-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.