[PATCH 0/2] tcm_fc: Generate TASK_SET_FULL for DataIN + response failures

[PATCH 0/2] tcm_fc: Generate TASK_SET_FULL for DataIN + response failures

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi Vasu,

This series generates SAM_STAT_TASK_SET_FULL status for lport->tt.seq_send()
failures in DataIN + response status codepaths, which is done in order to get
the initiator to reduce it's current queue_depth thus reducing the number of
outstanding I/Os permitted in flight on the wire.

For the DataIN case, once a lport->tt.seq_send() failure occurs it will stop
attempting to send subsequent DataIN payload data, and immediately attempt to
send a response packet with SAM_STAT_TASK_SET_FULL status.

For the response case, once a lport->tt.seq_send() failure occurs it will
set SAM_STAT_TASK_SET_FULL status and return -ENOMEM to the target, forcing
the response to be requeued by target-core

Note this series has been compile tested only, so please review + test.

Thank you,

--nab

Nicholas Bellinger (2):
  tcm_fc: Generate TASK_SET_FULL status for DataIN failures
  tcm_fc: Generate TASK_SET_FULL status for response failures

 drivers/target/tcm_fc/tfc_cmd.c |   19 ++++++++++++++++---
 drivers/target/tcm_fc/tfc_io.c  |   14 +++++++++++++-
 2 files changed, 29 insertions(+), 4 deletions(-)

-- 
1.7.10.4

[PATCH 1/2] tcm_fc: Generate TASK_SET_FULL status for DataIN failures

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch changes ft_queue_data_in() to set SAM_STAT_TASK_SET_FULL
status upon a lport->tt.seq_send() failure, where it will now stop
sending subsequent DataIN, and immediately attempt to send the
response with exception status.

Sending a response with SAM_STAT_TASK_SET_FULL status is useful in
order to signal the initiator that it should try to reduce it's
current queue_depth, to lower the number of outstanding I/Os on
the wire.

Also, add a check to skip sending DataIN if TASK_SET_FULL status
has already been set due to a response lport->tt.seq_send()
failure, that has asked target-core to requeue a response.

Reported-by: Vasu Dev <vasu.dev@linux.intel.com>
Cc: Vasu Dev <vasu.dev@linux.intel.com>
Cc: Jun Wu <jwu@stormojo.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/tcm_fc/tfc_io.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
index e415af3..140659f 100644
--- a/drivers/target/tcm_fc/tfc_io.c
+++ b/drivers/target/tcm_fc/tfc_io.c
@@ -82,6 +82,10 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
 
 	if (cmd->aborted)
 		return 0;
+
+	if (se_cmd->scsi_status == SAM_STAT_TASK_SET_FULL)
+		goto queue_status;
+
 	ep = fc_seq_exch(cmd->seq);
 	lport = ep->lp;
 	cmd->seq = lport->tt.seq_start_next(cmd->seq);
@@ -178,14 +182,22 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
 			       FC_TYPE_FCP, f_ctl, fh_off);
 		error = lport->tt.seq_send(lport, seq, fp);
 		if (error) {
-			/* XXX For now, initiator will retry */
 			pr_err_ratelimited("%s: Failed to send frame %p, "
 						"xid <0x%x>, remaining %zu, "
 						"lso_max <0x%x>\n",
 						__func__, fp, ep->xid,
 						remaining, lport->lso_max);
+			/*
+			 * Generate a TASK_SET_FULL status to notify the
+			 * initiator to reduce it's queue_depth, ignoring
+			 * the rest of the data-in and immediately attempt
+			 * to send the response.
+			 */
+			se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
+			break;
 		}
 	}
+queue_status:
 	return ft_queue_status(se_cmd);
 }
 
-- 
1.7.10.4

