[PATCH v2 0/6] hisi_sas: add abort and retry feature

[PATCH v2 0/6] hisi_sas: add abort and retry feature

[John Garry]

This patchset introduces support to abort
certain commands which have failed and retry.
Certain errors require that the command be
retried, like TRANS_TX_CREDIT_TIMEOUT_ERR in
v1 hw.
However, when these errors occur the IO may
still be in the device, so the IO must first
be aborted, and then retried. The HiSilicon
SAS controller has no FW to do this work, so
it needs to be done manually.

Differences to v1:
 - check for tmf success result removed
 - remove query for task prior to abort
 - add flag to slot structure for task abort
 - add removal of superflous debug statement

John Garry (6):
  hisi_sas: change tmf func complete check
  hisi_sas: add hisi_sas_slot_abort()
  hisi_sas: use slot abort in v1 hw
  hisi_sas: use slot abort in v2 hw
  hisi_sas: add hisi_sas_slave_configure()
  hisi_sas: update driver version to 1.3

 drivers/scsi/hisi_sas/hisi_sas.h       |  4 ++-
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 59 ++++++++++++++++++++++++++++++++--
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 13 ++++++++
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 15 +++++++--
 4 files changed, 85 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH v2 1/6] hisi_sas: change tmf func complete check

[John Garry]

In hisi_sas_exec_internal_tmf_task(), the check for
SAM_STAT_GOOD is replaced with
TMF_RESP_FUNC_COMPLETE, which is a genuine tmf
response code.
SAM_STAT_GOOD and TMF_RESP_FUNC_COMPLETE have the
same value, so this is why it worked before.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 2194917..58ca336 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -661,7 +661,7 @@ static int hisi_sas_exec_internal_tmf_task(struct domain_device *device,
 		}
 
 		if (task->task_status.resp == SAS_TASK_COMPLETE &&
-		    task->task_status.stat == SAM_STAT_GOOD) {
+		     task->task_status.stat == TMF_RESP_FUNC_COMPLETE) {
 			res = TMF_RESP_FUNC_COMPLETE;
 			break;
 		}
-- 
1.9.1

[PATCH v2 2/6] hisi_sas: add hisi_sas_slot_abort()

[John Garry]

