Hi Sagi & Co,
On Mon, 2015-02-09 at 18:07 +0200, Sagi Grimberg wrote:
> In case sendtargets response is larger than initiator MRDSL, we
> send a partial sendtargets response (setting F=0, C=1, TTT!=0xffffffff),
> accept a consecutive empty text message and send the rest of the payload.
> In case we are done, we set F=1, C=0, TTT=0xffffffff.
> We do that by storing the sendtargets response bytes done under
> the session.
>
> This patch makes iscsit_find_cmd_from_itt public for isert,
> and gets rid of maxcmdsn_inc variable.
>
> Signed-off-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>
> ---
> drivers/infiniband/ulp/isert/ib_isert.c | 18 +++++-
> drivers/target/iscsi/iscsi_target.c | 81 ++++++++++++++++++++--------
> drivers/target/iscsi/iscsi_target_device.c | 4 +-
> drivers/target/iscsi/iscsi_target_util.c | 1 +
> drivers/target/iscsi/iscsi_target_util.h | 1 -
> include/target/iscsi/iscsi_target_core.h | 3 +-
> 6 files changed, 77 insertions(+), 31 deletions(-)
>
Apologies for the delayed follow-up.
Looks fine, and thanks for updating -v2 series to reference cmd by ITT.
One comment below.
> diff --git a/drivers/target/iscsi/iscsi_target_device.c b/drivers/target/iscsi/iscsi_target_device.c
> index 34c3cd1..7e6d00c 100644
> --- a/drivers/target/iscsi/iscsi_target_device.c
> +++ b/drivers/target/iscsi/iscsi_target_device.c
> @@ -53,11 +53,9 @@ void iscsit_determine_maxcmdsn(struct iscsi_session *sess)
>
> void iscsit_increment_maxcmdsn(struct iscsi_cmd *cmd, struct iscsi_session *sess)
> {
> - if (cmd->immediate_cmd || cmd->maxcmdsn_inc)
> + if (cmd->immediate_cmd)
> return;
>
> - cmd->maxcmdsn_inc = 1;
> -
> mutex_lock(&sess->cmdsn_mutex);
> sess->max_cmd_sn += 1;
> pr_debug("Updated MaxCmdSN to 0x%08x\n", sess->max_cmd_sn);
IIRC there is a connection recovery codepath in iscsit_send_datain()
that can call iscsit_increment_maxcmdsn() more than once, for which this
check was originally added.
Since this is a special case for multi-part text payload exchange, the
following would make more sense:
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 4f00ad8..f533a0d 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3568,6 +3568,12 @@ iscsit_build_text_rsp(struct iscsi_cmd *cmd, struct iscsi_conn *conn,
hdr->statsn = cpu_to_be32(cmd->stat_sn);
iscsit_increment_maxcmdsn(cmd, conn->sess);
+ /*
+ * Reset maxcmdsn_inc in multi-part text payload exchanges to
+ * correctly increment MaxCmdSN for each response answering a
+ * non immediate text request with a valid CmdSN.
+ */
+ cmd->maxcmdsn_inc = 0;
hdr->exp_cmdsn = cpu_to_be32(conn->sess->exp_cmd_sn);
hdr->max_cmdsn = cpu_to_be32(conn->sess->max_cmd_sn);
If there are no objections, I'll fold this into your original patch and
apply to for-next.
Thank you,
--nab
--
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