[PATCH 2/2] tcm_fc: Generate TASK_SET_FULL status for response failures

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch changes ft_queue_status() to set SAM_STAT_TASK_SET_FULL
status upon lport->tt.seq_send( failure, and return -EAGAIN to notify
target-core to attempt to requeue the response.

It also does the same for a fc_frame_alloc() failures, in order to
signal the initiator that it should try to reduce it's current
queue_depth, to lower the number of outstanding I/Os on the wire.

Reported-by: Vasu Dev <vasu.dev@linux.intel.com>
Cc: Vasu Dev <vasu.dev@linux.intel.com>
Cc: Jun Wu <jwu@stormojo.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/tcm_fc/tfc_cmd.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index f5fd515..5585038 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -128,6 +128,7 @@ int ft_queue_status(struct se_cmd *se_cmd)
 	struct fc_lport *lport;
 	struct fc_exch *ep;
 	size_t len;
+	int rc;
 
 	if (cmd->aborted)
 		return 0;
@@ -137,9 +138,10 @@ int ft_queue_status(struct se_cmd *se_cmd)
 	len = sizeof(*fcp) + se_cmd->scsi_sense_length;
 	fp = fc_frame_alloc(lport, len);
 	if (!fp) {
-		/* XXX shouldn't just drop it - requeue and retry? */
-		return 0;
+		se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
+		return -ENOMEM;
 	}
+
 	fcp = fc_frame_payload_get(fp, len);
 	memset(fcp, 0, len);
 	fcp->resp.fr_status = se_cmd->scsi_status;
@@ -170,7 +172,18 @@ int ft_queue_status(struct se_cmd *se_cmd)
 	fc_fill_fc_hdr(fp, FC_RCTL_DD_CMD_STATUS, ep->did, ep->sid, FC_TYPE_FCP,
 		       FC_FC_EX_CTX | FC_FC_LAST_SEQ | FC_FC_END_SEQ, 0);
 
-	lport->tt.seq_send(lport, cmd->seq, fp);
+	rc = lport->tt.seq_send(lport, cmd->seq, fp);
+	if (rc) {
+		pr_err_ratelimited("%s: Failed to send response frame %p, "
+				   "xid <0x%x>\n", __func__, fp, ep->xid);
+		/*
+		 * Generate a TASK_SET_FULL status to notify the initiator
+		 * to reduce it's queue_depth after the se_cmd response has
+		 * been re-queued by target-core.
+		 */
+		se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
+		return -ENOMEM;
+	}
 	lport->tt.exch_done(cmd->seq);
 	return 0;
 }
-- 
1.7.10.4

Re: [PATCH 0/2] tcm_fc: Generate TASK_SET_FULL for DataIN + response failures

[Vasu Dev]

On Thu, 2014-06-05 at 23:30 +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi Vasu,
> 
> This series generates SAM_STAT_TASK_SET_FULL status for lport->tt.seq_send()
> failures in DataIN + response status codepaths, which is done in order to get
> the initiator to reduce it's current queue_depth thus reducing the number of
> outstanding I/Os permitted in flight on the wire.
> 
> For the DataIN case, once a lport->tt.seq_send() failure occurs it will stop
> attempting to send subsequent DataIN payload data, and immediately attempt to
> send a response packet with SAM_STAT_TASK_SET_FULL status.
> 
> For the response case, once a lport->tt.seq_send() failure occurs it will
> set SAM_STAT_TASK_SET_FULL status and return -ENOMEM to the target, forcing
> the response to be requeued by target-core
> 

Good change and it was missing in tcm_fc.

> Note this series has been compile tested only, so please review + test.

Just did review and basic test but should be stressed to really ensure
it does work & fixes issues resulting these patches.

Thanks,
Vasu

> Thank you,
> 
> --nab
> 
> Nicholas Bellinger (2):
>   tcm_fc: Generate TASK_SET_FULL status for DataIN failures
>   tcm_fc: Generate TASK_SET_FULL status for response failures
> 
>  drivers/target/tcm_fc/tfc_cmd.c |   19 ++++++++++++++++---
>  drivers/target/tcm_fc/tfc_io.c  |   14 +++++++++++++-
>  2 files changed, 29 insertions(+), 4 deletions(-)
> 

Re: [PATCH 1/2] tcm_fc: Generate TASK_SET_FULL status for DataIN failures

[Vasu Dev]

