[PATCH 4/6] target: simplify transport_put_cmd

[Christoph Hellwig <hch@xxxxxx>]

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.

Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Index: lio-core/drivers/target/target_core_transport.c
===================================================================
--- lio-core.orig/drivers/target/target_core_transport.c	2011-09-13 14:54:21.892088669 -0400
+++ lio-core/drivers/target/target_core_transport.c	2011-09-13 14:54:25.125089859 -0400
@@ -3754,36 +3754,6 @@ static inline void transport_free_pages(
 	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
@@ -3794,25 +3764,33 @@ static bool transport_put_cmd(struct se_
 {
 	unsigned long flags;
 
-	if (transport_dec_and_check(cmd))
-		return false;
-
 	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;
 	}
-	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 (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)) {
+		spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+		transport_free_pages(cmd);
+		transport_release_cmd(cmd);
+	} else {
+		atomic_set(&cmd->transport_dev_active, 0);
+		transport_all_task_dev_remove_state(cmd);
+		spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+
+		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
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html