On Tue, 2014-09-02 at 17:49 -0400, Joern Engel wrote:
> At this point in the function ret cannot possibly be -ENODEV. I
> strongly suspect that comparing rc to -ENODEV was meant when writing the
> code. Then again, we have lived with a technically non-existent
> condition for a long time and maybe the right thing is to just remove
> it.
>
> Nick, any opinion on this matter?
>
> Signed-off-by: Joern Engel <joern@xxxxxxxxx>
> ---
> drivers/target/iscsi/iscsi_target_login.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
> index b1ae5cbe70f8..0df2dcd0a273 100644
> --- a/drivers/target/iscsi/iscsi_target_login.c
> +++ b/drivers/target/iscsi/iscsi_target_login.c
> @@ -1267,7 +1267,7 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
> iscsit_put_transport(conn->conn_transport);
> kfree(conn);
> conn = NULL;
> - if (ret == -ENODEV)
> + if (rc == -ENODEV)
> goto out;
> /* Get another socket */
> return 1;
So yeah, in earlier code (v3.10) ENODEV was supposed to signal that
iser-target was shutting down, and that the NP thread was supposed to
bail out. In the same v3.10 code, there was another check at the out:
label to determine if the return value should be zero to signal a break
from the loop in iscsi_target_login_thread().
However, the ISCSI_NP_THREAD_SHUTDOWN check at the start of
__iscsi_target_login_thread() has the same effect as the original code
at the out: label, so I think it's safe to go ahead and drop the check
all-together.
Applying the following patch to target-pending/for-next.
--nab
commit 1d30686da4a40029cb48eab28442896b58aeceef
Author: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
Date: Wed Sep 17 13:17:55 2014 -0700
iscsi-target: Drop duplicate __iscsi_target_login_thread check
This patch drops the now duplicate + unnecessary check for -ENODEV from
iscsi_transport->iscsit_accept_np() for jumping to out:, or immediately
returning 1 in __iscsi_target_login_thread() code.
Since commit 81a9c5e72b the jump to out: and returning 1 have the same
effect, and end up hitting the ISCSI_NP_THREAD_SHUTDOWN check regardless
at the top of __iscsi_target_login_thread() during next loop iteration.
So that said, it's safe to go ahead and remove this duplicate check.
Reported-by: Joern Engel <joern@xxxxxxxxx>
Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_lo
index b1ae5cb..02d5ccd 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -1267,8 +1267,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
iscsit_put_transport(conn->conn_transport);
kfree(conn);
conn = NULL;
- if (ret == -ENODEV)
- goto out;
/* Get another socket */
return 1;
}
--
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
