[31/34] target: Inline transport_lun_remove_cmd()

Commit Message

Bart Van Assche Jan. 25, 2017, 11:36 p.m. UTC

Remove the code that clears .se_lun from
transport_cmd_check_stop_to_fabric() such that the
transport_lun_remove_cmd() call can be moved into
target_release_cmd_kref(). Because this guarantees that
transport_lun_remove_cmd() will be called exactly once, it is
safe to change the cmpxchg() call into a test of
se_cmd.lun_ref_active. Inline transport_lun_remove_cmd() because
it is not worth to keep it as a separate function.

Signed-off-by: Bart Van Assche <[e-mail]>
Cc: Christoph Hellwig <[e-mail]>
Cc: Andy Grover <[e-mail]>
Cc: David Disseldorp <[e-mail]>
---
 drivers/target/target_core_transport.c | 30 +++---------------------------
 1 file changed, 3 insertions(+), 27 deletions(-)

Comments

Hannes Reinecke Jan. 26, 2017, 8:37 a.m. UTC | #1

On 01/26/2017 12:36 AM, Bart Van Assche wrote:
> Remove the code that clears .se_lun from
> transport_cmd_check_stop_to_fabric() such that the
> transport_lun_remove_cmd() call can be moved into
> target_release_cmd_kref(). Because this guarantees that
> transport_lun_remove_cmd() will be called exactly once, it is
> safe to change the cmpxchg() call into a test of
> se_cmd.lun_ref_active. Inline transport_lun_remove_cmd() because
> it is not worth to keep it as a separate function.
> 
> Signed-off-by: Bart Van Assche <[e-mail]>
> Cc: Christoph Hellwig <[e-mail]>
> Cc: Andy Grover <[e-mail]>
> Cc: David Disseldorp <[e-mail]>
> ---
>  drivers/target/target_core_transport.c | 30 +++---------------------------
>  1 file changed, 3 insertions(+), 27 deletions(-)
> 
Reviewed-by: Hannes Reinecke <[e-mail]>

Cheers,

Hannes

Patch

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 1dde88b0c2c8..3472dcecfb28 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -590,11 +590,6 @@  static int transport_cmd_check_stop_to_fabric(struct se_cmd *cmd)
 
 	target_remove_from_state_list(cmd);
 
-	/*
-	 * Clear struct se_cmd->se_lun before the handoff to FE.
-	 */
-	cmd->se_lun = NULL;
-
 	spin_lock_irqsave(&cmd->t_state_lock, flags);
 	/*
 	 * Determine if frontend context caller is requesting the stopping of
@@ -621,17 +616,6 @@  static int transport_cmd_check_stop_to_fabric(struct se_cmd *cmd)
 		: 0;
 }
 
-static void transport_lun_remove_cmd(struct se_cmd *cmd)
-{
-	struct se_lun *lun = cmd->se_lun;
-
-	if (!lun)
-		return;
-
-	if (cmpxchg(&cmd->lun_ref_active, true, false))
-		percpu_ref_put(&lun->lun_ref);
-}
-
 static void target_complete_failure_work(struct work_struct *work)
 {
 	struct se_cmd *cmd = container_of(work, struct se_cmd, work);
@@ -667,8 +651,6 @@  static void transport_handle_abort(struct se_cmd *cmd)
 {
 	bool ack_kref = cmd->se_cmd_flags & SCF_ACK_KREF;
 
-	transport_lun_remove_cmd(cmd);
-
 	if (cmd->send_abort_response) {
 		cmd->scsi_status = SAM_STAT_TASK_ABORTED;
 		pr_debug("Setting SAM_STAT_TASK_ABORTED status for CDB: 0x%02x, ITT: 0x%08llx\n",
@@ -1738,7 +1720,6 @@  void transport_generic_request_failure(struct se_cmd *cmd,
 		goto queue_full;
 
 check_stop:
-	transport_lun_remove_cmd(cmd);
 	transport_cmd_check_stop_to_fabric(cmd);
 	return;
 
@@ -2016,7 +1997,6 @@  static void transport_complete_qf(struct se_cmd *cmd)
 		transport_handle_queue_full(cmd, cmd->se_dev);
 		return;
 	}
-	transport_lun_remove_cmd(cmd);
 	transport_cmd_check_stop_to_fabric(cmd);
 }
 
@@ -2090,7 +2070,6 @@  static void target_complete_ok_work(struct work_struct *work)
 		if (ret == -EAGAIN || ret == -ENOMEM)
 			goto queue_full;
 
-		transport_lun_remove_cmd(cmd);
 		transport_cmd_check_stop_to_fabric(cmd);
 		return;
 	}
@@ -2116,7 +2095,6 @@  static void target_complete_ok_work(struct work_struct *work)
 			if (ret == -EAGAIN || ret == -ENOMEM)
 				goto queue_full;
 
-			transport_lun_remove_cmd(cmd);
 			transport_cmd_check_stop_to_fabric(cmd);
 			return;
 		}
@@ -2141,7 +2119,6 @@  static void target_complete_ok_work(struct work_struct *work)
 			if (ret == -EAGAIN || ret == -ENOMEM)
 				goto queue_full;
 
-			transport_lun_remove_cmd(cmd);
 			transport_cmd_check_stop_to_fabric(cmd);
 			return;
 		}
@@ -2177,7 +2154,6 @@  static void target_complete_ok_work(struct work_struct *work)
 		break;
 	}
 
-	transport_lun_remove_cmd(cmd);
 	transport_cmd_check_stop_to_fabric(cmd);
 	return;
 
@@ -2492,9 +2468,6 @@  int transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks)
 		 */
 		if (cmd->state_active)
 			target_remove_from_state_list(cmd);
-
-		if (cmd->se_lun)
-			transport_lun_remove_cmd(cmd);
 	}
 	BUG_ON(!cmd->se_tfo);
 	return target_put_sess_cmd(cmd);
@@ -2555,6 +2528,9 @@  static void target_release_cmd_kref(struct kref *kref)
 	struct se_session *se_sess = se_cmd->se_sess;
 	unsigned long flags;
 
+	if (se_cmd->lun_ref_active)
+		percpu_ref_put(&se_cmd->se_lun->lun_ref);
+
 	complete_all(&se_cmd->finished);
 
 	spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);