From 9375b1bfd2555c8bc828d394a4419a212b46ba71 Mon Sep 17 00:00:00 2001 From: Jesper Juhl Date: Mon, 1 Aug 2011 23:29:11 +0200 Subject: target: Remove unneeded version.h includes It was pointed out by 'make versioncheck' that some includes of linux/version.h are not needed in drivers/target/. This patch removes them. Signed-off-by: Jesper Juhl Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_alua.c | 1 - drivers/target/target_core_configfs.c | 1 - drivers/target/target_core_fabric_configfs.c | 1 - drivers/target/target_core_file.c | 1 - drivers/target/target_core_iblock.c | 1 - drivers/target/target_core_pr.c | 1 - drivers/target/target_core_pscsi.c | 1 - drivers/target/target_core_rd.c | 1 - drivers/target/target_core_stat.c | 1 - drivers/target/target_core_tmr.c | 1 - drivers/target/target_core_transport.c | 1 - drivers/target/target_core_ua.c | 1 - drivers/target/tcm_fc/tfc_cmd.c | 1 - drivers/target/tcm_fc/tfc_conf.c | 1 - drivers/target/tcm_fc/tfc_io.c | 1 - drivers/target/tcm_fc/tfc_sess.c | 1 - 16 files changed, 16 deletions(-) diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c index 98c98a3a0250..007c6c298b8b 100644 --- a/drivers/target/target_core_alua.c +++ b/drivers/target/target_core_alua.c @@ -24,7 +24,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index b2575d8568cc..f37e2b9cbbd7 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -23,7 +23,6 @@ #include #include -#include #include #include #include diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c index 55bbe0847a6d..09b6f8729f91 100644 --- a/drivers/target/target_core_fabric_configfs.c +++ b/drivers/target/target_core_fabric_configfs.c @@ -22,7 +22,6 @@ #include #include -#include #include #include #include diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index bc1b33639b8d..99c9db003394 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -26,7 +26,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index 7e1234105442..7f0cc53b4581 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -27,7 +27,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index 7fd3a161f7cc..0c4f783f924c 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -25,7 +25,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c index 2b7b0da9146d..77d725886410 100644 --- a/drivers/target/target_core_pscsi.c +++ b/drivers/target/target_core_pscsi.c @@ -26,7 +26,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c index e567e129c697..1ab69f32e664 100644 --- a/drivers/target/target_core_rd.c +++ b/drivers/target/target_core_rd.c @@ -27,7 +27,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c index a8d6e1dee938..874152aed94a 100644 --- a/drivers/target/target_core_stat.c +++ b/drivers/target/target_core_stat.c @@ -32,7 +32,6 @@ #include #include #include -#include #include #include #include diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 27d4925e51c3..662473c54043 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -24,7 +24,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index a4b0a8d27f25..4b2d6bf87a12 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -26,7 +26,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/target_core_ua.c b/drivers/target/target_core_ua.c index 31e3c652527e..50a480db7a66 100644 --- a/drivers/target/target_core_ua.c +++ b/drivers/target/target_core_ua.c @@ -24,7 +24,6 @@ * ******************************************************************************/ -#include #include #include #include diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c index 80fbcde00cb6..3c6c5a1c7dd0 100644 --- a/drivers/target/tcm_fc/tfc_cmd.c +++ b/drivers/target/tcm_fc/tfc_cmd.c @@ -19,7 +19,6 @@ #include #include -#include #include #include #include diff --git a/drivers/target/tcm_fc/tfc_conf.c b/drivers/target/tcm_fc/tfc_conf.c index 8fa39b74f22c..b30eace70d35 100644 --- a/drivers/target/tcm_fc/tfc_conf.c +++ b/drivers/target/tcm_fc/tfc_conf.c @@ -23,7 +23,6 @@ #include #include -#include #include #include #include diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c index d35ea5a3d56c..1369b1cb103d 100644 --- a/drivers/target/tcm_fc/tfc_io.c +++ b/drivers/target/tcm_fc/tfc_io.c @@ -28,7 +28,6 @@ #include #include -#include #include #include #include diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c index dbb5eaeee399..326921385aff 100644 --- a/drivers/target/tcm_fc/tfc_sess.c +++ b/drivers/target/tcm_fc/tfc_sess.c @@ -19,7 +19,6 @@ #include #include -#include #include #include #include -- cgit v1.2.3 From 79a7fef26431830e22e282053d050af790117db8 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Wed, 28 Sep 2011 22:12:07 -0700 Subject: target: Prevent cmd->se_queue_node double add This patch addresses a bug with the lio-core-2.6.git conversion of transport_add_cmd_to_queue() to use a single embedded list_head, instead of individual struct se_queue_req allocations allowing a single se_cmd to be added to the queue mulitple times. This was changed in the following: commit 2a9e4d5ca5d99f4c600578d6285d45142e7e5208 Author: Andy Grover Date: Tue Apr 26 17:45:51 2011 -0700 target: Embed qr in struct se_cmd The problem is that some target code still assumes performing multiple adds is allowed via transport_add_cmd_to_queue(), which ends up causing list corruption in qobj->qobj_list code. This patch addresses this by removing an existing struct se_cmd from the list before the add, and removes an unnecessary list walk in transport_remove_cmd_from_queue() It also changes cmd->t_transport_queue_active to use explict sets intead of increment/decrement to prevent confusion during exception path handling. Signed-off-by: Roland Dreier Cc: Andy Grover Cc: stable@kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_tmr.c | 2 +- drivers/target/target_core_transport.c | 31 +++++++++++++++---------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 662473c54043..98b12a8923f0 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -339,7 +339,7 @@ int core_tmr_lun_reset( atomic_dec(&cmd->t_transport_queue_active); atomic_dec(&qobj->queue_cnt); - list_del(&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:" diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 4b2d6bf87a12..046da7f38230 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -620,8 +620,6 @@ static void transport_add_cmd_to_queue( struct se_queue_obj *qobj = &dev->dev_queue_obj; unsigned long flags; - INIT_LIST_HEAD(&cmd->se_queue_node); - if (t_state) { spin_lock_irqsave(&cmd->t_state_lock, flags); cmd->t_state = t_state; @@ -630,15 +628,21 @@ static void transport_add_cmd_to_queue( } spin_lock_irqsave(&qobj->cmd_queue_lock, flags); + + /* If the cmd is already on the list, remove it before we add it */ + if (!list_empty(&cmd->se_queue_node)) + list_del(&cmd->se_queue_node); + else + atomic_inc(&qobj->queue_cnt); + if (cmd->se_cmd_flags & SCF_EMULATE_QUEUE_FULL) { cmd->se_cmd_flags &= ~SCF_EMULATE_QUEUE_FULL; list_add(&cmd->se_queue_node, &qobj->qobj_list); } else list_add_tail(&cmd->se_queue_node, &qobj->qobj_list); - atomic_inc(&cmd->t_transport_queue_active); + atomic_set(&cmd->t_transport_queue_active, 1); spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags); - atomic_inc(&qobj->queue_cnt); wake_up_interruptible(&qobj->thread_wq); } @@ -655,9 +659,9 @@ transport_get_cmd_from_queue(struct se_queue_obj *qobj) } cmd = list_first_entry(&qobj->qobj_list, struct se_cmd, se_queue_node); - atomic_dec(&cmd->t_transport_queue_active); + atomic_set(&cmd->t_transport_queue_active, 0); - list_del(&cmd->se_queue_node); + list_del_init(&cmd->se_queue_node); atomic_dec(&qobj->queue_cnt); spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags); @@ -667,7 +671,6 @@ transport_get_cmd_from_queue(struct se_queue_obj *qobj) static void transport_remove_cmd_from_queue(struct se_cmd *cmd, struct se_queue_obj *qobj) { - struct se_cmd *t; unsigned long flags; spin_lock_irqsave(&qobj->cmd_queue_lock, flags); @@ -675,14 +678,9 @@ static void transport_remove_cmd_from_queue(struct se_cmd *cmd, spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags); return; } - - list_for_each_entry(t, &qobj->qobj_list, se_queue_node) - if (t == cmd) { - atomic_dec(&cmd->t_transport_queue_active); - atomic_dec(&qobj->queue_cnt); - list_del(&cmd->se_queue_node); - break; - } + atomic_set(&cmd->t_transport_queue_active, 0); + atomic_dec(&qobj->queue_cnt); + list_del_init(&cmd->se_queue_node); spin_unlock_irqrestore(&qobj->cmd_queue_lock, flags); if (atomic_read(&cmd->t_transport_queue_active)) { @@ -1066,7 +1064,7 @@ static void transport_release_all_cmds(struct se_device *dev) list_for_each_entry_safe(cmd, tcmd, &dev->dev_queue_obj.qobj_list, se_queue_node) { t_state = cmd->t_state; - list_del(&cmd->se_queue_node); + list_del_init(&cmd->se_queue_node); spin_unlock_irqrestore(&dev->dev_queue_obj.cmd_queue_lock, flags); @@ -1597,6 +1595,7 @@ void transport_init_se_cmd( INIT_LIST_HEAD(&cmd->se_delayed_node); INIT_LIST_HEAD(&cmd->se_ordered_node); INIT_LIST_HEAD(&cmd->se_qf_node); + INIT_LIST_HEAD(&cmd->se_queue_node); INIT_LIST_HEAD(&cmd->t_task_list); init_completion(&cmd->transport_lun_fe_stop_comp); -- cgit v1.2.3 From d050ffb922c782f092234611b9019e95024481ab Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Wed, 28 Sep 2011 21:37:29 -0700 Subject: target: Re-org of core_tmr_lun_reset 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(). (nab: Include target: Remove non-active tasks from execute list during LUN_RESET patch to address original breakage) Reported-by: Roland Dreier Cc: Roland Dreier Cc: Christoph Hellwig Cc: stable@kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_tmr.c | 197 +++++++++++++++++++++++++-------------- 1 file changed, 125 insertions(+), 72 deletions(-) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 98b12a8923f0..ed0b1ff99110 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -66,15 +66,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); } @@ -99,54 +100,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. @@ -168,29 +135,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; } + 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; + 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); + (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 @@ -235,9 +221,23 @@ 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); + /* + * Remove from task execute list before processing drain_task_list + */ + if (atomic_read(&task->task_execute_queue) != 0) { + list_del(&task->t_execute_list); + atomic_set(&task->task_execute_queue, 0); + atomic_dec(&dev->execute_tasks); + } + } + 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" @@ -274,20 +274,14 @@ int core_tmr_lun_reset( 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); + spin_unlock_irqrestore(&cmd->t_state_lock, flags); pr_debug("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); @@ -297,22 +291,31 @@ int core_tmr_lun_reset( " 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," " 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,10 +340,15 @@ int core_tmr_lun_reset( if (prout_cmd == cmd) continue; - 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:" " %d t_fe_count: %d\n", (preempt_and_abort_list) ? @@ -353,9 +361,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) { + 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); + + 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 @@ -378,3 +430,4 @@ int core_tmr_lun_reset( dev->transport->name); return 0; } + -- cgit v1.2.3 From b0e062aec578c756d1aea4b5809294488366a6e8 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Tue, 11 Oct 2011 06:02:48 +0000 Subject: target: Prevent TRANSPORT_FREE_CMD_INTR processing in core_tmr_drain_cmd_list This patch contains a bugfix for TMR LUN_RESET related to TRANSPORT_FREE_CMD_INTR operation, where core_tmr_drain_cmd_list() will now skip processing for this case to prevent an ABORT_TASK status from being returned for descriptors that are already queued up to be released by processing thread context. Cc: Roland Dreier Cc: Christoph Hellwig Cc: stable@kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_tmr.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index ed0b1ff99110..d04cc1016ebf 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -339,6 +339,16 @@ static void core_tmr_drain_cmd_list( */ 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_set(&cmd->t_transport_queue_active, 0); atomic_dec(&qobj->queue_cnt); -- cgit v1.2.3 From 77039d1eafbbc192df71ee84b157b8973766737d Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Thu, 29 Sep 2011 01:01:35 -0700 Subject: target: Fix transport_cmd_finish_abort queue removal bug This patch fixes a bug in LUN_RESET operation with transport_cmd_finish_abort() where transport_remove_cmd_from_queue() was incorrectly being called, causing descriptors with t_state == TRANSPORT_FREE_CMD_INTR to be incorrectly removed from qobj->qobj_list during process context release. This change ensures the descriptor is only removed via transport_remove_cmd_from_queue() when doing a direct release via transport_generic_remove(). Cc: stable@kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 046da7f38230..009547b35574 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -593,13 +593,14 @@ check_lun: void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) { - transport_remove_cmd_from_queue(cmd, &cmd->se_dev->dev_queue_obj); transport_lun_remove_cmd(cmd); if (transport_cmd_check_stop_to_fabric(cmd)) return; - if (remove) + if (remove) { + transport_remove_cmd_from_queue(cmd, &cmd->se_dev->dev_queue_obj); transport_generic_remove(cmd, 0); + } } void transport_cmd_finish_abort_tmr(struct se_cmd *cmd) -- cgit v1.2.3 From c252f003470a99d319db4ebd12f4a9e4710a65db Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Thu, 29 Sep 2011 14:22:13 -0700 Subject: target: Prevent transport_send_task_abort when CHECK_CONDITION status This patch fixes a bug where transport_send_task_abort() could be called during LUN_RESET to return SAM_STAT_TASK_ABORTED + tfo->queue_status(), when SCF_SENT_CHECK_CONDITION -> tfo->queue_status() has already been sent from within another context via transport_send_check_condition_and_sense(). Cc: stable@kernel.org Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 009547b35574..af1e0a5f9bc6 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -4919,6 +4919,15 @@ EXPORT_SYMBOL(transport_check_aborted_status); void transport_send_task_abort(struct se_cmd *cmd) { + unsigned long flags; + + spin_lock_irqsave(&cmd->t_state_lock, flags); + if (cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION) { + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + return; + } + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + /* * If there are still expected incoming fabric WRITEs, we wait * until until they have completed before sending a TASK_ABORTED -- cgit v1.2.3 From 4ca495e0630b6fd960f94f89a9f13ad32b91bc96 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 12 Sep 2011 21:50:49 +0200 Subject: tfm_fc: use transport_handle_cdb_direct ft_send_work is always called from workqueue context, which means we can handle the CDB directly instead of doing another context switch. Signed-off-by: Christoph Hellwig Cc: Kiran Patil Signed-off-by: Nicholas Bellinger --- drivers/target/tcm_fc/tfc_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c index 3c6c5a1c7dd0..c2d148d80cf7 100644 --- a/drivers/target/tcm_fc/tfc_cmd.c +++ b/drivers/target/tcm_fc/tfc_cmd.c @@ -639,7 +639,7 @@ static void ft_send_work(struct work_struct *work) transport_generic_free_cmd(se_cmd, 0, 0); return; } - transport_generic_handle_cdb(se_cmd); + transport_handle_cdb_direct(se_cmd); return; err: -- cgit v1.2.3 From acf3ecc4a1c7460662757c07ee1ec625760d3ae6 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 12 Sep 2011 21:50:56 +0200 Subject: iscsi-target: always call transport_handle_cdb_direct iscsit_task_reassign_complete is always called from the TX thread, so handle the CDB directly instead of offloading it. Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_tmr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/target/iscsi/iscsi_target_tmr.c b/drivers/target/iscsi/iscsi_target_tmr.c index db1fe1ec84df..c3617d8ff3ea 100644 --- a/drivers/target/iscsi/iscsi_target_tmr.c +++ b/drivers/target/iscsi/iscsi_target_tmr.c @@ -318,7 +318,7 @@ static int iscsit_task_reassign_complete_read( pr_debug("READ ITT: 0x%08x: t_state: %d never sent to" " transport\n", cmd->init_task_tag, cmd->se_cmd.t_state); - transport_generic_handle_cdb(se_cmd); + transport_handle_cdb_direct(se_cmd); return 0; } -- cgit v1.2.3 From 680b73c5f2fb60336707b53b2b2792d2c01b69dc Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 12 Sep 2011 21:51:14 +0200 Subject: target: remove transport_generic_handle_cdb Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 23 +++-------------------- include/target/target_core_transport.h | 1 - 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index af1e0a5f9bc6..f0f21bb28400 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1728,24 +1728,6 @@ int transport_generic_allocate_tasks( } EXPORT_SYMBOL(transport_generic_allocate_tasks); -/* - * Used by fabric module frontends not defining a TFO->new_cmd_map() - * to queue up a newly setup se_cmd w/ TRANSPORT_NEW_CMD statis - */ -int transport_generic_handle_cdb( - struct se_cmd *cmd) -{ - if (!cmd->se_lun) { - dump_stack(); - pr_err("cmd->se_lun is NULL\n"); - return -EINVAL; - } - - transport_add_cmd_to_queue(cmd, TRANSPORT_NEW_CMD); - return 0; -} -EXPORT_SYMBOL(transport_generic_handle_cdb); - static void transport_generic_request_failure(struct se_cmd *, struct se_device *, int, int); /* @@ -5205,6 +5187,9 @@ get_cmd: continue; switch (cmd->t_state) { + case TRANSPORT_NEW_CMD: + BUG(); + break; case TRANSPORT_NEW_CMD_MAP: if (!cmd->se_tfo->new_cmd_map) { pr_err("cmd->se_tfo->new_cmd_map is" @@ -5219,8 +5204,6 @@ get_cmd: DMA_TO_DEVICE)); break; } - /* Fall through */ - case TRANSPORT_NEW_CMD: ret = transport_generic_new_cmd(cmd); if (ret == -EAGAIN) break; diff --git a/include/target/target_core_transport.h b/include/target/target_core_transport.h index 46aae4f94ede..0482a28629ff 100644 --- a/include/target/target_core_transport.h +++ b/include/target/target_core_transport.h @@ -171,7 +171,6 @@ void *transport_kmap_first_data_page(struct se_cmd *cmd); void transport_kunmap_first_data_page(struct se_cmd *cmd); extern void transport_free_se_cmd(struct se_cmd *); extern int transport_generic_allocate_tasks(struct se_cmd *, unsigned char *); -extern int transport_generic_handle_cdb(struct se_cmd *); extern int transport_handle_cdb_direct(struct se_cmd *); extern int transport_generic_handle_cdb_map(struct se_cmd *); extern int transport_generic_handle_data(struct se_cmd *); -- cgit v1.2.3 From 31afc39c0c93edec5a117371f1bd2a264cceafac Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Sep 2011 23:08:11 +0200 Subject: target: don't opencode transport_release_cmd in transport_release_fe_cmd Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index f0f21bb28400..0c1d004548b1 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3794,8 +3794,7 @@ static void transport_release_fe_cmd(struct se_cmd *cmd) transport_release_tasks(cmd); free_pages: transport_free_pages(cmd); - transport_free_se_cmd(cmd); - cmd->se_tfo->release_cmd(cmd); + transport_release_cmd(cmd); } static int -- cgit v1.2.3 From 2dbc43d256c5371ebc294e3534620663eb80a5ce Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Sep 2011 23:08:19 +0200 Subject: target: remove transport_free_se_cmd It is only called by transport_release_cmd, so inline it there. Also add a kerneldoc comment for transport_release_cmd while we are at it. Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 25 +++++++++++-------------- include/target/target_core_transport.h | 1 - 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 0c1d004548b1..49df0971a37f 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1640,19 +1640,6 @@ static int transport_check_alloc_task_attr(struct se_cmd *cmd) return 0; } -void transport_free_se_cmd( - struct se_cmd *se_cmd) -{ - if (se_cmd->se_tmr_req) - core_tmr_release_req(se_cmd->se_tmr_req); - /* - * Check and free any extended CDB buffer that was allocated - */ - if (se_cmd->t_task_cdb != se_cmd->__t_task_cdb) - kfree(se_cmd->t_task_cdb); -} -EXPORT_SYMBOL(transport_free_se_cmd); - static void transport_generic_wait_for_tasks(struct se_cmd *, int, int); /* transport_generic_allocate_tasks(): @@ -4371,11 +4358,21 @@ queue_full: return ret; } +/** + * transport_release_cmd - free a command + * @cmd: command to free + * + * This routine unconditionally frees a command, and reference counting + * or list removal must be done in the caller. + */ void transport_release_cmd(struct se_cmd *cmd) { BUG_ON(!cmd->se_tfo); - transport_free_se_cmd(cmd); + if (cmd->se_tmr_req) + core_tmr_release_req(cmd->se_tmr_req); + if (cmd->t_task_cdb != cmd->__t_task_cdb) + kfree(cmd->t_task_cdb); cmd->se_tfo->release_cmd(cmd); } EXPORT_SYMBOL(transport_release_cmd); diff --git a/include/target/target_core_transport.h b/include/target/target_core_transport.h index 0482a28629ff..99a671e0f9da 100644 --- a/include/target/target_core_transport.h +++ b/include/target/target_core_transport.h @@ -169,7 +169,6 @@ extern void transport_init_se_cmd(struct se_cmd *, unsigned char *); void *transport_kmap_first_data_page(struct se_cmd *cmd); void transport_kunmap_first_data_page(struct se_cmd *cmd); -extern void transport_free_se_cmd(struct se_cmd *); extern int transport_generic_allocate_tasks(struct se_cmd *, unsigned char *); extern int transport_handle_cdb_direct(struct se_cmd *); extern int transport_generic_handle_cdb_map(struct se_cmd *); -- cgit v1.2.3 From d3df7825aed2e69e12732f9e32ef9093b01302d8 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Sep 2011 23:08:32 +0200 Subject: target: simplify transport_generic_remove Instead of duplicating the code from transport_release_fe_cmd re-use it by allowing transport_release_fe_cmd to return wether it actually freed the command or not. Also rename transport_release_fe_cmd to transport_put_cmd and add a kerneldoc comment for it to make the use case more obvious. Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 49df0971a37f..bee923fd5750 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -88,7 +88,7 @@ static u32 transport_allocate_tasks(struct se_cmd *cmd, static int transport_generic_get_mem(struct se_cmd *cmd); static int transport_generic_remove(struct se_cmd *cmd, int session_reinstatement); -static void transport_release_fe_cmd(struct se_cmd *cmd); +static bool transport_put_cmd(struct se_cmd *cmd); static void transport_remove_cmd_from_queue(struct se_cmd *cmd, struct se_queue_obj *qobj); static int transport_set_sense_codes(struct se_cmd *cmd, u8 asc, u8 ascq); @@ -1074,7 +1074,7 @@ static void transport_release_all_cmds(struct se_device *dev) cmd->se_tfo->get_task_tag(cmd), cmd->se_tfo->get_cmd_state(cmd), t_state); - transport_release_fe_cmd(cmd); + transport_put_cmd(cmd); bug_out = 1; spin_lock_irqsave(&dev->dev_queue_obj.cmd_queue_lock, flags); @@ -3762,12 +3762,18 @@ static inline int transport_dec_and_check(struct se_cmd *cmd) return 0; } -static void transport_release_fe_cmd(struct se_cmd *cmd) +/** + * transport_put_cmd - release a reference to a command + * @cmd: command to release + * + * This routine releases our reference to the command and frees it if possible. + */ +static bool transport_put_cmd(struct se_cmd *cmd) { unsigned long flags; if (transport_dec_and_check(cmd)) - return; + return false; spin_lock_irqsave(&cmd->t_state_lock, flags); if (!atomic_read(&cmd->transport_dev_active)) { @@ -3779,9 +3785,12 @@ static void transport_release_fe_cmd(struct se_cmd *cmd) spin_unlock_irqrestore(&cmd->t_state_lock, flags); transport_release_tasks(cmd); + return true; + free_pages: transport_free_pages(cmd); transport_release_cmd(cmd); + return true; } static int @@ -3789,7 +3798,7 @@ transport_generic_remove(struct se_cmd *cmd, int session_reinstatement) { unsigned long flags; - if (transport_dec_and_check(cmd)) { + if (!transport_put_cmd(cmd)) { if (session_reinstatement) { spin_lock_irqsave(&cmd->t_state_lock, flags); transport_all_task_dev_remove_state(cmd); @@ -3799,20 +3808,6 @@ transport_generic_remove(struct se_cmd *cmd, int session_reinstatement) return 1; } - spin_lock_irqsave(&cmd->t_state_lock, flags); - if (!atomic_read(&cmd->transport_dev_active)) { - spin_unlock_irqrestore(&cmd->t_state_lock, flags); - goto free_pages; - } - atomic_set(&cmd->transport_dev_active, 0); - transport_all_task_dev_remove_state(cmd); - spin_unlock_irqrestore(&cmd->t_state_lock, flags); - - transport_release_tasks(cmd); - -free_pages: - transport_free_pages(cmd); - transport_release_cmd(cmd); return 0; } -- cgit v1.2.3 From 4911e3ccbec047ed1f728e19a70ad87729a3fb01 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Sep 2011 23:08:42 +0200 Subject: target: simplify transport_put_cmd Inline two simple functions only used by it, and replace a goto with a simple if else construct. Note that the code moved from transport_dec_and_check seems fairly buggy - the atomic_read check on a variable where we'd do an atomic_dec_and_test looks racy if we'll ever get someone increment it without the lock held around them (which it looks like we do), and not decrementing the second counter if the first one doesn't hit zero also at least needs an explanation. (nab: Fix transport_put_cmd breakage) Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 61 +++++++++++----------------------- 1 file changed, 20 insertions(+), 41 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index bee923fd5750..a650100d997a 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3732,36 +3732,6 @@ static inline void transport_free_pages(struct se_cmd *cmd) cmd->t_bidi_data_nents = 0; } -static inline void transport_release_tasks(struct se_cmd *cmd) -{ - transport_free_dev_tasks(cmd); -} - -static inline int transport_dec_and_check(struct se_cmd *cmd) -{ - unsigned long flags; - - spin_lock_irqsave(&cmd->t_state_lock, flags); - if (atomic_read(&cmd->t_fe_count)) { - if (!atomic_dec_and_test(&cmd->t_fe_count)) { - spin_unlock_irqrestore(&cmd->t_state_lock, - flags); - return 1; - } - } - - if (atomic_read(&cmd->t_se_count)) { - if (!atomic_dec_and_test(&cmd->t_se_count)) { - spin_unlock_irqrestore(&cmd->t_state_lock, - flags); - return 1; - } - } - spin_unlock_irqrestore(&cmd->t_state_lock, flags); - - return 0; -} - /** * transport_put_cmd - release a reference to a command * @cmd: command to release @@ -3771,26 +3741,35 @@ static inline int transport_dec_and_check(struct se_cmd *cmd) static bool transport_put_cmd(struct se_cmd *cmd) { unsigned long flags; - - if (transport_dec_and_check(cmd)) - return false; + int free_tasks = 0; spin_lock_irqsave(&cmd->t_state_lock, flags); - if (!atomic_read(&cmd->transport_dev_active)) { - spin_unlock_irqrestore(&cmd->t_state_lock, flags); - goto free_pages; + if (atomic_read(&cmd->t_fe_count)) { + if (!atomic_dec_and_test(&cmd->t_fe_count)) + goto out_busy; + } + + if (atomic_read(&cmd->t_se_count)) { + if (!atomic_dec_and_test(&cmd->t_se_count)) + goto out_busy; + } + + if (atomic_read(&cmd->transport_dev_active)) { + atomic_set(&cmd->transport_dev_active, 0); + transport_all_task_dev_remove_state(cmd); + free_tasks = 1; } - atomic_set(&cmd->transport_dev_active, 0); - transport_all_task_dev_remove_state(cmd); spin_unlock_irqrestore(&cmd->t_state_lock, flags); - transport_release_tasks(cmd); - return true; + if (free_tasks != 0) + transport_free_dev_tasks(cmd); -free_pages: transport_free_pages(cmd); transport_release_cmd(cmd); return true; +out_busy: + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + return false; } static int -- cgit v1.2.3 From e6a2573f1f5d66f0456c433afdfc63f33fdf9008 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Sep 2011 23:08:50 +0200 Subject: target: remove transport_generic_remove All callers that never have the session_reinstatement flag set can trivially be converted to transport_put_cmd. Opencode the session reinstatement code in transport_generic_free_cmd, which was the only caller ever asking for it. Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_transport.c | 44 ++++++++++++---------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index a650100d997a..4f21b88b85b1 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -86,8 +86,6 @@ static u32 transport_allocate_tasks(struct se_cmd *cmd, enum dma_data_direction data_direction, struct scatterlist *sgl, unsigned int nents); static int transport_generic_get_mem(struct se_cmd *cmd); -static int transport_generic_remove(struct se_cmd *cmd, - int session_reinstatement); static bool transport_put_cmd(struct se_cmd *cmd); static void transport_remove_cmd_from_queue(struct se_cmd *cmd, struct se_queue_obj *qobj); @@ -599,7 +597,7 @@ void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) return; if (remove) { transport_remove_cmd_from_queue(cmd, &cmd->se_dev->dev_queue_obj); - transport_generic_remove(cmd, 0); + transport_put_cmd(cmd); } } @@ -610,7 +608,7 @@ void transport_cmd_finish_abort_tmr(struct se_cmd *cmd) if (transport_cmd_check_stop_to_fabric(cmd)) return; - transport_generic_remove(cmd, 0); + transport_put_cmd(cmd); } static void transport_add_cmd_to_queue( @@ -2072,7 +2070,7 @@ static void transport_generic_request_timeout(struct se_cmd *cmd) unsigned long flags; /* - * Reset cmd->t_se_count to allow transport_generic_remove() + * Reset cmd->t_se_count to allow transport_put_cmd() * to allow last call to free memory resources. */ spin_lock_irqsave(&cmd->t_state_lock, flags); @@ -2083,7 +2081,7 @@ static void transport_generic_request_timeout(struct se_cmd *cmd) } spin_unlock_irqrestore(&cmd->t_state_lock, flags); - transport_generic_remove(cmd, 0); + transport_put_cmd(cmd); } static inline u32 transport_lba_21(unsigned char *cdb) @@ -3772,24 +3770,6 @@ out_busy: return false; } -static int -transport_generic_remove(struct se_cmd *cmd, int session_reinstatement) -{ - unsigned long flags; - - if (!transport_put_cmd(cmd)) { - if (session_reinstatement) { - spin_lock_irqsave(&cmd->t_state_lock, flags); - transport_all_task_dev_remove_state(cmd); - spin_unlock_irqrestore(&cmd->t_state_lock, - flags); - } - return 1; - } - - return 0; -} - /* * transport_generic_map_mem_to_cmd - Use fabric-alloced pages instead of * allocating in the core. @@ -4379,7 +4359,13 @@ void transport_generic_free_cmd( transport_free_dev_tasks(cmd); - transport_generic_remove(cmd, session_reinstatement); + if (!transport_put_cmd(cmd) && session_reinstatement) { + unsigned long flags; + + spin_lock_irqsave(&cmd->t_state_lock, flags); + transport_all_task_dev_remove_state(cmd); + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + } } } EXPORT_SYMBOL(transport_generic_free_cmd); @@ -5066,7 +5052,7 @@ static void transport_processing_shutdown(struct se_device *dev) transport_lun_remove_cmd(cmd); if (transport_cmd_check_stop(cmd, 1, 0)) - transport_generic_remove(cmd, 0); + transport_put_cmd(cmd); } spin_lock_irqsave(&dev->execute_task_lock, flags); @@ -5094,7 +5080,7 @@ static void transport_processing_shutdown(struct se_device *dev) transport_lun_remove_cmd(cmd); if (transport_cmd_check_stop(cmd, 1, 0)) - transport_generic_remove(cmd, 0); + transport_put_cmd(cmd); } spin_lock_irqsave(&dev->execute_task_lock, flags); @@ -5117,7 +5103,7 @@ static void transport_processing_shutdown(struct se_device *dev) } else { transport_lun_remove_cmd(cmd); if (transport_cmd_check_stop(cmd, 1, 0)) - transport_generic_remove(cmd, 0); + transport_put_cmd(cmd); } } } @@ -5192,7 +5178,7 @@ get_cmd: transport_generic_complete_ok(cmd); break; case TRANSPORT_REMOVE: - transport_generic_remove(cmd, 0); + transport_put_cmd(cmd); break; case TRANSPORT_FREE_CMD_INTR: transport_generic_free_cmd(cmd, 0, 0); -- cgit v1.2.3 From 82f1c8a4e7739eae9f1c32c2c419efdc19b8af41 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 13 Sep 2011 23:09:01 +0200 Subject: target: push session reinstatement out of transport_generic_free_cmd Push session reinstatement out of transport_generic_free_cmd into the only caller that actually needs it. Clean up transport_generic_free_cmd a bit, and remove the useless comment. I'd love to add a more useful kerneldoc comment for it, but as this point I'm still a bit confused in where it stands in the command release stack. Signed-off-by: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target.c | 2 +- drivers/target/loopback/tcm_loop.c | 4 ++-- drivers/target/target_core_transport.c | 37 ++++++++++++---------------------- drivers/target/tcm_fc/tfc_cmd.c | 12 +++++------ include/target/target_core_transport.h | 2 +- 5 files changed, 23 insertions(+), 34 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 6a4ea29c2f36..7dc2cfe9431c 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -3547,7 +3547,7 @@ get_immediate: iscsit_release_cmd(cmd); else transport_generic_free_cmd(&cmd->se_cmd, - 1, 0); + 1); goto get_immediate; case ISTATE_SEND_NOPIN_WANT_RESPONSE: spin_unlock_bh(&cmd->istate_lock); diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c index aa2d67997235..f0e701d27bd1 100644 --- a/drivers/target/loopback/tcm_loop.c +++ b/drivers/target/loopback/tcm_loop.c @@ -200,7 +200,7 @@ static void tcm_loop_check_stop_free(struct se_cmd *se_cmd) * Release the struct se_cmd, which will make a callback to release * struct tcm_loop_cmd * in tcm_loop_deallocate_core_cmd() */ - transport_generic_free_cmd(se_cmd, 0, 0); + transport_generic_free_cmd(se_cmd, 0); } static void tcm_loop_release_cmd(struct se_cmd *se_cmd) @@ -388,7 +388,7 @@ static int tcm_loop_device_reset(struct scsi_cmnd *sc) SUCCESS : FAILED; release: if (se_cmd) - transport_generic_free_cmd(se_cmd, 1, 0); + transport_generic_free_cmd(se_cmd, 1); else kmem_cache_free(tcm_loop_cmd_cache, tl_cmd); kfree(tl_tmr); diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 4f21b88b85b1..db2f8987a046 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -4331,42 +4331,25 @@ void transport_release_cmd(struct se_cmd *cmd) } EXPORT_SYMBOL(transport_release_cmd); -/* transport_generic_free_cmd(): - * - * Called from processing frontend to release storage engine resources - */ -void transport_generic_free_cmd( - struct se_cmd *cmd, - int wait_for_tasks, - int session_reinstatement) +bool transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks) { if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD)) transport_release_cmd(cmd); else { core_dec_lacl_count(cmd->se_sess->se_node_acl, cmd); - if (cmd->se_lun) { -#if 0 - pr_debug("cmd: %p ITT: 0x%08x contains" - " cmd->se_lun\n", cmd, - cmd->se_tfo->get_task_tag(cmd)); -#endif + if (cmd->se_lun) transport_lun_remove_cmd(cmd); - } if (wait_for_tasks && cmd->transport_wait_for_tasks) cmd->transport_wait_for_tasks(cmd, 0, 0); transport_free_dev_tasks(cmd); - if (!transport_put_cmd(cmd) && session_reinstatement) { - unsigned long flags; - - spin_lock_irqsave(&cmd->t_state_lock, flags); - transport_all_task_dev_remove_state(cmd); - spin_unlock_irqrestore(&cmd->t_state_lock, flags); - } + return transport_put_cmd(cmd); } + + return true; } EXPORT_SYMBOL(transport_generic_free_cmd); @@ -4631,7 +4614,13 @@ remove: if (!remove_cmd) return; - transport_generic_free_cmd(cmd, 0, session_reinstatement); + if (!transport_generic_free_cmd(cmd, 0) && session_reinstatement) { + unsigned long flags; + + spin_lock_irqsave(&cmd->t_state_lock, flags); + transport_all_task_dev_remove_state(cmd); + spin_unlock_irqrestore(&cmd->t_state_lock, flags); + } } static int transport_get_sense_codes( @@ -5181,7 +5170,7 @@ get_cmd: transport_put_cmd(cmd); break; case TRANSPORT_FREE_CMD_INTR: - transport_generic_free_cmd(cmd, 0, 0); + transport_generic_free_cmd(cmd, 0); break; case TRANSPORT_PROCESS_TMR: transport_generic_do_tmr(cmd); diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c index c2d148d80cf7..7f2ee5a0ed79 100644 --- a/drivers/target/tcm_fc/tfc_cmd.c +++ b/drivers/target/tcm_fc/tfc_cmd.c @@ -114,7 +114,7 @@ void ft_release_cmd(struct se_cmd *se_cmd) void ft_check_stop_free(struct se_cmd *se_cmd) { - transport_generic_free_cmd(se_cmd, 0, 0); + transport_generic_free_cmd(se_cmd, 0); } /* @@ -269,7 +269,7 @@ static void ft_recv_seq(struct fc_seq *sp, struct fc_frame *fp, void *arg) /* XXX need to find cmd if queued */ cmd->se_cmd.t_state = TRANSPORT_REMOVE; cmd->seq = NULL; - transport_generic_free_cmd(&cmd->se_cmd, 0, 0); + transport_generic_free_cmd(&cmd->se_cmd, 0); return; } @@ -287,7 +287,7 @@ static void ft_recv_seq(struct fc_seq *sp, struct fc_frame *fp, void *arg) __func__, fh->fh_r_ctl); ft_invl_hw_context(cmd); fc_frame_free(fp); - transport_generic_free_cmd(&cmd->se_cmd, 0, 0); + transport_generic_free_cmd(&cmd->se_cmd, 0); break; } } @@ -420,7 +420,7 @@ static void ft_send_tm(struct ft_cmd *cmd) sess = cmd->sess; transport_send_check_condition_and_sense(&cmd->se_cmd, cmd->se_cmd.scsi_sense_reason, 0); - transport_generic_free_cmd(&cmd->se_cmd, 0, 0); + transport_generic_free_cmd(&cmd->se_cmd, 0); ft_sess_put(sess); return; } @@ -627,7 +627,7 @@ static void ft_send_work(struct work_struct *work) if (ret == -ENOMEM) { transport_send_check_condition_and_sense(se_cmd, TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0); - transport_generic_free_cmd(se_cmd, 0, 0); + transport_generic_free_cmd(se_cmd, 0); return; } if (ret == -EINVAL) { @@ -636,7 +636,7 @@ static void ft_send_work(struct work_struct *work) else transport_send_check_condition_and_sense(se_cmd, se_cmd->scsi_sense_reason, 0); - transport_generic_free_cmd(se_cmd, 0, 0); + transport_generic_free_cmd(se_cmd, 0); return; } transport_handle_cdb_direct(se_cmd); diff --git a/include/target/target_core_transport.h b/include/target/target_core_transport.h index 99a671e0f9da..a113129fa22e 100644 --- a/include/target/target_core_transport.h +++ b/include/target/target_core_transport.h @@ -184,7 +184,7 @@ extern int transport_check_aborted_status(struct se_cmd *, int); extern int transport_send_check_condition_and_sense(struct se_cmd *, u8, int); extern void transport_send_task_abort(struct se_cmd *); extern void transport_release_cmd(struct se_cmd *); -extern void transport_generic_free_cmd(struct se_cmd *, int, int); +extern bool transport_generic_free_cmd(struct se_cmd *, int); extern void transport_generic_wait_for_cmds(struct se_cmd *, int); extern int transport_init_task_sg(struct se_task *, struct se_mem *, u32); extern int transport_map_mem_to_sg(struct se_task *, struct list_head *, -- cgit v1.2.3 From 39c05f321a4b27f3036392eed68bd94ce2267155 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Sat, 8 Oct 2011 13:59:52 -0700 Subject: target: Remove session_reinstatement parameter from ->transport_wait_for_tasks This patch removes the unnecessary session_reinstatement parameter from se_cmd->transport_wait_for_tasks(), logic in transport_generic_wait_for_tasks, and usage within iscsi-target code. This also includes the removal of the 'bool' return from transport_put_cmd() + transport_generic_free_cmd() that is no longer necessary. Cc: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target.c | 4 ++-- drivers/target/iscsi/iscsi_target_erl2.c | 15 +++++++-------- drivers/target/target_core_transport.c | 33 +++++++++++--------------------- include/target/target_core_base.h | 2 +- include/target/target_core_transport.h | 2 +- 5 files changed, 22 insertions(+), 34 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 7dc2cfe9431c..354a8339a3f9 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -3960,7 +3960,7 @@ static void iscsit_release_commands_from_conn(struct iscsi_conn *conn) * iscsit_get_lun_for_cmd() in iscsit_handle_scsi_cmd(). */ if (cmd->tmr_req && se_cmd->transport_wait_for_tasks) - se_cmd->transport_wait_for_tasks(se_cmd, 1, 1); + se_cmd->transport_wait_for_tasks(se_cmd, 1); else if (cmd->se_cmd.se_cmd_flags & SCF_SE_LUN_CMD) transport_release_cmd(se_cmd); else @@ -3976,7 +3976,7 @@ static void iscsit_release_commands_from_conn(struct iscsi_conn *conn) se_cmd = &cmd->se_cmd; if (se_cmd->transport_wait_for_tasks) - se_cmd->transport_wait_for_tasks(se_cmd, 1, 1); + se_cmd->transport_wait_for_tasks(se_cmd, 1); spin_lock_bh(&conn->cmd_lock); } diff --git a/drivers/target/iscsi/iscsi_target_erl2.c b/drivers/target/iscsi/iscsi_target_erl2.c index 91a4d170bda4..000356baf0b5 100644 --- a/drivers/target/iscsi/iscsi_target_erl2.c +++ b/drivers/target/iscsi/iscsi_target_erl2.c @@ -148,7 +148,7 @@ void iscsit_free_connection_recovery_entires(struct iscsi_session *sess) iscsit_release_cmd(cmd); else cmd->se_cmd.transport_wait_for_tasks( - &cmd->se_cmd, 1, 1); + &cmd->se_cmd, 1); spin_lock(&cr->conn_recovery_cmd_lock); } spin_unlock(&cr->conn_recovery_cmd_lock); @@ -175,7 +175,7 @@ void iscsit_free_connection_recovery_entires(struct iscsi_session *sess) iscsit_release_cmd(cmd); else cmd->se_cmd.transport_wait_for_tasks( - &cmd->se_cmd, 1, 1); + &cmd->se_cmd, 1); spin_lock(&cr->conn_recovery_cmd_lock); } spin_unlock(&cr->conn_recovery_cmd_lock); @@ -265,7 +265,7 @@ void iscsit_discard_cr_cmds_by_expstatsn( iscsit_release_cmd(cmd); else cmd->se_cmd.transport_wait_for_tasks( - &cmd->se_cmd, 1, 0); + &cmd->se_cmd, 1); spin_lock(&cr->conn_recovery_cmd_lock); } spin_unlock(&cr->conn_recovery_cmd_lock); @@ -324,7 +324,7 @@ int iscsit_discard_unacknowledged_ooo_cmdsns_for_conn(struct iscsi_conn *conn) iscsit_release_cmd(cmd); else cmd->se_cmd.transport_wait_for_tasks( - &cmd->se_cmd, 1, 1); + &cmd->se_cmd, 1); spin_lock_bh(&conn->cmd_lock); } spin_unlock_bh(&conn->cmd_lock); @@ -383,7 +383,7 @@ int iscsit_prepare_cmds_for_realligance(struct iscsi_conn *conn) iscsit_release_cmd(cmd); else cmd->se_cmd.transport_wait_for_tasks( - &cmd->se_cmd, 1, 0); + &cmd->se_cmd, 1); spin_lock_bh(&conn->cmd_lock); continue; } @@ -409,7 +409,7 @@ int iscsit_prepare_cmds_for_realligance(struct iscsi_conn *conn) iscsit_release_cmd(cmd); else cmd->se_cmd.transport_wait_for_tasks( - &cmd->se_cmd, 1, 1); + &cmd->se_cmd, 1); spin_lock_bh(&conn->cmd_lock); continue; } @@ -436,8 +436,7 @@ int iscsit_prepare_cmds_for_realligance(struct iscsi_conn *conn) if ((cmd->se_cmd.se_cmd_flags & SCF_SE_LUN_CMD) && cmd->se_cmd.transport_wait_for_tasks) - cmd->se_cmd.transport_wait_for_tasks(&cmd->se_cmd, - 0, 0); + cmd->se_cmd.transport_wait_for_tasks(&cmd->se_cmd, 0); /* * Add the struct iscsi_cmd to the connection recovery cmd list */ diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index db2f8987a046..e5905269c850 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -86,7 +86,7 @@ static u32 transport_allocate_tasks(struct se_cmd *cmd, enum dma_data_direction data_direction, struct scatterlist *sgl, unsigned int nents); static int transport_generic_get_mem(struct se_cmd *cmd); -static bool transport_put_cmd(struct se_cmd *cmd); +static void transport_put_cmd(struct se_cmd *cmd); static void transport_remove_cmd_from_queue(struct se_cmd *cmd, struct se_queue_obj *qobj); static int transport_set_sense_codes(struct se_cmd *cmd, u8 asc, u8 ascq); @@ -1638,7 +1638,7 @@ static int transport_check_alloc_task_attr(struct se_cmd *cmd) return 0; } -static void transport_generic_wait_for_tasks(struct se_cmd *, int, int); +static void transport_generic_wait_for_tasks(struct se_cmd *, int); /* transport_generic_allocate_tasks(): * @@ -2504,7 +2504,7 @@ void transport_new_cmd_failure(struct se_cmd *se_cmd) spin_unlock_irqrestore(&se_cmd->t_state_lock, flags); } -static void transport_nop_wait_for_tasks(struct se_cmd *, int, int); +static void transport_nop_wait_for_tasks(struct se_cmd *, int); static inline u32 transport_get_sectors_6( unsigned char *cdb, @@ -3736,7 +3736,7 @@ static inline void transport_free_pages(struct se_cmd *cmd) * * This routine releases our reference to the command and frees it if possible. */ -static bool transport_put_cmd(struct se_cmd *cmd) +static void transport_put_cmd(struct se_cmd *cmd) { unsigned long flags; int free_tasks = 0; @@ -3764,10 +3764,9 @@ static bool transport_put_cmd(struct se_cmd *cmd) transport_free_pages(cmd); transport_release_cmd(cmd); - return true; + return; out_busy: spin_unlock_irqrestore(&cmd->t_state_lock, flags); - return false; } /* @@ -4331,7 +4330,7 @@ void transport_release_cmd(struct se_cmd *cmd) } EXPORT_SYMBOL(transport_release_cmd); -bool transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks) +void transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks) { if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD)) transport_release_cmd(cmd); @@ -4342,21 +4341,18 @@ bool transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks) transport_lun_remove_cmd(cmd); if (wait_for_tasks && cmd->transport_wait_for_tasks) - cmd->transport_wait_for_tasks(cmd, 0, 0); + cmd->transport_wait_for_tasks(cmd, 0); transport_free_dev_tasks(cmd); - return transport_put_cmd(cmd); + transport_put_cmd(cmd); } - - return true; } EXPORT_SYMBOL(transport_generic_free_cmd); static void transport_nop_wait_for_tasks( struct se_cmd *cmd, - int remove_cmd, - int session_reinstatement) + int remove_cmd) { return; } @@ -4537,8 +4533,7 @@ int transport_clear_lun_from_sessions(struct se_lun *lun) */ static void transport_generic_wait_for_tasks( struct se_cmd *cmd, - int remove_cmd, - int session_reinstatement) + int remove_cmd) { unsigned long flags; @@ -4614,13 +4609,7 @@ remove: if (!remove_cmd) return; - if (!transport_generic_free_cmd(cmd, 0) && session_reinstatement) { - unsigned long flags; - - spin_lock_irqsave(&cmd->t_state_lock, flags); - transport_all_task_dev_remove_state(cmd); - spin_unlock_irqrestore(&cmd->t_state_lock, flags); - } + transport_generic_free_cmd(cmd, 0); } static int transport_get_sense_codes( diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 27040653005e..c10e351bc1f5 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -480,7 +480,7 @@ struct se_cmd { struct target_core_fabric_ops *se_tfo; int (*transport_emulate_cdb)(struct se_cmd *); void (*transport_split_cdb)(unsigned long long, u32, unsigned char *); - void (*transport_wait_for_tasks)(struct se_cmd *, int, int); + void (*transport_wait_for_tasks)(struct se_cmd *, int); void (*transport_complete_callback)(struct se_cmd *); int (*transport_qf_callback)(struct se_cmd *); diff --git a/include/target/target_core_transport.h b/include/target/target_core_transport.h index a113129fa22e..e67feeb88b69 100644 --- a/include/target/target_core_transport.h +++ b/include/target/target_core_transport.h @@ -184,7 +184,7 @@ extern int transport_check_aborted_status(struct se_cmd *, int); extern int transport_send_check_condition_and_sense(struct se_cmd *, u8, int); extern void transport_send_task_abort(struct se_cmd *); exter