On Thu, 2014-06-05 at 23:30 +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch changes ft_queue_data_in() to set SAM_STAT_TASK_SET_FULL
> status upon a lport->tt.seq_send() failure, where it will now stop
> sending subsequent DataIN, and immediately attempt to send the
> response with exception status.
> 
> Sending a response with SAM_STAT_TASK_SET_FULL status is useful in
> order to signal the initiator that it should try to reduce it's
> current queue_depth, to lower the number of outstanding I/Os on
> the wire.
> 
> Also, add a check to skip sending DataIN if TASK_SET_FULL status
> has already been set due to a response lport->tt.seq_send()
> failure, that has asked target-core to requeue a response.
> 
> Reported-by: Vasu Dev <vasu.dev@linux.intel.com>
> Cc: Vasu Dev <vasu.dev@linux.intel.com>
> Cc: Jun Wu <jwu@stormojo.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/target/tcm_fc/tfc_io.c |   14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
> index e415af3..140659f 100644
> --- a/drivers/target/tcm_fc/tfc_io.c
> +++ b/drivers/target/tcm_fc/tfc_io.c
> @@ -82,6 +82,10 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
>  
>  	if (cmd->aborted)
>  		return 0;
> +
> +	if (se_cmd->scsi_status == SAM_STAT_TASK_SET_FULL)
> +		goto queue_status;
> +
>  	ep = fc_seq_exch(cmd->seq);
>  	lport = ep->lp;
>  	cmd->seq = lport->tt.seq_start_next(cmd->seq);
> @@ -178,14 +182,22 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
>  			       FC_TYPE_FCP, f_ctl, fh_off);
>  		error = lport->tt.seq_send(lport, seq, fp);
>  		if (error) {
> -			/* XXX For now, initiator will retry */
>  			pr_err_ratelimited("%s: Failed to send frame %p, "
>  						"xid <0x%x>, remaining %zu, "
>  						"lso_max <0x%x>\n",
>  						__func__, fp, ep->xid,
>  						remaining, lport->lso_max);
> +			/*
> +			 * Generate a TASK_SET_FULL status to notify the
> +			 * initiator to reduce it's queue_depth, ignoring
> +			 * the rest of the data-in and immediately attempt
> +			 * to send the response.
> +			 */

I see above added check will drop rest of the frames but cannot find
notifying to initiator with the TASK_SET_FULL status as in comments
above, may be just the comment needs update otherwise code changes are
fine.

> +			se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
> +			break;
>  		}
>  	}
> +queue_status:
>  	return ft_queue_status(se_cmd);
>  }
>  

Reviewed-by: Vasu Dev <vasu.dev@intel.com> 

Re: [PATCH 1/2] tcm_fc: Generate TASK_SET_FULL status for DataIN failures

[Nicholas A. Bellinger]

On Fri, 2014-06-06 at 13:51 -0700, Vasu Dev wrote:
> On Thu, 2014-06-05 at 23:30 +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
> > 
> > This patch changes ft_queue_data_in() to set SAM_STAT_TASK_SET_FULL
> > status upon a lport->tt.seq_send() failure, where it will now stop
> > sending subsequent DataIN, and immediately attempt to send the
> > response with exception status.
> > 
> > Sending a response with SAM_STAT_TASK_SET_FULL status is useful in
> > order to signal the initiator that it should try to reduce it's
> > current queue_depth, to lower the number of outstanding I/Os on
> > the wire.
> > 
> > Also, add a check to skip sending DataIN if TASK_SET_FULL status
> > has already been set due to a response lport->tt.seq_send()
> > failure, that has asked target-core to requeue a response.
> > 
> > Reported-by: Vasu Dev <vasu.dev-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > Cc: Vasu Dev <vasu.dev-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > Cc: Jun Wu <jwu-kEM2MQtWpIJWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Nicholas Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
> > ---
> >  drivers/target/tcm_fc/tfc_io.c |   14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
> > index e415af3..140659f 100644
> > --- a/drivers/target/tcm_fc/tfc_io.c
> > +++ b/drivers/target/tcm_fc/tfc_io.c
> > @@ -82,6 +82,10 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
> >  
> >  	if (cmd->aborted)
> >  		return 0;
> > +
> > +	if (se_cmd->scsi_status == SAM_STAT_TASK_SET_FULL)
> > +		goto queue_status;
> > +
> >  	ep = fc_seq_exch(cmd->seq);
> >  	lport = ep->lp;
> >  	cmd->seq = lport->tt.seq_start_next(cmd->seq);
> > @@ -178,14 +182,22 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
> >  			       FC_TYPE_FCP, f_ctl, fh_off);
> >  		error = lport->tt.seq_send(lport, seq, fp);
> >  		if (error) {
> > -			/* XXX For now, initiator will retry */
> >  			pr_err_ratelimited("%s: Failed to send frame %p, "
> >  						"xid <0x%x>, remaining %zu, "
> >  						"lso_max <0x%x>\n",
> >  						__func__, fp, ep->xid,
> >  						remaining, lport->lso_max);
> > +			/*
> > +			 * Generate a TASK_SET_FULL status to notify the
> > +			 * initiator to reduce it's queue_depth, ignoring
> > +			 * the rest of the data-in and immediately attempt
> > +			 * to send the response.
> > +			 */
> 
> I see above added check will drop rest of the frames but cannot find
> notifying to initiator with the TASK_SET_FULL status as in comments
> above, may be just the comment needs update otherwise code changes are
> fine.
> 

The break aborts the DataIN send loop and invokes ft_queue_status()
below in an attempt to send TASK_SET_FULL status.

If the ft_queue_status() -> lport->tt.seq_send() also fails, then
-ENOMEM will be returned to the target and a delayed re-queue attempt
will be made.

In any event, updating the comment above to be more precise.. 

> > +			se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
> > +			break;
> >  		}
> >  	}
> > +queue_status:
> >  	return ft_queue_status(se_cmd);
> >  }
> >  
> 
> Reviewed-by: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 
> 

Thanks Vasu!

--nab

Re: [PATCH 2/2] tcm_fc: Generate TASK_SET_FULL status for response failures

[Vasu Dev]

On Thu, 2014-06-05 at 23:30 +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
> 
> This patch changes ft_queue_status() to set SAM_STAT_TASK_SET_FULL
> status upon lport->tt.seq_send( failure, and return -EAGAIN to notify
> target-core to attempt to requeue the response.
> 
> It also does the same for a fc_frame_alloc() failures, in order to
> signal the initiator that it should try to reduce it's current
> queue_depth, to lower the number of outstanding I/Os on the wire.
> 
> Reported-by: Vasu Dev <vasu.dev-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Cc: Vasu Dev <vasu.dev-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Cc: Jun Wu <jwu-kEM2MQtWpIJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Nicholas Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
> ---
>  drivers/target/tcm_fc/tfc_cmd.c |   19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
> index f5fd515..5585038 100644
> --- a/drivers/target/tcm_fc/tfc_cmd.c
> +++ b/drivers/target/tcm_fc/tfc_cmd.c
> @@ -128,6 +128,7 @@ int ft_queue_status(struct se_cmd *se_cmd)
>  	struct fc_lport *lport;
>  	struct fc_exch *ep;
>  	size_t len;
> +	int rc;
>  
>  	if (cmd->aborted)
>  		return 0;
> @@ -137,9 +138,10 @@ int ft_queue_status(struct se_cmd *se_cmd)
>  	len = sizeof(*fcp) + se_cmd->scsi_sense_length;
>  	fp = fc_frame_alloc(lport, len);
>  	if (!fp) {
> -		/* XXX shouldn't just drop it - requeue and retry? */
> -		return 0;
> +		se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
> +		return -ENOMEM;
>  	}
> +
>  	fcp = fc_frame_payload_get(fp, len);
>  	memset(fcp, 0, len);
>  	fcp->resp.fr_status = se_cmd->scsi_status;
> @@ -170,7 +172,18 @@ int ft_queue_status(struct se_cmd *se_cmd)
>  	fc_fill_fc_hdr(fp, FC_RCTL_DD_CMD_STATUS, ep->did, ep->sid, FC_TYPE_FCP,
>  		       FC_FC_EX_CTX | FC_FC_LAST_SEQ | FC_FC_END_SEQ, 0);
>  
> -	lport->tt.seq_send(lport, cmd->seq, fp);
> +	rc = lport->tt.seq_send(lport, cmd->seq, fp);
> +	if (rc) {
> +		pr_err_ratelimited("%s: Failed to send response frame %p, "
> +				   "xid <0x%x>\n", __func__, fp, ep->xid);

May be should be pr_info_ratelimited since this is not really error and
IO could get aborted due to timeouts in any setups or stressed
workloads.

> +		/*
> +		 * Generate a TASK_SET_FULL status to notify the initiator
> +		 * to reduce it's queue_depth after the se_cmd response has
> +		 * been re-queued by target-core.
> +		 */
> +		se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
> +		return -ENOMEM;
> +	}
>  	lport->tt.exch_done(cmd->seq);
>  	return 0;
>  }

