[02/14] target: drop kref_get_unless_zero in target_get_sess_cmd

Commit Message

Mike Christie Feb. 11, 2021, 12:27 p.m. UTC

The kref_get_unless_zero use in target_get_sess_cmd was added
in:

commit 1b4c59b7a1d0 ("target: fix potential race window in
target_sess_cmd_list_waiting()")'

but it does not seem to do anything.

I think the original patch might have thought we could have added the
cmd to the sess_wait_list and then target_wait_for_sess_cmds could
do a put before target_get_sess_cmd did it's get. That wouldn't
happen because we do the get first then grab the sess lock and put
it on the list.

It's also not needed now, because the sess_cmd_list does not exist
anymore and we instead wait on the session cmd_count.

The other problem with the patch is that target_submit_cmd_map_sgls/
target_submit_cmd callers do not handle the error case properly if
it were to ever happen. The drivers assume they have a refcount
on the cmd and in many cases do a target_put_sess_cmd on it either
directly or via transport_generic_free_cmd.

So this patch just changes the kref_get_unless_zero to kref_get.

Signed-off-by: Mike Christie <[e-mail]>
---
 drivers/target/target_core_transport.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Christoph Hellwig Feb. 11, 2021, 3:09 p.m. UTC | #1

Looks good,

Reviewed-by: Christoph Hellwig <[e-mail]>

Patch

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 5c4adde96d5e..b5427e26187b 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2775,9 +2775,7 @@  int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref)
 	 * invocations before se_cmd descriptor release.
 	 */
 	if (ack_kref) {
-		if (!kref_get_unless_zero(&se_cmd->cmd_kref))
-			return -EINVAL;
-
+		kref_get(&se_cmd->cmd_kref);
 		se_cmd->se_cmd_flags |= SCF_ACK_KREF;
 	}