On 2/12/21 1:26 AM, Mike Christie wrote:
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 several
target_submit_cmd_map_sgls/ target_submit_cmd callers do not handle the
error case properly if it were to ever happen. These drivers think
they have their normal refcount on the cmd and in many cases do a
transport_generic_free_cmd plus target_put_sess_cmd so they would
have fired off the refcount WARN/BUGs.
So this patch just changes the kref_get_unless_zero to kref_get.
Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
---
drivers/target/target_core_transport.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
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;
}
Looks Good.
Reviewed-by: Himanshu Madhani <himanshu.madhani@xxxxxxxxxx>
--
Himanshu Madhani Oracle Linux Engineering