Reviewed-by: Vasu Dev <vasu.dev-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 

Re: [PATCH 0/2] tcm_fc: Generate TASK_SET_FULL for DataIN + response failures

[Nicholas A. Bellinger]

On Fri, 2014-06-06 at 13:38 -0700, Vasu Dev wrote:
> On Thu, 2014-06-05 at 23:30 +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > Hi Vasu,
> > 
> > This series generates SAM_STAT_TASK_SET_FULL status for lport->tt.seq_send()
> > failures in DataIN + response status codepaths, which is done in order to get
> > the initiator to reduce it's current queue_depth thus reducing the number of
> > outstanding I/Os permitted in flight on the wire.
> > 
> > For the DataIN case, once a lport->tt.seq_send() failure occurs it will stop
> > attempting to send subsequent DataIN payload data, and immediately attempt to
> > send a response packet with SAM_STAT_TASK_SET_FULL status.
> > 
> > For the response case, once a lport->tt.seq_send() failure occurs it will
> > set SAM_STAT_TASK_SET_FULL status and return -ENOMEM to the target, forcing
> > the response to be requeued by target-core
> > 
> 
> Good change and it was missing in tcm_fc.
> 
> > Note this series has been compile tested only, so please review + test.
> 
> Just did review and basic test but should be stressed to really ensure
> it does work & fixes issues resulting these patches.

So if you don't mind I'll go ahead and queue these up for now in
target-pending/for-next, given they are pretty straight-forward fixes.

If they end up being problematic, they can be dropped before the v3.16
PULL request goes out next week.

Jun can you please test both cases..?  Eg: the first with an explicitly
reduced fcoe initiator queue_depth setting mentioned earlier, and the
second with these two patches to return TASK_SET_FULL when the sequence
failures start occurring.

Thanks,

--nab

Re: [PATCH 2/2] tcm_fc: Generate TASK_SET_FULL status for response failures

[Nicholas A. Bellinger]

