Re: [PATCH 27/29] target/iscsi: Do not touch se_cmd in initializing pdu and seq lists

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


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


[Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Photos]     [Yosemite]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

Add to Google Powered by Linux