On Tue, 2012-04-03 at 15:51 -0700, Andy Grover wrote:
> This is so we can hoist it before se_cmd init in a later patch. Instead,
> pass it into build_pdu_and_seq_lists, and then stick it in the
> struct iscsi_build_list that gets passed around.
>
This assumption is incorrect because target_setup_cmd_from_cdb() ->
transport_generic_cmd_sequencer() may end up changing
se_cmd->data_length if what the fabric declares for data_length does not
match what is extracted from SCSI CDB length at the bottom on the
sequencer code..
Which means that iscsit_build_pdu_and_seq_lists() cannot depend upon
this value until transport_generic_cmd_sequencer() has been called to
verify the two values.
So given that fact, I'm not going to merge this one for now.
--nab
> Signed-off-by: Andy Grover <agrover@xxxxxxxxxx>
> ---
> drivers/target/iscsi/iscsi_target.c | 4 ++-
> drivers/target/iscsi/iscsi_target_seq_pdu_list.c | 41 ++++++++++++----------
> drivers/target/iscsi/iscsi_target_seq_pdu_list.h | 4 ++-
> 3 files changed, 28 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 764fde5..da4e779 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -1059,7 +1059,9 @@ done:
> */
> send_check_condition = 1;
> } else {
> - if (iscsit_build_pdu_and_seq_lists(cmd, payload_length) < 0)
> + ret = iscsit_build_pdu_and_seq_lists(cmd, payload_length,
> + hdr->data_length);
> + if (ret < 0)
> return iscsit_add_reject_from_cmd(
> ISCSI_REASON_BOOKMARK_NO_RESOURCES,
> 1, 1, buf, cmd);
> diff --git a/drivers/target/iscsi/iscsi_target_seq_pdu_list.c b/drivers/target/iscsi/iscsi_target_seq_pdu_list.c
> index 7d0effa..d3e552d 100644
> --- a/drivers/target/iscsi/iscsi_target_seq_pdu_list.c
> +++ b/drivers/target/iscsi/iscsi_target_seq_pdu_list.c
> @@ -227,10 +227,10 @@ static void iscsit_determine_counts_for_list(
>
> if ((bl->type == PDULIST_UNSOLICITED) ||
> (bl->type == PDULIST_IMMEDIATE_AND_UNSOLICITED))
> - unsolicited_data_length = min(cmd->se_cmd.data_length,
> + unsolicited_data_length = min(bl->data_length,
> conn->sess->sess_ops->FirstBurstLength);
>
> - while (offset < cmd->se_cmd.data_length) {
> + while (offset < bl->data_length) {
> *pdu_count += 1;
>
> if (check_immediate) {
> @@ -244,10 +244,10 @@ static void iscsit_determine_counts_for_list(
> }
> if (unsolicited_data_length > 0) {
> if ((offset + conn->conn_ops->MaxRecvDataSegmentLength)
> - >= cmd->se_cmd.data_length) {
> + >= bl->data_length) {
> unsolicited_data_length -=
> - (cmd->se_cmd.data_length - offset);
> - offset += (cmd->se_cmd.data_length - offset);
> + (bl->data_length - offset);
> + offset += (bl->data_length - offset);
> continue;
> }
> if ((offset + conn->conn_ops->MaxRecvDataSegmentLength)
> @@ -268,8 +268,8 @@ static void iscsit_determine_counts_for_list(
> continue;
> }
> if ((offset + conn->conn_ops->MaxRecvDataSegmentLength) >=
> - cmd->se_cmd.data_length) {
> - offset += (cmd->se_cmd.data_length - offset);
> + bl->data_length) {
> + offset += (bl->data_length - offset);
> continue;
> }
> if ((burstlength + conn->conn_ops->MaxRecvDataSegmentLength) >=
> @@ -311,10 +311,10 @@ static int iscsit_do_build_pdu_and_seq_lists(
>
> if ((bl->type == PDULIST_UNSOLICITED) ||
> (bl->type == PDULIST_IMMEDIATE_AND_UNSOLICITED))
> - unsolicited_data_length = min(cmd->se_cmd.data_length,
> + unsolicited_data_length = min(bl->data_length,
> conn->sess->sess_ops->FirstBurstLength);
>
> - while (offset < cmd->se_cmd.data_length) {
> + while (offset < bl->data_length) {
> pdu_count++;
> if (!datapduinorder) {
> pdu[i].offset = offset;
> @@ -350,21 +350,21 @@ static int iscsit_do_build_pdu_and_seq_lists(
> if (unsolicited_data_length > 0) {
> if ((offset +
> conn->conn_ops->MaxRecvDataSegmentLength) >=
> - cmd->se_cmd.data_length) {
> + bl->data_length) {
> if (!datapduinorder) {
> pdu[i].type = PDUTYPE_UNSOLICITED;
> pdu[i].length =
> - (cmd->se_cmd.data_length - offset);
> + (bl->data_length - offset);
> }
> if (!datasequenceinorder) {
> seq[seq_no].type = SEQTYPE_UNSOLICITED;
> seq[seq_no].pdu_count = pdu_count;
> seq[seq_no].xfer_len = (burstlength +
> - (cmd->se_cmd.data_length - offset));
> + (bl->data_length - offset));
> }
> unsolicited_data_length -=
> - (cmd->se_cmd.data_length - offset);
> - offset += (cmd->se_cmd.data_length - offset);
> + (bl->data_length - offset);
> + offset += (bl->data_length - offset);
> continue;
> }
> if ((offset +
> @@ -406,18 +406,18 @@ static int iscsit_do_build_pdu_and_seq_lists(
> continue;
> }
> if ((offset + conn->conn_ops->MaxRecvDataSegmentLength) >=
> - cmd->se_cmd.data_length) {
> + bl->data_length) {
> if (!datapduinorder) {
> pdu[i].type = PDUTYPE_NORMAL;
> - pdu[i].length = (cmd->se_cmd.data_length - offset);
> + pdu[i].length = (bl->data_length - offset);
> }
> if (!datasequenceinorder) {
> seq[seq_no].type = SEQTYPE_NORMAL;
> seq[seq_no].pdu_count = pdu_count;
> seq[seq_no].xfer_len = (burstlength +
> - (cmd->se_cmd.data_length - offset));
> + (bl->data_length - offset));
> }
> - offset += (cmd->se_cmd.data_length - offset);
> + offset += (bl->data_length - offset);
> continue;
> }
> if ((burstlength + conn->conn_ops->MaxRecvDataSegmentLength) >=
> @@ -496,7 +496,8 @@ static int iscsit_do_build_pdu_and_seq_lists(
>
> int iscsit_build_pdu_and_seq_lists(
> struct iscsi_cmd *cmd,
> - u32 immediate_data_length)
> + u32 immediate_data_length,
> + u32 data_length)
> {
> struct iscsi_build_list bl;
> u32 pdu_count = 0, seq_count = 1;
> @@ -518,7 +519,9 @@ int iscsit_build_pdu_and_seq_lists(
> return 0;
>
> na = iscsit_tpg_get_node_attrib(sess);
> +
> memset(&bl, 0, sizeof(struct iscsi_build_list));
> + bl.data_length = data_length;
>
> if (cmd->data_direction == DMA_FROM_DEVICE) {
> bl.data_direction = ISCSI_PDU_READ;
> diff --git a/drivers/target/iscsi/iscsi_target_seq_pdu_list.h b/drivers/target/iscsi/iscsi_target_seq_pdu_list.h
> index d5b1537..6334ec8 100644
> --- a/drivers/target/iscsi/iscsi_target_seq_pdu_list.h
> +++ b/drivers/target/iscsi/iscsi_target_seq_pdu_list.h
> @@ -46,6 +46,7 @@ struct iscsi_build_list {
> int randomize;
> int type;
> int immediate_data_length;
> + u32 data_length;
> };
>
> struct iscsi_pdu {
> @@ -78,7 +79,8 @@ struct iscsi_seq {
> u32 xfer_len;
> } ____cacheline_aligned;
>
> -extern int iscsit_build_pdu_and_seq_lists(struct iscsi_cmd *, u32);
> +int iscsit_build_pdu_and_seq_lists(struct iscsi_cmd *cmd, u32 immediate_data_length,
> + u32 data_length);
> extern struct iscsi_pdu *iscsit_get_pdu_holder(struct iscsi_cmd *, u32, u32);
> extern struct iscsi_pdu *iscsit_get_pdu_holder_for_seq(struct iscsi_cmd *, struct iscsi_seq *);
> extern struct iscsi_seq *iscsit_get_seq_holder(struct iscsi_cmd *, u32, u32);
--
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