On Fri, 2014-06-06 at 14:03 -0700, Vasu Dev wrote:
> On Thu, 2014-06-05 at 23:30 +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > This patch changes ft_queue_status() to set SAM_STAT_TASK_SET_FULL
> > status upon lport->tt.seq_send( failure, and return -EAGAIN to notify
> > target-core to attempt to requeue the response.
> > 
> > It also does the same for a fc_frame_alloc() failures, in order to
> > signal the initiator that it should try to reduce it's current
> > queue_depth, to lower the number of outstanding I/Os on the wire.
> > 
> > Reported-by: Vasu Dev <vasu.dev@linux.intel.com>
> > Cc: Vasu Dev <vasu.dev@linux.intel.com>
> > Cc: Jun Wu <jwu@stormojo.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > ---
> >  drivers/target/tcm_fc/tfc_cmd.c |   19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
> > index f5fd515..5585038 100644
> > --- a/drivers/target/tcm_fc/tfc_cmd.c
> > +++ b/drivers/target/tcm_fc/tfc_cmd.c
> > @@ -128,6 +128,7 @@ int ft_queue_status(struct se_cmd *se_cmd)
> >  	struct fc_lport *lport;
> >  	struct fc_exch *ep;
> >  	size_t len;
> > +	int rc;
> >  
> >  	if (cmd->aborted)
> >  		return 0;
> > @@ -137,9 +138,10 @@ int ft_queue_status(struct se_cmd *se_cmd)
> >  	len = sizeof(*fcp) + se_cmd->scsi_sense_length;
> >  	fp = fc_frame_alloc(lport, len);
> >  	if (!fp) {
> > -		/* XXX shouldn't just drop it - requeue and retry? */
> > -		return 0;
> > +		se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
> > +		return -ENOMEM;
> >  	}
> > +
> >  	fcp = fc_frame_payload_get(fp, len);
> >  	memset(fcp, 0, len);
> >  	fcp->resp.fr_status = se_cmd->scsi_status;
> > @@ -170,7 +172,18 @@ int ft_queue_status(struct se_cmd *se_cmd)
> >  	fc_fill_fc_hdr(fp, FC_RCTL_DD_CMD_STATUS, ep->did, ep->sid, FC_TYPE_FCP,
> >  		       FC_FC_EX_CTX | FC_FC_LAST_SEQ | FC_FC_END_SEQ, 0);
> >  
> > -	lport->tt.seq_send(lport, cmd->seq, fp);
> > +	rc = lport->tt.seq_send(lport, cmd->seq, fp);
> > +	if (rc) {
> > +		pr_err_ratelimited("%s: Failed to send response frame %p, "
> > +				   "xid <0x%x>\n", __func__, fp, ep->xid);
> 
> May be should be pr_info_ratelimited since this is not really error and
> IO could get aborted due to timeouts in any setups or stressed
> workloads.

<nod>, changing this to pr_info_ratelimited and doing the same in
ft_queue_data_in() well.

> 
> > +		/*
> > +		 * Generate a TASK_SET_FULL status to notify the initiator
> > +		 * to reduce it's queue_depth after the se_cmd response has
> > +		 * been re-queued by target-core.
> > +		 */
> > +		se_cmd->scsi_status = SAM_STAT_TASK_SET_FULL;
> > +		return -ENOMEM;
> > +	}
> >  	lport->tt.exch_done(cmd->seq);
> >  	return 0;
> >  }
> 
> Reviewed-by: Vasu Dev <vasu.dev@intel.com> 
> 

Thanks Vasu!

--nab

Re: [PATCH 0/2] tcm_fc: Generate TASK_SET_FULL for DataIN + response failures

[Vasu Dev]

On Fri, 2014-06-06 at 14:09 -0700, Nicholas A. Bellinger wrote:
> 
> So if you don't mind I'll go ahead and queue these up for now in
> target-pending/for-next, given they are pretty straight-forward fixes.
> 
> If they end up being problematic, they can be dropped before the v3.16
> PULL request goes out next week. 

All make sense, so go head with the push and I already added reviewed by
to both patches.

//Vasu

Re: [PATCH 2/2] tcm_fc: Generate TASK_SET_FULL status for response failures

[Vasu Dev]

On Fri, 2014-06-06 at 14:11 -0700, Nicholas A. Bellinger wrote:
> <nod>, changing this to pr_info_ratelimited and doing the same in
> ft_queue_data_in() well. 

Since doing more places and therefore can be done in separate patch.

//Vasu

Re: [PATCH 1/2] tcm_fc: Generate TASK_SET_FULL status for DataIN failures

[Vasu Dev]

On Fri, 2014-06-06 at 14:02 -0700, Nicholas A. Bellinger wrote:
> The break aborts the DataIN send loop and invokes ft_queue_status()
> below in an attempt to send TASK_SET_FULL status.
> 
> If the ft_queue_status() -> lport->tt.seq_send() also fails, then
> -ENOMEM will be returned to the target and a delayed re-queue attempt
> will be made.

I see.

> In any event, updating the comment above to be more precise 

I'm ok with leaving comment as is for now, just to avoid patch re-spin
for this after above clarification. Thanks for detailed clarification

//Vasu