Add a function to abort a slot (task) in the target
device and then cleanup and complete the task.
The function is called from work queue context as
it cannot be called from the context where it is
triggered (interrupt).
Flag hisi_sas_slot.abort is added as the flag used
in the slot error handler to indicate whether the
slot needs to be aborted in the sdev prior to
cleanup and finish.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas.h      |  2 ++
 drivers/scsi/hisi_sas/hisi_sas_main.c | 42 +++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 02da7e4..c92e65b 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -112,6 +112,7 @@ struct hisi_sas_slot {
 	int	cmplt_queue;
 	int	cmplt_queue_slot;
 	int	idx;
+	int	abort;
 	void	*cmd_hdr;
 	dma_addr_t cmd_hdr_dma;
 	void	*status_buffer;
@@ -120,6 +121,7 @@ struct hisi_sas_slot {
 	dma_addr_t command_table_dma;
 	struct hisi_sas_sge_page *sge_page;
 	dma_addr_t sge_page_dma;
+	struct work_struct abort_slot;
 };
 
 struct hisi_sas_tmf_task {
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 58ca336..e51612f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -15,6 +15,9 @@
 #define DEV_IS_GONE(dev) \
 	((!dev) || (dev->dev_type == SAS_PHY_UNUSED))
 
+static int hisi_sas_debug_issue_ssp_tmf(struct domain_device *device,
+				u8 *lun, struct hisi_sas_tmf_task *tmf);
+
 static struct hisi_hba *dev_to_hisi_hba(struct domain_device *device)
 {
 	return device->port->ha->lldd_ha;
@@ -113,6 +116,44 @@ static int hisi_sas_task_prep_ata(struct hisi_hba *hisi_hba,
 	return hisi_hba->hw->prep_stp(hisi_hba, slot);
 }
 
+/*
+ * This function will issue an abort TMF regardless of whether the
+ * task is in the sdev or not. Then it will do the task complete
+ * cleanup and callbacks.
+ */
+static void hisi_sas_slot_abort(struct work_struct *work)
+{
+	struct hisi_sas_slot *abort_slot =
+		container_of(work, struct hisi_sas_slot, abort_slot);
+	struct sas_task *task = abort_slot->task;
+	struct hisi_hba *hisi_hba = dev_to_hisi_hba(task->dev);
+	struct scsi_cmnd *cmnd = task->uldd_task;
+	struct hisi_sas_tmf_task tmf_task;
+	struct domain_device *device = task->dev;
+	struct hisi_sas_device *sas_dev = device->lldd_dev;
+	struct scsi_lun lun;
+	struct device *dev = &hisi_hba->pdev->dev;
+	int tag = abort_slot->idx;
+
+	if (!(task->task_proto & SAS_PROTOCOL_SSP)) {
+		dev_err(dev, "cannot abort slot for non-ssp task\n");
+		goto out;
+	}
+
+	int_to_scsilun(cmnd->device->lun, &lun);
+	tmf_task.tmf = TMF_ABORT_TASK;
+	tmf_task.tag_of_task_to_be_managed = cpu_to_le16(tag);
+
+	hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun, &tmf_task);
+out:
+	/* Do cleanup for this task */
+	hisi_sas_slot_task_free(hisi_hba, task, abort_slot);
+	if (task->task_done)
+		task->task_done(task);
+	if (sas_dev && sas_dev->running_req)
+		sas_dev->running_req--;
+}
+
 static int hisi_sas_task_prep(struct sas_task *task, struct hisi_hba *hisi_hba,
 			      int is_tmf, struct hisi_sas_tmf_task *tmf,
 			      int *pass)
@@ -206,6 +247,7 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_hba *hisi_hba,
 	slot->task = task;
 	slot->port = port;
 	task->lldd_task = slot;
+	INIT_WORK(&slot->abort_slot, hisi_sas_slot_abort);
 
 	slot->status_buffer = dma_pool_alloc(hisi_hba->status_buffer_pool,
 					     GFP_ATOMIC,
-- 
1.9.1

[PATCH v2 3/6] hisi_sas: use slot abort in v1 hw

[John Garry]

When TRANS_TX_CREDIT_TIMEOUT_ERR or
TRANS_TX_CLOSE_NORMAL_ERR error occur in
a slot, the command should be re-attempted.
This error is equivalent to meaning that the queue
is full in the sdev (and not the host).

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index ce5f65d..1abbc2e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1212,6 +1212,14 @@ static void slot_err_v1_hw(struct hisi_hba *hisi_hba,
 			ts->stat = SAS_NAK_R_ERR;
 			break;
 		}
+		case TRANS_TX_CREDIT_TIMEOUT_ERR:
+		case TRANS_TX_CLOSE_NORMAL_ERR:
+		{
+			/* This will request a retry */
+			ts->stat = SAS_QUEUE_FULL;
+			slot->abort = 1;
+			break;
+		}
 		default:
 		{
 			ts->stat = SAM_STAT_CHECK_CONDITION;
@@ -1319,6 +1327,11 @@ static int slot_complete_v1_hw(struct hisi_hba *hisi_hba,
 		!(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
 
 		slot_err_v1_hw(hisi_hba, task, slot);
+		if (unlikely(slot->abort)) {
+			queue_work(hisi_hba->wq, &slot->abort_slot);
+			/* immediately return and do not complete */
+			return ts->stat;
+		}
 		goto out;
 	}
 
-- 
1.9.1

[PATCH v2 4/6] hisi_sas: use slot abort in v2 hw

[John Garry]

When TRANS_TX_ERR_FRAME_TXED error occurs in
a slot, the command should be re-attempted.
This error is equivalent to meaning that the queue
is full in the sdev (and not the host).
A superflous debug statement is also removed in the
slot complete handler.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 58e1956..b733747 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1299,6 +1299,13 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
 			ts->stat = SAS_DATA_UNDERRUN;
 			break;
 		}
+		case TRANS_TX_ERR_FRAME_TXED:
+		{
+			/* This will request a retry */
+			ts->stat = SAS_QUEUE_FULL;
+			slot->abort = 1;
+			break;
+		}
 		case TRANS_TX_OPEN_FAIL_WITH_IT_NEXUS_LOSS:
 		case TRANS_TX_ERR_PHY_NOT_ENABLE:
 		case TRANS_TX_OPEN_CNX_ERR_BY_OTHER:
@@ -1491,11 +1498,13 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_slot *slot,
 
 	if ((complete_hdr->dw0 & CMPLT_HDR_ERX_MSK) &&
 		(!(complete_hdr->dw0 & CMPLT_HDR_RSPNS_XFRD_MSK))) {
-		dev_dbg(dev, "%s slot %d has error info 0x%x\n",
-			__func__, slot->cmplt_queue_slot,
-			complete_hdr->dw0 & CMPLT_HDR_ERX_MSK);
 
 		slot_err_v2_hw(hisi_hba, task, slot);
+		if (unlikely(slot->abort)) {
+			queue_work(hisi_hba->wq, &slot->abort_slot);
+			/* immediately return and do not complete */
+			return ts->stat;
+		}
 		goto out;
 	}
 
-- 
1.9.1

[PATCH v2 5/6] hisi_sas: add hisi_sas_slave_configure()

[John Garry]

In high-datarate aging tests, it is found that the
SCSI framework can periodically issue lu resets as
some commands timeout.
Response TASK SET FULL and SAS_QUEUE_FULL may be
returned many times for the same command, causing the
timeouts.
The SAS_QUEUE_FULL errors come from
TRANS_TX_CREDIT_TIMEOUT_ERR, TRANS_TX_CLOSE_NORMAL_ERR,
and TRANS_TX_ERR_FRAME_TXED errors. They do not mean
that the queue is full in the host, but rather it is
equivalent to meaning the queue is full for the sdev.
To overcome this, the queue depth for the sdev is
reduced to 64 (from 256, set in sas_slave_configure()).
Normally error code SAS_QUEUE_FULL will result in the
sdev queue depth falling, but it falls too slowly during
high-datarate tests and commands timeout before it
has fallen to an adequete level from original value.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index e51612f..097ab4f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -453,6 +453,19 @@ static int hisi_sas_dev_found(struct domain_device *device)
 	return 0;
 }
 
+static int hisi_sas_slave_configure(struct scsi_device *sdev)
+{
+	struct domain_device *dev = sdev_to_domain_dev(sdev);
+	int ret = sas_slave_configure(sdev);
+
+	if (ret)
+		return ret;
+	if (!dev_is_sata(dev))
+		sas_change_queue_depth(sdev, 64);
+
+	return 0;
+}
+
 static void hisi_sas_scan_start(struct Scsi_Host *shost)
 {
 	struct hisi_hba *hisi_hba = shost_priv(shost);
@@ -990,7 +1003,7 @@ static struct scsi_host_template hisi_sas_sht = {
 	.name			= DRV_NAME,
 	.queuecommand		= sas_queuecommand,
 	.target_alloc		= sas_target_alloc,
-	.slave_configure	= sas_slave_configure,
+	.slave_configure	= hisi_sas_slave_configure,
 	.scan_finished		= hisi_sas_scan_finished,
 	.scan_start		= hisi_sas_scan_start,
 	.change_queue_depth	= sas_change_queue_depth,
-- 
1.9.1

[PATCH v2 6/6] hisi_sas: update driver version to 1.3

[John Garry]

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index c92e65b..29e89f3 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -23,7 +23,7 @@
 #include <scsi/sas_ata.h>
 #include <scsi/libsas.h>
 
-#define DRV_VERSION "v1.2"
+#define DRV_VERSION "v1.3"
 
 #define HISI_SAS_MAX_PHYS	9
 #define HISI_SAS_MAX_QUEUES	32
-- 
1.9.1

Re: [PATCH v2 1/6] hisi_sas: change tmf func complete check

[Hannes Reinecke]

On 02/25/2016 05:42 PM, John Garry wrote:
> In hisi_sas_exec_internal_tmf_task(), the check for
> SAM_STAT_GOOD is replaced with
> TMF_RESP_FUNC_COMPLETE, which is a genuine tmf
> response code.
> SAM_STAT_GOOD and TMF_RESP_FUNC_COMPLETE have the
> same value, so this is why it worked before.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH v2 2/6] hisi_sas: add hisi_sas_slot_abort()

[Hannes Reinecke]

On 02/25/2016 05:42 PM, John Garry wrote:
> Add a function to abort a slot (task) in the target
> device and then cleanup and complete the task.
> The function is called from work queue context as
> it cannot be called from the context where it is
> triggered (interrupt).
> Flag hisi_sas_slot.abort is added as the flag used
> in the slot error handler to indicate whether the
> slot needs to be aborted in the sdev prior to
> cleanup and finish.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH v2 3/6] hisi_sas: use slot abort in v1 hw

[Hannes Reinecke]

On 02/25/2016 05:42 PM, John Garry wrote:
> When TRANS_TX_CREDIT_TIMEOUT_ERR or
> TRANS_TX_CLOSE_NORMAL_ERR error occur in
> a slot, the command should be re-attempted.
> This error is equivalent to meaning that the queue
> is full in the sdev (and not the host).
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH v2 4/6] hisi_sas: use slot abort in v2 hw

[Hannes Reinecke]

On 02/25/2016 05:42 PM, John Garry wrote:
> When TRANS_TX_ERR_FRAME_TXED error occurs in
> a slot, the command should be re-attempted.
> This error is equivalent to meaning that the queue
> is full in the sdev (and not the host).
> A superflous debug statement is also removed in the
> slot complete handler.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH v2 5/6] hisi_sas: add hisi_sas_slave_configure()

[Hannes Reinecke]

On 02/25/2016 05:42 PM, John Garry wrote:
> In high-datarate aging tests, it is found that the
> SCSI framework can periodically issue lu resets as
> some commands timeout.
> Response TASK SET FULL and SAS_QUEUE_FULL may be
> returned many times for the same command, causing the
> timeouts.
> The SAS_QUEUE_FULL errors come from
> TRANS_TX_CREDIT_TIMEOUT_ERR, TRANS_TX_CLOSE_NORMAL_ERR,
> and TRANS_TX_ERR_FRAME_TXED errors. They do not mean
> that the queue is full in the host, but rather it is
> equivalent to meaning the queue is full for the sdev.
> To overcome this, the queue depth for the sdev is
> reduced to 64 (from 256, set in sas_slave_configure()).
> Normally error code SAS_QUEUE_FULL will result in the
> sdev queue depth falling, but it falls too slowly during
> high-datarate tests and commands timeout before it
> has fallen to an adequete level from original value.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH v2 6/6] hisi_sas: update driver version to 1.3

[Hannes Reinecke]

On 02/25/2016 05:42 PM, John Garry wrote:
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
> index c92e65b..29e89f3 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas.h
> +++ b/drivers/scsi/hisi_sas/hisi_sas.h
> @@ -23,7 +23,7 @@
>  #include <scsi/sas_ata.h>
>  #include <scsi/libsas.h>
>  
> -#define DRV_VERSION "v1.2"
> +#define DRV_VERSION "v1.3"
>  
>  #define HISI_SAS_MAX_PHYS	9
>  #define HISI_SAS_MAX_QUEUES	32
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH v2 0/6] hisi_sas: add abort and retry feature

[Martin K. Petersen]

>>>>> "John" == John Garry <john.garry@huawei.com> writes:

John> This patchset introduces support to abort certain commands which
John> have failed and retry.  Certain errors require that the command be
John> retried, like TRANS_TX_CREDIT_TIMEOUT_ERR in v1 hw.  However, when
John> these errors occur the IO may still be in the device, so the IO
John> must first be aborted, and then retried. The HiSilicon SAS
John> controller has no FW to do this work, so it needs to be done
John> manually.

Applied to 4.6/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering