[PATCH 0/6] hisi_sas: improve DQ locking

[PATCH 0/6] hisi_sas: improve DQ locking

[John Garry]

This patchset introduces some patches to much
improve DQ lockout for sending commands to the
HW.

Currently we lockout the complete DQ when building
and sending a task to the HW.

The reason we did this was because once we allocate
a slot in the DQ to use, if we fail the send the
slot then the slot must be reused. We cannot simply
forget about the slot and allow subsequent slots
in the DQ to be used.

To improve this and reduce this DQ lockout, we change
the order in which we allocate and build a slot.

We must now do any steps *which may fail* before
allocating the slot. So this means that once we
allocate the slot we cannot fail to send it.

By doing this we can greatly reduce the periods
in which we lock the DQ for building and sending
a slot, allowing more parallelism in sending
commands to HW.

Overall this DQ locking improvement has shown to
improve performance, and also will make MQ perform
better if ever turned on.

Xiang Chen (5):
  scsi: hisi_sas: relocate smp sg map
  scsi: hisi_sas: make return type of prep functions void
  scsi: hisi_sas: allocate slot buffer earlier
  scsi: hisi_sas: Don't lock DQ for complete task sending
  scsi: hisi_sas: Use device lock to protect slot alloc/free

Xiaofei Tan (1):
  scsi: hisi_sas: add check of device in hisi_sas_task_exec()

 drivers/scsi/hisi_sas/hisi_sas.h       |  12 +-
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 264 ++++++++++++++++++---------------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c |  88 ++++-------
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 109 +++++---------
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 107 +++++--------
 5 files changed, 248 insertions(+), 332 deletions(-)

-- 
1.9.1

[PATCH 0/6] hisi_sas: improve DQ locking

[John Garry]

This patchset introduces some patches to much
improve DQ lockout for sending commands to the
HW.

Currently we lockout the complete DQ when building
and sending a task to the HW.

The reason we did this was because once we allocate
a slot in the DQ to use, if we fail the send the
slot then the slot must be reused. We cannot simply
forget about the slot and allow subsequent slots
in the DQ to be used.

To improve this and reduce this DQ lockout, we change
the order in which we allocate and build a slot.

We must now do any steps *which may fail* before
allocating the slot. So this means that once we
allocate the slot we cannot fail to send it.

By doing this we can greatly reduce the periods
in which we lock the DQ for building and sending
a slot, allowing more parallelism in sending
commands to HW.

Overall this DQ locking improvement has shown to
improve performance, and also will make MQ perform
better if ever turned on.

Xiang Chen (5):
  scsi: hisi_sas: relocate smp sg map
  scsi: hisi_sas: make return type of prep functions void
  scsi: hisi_sas: allocate slot buffer earlier
  scsi: hisi_sas: Don't lock DQ for complete task sending
  scsi: hisi_sas: Use device lock to protect slot alloc/free

Xiaofei Tan (1):
  scsi: hisi_sas: add check of device in hisi_sas_task_exec()

 drivers/scsi/hisi_sas/hisi_sas.h       |  12 +-
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 264 ++++++++++++++++++---------------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c |  88 ++++-------
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 109 +++++---------
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 107 +++++--------
 5 files changed, 248 insertions(+), 332 deletions(-)

-- 
1.9.1

[PATCH 1/6] scsi: hisi_sas: relocate smp sg map

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

Currently we use DQ lock to protect delivery of DQ
entry one by one.

To optimise to allow more than one slot to be built
for a single DQ in parallel, we need to remove the DQ
lock when preparing slots, prior to delivery.

To achieve this, we rearrange the slot build order to
ensure that once we allocate a slot for a task, we do
cannot fail to deliver the task.

In this patch, we rearrange the slot building for SMP
tasks to ensure that sg mapping part (which can fail)
happens before we allocate the slot in the DQ.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 56 ++++++++++++++++++++++++++--------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 33 ++------------------
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 35 ++-------------------
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 33 ++------------------
 4 files changed, 51 insertions(+), 106 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index ff5b8d7..0ce7c71 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -319,7 +319,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	struct hisi_sas_cmd_hdr	*cmd_hdr_base;
 	struct asd_sas_port *sas_port = device->port;
 	struct device *dev = hisi_hba->dev;
-	int dlvry_queue_slot, dlvry_queue, n_elem = 0, rc, slot_idx;
+	int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
+	int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
 	unsigned long flags;
 
 	if (!sas_port) {
@@ -358,6 +359,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	}
 
 	if (!sas_protocol_ata(task->task_proto)) {
+		unsigned int req_len, resp_len;
+
 		if (task->num_scatter) {
 			n_elem = dma_map_sg(dev, task->scatter,
 					    task->num_scatter, task->data_dir);
@@ -365,6 +368,29 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 				rc = -ENOMEM;
 				goto prep_out;
 			}
+		} else if (task->task_proto & SAS_PROTOCOL_SMP) {
+			n_elem_req = dma_map_sg(dev, &task->smp_task.smp_req,
+						1, DMA_TO_DEVICE);
+			if (!n_elem_req) {
+				rc = -ENOMEM;
+				goto prep_out;
+			}
+			req_len = sg_dma_len(&task->smp_task.smp_req);
+			if (req_len & 0x3) {
+				rc = -EINVAL;
+				goto err_out_dma_unmap;
+			}
+			n_elem_resp = dma_map_sg(dev, &task->smp_task.smp_resp,
+						 1, DMA_FROM_DEVICE);
+			if (!n_elem_req) {
+				rc = -ENOMEM;
+				goto err_out_dma_unmap;
+			}
+			resp_len = sg_dma_len(&task->smp_task.smp_resp);
+			if (resp_len & 0x3) {
+				rc = -EINVAL;
+				goto err_out_dma_unmap;
+			}
 		}
 	} else
 		n_elem = task->num_scatter;
@@ -375,11 +401,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 						    device);
 	else
 		rc = hisi_sas_slot_index_alloc(hisi_hba, &slot_idx);
-	if (rc) {
-		spin_unlock_irqrestore(&hisi_hba->lock, flags);
-		goto err_out;
-	}
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
+	if (rc)
+		goto err_out_dma_unmap;
 
 	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
 	if (rc)
@@ -458,14 +482,22 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot_idx);
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
-err_out:
-	dev_err(dev, "task prep: failed[%d]!\n", rc);
-	if (!sas_protocol_ata(task->task_proto))
-		if (n_elem)
-			dma_unmap_sg(dev, task->scatter,
-				     task->num_scatter,
-				     task->data_dir);
+err_out_dma_unmap:
+	if (!sas_protocol_ata(task->task_proto)) {
+		if (task->num_scatter) {
+			dma_unmap_sg(dev, task->scatter, task->num_scatter,
+			     task->data_dir);
+		} else if (task->task_proto & SAS_PROTOCOL_SMP) {
+			if (n_elem_req)
+				dma_unmap_sg(dev, &task->smp_task.smp_req,
+					     1, DMA_TO_DEVICE);
+			if (n_elem_resp)
+				dma_unmap_sg(dev, &task->smp_task.smp_resp,
+					     1, DMA_FROM_DEVICE);
+		}
+	}
 prep_out:
+	dev_err(dev, "task prep: failed[%d]!\n", rc);
 	return rc;
 }
 
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 84a0ccc..3769d70 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -974,38 +974,17 @@ static int prep_smp_v1_hw(struct hisi_hba *hisi_hba,
 	struct sas_task *task = slot->task;
 	struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr;
 	struct domain_device *device = task->dev;
-	struct device *dev = hisi_hba->dev;
 	struct hisi_sas_port *port = slot->port;
-	struct scatterlist *sg_req, *sg_resp;
+	struct scatterlist *sg_req;
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	dma_addr_t req_dma_addr;
-	unsigned int req_len, resp_len;
-	int elem, rc;
+	unsigned int req_len;
 
-	/*
-	* DMA-map SMP request, response buffers
-	*/
 	/* req */
 	sg_req = &task->smp_task.smp_req;
-	elem = dma_map_sg(dev, sg_req, 1, DMA_TO_DEVICE);
-	if (!elem)
-		return -ENOMEM;
 	req_len = sg_dma_len(sg_req);
 	req_dma_addr = sg_dma_address(sg_req);
 
-	/* resp */
-	sg_resp = &task->smp_task.smp_resp;
-	elem = dma_map_sg(dev, sg_resp, 1, DMA_FROM_DEVICE);
-	if (!elem) {
-		rc = -ENOMEM;
-		goto err_out_req;
-	}
-	resp_len = sg_dma_len(sg_resp);
-	if ((req_len & 0x3) || (resp_len & 0x3)) {
-		rc = -EINVAL;
-		goto err_out_resp;
-	}
-
 	/* create header */
 	/* dw0 */
 	hdr->dw0 = cpu_to_le32((port->id << CMD_HDR_PORT_OFF) |
@@ -1027,14 +1006,6 @@ static int prep_smp_v1_hw(struct hisi_hba *hisi_hba,
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
 
 	return 0;
-
-err_out_resp:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_resp, 1,
-		     DMA_FROM_DEVICE);
-err_out_req:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_req, 1,
-		     DMA_TO_DEVICE);
-	return rc;
 }
 
 static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 9e68731..ac0a0b2 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1721,37 +1721,16 @@ static int prep_smp_v2_hw(struct hisi_hba *hisi_hba,
 	struct sas_task *task = slot->task;
 	struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr;
 	struct domain_device *device = task->dev;
-	struct device *dev = hisi_hba->dev;
 	struct hisi_sas_port *port = slot->port;
-	struct scatterlist *sg_req, *sg_resp;
+	struct scatterlist *sg_req;
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	dma_addr_t req_dma_addr;
-	unsigned int req_len, resp_len;
-	int elem, rc;
+	unsigned int req_len;
 
-	/*
-	* DMA-map SMP request, response buffers
-	*/
 	/* req */
 	sg_req = &task->smp_task.smp_req;
-	elem = dma_map_sg(dev, sg_req, 1, DMA_TO_DEVICE);
-	if (!elem)
-		return -ENOMEM;
-	req_len = sg_dma_len(sg_req);
 	req_dma_addr = sg_dma_address(sg_req);
-
-	/* resp */
-	sg_resp = &task->smp_task.smp_resp;
-	elem = dma_map_sg(dev, sg_resp, 1, DMA_FROM_DEVICE);
-	if (!elem) {
-		rc = -ENOMEM;
-		goto err_out_req;
-	}
-	resp_len = sg_dma_len(sg_resp);
-	if ((req_len & 0x3) || (resp_len & 0x3)) {
-		rc = -EINVAL;
-		goto err_out_resp;
-	}
+	req_len = sg_dma_len(&task->smp_task.smp_req);
 
 	/* create header */
 	/* dw0 */
@@ -1775,14 +1754,6 @@ static int prep_smp_v2_hw(struct hisi_hba *hisi_hba,
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
 
 	return 0;
-
-err_out_resp:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_resp, 1,
-		     DMA_FROM_DEVICE);
-err_out_req:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_req, 1,
-		     DMA_TO_DEVICE);
-	return rc;
 }
 
 static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 492c3be..cea5354 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -986,38 +986,17 @@ static int prep_smp_v3_hw(struct hisi_hba *hisi_hba,
 	struct sas_task *task = slot->task;
 	struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr;
 	struct domain_device *device = task->dev;
-	struct device *dev = hisi_hba->dev;
 	struct hisi_sas_port *port = slot->port;
-	struct scatterlist *sg_req, *sg_resp;
+	struct scatterlist *sg_req;
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	dma_addr_t req_dma_addr;
-	unsigned int req_len, resp_len;
-	int elem, rc;
+	unsigned int req_len;
 
-	/*
-	 * DMA-map SMP request, response buffers
-	 */
 	/* req */
 	sg_req = &task->smp_task.smp_req;
-	elem = dma_map_sg(dev, sg_req, 1, DMA_TO_DEVICE);
-	if (!elem)
-		return -ENOMEM;
 	req_len = sg_dma_len(sg_req);
 	req_dma_addr = sg_dma_address(sg_req);
 
-	/* resp */
-	sg_resp = &task->smp_task.smp_resp;
-	elem = dma_map_sg(dev, sg_resp, 1, DMA_FROM_DEVICE);
-	if (!elem) {
-		rc = -ENOMEM;
-		goto err_out_req;
-	}
-	resp_len = sg_dma_len(sg_resp);
-	if ((req_len & 0x3) || (resp_len & 0x3)) {
-		rc = -EINVAL;
-		goto err_out_resp;
-	}
-
 	/* create header */
 	/* dw0 */
 	hdr->dw0 = cpu_to_le32((port->id << CMD_HDR_PORT_OFF) |
@@ -1040,14 +1019,6 @@ static int prep_smp_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
 
 	return 0;
-
-err_out_resp:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_resp, 1,
-		     DMA_FROM_DEVICE);
-err_out_req:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_req, 1,
-		     DMA_TO_DEVICE);
-	return rc;
 }
 
 static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
-- 
1.9.1

[PATCH 1/6] scsi: hisi_sas: relocate smp sg map

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

Currently we use DQ lock to protect delivery of DQ
entry one by one.

To optimise to allow more than one slot to be built
for a single DQ in parallel, we need to remove the DQ
lock when preparing slots, prior to delivery.

To achieve this, we rearrange the slot build order to
ensure that once we allocate a slot for a task, we do
cannot fail to deliver the task.

In this patch, we rearrange the slot building for SMP
tasks to ensure that sg mapping part (which can fail)
happens before we allocate the slot in the DQ.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 56 ++++++++++++++++++++++++++--------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 33 ++------------------
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 35 ++-------------------
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 33 ++------------------
 4 files changed, 51 insertions(+), 106 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index ff5b8d7..0ce7c71 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -319,7 +319,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	struct hisi_sas_cmd_hdr	*cmd_hdr_base;
 	struct asd_sas_port *sas_port = device->port;
 	struct device *dev = hisi_hba->dev;
-	int dlvry_queue_slot, dlvry_queue, n_elem = 0, rc, slot_idx;
+	int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
+	int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
 	unsigned long flags;
 
 	if (!sas_port) {
@@ -358,6 +359,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	}
 
 	if (!sas_protocol_ata(task->task_proto)) {
+		unsigned int req_len, resp_len;
+
 		if (task->num_scatter) {
 			n_elem = dma_map_sg(dev, task->scatter,
 					    task->num_scatter, task->data_dir);
@@ -365,6 +368,29 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 				rc = -ENOMEM;
 				goto prep_out;
 			}
+		} else if (task->task_proto & SAS_PROTOCOL_SMP) {
+			n_elem_req = dma_map_sg(dev, &task->smp_task.smp_req,
+						1, DMA_TO_DEVICE);
+			if (!n_elem_req) {
+				rc = -ENOMEM;
+				goto prep_out;
+			}
+			req_len = sg_dma_len(&task->smp_task.smp_req);
+			if (req_len & 0x3) {
+				rc = -EINVAL;
+				goto err_out_dma_unmap;
+			}
+			n_elem_resp = dma_map_sg(dev, &task->smp_task.smp_resp,
+						 1, DMA_FROM_DEVICE);
+			if (!n_elem_req) {
+				rc = -ENOMEM;
+				goto err_out_dma_unmap;
+			}
+			resp_len = sg_dma_len(&task->smp_task.smp_resp);
+			if (resp_len & 0x3) {
+				rc = -EINVAL;
+				goto err_out_dma_unmap;
+			}
 		}
 	} else
 		n_elem = task->num_scatter;
@@ -375,11 +401,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 						    device);
 	else
 		rc = hisi_sas_slot_index_alloc(hisi_hba, &slot_idx);
-	if (rc) {
-		spin_unlock_irqrestore(&hisi_hba->lock, flags);
-		goto err_out;
-	}
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
+	if (rc)
+		goto err_out_dma_unmap;
 
 	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
 	if (rc)
@@ -458,14 +482,22 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot_idx);
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
-err_out:
-	dev_err(dev, "task prep: failed[%d]!\n", rc);
-	if (!sas_protocol_ata(task->task_proto))
-		if (n_elem)
-			dma_unmap_sg(dev, task->scatter,
-				     task->num_scatter,
-				     task->data_dir);
+err_out_dma_unmap:
+	if (!sas_protocol_ata(task->task_proto)) {
+		if (task->num_scatter) {
+			dma_unmap_sg(dev, task->scatter, task->num_scatter,
+			     task->data_dir);
+		} else if (task->task_proto & SAS_PROTOCOL_SMP) {
+			if (n_elem_req)
+				dma_unmap_sg(dev, &task->smp_task.smp_req,
+					     1, DMA_TO_DEVICE);
+			if (n_elem_resp)
+				dma_unmap_sg(dev, &task->smp_task.smp_resp,
+					     1, DMA_FROM_DEVICE);
+		}
+	}
 prep_out:
+	dev_err(dev, "task prep: failed[%d]!\n", rc);
 	return rc;
 }
 
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 84a0ccc..3769d70 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -974,38 +974,17 @@ static int prep_smp_v1_hw(struct hisi_hba *hisi_hba,
 	struct sas_task *task = slot->task;
 	struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr;
 	struct domain_device *device = task->dev;
-	struct device *dev = hisi_hba->dev;
 	struct hisi_sas_port *port = slot->port;
-	struct scatterlist *sg_req, *sg_resp;
+	struct scatterlist *sg_req;
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	dma_addr_t req_dma_addr;
-	unsigned int req_len, resp_len;
-	int elem, rc;
+	unsigned int req_len;
 
-	/*
-	* DMA-map SMP request, response buffers
-	*/
 	/* req */
 	sg_req = &task->smp_task.smp_req;
-	elem = dma_map_sg(dev, sg_req, 1, DMA_TO_DEVICE);
-	if (!elem)
-		return -ENOMEM;
 	req_len = sg_dma_len(sg_req);
 	req_dma_addr = sg_dma_address(sg_req);
 
-	/* resp */
-	sg_resp = &task->smp_task.smp_resp;
-	elem = dma_map_sg(dev, sg_resp, 1, DMA_FROM_DEVICE);
-	if (!elem) {
-		rc = -ENOMEM;
-		goto err_out_req;
-	}
-	resp_len = sg_dma_len(sg_resp);
-	if ((req_len & 0x3) || (resp_len & 0x3)) {
-		rc = -EINVAL;
-		goto err_out_resp;
-	}
-
 	/* create header */
 	/* dw0 */
 	hdr->dw0 = cpu_to_le32((port->id << CMD_HDR_PORT_OFF) |
@@ -1027,14 +1006,6 @@ static int prep_smp_v1_hw(struct hisi_hba *hisi_hba,
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
 
 	return 0;
-
-err_out_resp:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_resp, 1,
-		     DMA_FROM_DEVICE);
-err_out_req:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_req, 1,
-		     DMA_TO_DEVICE);
-	return rc;
 }
 
 static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 9e68731..ac0a0b2 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1721,37 +1721,16 @@ static int prep_smp_v2_hw(struct hisi_hba *hisi_hba,
 	struct sas_task *task = slot->task;
 	struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr;
 	struct domain_device *device = task->dev;
-	struct device *dev = hisi_hba->dev;
 	struct hisi_sas_port *port = slot->port;
-	struct scatterlist *sg_req, *sg_resp;
+	struct scatterlist *sg_req;
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	dma_addr_t req_dma_addr;
-	unsigned int req_len, resp_len;
-	int elem, rc;
+	unsigned int req_len;
 
-	/*
-	* DMA-map SMP request, response buffers
-	*/
 	/* req */
 	sg_req = &task->smp_task.smp_req;
-	elem = dma_map_sg(dev, sg_req, 1, DMA_TO_DEVICE);
-	if (!elem)
-		return -ENOMEM;
-	req_len = sg_dma_len(sg_req);
 	req_dma_addr = sg_dma_address(sg_req);
-
-	/* resp */
-	sg_resp = &task->smp_task.smp_resp;
-	elem = dma_map_sg(dev, sg_resp, 1, DMA_FROM_DEVICE);
-	if (!elem) {
-		rc = -ENOMEM;
-		goto err_out_req;
-	}
-	resp_len = sg_dma_len(sg_resp);
-	if ((req_len & 0x3) || (resp_len & 0x3)) {
-		rc = -EINVAL;
-		goto err_out_resp;
-	}
+	req_len = sg_dma_len(&task->smp_task.smp_req);
 
 	/* create header */
 	/* dw0 */
@@ -1775,14 +1754,6 @@ static int prep_smp_v2_hw(struct hisi_hba *hisi_hba,
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
 
 	return 0;
-
-err_out_resp:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_resp, 1,
-		     DMA_FROM_DEVICE);
-err_out_req:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_req, 1,
-		     DMA_TO_DEVICE);
-	return rc;
 }
 
 static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 492c3be..cea5354 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -986,38 +986,17 @@ static int prep_smp_v3_hw(struct hisi_hba *hisi_hba,
 	struct sas_task *task = slot->task;
 	struct hisi_sas_cmd_hdr *hdr = slot->cmd_hdr;
 	struct domain_device *device = task->dev;
-	struct device *dev = hisi_hba->dev;
 	struct hisi_sas_port *port = slot->port;
-	struct scatterlist *sg_req, *sg_resp;
+	struct scatterlist *sg_req;
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	dma_addr_t req_dma_addr;
-	unsigned int req_len, resp_len;
-	int elem, rc;
+	unsigned int req_len;
 
-	/*
-	 * DMA-map SMP request, response buffers
-	 */
 	/* req */
 	sg_req = &task->smp_task.smp_req;
-	elem = dma_map_sg(dev, sg_req, 1, DMA_TO_DEVICE);
-	if (!elem)
-		return -ENOMEM;
 	req_len = sg_dma_len(sg_req);
 	req_dma_addr = sg_dma_address(sg_req);
 
-	/* resp */
-	sg_resp = &task->smp_task.smp_resp;
-	elem = dma_map_sg(dev, sg_resp, 1, DMA_FROM_DEVICE);
-	if (!elem) {
-		rc = -ENOMEM;
-		goto err_out_req;
-	}
-	resp_len = sg_dma_len(sg_resp);
-	if ((req_len & 0x3) || (resp_len & 0x3)) {
-		rc = -EINVAL;
-		goto err_out_resp;
-	}
-
 	/* create header */
 	/* dw0 */
 	hdr->dw0 = cpu_to_le32((port->id << CMD_HDR_PORT_OFF) |
@@ -1040,14 +1019,6 @@ static int prep_smp_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
 
 	return 0;
-
-err_out_resp:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_resp, 1,
-		     DMA_FROM_DEVICE);
-err_out_req:
-	dma_unmap_sg(dev, &slot->task->smp_task.smp_req, 1,
-		     DMA_TO_DEVICE);
-	return rc;
 }
 
 static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
-- 
1.9.1

[PATCH 2/6] scsi: hisi_sas: make return type of prep functions void

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

Since the task prep functions now should not fail, adjust
the return types to void.

In addition, some checks in the task prep functions are
relocated to the main module; this is specifically the
check for the number of elements in an sg list exceeded
the HW SGE limit.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas.h       |  8 +++---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 45 ++++++++++++++--------------------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 28 +++++----------------
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 45 +++++++++-------------------------
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 43 +++++++++-----------------------
 5 files changed, 51 insertions(+), 118 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 4410538..3a1caa04 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -214,14 +214,14 @@ struct hisi_sas_hw {
 	void (*sl_notify)(struct hisi_hba *hisi_hba, int phy_no);
 	int (*get_free_slot)(struct hisi_hba *hisi_hba, struct hisi_sas_dq *dq);
 	void (*start_delivery)(struct hisi_sas_dq *dq);
-	int (*prep_ssp)(struct hisi_hba *hisi_hba,
+	void (*prep_ssp)(struct hisi_hba *hisi_hba,
 			struct hisi_sas_slot *slot, int is_tmf,
 			struct hisi_sas_tmf_task *tmf);
-	int (*prep_smp)(struct hisi_hba *hisi_hba,
+	void (*prep_smp)(struct hisi_hba *hisi_hba,
 			struct hisi_sas_slot *slot);
-	int (*prep_stp)(struct hisi_hba *hisi_hba,
+	void (*prep_stp)(struct hisi_hba *hisi_hba,
 			struct hisi_sas_slot *slot);
-	int (*prep_abort)(struct hisi_hba *hisi_hba,
+	void (*prep_abort)(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot,
 			  int device_id, int abort_flag, int tag_to_abort);
 	int (*slot_complete)(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 0ce7c71..2772e920 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -243,30 +243,30 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 }
 EXPORT_SYMBOL_GPL(hisi_sas_slot_task_free);
 
-static int hisi_sas_task_prep_smp(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_smp(struct hisi_hba *hisi_hba,
 				  struct hisi_sas_slot *slot)
 {
-	return hisi_hba->hw->prep_smp(hisi_hba, slot);
+	hisi_hba->hw->prep_smp(hisi_hba, slot);
 }
 
-static int hisi_sas_task_prep_ssp(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_ssp(struct hisi_hba *hisi_hba,
 				  struct hisi_sas_slot *slot, int is_tmf,
 				  struct hisi_sas_tmf_task *tmf)
 {
-	return hisi_hba->hw->prep_ssp(hisi_hba, slot, is_tmf, tmf);
+	hisi_hba->hw->prep_ssp(hisi_hba, slot, is_tmf, tmf);
 }
 
-static int hisi_sas_task_prep_ata(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_ata(struct hisi_hba *hisi_hba,
 				  struct hisi_sas_slot *slot)
 {
-	return hisi_hba->hw->prep_stp(hisi_hba, slot);
+	hisi_hba->hw->prep_stp(hisi_hba, slot);
 }
 
-static int hisi_sas_task_prep_abort(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_abort(struct hisi_hba *hisi_hba,
 		struct hisi_sas_slot *slot,
 		int device_id, int abort_flag, int tag_to_abort)
 {
-	return hisi_hba->hw->prep_abort(hisi_hba, slot,
+	hisi_hba->hw->prep_abort(hisi_hba, slot,
 			device_id, abort_flag, tag_to_abort);
 }
 
@@ -395,6 +395,13 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	} else
 		n_elem = task->num_scatter;
 
+	if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
+		dev_err(dev, "task prep: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
+			n_elem);
+		rc = -EINVAL;
+		goto err_out_dma_unmap;
+	}
+
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	if (hisi_hba->hw->slot_index_alloc)
 		rc = hisi_hba->hw->slot_index_alloc(hisi_hba, &slot_idx,
@@ -439,28 +446,22 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 
 	switch (task->task_proto) {
 	case SAS_PROTOCOL_SMP:
-		rc = hisi_sas_task_prep_smp(hisi_hba, slot);
+		hisi_sas_task_prep_smp(hisi_hba, slot);
 		break;
 	case SAS_PROTOCOL_SSP:
-		rc = hisi_sas_task_prep_ssp(hisi_hba, slot, is_tmf, tmf);
+		hisi_sas_task_prep_ssp(hisi_hba, slot, is_tmf, tmf);
 		break;
 	case SAS_PROTOCOL_SATA:
 	case SAS_PROTOCOL_STP:
 	case SAS_PROTOCOL_SATA | SAS_PROTOCOL_STP:
-		rc = hisi_sas_task_prep_ata(hisi_hba, slot);
+		hisi_sas_task_prep_ata(hisi_hba, slot);
 		break;
 	default:
 		dev_err(dev, "task prep: unknown/unsupported proto (0x%x)\n",
 			task->task_proto);
-		rc = -EINVAL;
 		break;
 	}
 
-	if (rc) {
-		dev_err(dev, "task prep: rc = 0x%x\n", rc);
-		goto err_out_buf;
-	}
-
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	list_add_tail(&slot->entry, &sas_dev->list);
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
@@ -473,9 +474,6 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 
 	return 0;
 
-err_out_buf:
-	dma_pool_free(hisi_hba->buffer_pool, slot->buf,
-		slot->buf_dma);
 err_out_slot_buf:
 	/* Nothing to be done */
 err_out_tag:
@@ -1554,10 +1552,8 @@ static int hisi_sas_query_task(struct sas_task *task)
 	memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
 	memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
 
-	rc = hisi_sas_task_prep_abort(hisi_hba, slot, device_id,
+	hisi_sas_task_prep_abort(hisi_hba, slot, device_id,
 				      abort_flag, task_tag);
-	if (rc)
-		goto err_out_buf;
 
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	list_add_tail(&slot->entry, &sas_dev->list);
@@ -1574,9 +1570,6 @@ static int hisi_sas_query_task(struct sas_task *task)
 
 	return 0;
 
-err_out_buf:
-	dma_pool_free(hisi_hba->buffer_pool, slot->buf,
-		slot->buf_dma);
 err_out_tag:
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot_idx);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 3769d70..7781a01 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -935,23 +935,16 @@ static void start_delivery_v1_hw(struct hisi_sas_dq *dq)
 			 dq->wr_point);
 }
 
-static int prep_prd_sge_v1_hw(struct hisi_hba *hisi_hba,
+static void prep_prd_sge_v1_hw(struct hisi_hba *hisi_hba,
 			      struct hisi_sas_slot *slot,
 			      struct hisi_sas_cmd_hdr *hdr,
 			      struct scatterlist *scatter,
 			      int n_elem)
 {
 	struct hisi_sas_sge_page *sge_page = hisi_sas_sge_addr_mem(slot);
-	struct device *dev = hisi_hba->dev;
 	struct scatterlist *sg;
 	int i;
 
-	if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
-		dev_err(dev, "prd err: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
-			n_elem);
-		return -EINVAL;
-	}
-
 	for_each_sg(scatter, sg, n_elem, i) {
 		struct hisi_sas_sge *entry = &sge_page->sge[i];
 
@@ -964,11 +957,9 @@ static int prep_prd_sge_v1_hw(struct hisi_hba *hisi_hba,
 	hdr->prd_table_addr = cpu_to_le64(hisi_sas_sge_addr_dma(slot));
 
 	hdr->sg_len = cpu_to_le32(n_elem << CMD_HDR_DATA_SGL_LEN_OFF);
-
-	return 0;
 }
 
-static int prep_smp_v1_hw(struct hisi_hba *hisi_hba,
+static void prep_smp_v1_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -1004,11 +995,9 @@ static int prep_smp_v1_hw(struct hisi_hba *hisi_hba,
 
 	hdr->cmd_table_addr = cpu_to_le64(req_dma_addr);
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
-
-	return 0;
 }
 
-static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
+static void prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot, int is_tmf,
 			  struct hisi_sas_tmf_task *tmf)
 {
@@ -1019,7 +1008,7 @@ static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
 	struct hisi_sas_port *port = slot->port;
 	struct sas_ssp_task *ssp_task = &task->ssp_task;
 	struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
-	int has_data = 0, rc, priority = is_tmf;
+	int has_data = 0, priority = is_tmf;
 	u8 *buf_cmd, fburst = 0;
 	u32 dw1, dw2;
 
@@ -1068,12 +1057,9 @@ static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
 
 	hdr->transfer_tags = cpu_to_le32(slot->idx << CMD_HDR_IPTT_OFF);
 
-	if (has_data) {
-		rc = prep_prd_sge_v1_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v1_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -1107,8 +1093,6 @@ static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
 			break;
 		}
 	}
-
-	return 0;
 }
 
 /* by default, task resp is complete */
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index ac0a0b2..9f77fd8 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1682,23 +1682,16 @@ static void start_delivery_v2_hw(struct hisi_sas_dq *dq)
 			 dq->wr_point);
 }
 
-static int prep_prd_sge_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_prd_sge_v2_hw(struct hisi_hba *hisi_hba,
 			      struct hisi_sas_slot *slot,
 			      struct hisi_sas_cmd_hdr *hdr,
 			      struct scatterlist *scatter,
 			      int n_elem)
 {
 	struct hisi_sas_sge_page *sge_page = hisi_sas_sge_addr_mem(slot);
-	struct device *dev = hisi_hba->dev;
 	struct scatterlist *sg;
 	int i;
 
-	if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
-		dev_err(dev, "prd err: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
-			n_elem);
-		return -EINVAL;
-	}
-
 	for_each_sg(scatter, sg, n_elem, i) {
 		struct hisi_sas_sge *entry = &sge_page->sge[i];
 
@@ -1711,11 +1704,9 @@ static int prep_prd_sge_v2_hw(struct hisi_hba *hisi_hba,
 	hdr->prd_table_addr = cpu_to_le64(hisi_sas_sge_addr_dma(slot));
 
 	hdr->sg_len = cpu_to_le32(n_elem << CMD_HDR_DATA_SGL_LEN_OFF);
-
-	return 0;
 }
 
-static int prep_smp_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_smp_v2_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -1752,11 +1743,9 @@ static int prep_smp_v2_hw(struct hisi_hba *hisi_hba,
 
 	hdr->cmd_table_addr = cpu_to_le64(req_dma_addr);
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
-
-	return 0;
 }
 
-static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot, int is_tmf,
 			  struct hisi_sas_tmf_task *tmf)
 {
@@ -1767,7 +1756,7 @@ static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
 	struct hisi_sas_port *port = slot->port;
 	struct sas_ssp_task *ssp_task = &task->ssp_task;
 	struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
-	int has_data = 0, rc, priority = is_tmf;
+	int has_data = 0, priority = is_tmf;
 	u8 *buf_cmd;
 	u32 dw1 = 0, dw2 = 0;
 
@@ -1809,12 +1798,9 @@ static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
 
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	if (has_data) {
-		rc = prep_prd_sge_v2_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v2_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -1843,8 +1829,6 @@ static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
 			break;
 		}
 	}
-
-	return 0;
 }
 
 #define TRANS_TX_ERR	0
@@ -2519,7 +2503,7 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
 	return sts;
 }
 
-static int prep_ata_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_ata_v2_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -2530,7 +2514,7 @@ static int prep_ata_v2_hw(struct hisi_hba *hisi_hba,
 	struct asd_sas_port *sas_port = device->port;
 	struct hisi_sas_port *port = to_hisi_sas_port(sas_port);
 	u8 *buf_cmd;
-	int has_data = 0, rc = 0, hdr_tag = 0;
+	int has_data = 0, hdr_tag = 0;
 	u32 dw1 = 0, dw2 = 0;
 
 	/* create header */
@@ -2578,12 +2562,9 @@ static int prep_ata_v2_hw(struct hisi_hba *hisi_hba,
 	/* dw3 */
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	if (has_data) {
-		rc = prep_prd_sge_v2_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v2_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -2595,8 +2576,6 @@ static int prep_ata_v2_hw(struct hisi_hba *hisi_hba,
 		task->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
 	/* fill in command FIS */
 	memcpy(buf_cmd, &task->ata_task.fis, sizeof(struct host_to_dev_fis));
-
-	return 0;
 }
 
 static void hisi_sas_internal_abort_quirk_timeout(struct timer_list *t)
@@ -2633,7 +2612,7 @@ static void hisi_sas_internal_abort_quirk_timeout(struct timer_list *t)
 	}
 }
 
-static int prep_abort_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_abort_v2_hw(struct hisi_hba *hisi_hba,
 		struct hisi_sas_slot *slot,
 		int device_id, int abort_flag, int tag_to_abort)
 {
@@ -2661,8 +2640,6 @@ static int prep_abort_v2_hw(struct hisi_hba *hisi_hba,
 	/* dw7 */
 	hdr->dw7 = cpu_to_le32(tag_to_abort << CMD_HDR_ABORT_IPTT_OFF);
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
-
-	return 0;
 }
 
 static int phy_up_v2_hw(int phy_no, struct hisi_hba *hisi_hba)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index cea5354..8cd1374 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -859,23 +859,16 @@ static void start_delivery_v3_hw(struct hisi_sas_dq *dq)
 			 dq->wr_point);
 }
 
-static int prep_prd_sge_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_prd_sge_v3_hw(struct hisi_hba *hisi_hba,
 			      struct hisi_sas_slot *slot,
 			      struct hisi_sas_cmd_hdr *hdr,
 			      struct scatterlist *scatter,
 			      int n_elem)
 {
 	struct hisi_sas_sge_page *sge_page = hisi_sas_sge_addr_mem(slot);
-	struct device *dev = hisi_hba->dev;
 	struct scatterlist *sg;
 	int i;
 
-	if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
-		dev_err(dev, "prd err: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
-			n_elem);
-		return -EINVAL;
-	}
-
 	for_each_sg(scatter, sg, n_elem, i) {
 		struct hisi_sas_sge *entry = &sge_page->sge[i];
 
@@ -888,11 +881,9 @@ static int prep_prd_sge_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->prd_table_addr = cpu_to_le64(hisi_sas_sge_addr_dma(slot));
 
 	hdr->sg_len = cpu_to_le32(n_elem << CMD_HDR_DATA_SGL_LEN_OFF);
-
-	return 0;
 }
 
-static int prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot, int is_tmf,
 			  struct hisi_sas_tmf_task *tmf)
 {
@@ -903,7 +894,7 @@ static int prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
 	struct hisi_sas_port *port = slot->port;
 	struct sas_ssp_task *ssp_task = &task->ssp_task;
 	struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
-	int has_data = 0, rc, priority = is_tmf;
+	int has_data = 0, priority = is_tmf;
 	u8 *buf_cmd;
 	u32 dw1 = 0, dw2 = 0;
 
@@ -944,12 +935,9 @@ static int prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->dw2 = cpu_to_le32(dw2);
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	if (has_data) {
-		rc = prep_prd_sge_v3_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v3_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -976,11 +964,9 @@ static int prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
 			break;
 		}
 	}
-
-	return 0;
 }
 
-static int prep_smp_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_smp_v3_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -1018,10 +1004,9 @@ static int prep_smp_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->cmd_table_addr = cpu_to_le64(req_dma_addr);
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
 
-	return 0;
 }
 
-static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_ata_v3_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -1032,7 +1017,7 @@ static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
 	struct asd_sas_port *sas_port = device->port;
 	struct hisi_sas_port *port = to_hisi_sas_port(sas_port);
 	u8 *buf_cmd;
-	int has_data = 0, rc = 0, hdr_tag = 0;
+	int has_data = 0, hdr_tag = 0;
 	u32 dw1 = 0, dw2 = 0;
 
 	hdr->dw0 = cpu_to_le32(port->id << CMD_HDR_PORT_OFF);
@@ -1081,12 +1066,9 @@ static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
 	/* dw3 */
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	if (has_data) {
-		rc = prep_prd_sge_v3_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v3_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -1098,11 +1080,9 @@ static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
 		task->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
 	/* fill in command FIS */
 	memcpy(buf_cmd, &task->ata_task.fis, sizeof(struct host_to_dev_fis));
-
-	return 0;
 }
 
-static int prep_abort_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_abort_v3_hw(struct hisi_hba *hisi_hba,
 		struct hisi_sas_slot *slot,
 		int device_id, int abort_flag, int tag_to_abort)
 {
@@ -1127,7 +1107,6 @@ static int prep_abort_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->dw7 = cpu_to_le32(tag_to_abort << CMD_HDR_ABORT_IPTT_OFF);
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	return 0;
 }
 
 static irqreturn_t phy_up_v3_hw(int phy_no, struct hisi_hba *hisi_hba)
-- 
1.9.1

[PATCH 2/6] scsi: hisi_sas: make return type of prep functions void

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

Since the task prep functions now should not fail, adjust
the return types to void.

In addition, some checks in the task prep functions are
relocated to the main module; this is specifically the
check for the number of elements in an sg list exceeded
the HW SGE limit.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas.h       |  8 +++---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 45 ++++++++++++++--------------------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 28 +++++----------------
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 45 +++++++++-------------------------
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 43 +++++++++-----------------------
 5 files changed, 51 insertions(+), 118 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 4410538..3a1caa04 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -214,14 +214,14 @@ struct hisi_sas_hw {
 	void (*sl_notify)(struct hisi_hba *hisi_hba, int phy_no);
 	int (*get_free_slot)(struct hisi_hba *hisi_hba, struct hisi_sas_dq *dq);
 	void (*start_delivery)(struct hisi_sas_dq *dq);
-	int (*prep_ssp)(struct hisi_hba *hisi_hba,
+	void (*prep_ssp)(struct hisi_hba *hisi_hba,
 			struct hisi_sas_slot *slot, int is_tmf,
 			struct hisi_sas_tmf_task *tmf);
-	int (*prep_smp)(struct hisi_hba *hisi_hba,
+	void (*prep_smp)(struct hisi_hba *hisi_hba,
 			struct hisi_sas_slot *slot);
-	int (*prep_stp)(struct hisi_hba *hisi_hba,
+	void (*prep_stp)(struct hisi_hba *hisi_hba,
 			struct hisi_sas_slot *slot);
-	int (*prep_abort)(struct hisi_hba *hisi_hba,
+	void (*prep_abort)(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot,
 			  int device_id, int abort_flag, int tag_to_abort);
 	int (*slot_complete)(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 0ce7c71..2772e920 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -243,30 +243,30 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 }
 EXPORT_SYMBOL_GPL(hisi_sas_slot_task_free);
 
-static int hisi_sas_task_prep_smp(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_smp(struct hisi_hba *hisi_hba,
 				  struct hisi_sas_slot *slot)
 {
-	return hisi_hba->hw->prep_smp(hisi_hba, slot);
+	hisi_hba->hw->prep_smp(hisi_hba, slot);
 }
 
-static int hisi_sas_task_prep_ssp(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_ssp(struct hisi_hba *hisi_hba,
 				  struct hisi_sas_slot *slot, int is_tmf,
 				  struct hisi_sas_tmf_task *tmf)
 {
-	return hisi_hba->hw->prep_ssp(hisi_hba, slot, is_tmf, tmf);
+	hisi_hba->hw->prep_ssp(hisi_hba, slot, is_tmf, tmf);
 }
 
-static int hisi_sas_task_prep_ata(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_ata(struct hisi_hba *hisi_hba,
 				  struct hisi_sas_slot *slot)
 {
-	return hisi_hba->hw->prep_stp(hisi_hba, slot);
+	hisi_hba->hw->prep_stp(hisi_hba, slot);
 }
 
-static int hisi_sas_task_prep_abort(struct hisi_hba *hisi_hba,
+static void hisi_sas_task_prep_abort(struct hisi_hba *hisi_hba,
 		struct hisi_sas_slot *slot,
 		int device_id, int abort_flag, int tag_to_abort)
 {
-	return hisi_hba->hw->prep_abort(hisi_hba, slot,
+	hisi_hba->hw->prep_abort(hisi_hba, slot,
 			device_id, abort_flag, tag_to_abort);
 }
 
@@ -395,6 +395,13 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	} else
 		n_elem = task->num_scatter;
 
+	if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
+		dev_err(dev, "task prep: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
+			n_elem);
+		rc = -EINVAL;
+		goto err_out_dma_unmap;
+	}
+
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	if (hisi_hba->hw->slot_index_alloc)
 		rc = hisi_hba->hw->slot_index_alloc(hisi_hba, &slot_idx,
@@ -439,28 +446,22 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 
 	switch (task->task_proto) {
 	case SAS_PROTOCOL_SMP:
-		rc = hisi_sas_task_prep_smp(hisi_hba, slot);
+		hisi_sas_task_prep_smp(hisi_hba, slot);
 		break;
 	case SAS_PROTOCOL_SSP:
-		rc = hisi_sas_task_prep_ssp(hisi_hba, slot, is_tmf, tmf);
+		hisi_sas_task_prep_ssp(hisi_hba, slot, is_tmf, tmf);
 		break;
 	case SAS_PROTOCOL_SATA:
 	case SAS_PROTOCOL_STP:
 	case SAS_PROTOCOL_SATA | SAS_PROTOCOL_STP:
-		rc = hisi_sas_task_prep_ata(hisi_hba, slot);
+		hisi_sas_task_prep_ata(hisi_hba, slot);
 		break;
 	default:
 		dev_err(dev, "task prep: unknown/unsupported proto (0x%x)\n",
 			task->task_proto);
-		rc = -EINVAL;
 		break;
 	}
 
-	if (rc) {
-		dev_err(dev, "task prep: rc = 0x%x\n", rc);
-		goto err_out_buf;
-	}
-
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	list_add_tail(&slot->entry, &sas_dev->list);
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
@@ -473,9 +474,6 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 
 	return 0;
 
-err_out_buf:
-	dma_pool_free(hisi_hba->buffer_pool, slot->buf,
-		slot->buf_dma);
 err_out_slot_buf:
 	/* Nothing to be done */
 err_out_tag:
@@ -1554,10 +1552,8 @@ static int hisi_sas_query_task(struct sas_task *task)
 	memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
 	memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
 
-	rc = hisi_sas_task_prep_abort(hisi_hba, slot, device_id,
+	hisi_sas_task_prep_abort(hisi_hba, slot, device_id,
 				      abort_flag, task_tag);
-	if (rc)
-		goto err_out_buf;
 
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	list_add_tail(&slot->entry, &sas_dev->list);
@@ -1574,9 +1570,6 @@ static int hisi_sas_query_task(struct sas_task *task)
 
 	return 0;
 
-err_out_buf:
-	dma_pool_free(hisi_hba->buffer_pool, slot->buf,
-		slot->buf_dma);
 err_out_tag:
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot_idx);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 3769d70..7781a01 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -935,23 +935,16 @@ static void start_delivery_v1_hw(struct hisi_sas_dq *dq)
 			 dq->wr_point);
 }
 
-static int prep_prd_sge_v1_hw(struct hisi_hba *hisi_hba,
+static void prep_prd_sge_v1_hw(struct hisi_hba *hisi_hba,
 			      struct hisi_sas_slot *slot,
 			      struct hisi_sas_cmd_hdr *hdr,
 			      struct scatterlist *scatter,
 			      int n_elem)
 {
 	struct hisi_sas_sge_page *sge_page = hisi_sas_sge_addr_mem(slot);
-	struct device *dev = hisi_hba->dev;
 	struct scatterlist *sg;
 	int i;
 
-	if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
-		dev_err(dev, "prd err: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
-			n_elem);
-		return -EINVAL;
-	}
-
 	for_each_sg(scatter, sg, n_elem, i) {
 		struct hisi_sas_sge *entry = &sge_page->sge[i];
 
@@ -964,11 +957,9 @@ static int prep_prd_sge_v1_hw(struct hisi_hba *hisi_hba,
 	hdr->prd_table_addr = cpu_to_le64(hisi_sas_sge_addr_dma(slot));
 
 	hdr->sg_len = cpu_to_le32(n_elem << CMD_HDR_DATA_SGL_LEN_OFF);
-
-	return 0;
 }
 
-static int prep_smp_v1_hw(struct hisi_hba *hisi_hba,
+static void prep_smp_v1_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -1004,11 +995,9 @@ static int prep_smp_v1_hw(struct hisi_hba *hisi_hba,
 
 	hdr->cmd_table_addr = cpu_to_le64(req_dma_addr);
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
-
-	return 0;
 }
 
-static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
+static void prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot, int is_tmf,
 			  struct hisi_sas_tmf_task *tmf)
 {
@@ -1019,7 +1008,7 @@ static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
 	struct hisi_sas_port *port = slot->port;
 	struct sas_ssp_task *ssp_task = &task->ssp_task;
 	struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
-	int has_data = 0, rc, priority = is_tmf;
+	int has_data = 0, priority = is_tmf;
 	u8 *buf_cmd, fburst = 0;
 	u32 dw1, dw2;
 
@@ -1068,12 +1057,9 @@ static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
 
 	hdr->transfer_tags = cpu_to_le32(slot->idx << CMD_HDR_IPTT_OFF);
 
-	if (has_data) {
-		rc = prep_prd_sge_v1_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v1_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -1107,8 +1093,6 @@ static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
 			break;
 		}
 	}
-
-	return 0;
 }
 
 /* by default, task resp is complete */
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index ac0a0b2..9f77fd8 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1682,23 +1682,16 @@ static void start_delivery_v2_hw(struct hisi_sas_dq *dq)
 			 dq->wr_point);
 }
 
-static int prep_prd_sge_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_prd_sge_v2_hw(struct hisi_hba *hisi_hba,
 			      struct hisi_sas_slot *slot,
 			      struct hisi_sas_cmd_hdr *hdr,
 			      struct scatterlist *scatter,
 			      int n_elem)
 {
 	struct hisi_sas_sge_page *sge_page = hisi_sas_sge_addr_mem(slot);
-	struct device *dev = hisi_hba->dev;
 	struct scatterlist *sg;
 	int i;
 
-	if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
-		dev_err(dev, "prd err: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
-			n_elem);
-		return -EINVAL;
-	}
-
 	for_each_sg(scatter, sg, n_elem, i) {
 		struct hisi_sas_sge *entry = &sge_page->sge[i];
 
@@ -1711,11 +1704,9 @@ static int prep_prd_sge_v2_hw(struct hisi_hba *hisi_hba,
 	hdr->prd_table_addr = cpu_to_le64(hisi_sas_sge_addr_dma(slot));
 
 	hdr->sg_len = cpu_to_le32(n_elem << CMD_HDR_DATA_SGL_LEN_OFF);
-
-	return 0;
 }
 
-static int prep_smp_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_smp_v2_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -1752,11 +1743,9 @@ static int prep_smp_v2_hw(struct hisi_hba *hisi_hba,
 
 	hdr->cmd_table_addr = cpu_to_le64(req_dma_addr);
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
-
-	return 0;
 }
 
-static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot, int is_tmf,
 			  struct hisi_sas_tmf_task *tmf)
 {
@@ -1767,7 +1756,7 @@ static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
 	struct hisi_sas_port *port = slot->port;
 	struct sas_ssp_task *ssp_task = &task->ssp_task;
 	struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
-	int has_data = 0, rc, priority = is_tmf;
+	int has_data = 0, priority = is_tmf;
 	u8 *buf_cmd;
 	u32 dw1 = 0, dw2 = 0;
 
@@ -1809,12 +1798,9 @@ static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
 
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	if (has_data) {
-		rc = prep_prd_sge_v2_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v2_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -1843,8 +1829,6 @@ static int prep_ssp_v2_hw(struct hisi_hba *hisi_hba,
 			break;
 		}
 	}
-
-	return 0;
 }
 
 #define TRANS_TX_ERR	0
@@ -2519,7 +2503,7 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
 	return sts;
 }
 
-static int prep_ata_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_ata_v2_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -2530,7 +2514,7 @@ static int prep_ata_v2_hw(struct hisi_hba *hisi_hba,
 	struct asd_sas_port *sas_port = device->port;
 	struct hisi_sas_port *port = to_hisi_sas_port(sas_port);
 	u8 *buf_cmd;
-	int has_data = 0, rc = 0, hdr_tag = 0;
+	int has_data = 0, hdr_tag = 0;
 	u32 dw1 = 0, dw2 = 0;
 
 	/* create header */
@@ -2578,12 +2562,9 @@ static int prep_ata_v2_hw(struct hisi_hba *hisi_hba,
 	/* dw3 */
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	if (has_data) {
-		rc = prep_prd_sge_v2_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v2_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -2595,8 +2576,6 @@ static int prep_ata_v2_hw(struct hisi_hba *hisi_hba,
 		task->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
 	/* fill in command FIS */
 	memcpy(buf_cmd, &task->ata_task.fis, sizeof(struct host_to_dev_fis));
-
-	return 0;
 }
 
 static void hisi_sas_internal_abort_quirk_timeout(struct timer_list *t)
@@ -2633,7 +2612,7 @@ static void hisi_sas_internal_abort_quirk_timeout(struct timer_list *t)
 	}
 }
 
-static int prep_abort_v2_hw(struct hisi_hba *hisi_hba,
+static void prep_abort_v2_hw(struct hisi_hba *hisi_hba,
 		struct hisi_sas_slot *slot,
 		int device_id, int abort_flag, int tag_to_abort)
 {
@@ -2661,8 +2640,6 @@ static int prep_abort_v2_hw(struct hisi_hba *hisi_hba,
 	/* dw7 */
 	hdr->dw7 = cpu_to_le32(tag_to_abort << CMD_HDR_ABORT_IPTT_OFF);
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
-
-	return 0;
 }
 
 static int phy_up_v2_hw(int phy_no, struct hisi_hba *hisi_hba)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index cea5354..8cd1374 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -859,23 +859,16 @@ static void start_delivery_v3_hw(struct hisi_sas_dq *dq)
 			 dq->wr_point);
 }
 
-static int prep_prd_sge_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_prd_sge_v3_hw(struct hisi_hba *hisi_hba,
 			      struct hisi_sas_slot *slot,
 			      struct hisi_sas_cmd_hdr *hdr,
 			      struct scatterlist *scatter,
 			      int n_elem)
 {
 	struct hisi_sas_sge_page *sge_page = hisi_sas_sge_addr_mem(slot);
-	struct device *dev = hisi_hba->dev;
 	struct scatterlist *sg;
 	int i;
 
-	if (n_elem > HISI_SAS_SGE_PAGE_CNT) {
-		dev_err(dev, "prd err: n_elem(%d) > HISI_SAS_SGE_PAGE_CNT",
-			n_elem);
-		return -EINVAL;
-	}
-
 	for_each_sg(scatter, sg, n_elem, i) {
 		struct hisi_sas_sge *entry = &sge_page->sge[i];
 
@@ -888,11 +881,9 @@ static int prep_prd_sge_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->prd_table_addr = cpu_to_le64(hisi_sas_sge_addr_dma(slot));
 
 	hdr->sg_len = cpu_to_le32(n_elem << CMD_HDR_DATA_SGL_LEN_OFF);
-
-	return 0;
 }
 
-static int prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot, int is_tmf,
 			  struct hisi_sas_tmf_task *tmf)
 {
@@ -903,7 +894,7 @@ static int prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
 	struct hisi_sas_port *port = slot->port;
 	struct sas_ssp_task *ssp_task = &task->ssp_task;
 	struct scsi_cmnd *scsi_cmnd = ssp_task->cmd;
-	int has_data = 0, rc, priority = is_tmf;
+	int has_data = 0, priority = is_tmf;
 	u8 *buf_cmd;
 	u32 dw1 = 0, dw2 = 0;
 
@@ -944,12 +935,9 @@ static int prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->dw2 = cpu_to_le32(dw2);
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	if (has_data) {
-		rc = prep_prd_sge_v3_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v3_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -976,11 +964,9 @@ static int prep_ssp_v3_hw(struct hisi_hba *hisi_hba,
 			break;
 		}
 	}
-
-	return 0;
 }
 
-static int prep_smp_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_smp_v3_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -1018,10 +1004,9 @@ static int prep_smp_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->cmd_table_addr = cpu_to_le64(req_dma_addr);
 	hdr->sts_buffer_addr = cpu_to_le64(hisi_sas_status_buf_addr_dma(slot));
 
-	return 0;
 }
 
-static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_ata_v3_hw(struct hisi_hba *hisi_hba,
 			  struct hisi_sas_slot *slot)
 {
 	struct sas_task *task = slot->task;
@@ -1032,7 +1017,7 @@ static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
 	struct asd_sas_port *sas_port = device->port;
 	struct hisi_sas_port *port = to_hisi_sas_port(sas_port);
 	u8 *buf_cmd;
-	int has_data = 0, rc = 0, hdr_tag = 0;
+	int has_data = 0, hdr_tag = 0;
 	u32 dw1 = 0, dw2 = 0;
 
 	hdr->dw0 = cpu_to_le32(port->id << CMD_HDR_PORT_OFF);
@@ -1081,12 +1066,9 @@ static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
 	/* dw3 */
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	if (has_data) {
-		rc = prep_prd_sge_v3_hw(hisi_hba, slot, hdr, task->scatter,
+	if (has_data)
+		prep_prd_sge_v3_hw(hisi_hba, slot, hdr, task->scatter,
 					slot->n_elem);
-		if (rc)
-			return rc;
-	}
 
 	hdr->data_transfer_len = cpu_to_le32(task->total_xfer_len);
 	hdr->cmd_table_addr = cpu_to_le64(hisi_sas_cmd_hdr_addr_dma(slot));
@@ -1098,11 +1080,9 @@ static int prep_ata_v3_hw(struct hisi_hba *hisi_hba,
 		task->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
 	/* fill in command FIS */
 	memcpy(buf_cmd, &task->ata_task.fis, sizeof(struct host_to_dev_fis));
-
-	return 0;
 }
 
-static int prep_abort_v3_hw(struct hisi_hba *hisi_hba,
+static void prep_abort_v3_hw(struct hisi_hba *hisi_hba,
 		struct hisi_sas_slot *slot,
 		int device_id, int abort_flag, int tag_to_abort)
 {
@@ -1127,7 +1107,6 @@ static int prep_abort_v3_hw(struct hisi_hba *hisi_hba,
 	hdr->dw7 = cpu_to_le32(tag_to_abort << CMD_HDR_ABORT_IPTT_OFF);
 	hdr->transfer_tags = cpu_to_le32(slot->idx);
 
-	return 0;
 }
 
 static irqreturn_t phy_up_v3_hw(int phy_no, struct hisi_hba *hisi_hba)
-- 
1.9.1

[PATCH 3/6] scsi: hisi_sas: allocate slot buffer earlier

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

Currently we allocate the slot's memory buffer
after allocating the DQ slot.

To aid DQ lockout reduction, and allow slots to be
built in parallel, move this step (which can fail)
prior to allocating the slot.

Also a stray spin_unlock_irqrestore() is removed
from internal task exec function.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 55 ++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 2772e920..58cbe1f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -412,14 +412,22 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	if (rc)
 		goto err_out_dma_unmap;
 
+	slot = &hisi_hba->slot_info[slot_idx];
+	memset(slot, 0, sizeof(struct hisi_sas_slot));
+
+	slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
+				   GFP_ATOMIC, &slot->buf_dma);
+	if (!slot->buf) {
+		rc = -ENOMEM;
+		goto err_out_tag;
+	}
+
 	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
 	if (rc)
-		goto err_out_tag;
+		goto err_out_buf;
 
 	dlvry_queue = dq->id;
 	dlvry_queue_slot = dq->wr_point;
-	slot = &hisi_hba->slot_info[slot_idx];
-	memset(slot, 0, sizeof(struct hisi_sas_slot));
 
 	slot->idx = slot_idx;
 	slot->n_elem = n_elem;
@@ -434,12 +442,6 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	task->lldd_task = slot;
 	INIT_WORK(&slot->abort_slot, hisi_sas_slot_abort);
 
-	slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
-				   GFP_ATOMIC, &slot->buf_dma);
-	if (!slot->buf) {
-		rc = -ENOMEM;
-		goto err_out_slot_buf;
-	}
 	memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
 	memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
 	memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
@@ -474,8 +476,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 
 	return 0;
 
-err_out_slot_buf:
-	/* Nothing to be done */
+err_out_buf:
+	dma_pool_free(hisi_hba->buffer_pool, slot->buf,
+		      slot->buf_dma);
 err_out_tag:
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot_idx);
@@ -1519,17 +1522,26 @@ static int hisi_sas_query_task(struct sas_task *task)
 	}
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
+	slot = &hisi_hba->slot_info[slot_idx];
+	memset(slot, 0, sizeof(struct hisi_sas_slot));
+
+	slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
+			GFP_ATOMIC, &slot->buf_dma);
+	if (!slot->buf) {
+		rc = -ENOMEM;
+		goto err_out_tag;
+	}
 	spin_lock_irqsave(&dq->lock, flags_dq);
 	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
-	if (rc)
-		goto err_out_tag;
+	if (rc) {
+		rc = -ENOMEM;
+		spin_unlock_irqrestore(&dq->lock, flags_dq);
+		goto err_out_buf;
+	}
 
 	dlvry_queue = dq->id;
 	dlvry_queue_slot = dq->wr_point;
 
-	slot = &hisi_hba->slot_info[slot_idx];
-	memset(slot, 0, sizeof(struct hisi_sas_slot));
-
 	slot->idx = slot_idx;
 	slot->n_elem = n_elem;
 	slot->dlvry_queue = dlvry_queue;
@@ -1541,13 +1553,6 @@ static int hisi_sas_query_task(struct sas_task *task)
 	slot->is_internal = true;
 	task->lldd_task = slot;
 
-	slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
-			GFP_ATOMIC, &slot->buf_dma);
-	if (!slot->buf) {
-		rc = -ENOMEM;
-		goto err_out_tag;
-	}
-
 	memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
 	memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
 	memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
@@ -1570,11 +1575,13 @@ static int hisi_sas_query_task(struct sas_task *task)
 
 	return 0;
 
+err_out_buf:
+	dma_pool_free(hisi_hba->buffer_pool, slot->buf,
+		      slot->buf_dma);
 err_out_tag:
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot_idx);
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
-	spin_unlock_irqrestore(&dq->lock, flags_dq);
 err_out:
 	dev_err(dev, "internal abort task prep: failed[%d]!\n", rc);
 
-- 
1.9.1

[PATCH 3/6] scsi: hisi_sas: allocate slot buffer earlier

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

Currently we allocate the slot's memory buffer
after allocating the DQ slot.

To aid DQ lockout reduction, and allow slots to be
built in parallel, move this step (which can fail)
prior to allocating the slot.

Also a stray spin_unlock_irqrestore() is removed
from internal task exec function.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 55 ++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 2772e920..58cbe1f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -412,14 +412,22 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	if (rc)
 		goto err_out_dma_unmap;
 
+	slot = &hisi_hba->slot_info[slot_idx];
+	memset(slot, 0, sizeof(struct hisi_sas_slot));
+
+	slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
+				   GFP_ATOMIC, &slot->buf_dma);
+	if (!slot->buf) {
+		rc = -ENOMEM;
+		goto err_out_tag;
+	}
+
 	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
 	if (rc)
-		goto err_out_tag;
+		goto err_out_buf;
 
 	dlvry_queue = dq->id;
 	dlvry_queue_slot = dq->wr_point;
-	slot = &hisi_hba->slot_info[slot_idx];
-	memset(slot, 0, sizeof(struct hisi_sas_slot));
 
 	slot->idx = slot_idx;
 	slot->n_elem = n_elem;
@@ -434,12 +442,6 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	task->lldd_task = slot;
 	INIT_WORK(&slot->abort_slot, hisi_sas_slot_abort);
 
-	slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
-				   GFP_ATOMIC, &slot->buf_dma);
-	if (!slot->buf) {
-		rc = -ENOMEM;
-		goto err_out_slot_buf;
-	}
 	memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
 	memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
 	memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
@@ -474,8 +476,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 
 	return 0;
 
-err_out_slot_buf:
-	/* Nothing to be done */
+err_out_buf:
+	dma_pool_free(hisi_hba->buffer_pool, slot->buf,
+		      slot->buf_dma);
 err_out_tag:
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot_idx);
@@ -1519,17 +1522,26 @@ static int hisi_sas_query_task(struct sas_task *task)
 	}
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
+	slot = &hisi_hba->slot_info[slot_idx];
+	memset(slot, 0, sizeof(struct hisi_sas_slot));
+
+	slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
+			GFP_ATOMIC, &slot->buf_dma);
+	if (!slot->buf) {
+		rc = -ENOMEM;
+		goto err_out_tag;
+	}
 	spin_lock_irqsave(&dq->lock, flags_dq);
 	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
-	if (rc)
-		goto err_out_tag;
+	if (rc) {
+		rc = -ENOMEM;
+		spin_unlock_irqrestore(&dq->lock, flags_dq);
+		goto err_out_buf;
+	}
 
 	dlvry_queue = dq->id;
 	dlvry_queue_slot = dq->wr_point;
 
-	slot = &hisi_hba->slot_info[slot_idx];
-	memset(slot, 0, sizeof(struct hisi_sas_slot));
-
 	slot->idx = slot_idx;
 	slot->n_elem = n_elem;
 	slot->dlvry_queue = dlvry_queue;
@@ -1541,13 +1553,6 @@ static int hisi_sas_query_task(struct sas_task *task)
 	slot->is_internal = true;
 	task->lldd_task = slot;
 
-	slot->buf = dma_pool_alloc(hisi_hba->buffer_pool,
-			GFP_ATOMIC, &slot->buf_dma);
-	if (!slot->buf) {
-		rc = -ENOMEM;
-		goto err_out_tag;
-	}
-
 	memset(slot->cmd_hdr, 0, sizeof(struct hisi_sas_cmd_hdr));
 	memset(hisi_sas_cmd_hdr_addr_mem(slot), 0, HISI_SAS_COMMAND_TABLE_SZ);
 	memset(hisi_sas_status_buf_addr_mem(slot), 0, HISI_SAS_STATUS_BUF_SZ);
@@ -1570,11 +1575,13 @@ static int hisi_sas_query_task(struct sas_task *task)
 
 	return 0;
 
+err_out_buf:
+	dma_pool_free(hisi_hba->buffer_pool, slot->buf,
+		      slot->buf_dma);
 err_out_tag:
 	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot_idx);
 	spin_unlock_irqrestore(&hisi_hba->lock, flags);
-	spin_unlock_irqrestore(&dq->lock, flags_dq);
 err_out:
 	dev_err(dev, "internal abort task prep: failed[%d]!\n", rc);
 
-- 
1.9.1

[PATCH 4/6] scsi: hisi_sas: Don't lock DQ for complete task sending

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

Currently we lock the DQ to protect whole delivery process.
So this stops us building slots for the same queue in
parallel, and can affect performance.

To optimise it, only lock the DQ during special periods,
specifically when allocating a slot from the DQ and when
delivering a slot to the HW.

This approach is now safe, thanks to the previous patches
to ensure that we always deliver a slot to the HW once
allocated.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas.h       |  4 ++-
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 49 ++++++++++++++++++++--------------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 27 ++++++++++++++-----
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 29 +++++++++++++++-----
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 28 ++++++++++++++-----
 5 files changed, 96 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 3a1caa04..52fc709 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -161,7 +161,7 @@ struct hisi_sas_cq {
 
 struct hisi_sas_dq {
 	struct hisi_hba *hisi_hba;
-	struct hisi_sas_slot	*slot_prep;
+	struct list_head list;
 	spinlock_t lock;
 	int	wr_point;
 	int	id;
@@ -181,6 +181,7 @@ struct hisi_sas_device {
 
 struct hisi_sas_slot {
 	struct list_head entry;
+	struct list_head delivery;
 	struct sas_task *task;
 	struct hisi_sas_port	*port;
 	u64	n_elem;
@@ -190,6 +191,7 @@ struct hisi_sas_slot {
 	int	cmplt_queue_slot;
 	int	idx;
 	int	abort;
+	int	ready;
 	void	*buf;
 	dma_addr_t buf_dma;
 	void	*cmd_hdr;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 58cbe1f..bf374a7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -307,9 +307,9 @@ static void hisi_sas_slot_abort(struct work_struct *work)
 		task->task_done(task);
 }
 
-static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
-		*dq, int is_tmf, struct hisi_sas_tmf_task *tmf,
-		int *pass)
+static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
+			      int is_tmf, struct hisi_sas_tmf_task *tmf,
+			      int *pass)
 {
 	struct hisi_hba *hisi_hba = dq->hisi_hba;
 	struct domain_device *device = task->dev;
@@ -321,7 +321,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	struct device *dev = hisi_hba->dev;
 	int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
 	int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
-	unsigned long flags;
+	unsigned long flags, flags_dq;
+	int wr_q_index;
 
 	if (!sas_port) {
 		struct task_status_struct *ts = &task->task_status;
@@ -422,12 +423,18 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 		goto err_out_tag;
 	}
 
-	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
-	if (rc)
+	spin_lock_irqsave(&dq->lock, flags_dq);
+	wr_q_index = hisi_hba->hw->get_free_slot(hisi_hba, dq);
+	if (wr_q_index < 0) {
+		spin_unlock_irqrestore(&dq->lock, flags_dq);
 		goto err_out_buf;
+	}
+
+	list_add_tail(&slot->delivery, &dq->list);
+	spin_unlock_irqrestore(&dq->lock, flags_dq);
 
 	dlvry_queue = dq->id;
-	dlvry_queue_slot = dq->wr_point;
+	dlvry_queue_slot = wr_q_index;
 
 	slot->idx = slot_idx;
 	slot->n_elem = n_elem;
@@ -471,8 +478,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	task->task_state_flags |= SAS_TASK_AT_INITIATOR;
 	spin_unlock_irqrestore(&task->task_state_lock, flags);
 
-	dq->slot_prep = slot;
 	++(*pass);
+	slot->ready = 1;
 
 	return 0;
 
@@ -518,11 +525,11 @@ static int hisi_sas_task_exec(struct sas_task *task, gfp_t gfp_flags,
 		return -EINVAL;
 
 	/* protect task_prep and start_delivery sequence */
-	spin_lock_irqsave(&dq->lock, flags);
 	rc = hisi_sas_task_prep(task, dq, is_tmf, tmf, &pass);
 	if (rc)
 		dev_err(dev, "task exec: failed[%d]!\n", rc);
 
+	spin_lock_irqsave(&dq->lock, flags);
 	if (likely(pass))
 		hisi_hba->hw->start_delivery(dq);
 	spin_unlock_irqrestore(&dq->lock, flags);
@@ -1503,7 +1510,8 @@ static int hisi_sas_query_task(struct sas_task *task)
 	struct hisi_sas_cmd_hdr *cmd_hdr_base;
 	struct hisi_sas_dq *dq = sas_dev->dq;
 	int dlvry_queue_slot, dlvry_queue, n_elem = 0, rc, slot_idx;
-	unsigned long flags, flags_dq;
+	unsigned long flags, flags_dq = 0;
+	int wr_q_index;
 
 	if (unlikely(test_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags)))
 		return -EINVAL;
@@ -1531,16 +1539,18 @@ static int hisi_sas_query_task(struct sas_task *task)
 		rc = -ENOMEM;
 		goto err_out_tag;
 	}
+
 	spin_lock_irqsave(&dq->lock, flags_dq);
-	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
-	if (rc) {
-		rc = -ENOMEM;
+	wr_q_index = hisi_hba->hw->get_free_slot(hisi_hba, dq);
+	if (wr_q_index < 0) {
 		spin_unlock_irqrestore(&dq->lock, flags_dq);
 		goto err_out_buf;
 	}
+	list_add_tail(&slot->delivery, &dq->list);
+	spin_unlock_irqrestore(&dq->lock, flags_dq);
 
 	dlvry_queue = dq->id;
-	dlvry_queue_slot = dq->wr_point;
+	dlvry_queue_slot = wr_q_index;
 
 	slot->idx = slot_idx;
 	slot->n_elem = n_elem;
@@ -1560,18 +1570,16 @@ static int hisi_sas_query_task(struct sas_task *task)
 	hisi_sas_task_prep_abort(hisi_hba, slot, device_id,
 				      abort_flag, task_tag);
 
-	spin_lock_irqsave(&hisi_hba->lock, flags);
-	list_add_tail(&slot->entry, &sas_dev->list);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 	spin_lock_irqsave(&task->task_state_lock, flags);
 	task->task_state_flags |= SAS_TASK_AT_INITIATOR;
 	spin_unlock_irqrestore(&task->task_state_lock, flags);
 
-	dq->slot_prep = slot;
-
+	slot->ready = 1;
 	/* send abort command to the chip */
+	spin_lock_irqsave(&dq->lock, flags);
+	list_add_tail(&slot->entry, &sas_dev->list);
 	hisi_hba->hw->start_delivery(dq);
-	spin_unlock_irqrestore(&dq->lock, flags_dq);
+	spin_unlock_irqrestore(&dq->lock, flags);
 
 	return 0;
 
@@ -1856,6 +1864,7 @@ int hisi_sas_alloc(struct hisi_hba *hisi_hba, struct Scsi_Host *shost)
 
 		/* Delivery queue structure */
 		spin_lock_init(&dq->lock);
+		INIT_LIST_HEAD(&dq->list);
 		dq->id = i;
 		dq->hisi_hba = hisi_hba;
 
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 7781a01..abe175f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -921,18 +921,33 @@ static int get_wideport_bitmap_v1_hw(struct hisi_hba *hisi_hba, int port_id)
 		return -EAGAIN;
 	}
 
-	return 0;
+	dq->wr_point = (dq->wr_point + 1) % HISI_SAS_QUEUE_SLOTS;
+
+	return w;
 }
 
+/* DQ lock must be taken here */
 static void start_delivery_v1_hw(struct hisi_sas_dq *dq)
 {
 	struct hisi_hba *hisi_hba = dq->hisi_hba;
-	int dlvry_queue = dq->slot_prep->dlvry_queue;
-	int dlvry_queue_slot = dq->slot_prep->dlvry_queue_slot;
+	struct hisi_sas_slot *s, *s1;
+	struct list_head *dq_list;
+	int dlvry_queue = dq->id;
+	int wp, count = 0;
+
+	dq_list = &dq->list;
+	list_for_each_entry_safe(s, s1, &dq->list, delivery) {
+		if (!s->ready)
+			break;
+		count++;
+		wp = (s->dlvry_queue_slot + 1) % HISI_SAS_QUEUE_SLOTS;
+		list_del(&s->delivery);
+	}
+
+	if (!count)
+		return;
 
-	dq->wr_point = ++dlvry_queue_slot % HISI_SAS_QUEUE_SLOTS;
-	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14),
-			 dq->wr_point);
+	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14), wp);
 }
 
 static void prep_prd_sge_v1_hw(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 9f77fd8..911bb76 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1663,23 +1663,38 @@ static int get_wideport_bitmap_v2_hw(struct hisi_hba *hisi_hba, int port_id)
 	r = hisi_sas_read32_relaxed(hisi_hba,
 				DLVRY_Q_0_RD_PTR + (queue * 0x14));
 	if (r == (w+1) % HISI_SAS_QUEUE_SLOTS) {
-		dev_warn(dev, "full queue=%d r=%d w=%d\n\n",
+		dev_warn(dev, "full queue=%d r=%d w=%d\n",
 				queue, r, w);
 		return -EAGAIN;
 	}
 
-	return 0;
+	dq->wr_point = (dq->wr_point + 1) % HISI_SAS_QUEUE_SLOTS;
+
+	return w;
 }
 
+/* DQ lock must be taken here */
 static void start_delivery_v2_hw(struct hisi_sas_dq *dq)
 {
 	struct hisi_hba *hisi_hba = dq->hisi_hba;
-	int dlvry_queue = dq->slot_prep->dlvry_queue;
-	int dlvry_queue_slot = dq->slot_prep->dlvry_queue_slot;
+	struct hisi_sas_slot *s, *s1;
+	struct list_head *dq_list;
+	int dlvry_queue = dq->id;
+	int wp, count = 0;
+
+	dq_list = &dq->list;
+	list_for_each_entry_safe(s, s1, &dq->list, delivery) {
+		if (!s->ready)
+			break;
+		count++;
+		wp = (s->dlvry_queue_slot + 1) % HISI_SAS_QUEUE_SLOTS;
+		list_del(&s->delivery);
+	}
+
+	if (!count)
+		return;
 
-	dq->wr_point = ++dlvry_queue_slot % HISI_SAS_QUEUE_SLOTS;
-	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14),
-			 dq->wr_point);
+	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14), wp);
 }
 
 static void prep_prd_sge_v2_hw(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 8cd1374..56f1046 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -840,23 +840,37 @@ static int get_wideport_bitmap_v3_hw(struct hisi_hba *hisi_hba, int port_id)
 	r = hisi_sas_read32_relaxed(hisi_hba,
 				DLVRY_Q_0_RD_PTR + (queue * 0x14));
 	if (r == (w+1) % HISI_SAS_QUEUE_SLOTS) {
-		dev_warn(dev, "full queue=%d r=%d w=%d\n\n",
+		dev_warn(dev, "full queue=%d r=%d w=%d\n",
 				queue, r, w);
 		return -EAGAIN;
 	}
 
-	return 0;
+	dq->wr_point = (dq->wr_point + 1) % HISI_SAS_QUEUE_SLOTS;
+
+	return w;
 }
 
 static void start_delivery_v3_hw(struct hisi_sas_dq *dq)
 {
 	struct hisi_hba *hisi_hba = dq->hisi_hba;
-	int dlvry_queue = dq->slot_prep->dlvry_queue;
-	int dlvry_queue_slot = dq->slot_prep->dlvry_queue_slot;
+	struct hisi_sas_slot *s, *s1;
+	struct list_head *dq_list;
+	int dlvry_queue = dq->id;
+	int wp, count = 0;
+
+	dq_list = &dq->list;
+	list_for_each_entry_safe(s, s1, &dq->list, delivery) {
+		if (!s->ready)
+			break;
+		count++;
+		wp = (s->dlvry_queue_slot + 1) % HISI_SAS_QUEUE_SLOTS;
+		list_del(&s->delivery);
+	}
+
+	if (!count)
+		return;
 
-	dq->wr_point = ++dlvry_queue_slot % HISI_SAS_QUEUE_SLOTS;
-	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14),
-			 dq->wr_point);
+	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14), wp);
 }
 
 static void prep_prd_sge_v3_hw(struct hisi_hba *hisi_hba,
-- 
1.9.1

[PATCH 4/6] scsi: hisi_sas: Don't lock DQ for complete task sending

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

Currently we lock the DQ to protect whole delivery process.
So this stops us building slots for the same queue in
parallel, and can affect performance.

To optimise it, only lock the DQ during special periods,
specifically when allocating a slot from the DQ and when
delivering a slot to the HW.

This approach is now safe, thanks to the previous patches
to ensure that we always deliver a slot to the HW once
allocated.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas.h       |  4 ++-
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 49 ++++++++++++++++++++--------------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 27 ++++++++++++++-----
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 29 +++++++++++++++-----
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 28 ++++++++++++++-----
 5 files changed, 96 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 3a1caa04..52fc709 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -161,7 +161,7 @@ struct hisi_sas_cq {
 
 struct hisi_sas_dq {
 	struct hisi_hba *hisi_hba;
-	struct hisi_sas_slot	*slot_prep;
+	struct list_head list;
 	spinlock_t lock;
 	int	wr_point;
 	int	id;
@@ -181,6 +181,7 @@ struct hisi_sas_device {
 
 struct hisi_sas_slot {
 	struct list_head entry;
+	struct list_head delivery;
 	struct sas_task *task;
 	struct hisi_sas_port	*port;
 	u64	n_elem;
@@ -190,6 +191,7 @@ struct hisi_sas_slot {
 	int	cmplt_queue_slot;
 	int	idx;
 	int	abort;
+	int	ready;
 	void	*buf;
 	dma_addr_t buf_dma;
 	void	*cmd_hdr;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 58cbe1f..bf374a7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -307,9 +307,9 @@ static void hisi_sas_slot_abort(struct work_struct *work)
 		task->task_done(task);
 }
 
-static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
-		*dq, int is_tmf, struct hisi_sas_tmf_task *tmf,
-		int *pass)
+static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
+			      int is_tmf, struct hisi_sas_tmf_task *tmf,
+			      int *pass)
 {
 	struct hisi_hba *hisi_hba = dq->hisi_hba;
 	struct domain_device *device = task->dev;
@@ -321,7 +321,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	struct device *dev = hisi_hba->dev;
 	int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
 	int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
-	unsigned long flags;
+	unsigned long flags, flags_dq;
+	int wr_q_index;
 
 	if (!sas_port) {
 		struct task_status_struct *ts = &task->task_status;
@@ -422,12 +423,18 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 		goto err_out_tag;
 	}
 
-	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
-	if (rc)
+	spin_lock_irqsave(&dq->lock, flags_dq);
+	wr_q_index = hisi_hba->hw->get_free_slot(hisi_hba, dq);
+	if (wr_q_index < 0) {
+		spin_unlock_irqrestore(&dq->lock, flags_dq);
 		goto err_out_buf;
+	}
+
+	list_add_tail(&slot->delivery, &dq->list);
+	spin_unlock_irqrestore(&dq->lock, flags_dq);
 
 	dlvry_queue = dq->id;
-	dlvry_queue_slot = dq->wr_point;
+	dlvry_queue_slot = wr_q_index;
 
 	slot->idx = slot_idx;
 	slot->n_elem = n_elem;
@@ -471,8 +478,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq
 	task->task_state_flags |= SAS_TASK_AT_INITIATOR;
 	spin_unlock_irqrestore(&task->task_state_lock, flags);
 
-	dq->slot_prep = slot;
 	++(*pass);
+	slot->ready = 1;
 
 	return 0;
 
@@ -518,11 +525,11 @@ static int hisi_sas_task_exec(struct sas_task *task, gfp_t gfp_flags,
 		return -EINVAL;
 
 	/* protect task_prep and start_delivery sequence */
-	spin_lock_irqsave(&dq->lock, flags);
 	rc = hisi_sas_task_prep(task, dq, is_tmf, tmf, &pass);
 	if (rc)
 		dev_err(dev, "task exec: failed[%d]!\n", rc);
 
+	spin_lock_irqsave(&dq->lock, flags);
 	if (likely(pass))
 		hisi_hba->hw->start_delivery(dq);
 	spin_unlock_irqrestore(&dq->lock, flags);
@@ -1503,7 +1510,8 @@ static int hisi_sas_query_task(struct sas_task *task)
 	struct hisi_sas_cmd_hdr *cmd_hdr_base;
 	struct hisi_sas_dq *dq = sas_dev->dq;
 	int dlvry_queue_slot, dlvry_queue, n_elem = 0, rc, slot_idx;
-	unsigned long flags, flags_dq;
+	unsigned long flags, flags_dq = 0;
+	int wr_q_index;
 
 	if (unlikely(test_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags)))
 		return -EINVAL;
@@ -1531,16 +1539,18 @@ static int hisi_sas_query_task(struct sas_task *task)
 		rc = -ENOMEM;
 		goto err_out_tag;
 	}
+
 	spin_lock_irqsave(&dq->lock, flags_dq);
-	rc = hisi_hba->hw->get_free_slot(hisi_hba, dq);
-	if (rc) {
-		rc = -ENOMEM;
+	wr_q_index = hisi_hba->hw->get_free_slot(hisi_hba, dq);
+	if (wr_q_index < 0) {
 		spin_unlock_irqrestore(&dq->lock, flags_dq);
 		goto err_out_buf;
 	}
+	list_add_tail(&slot->delivery, &dq->list);
+	spin_unlock_irqrestore(&dq->lock, flags_dq);
 
 	dlvry_queue = dq->id;
-	dlvry_queue_slot = dq->wr_point;
+	dlvry_queue_slot = wr_q_index;
 
 	slot->idx = slot_idx;
 	slot->n_elem = n_elem;
@@ -1560,18 +1570,16 @@ static int hisi_sas_query_task(struct sas_task *task)
 	hisi_sas_task_prep_abort(hisi_hba, slot, device_id,
 				      abort_flag, task_tag);
 
-	spin_lock_irqsave(&hisi_hba->lock, flags);
-	list_add_tail(&slot->entry, &sas_dev->list);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 	spin_lock_irqsave(&task->task_state_lock, flags);
 	task->task_state_flags |= SAS_TASK_AT_INITIATOR;
 	spin_unlock_irqrestore(&task->task_state_lock, flags);
 
-	dq->slot_prep = slot;
-
+	slot->ready = 1;
 	/* send abort command to the chip */
+	spin_lock_irqsave(&dq->lock, flags);
+	list_add_tail(&slot->entry, &sas_dev->list);
 	hisi_hba->hw->start_delivery(dq);
-	spin_unlock_irqrestore(&dq->lock, flags_dq);
+	spin_unlock_irqrestore(&dq->lock, flags);
 
 	return 0;
 
@@ -1856,6 +1864,7 @@ int hisi_sas_alloc(struct hisi_hba *hisi_hba, struct Scsi_Host *shost)
 
 		/* Delivery queue structure */
 		spin_lock_init(&dq->lock);
+		INIT_LIST_HEAD(&dq->list);
 		dq->id = i;
 		dq->hisi_hba = hisi_hba;
 
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 7781a01..abe175f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -921,18 +921,33 @@ static int get_wideport_bitmap_v1_hw(struct hisi_hba *hisi_hba, int port_id)
 		return -EAGAIN;
 	}
 
-	return 0;
+	dq->wr_point = (dq->wr_point + 1) % HISI_SAS_QUEUE_SLOTS;
+
+	return w;
 }
 
+/* DQ lock must be taken here */
 static void start_delivery_v1_hw(struct hisi_sas_dq *dq)
 {
 	struct hisi_hba *hisi_hba = dq->hisi_hba;
-	int dlvry_queue = dq->slot_prep->dlvry_queue;
-	int dlvry_queue_slot = dq->slot_prep->dlvry_queue_slot;
+	struct hisi_sas_slot *s, *s1;
+	struct list_head *dq_list;
+	int dlvry_queue = dq->id;
+	int wp, count = 0;
+
+	dq_list = &dq->list;
+	list_for_each_entry_safe(s, s1, &dq->list, delivery) {
+		if (!s->ready)
+			break;
+		count++;
+		wp = (s->dlvry_queue_slot + 1) % HISI_SAS_QUEUE_SLOTS;
+		list_del(&s->delivery);
+	}
+
+	if (!count)
+		return;
 
-	dq->wr_point = ++dlvry_queue_slot % HISI_SAS_QUEUE_SLOTS;
-	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14),
-			 dq->wr_point);
+	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14), wp);
 }
 
 static void prep_prd_sge_v1_hw(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 9f77fd8..911bb76 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1663,23 +1663,38 @@ static int get_wideport_bitmap_v2_hw(struct hisi_hba *hisi_hba, int port_id)
 	r = hisi_sas_read32_relaxed(hisi_hba,
 				DLVRY_Q_0_RD_PTR + (queue * 0x14));
 	if (r == (w+1) % HISI_SAS_QUEUE_SLOTS) {
-		dev_warn(dev, "full queue=%d r=%d w=%d\n\n",
+		dev_warn(dev, "full queue=%d r=%d w=%d\n",
 				queue, r, w);
 		return -EAGAIN;
 	}
 
-	return 0;
+	dq->wr_point = (dq->wr_point + 1) % HISI_SAS_QUEUE_SLOTS;
+
+	return w;
 }
 
+/* DQ lock must be taken here */
 static void start_delivery_v2_hw(struct hisi_sas_dq *dq)
 {
 	struct hisi_hba *hisi_hba = dq->hisi_hba;
-	int dlvry_queue = dq->slot_prep->dlvry_queue;
-	int dlvry_queue_slot = dq->slot_prep->dlvry_queue_slot;
+	struct hisi_sas_slot *s, *s1;
+	struct list_head *dq_list;
+	int dlvry_queue = dq->id;
+	int wp, count = 0;
+
+	dq_list = &dq->list;
+	list_for_each_entry_safe(s, s1, &dq->list, delivery) {
+		if (!s->ready)
+			break;
+		count++;
+		wp = (s->dlvry_queue_slot + 1) % HISI_SAS_QUEUE_SLOTS;
+		list_del(&s->delivery);
+	}
+
+	if (!count)
+		return;
 
-	dq->wr_point = ++dlvry_queue_slot % HISI_SAS_QUEUE_SLOTS;
-	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14),
-			 dq->wr_point);
+	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14), wp);
 }
 
 static void prep_prd_sge_v2_hw(struct hisi_hba *hisi_hba,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 8cd1374..56f1046 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -840,23 +840,37 @@ static int get_wideport_bitmap_v3_hw(struct hisi_hba *hisi_hba, int port_id)
 	r = hisi_sas_read32_relaxed(hisi_hba,
 				DLVRY_Q_0_RD_PTR + (queue * 0x14));
 	if (r == (w+1) % HISI_SAS_QUEUE_SLOTS) {
-		dev_warn(dev, "full queue=%d r=%d w=%d\n\n",
+		dev_warn(dev, "full queue=%d r=%d w=%d\n",
 				queue, r, w);
 		return -EAGAIN;
 	}
 
-	return 0;
+	dq->wr_point = (dq->wr_point + 1) % HISI_SAS_QUEUE_SLOTS;
+
+	return w;
 }
 
 static void start_delivery_v3_hw(struct hisi_sas_dq *dq)
 {
 	struct hisi_hba *hisi_hba = dq->hisi_hba;
-	int dlvry_queue = dq->slot_prep->dlvry_queue;
-	int dlvry_queue_slot = dq->slot_prep->dlvry_queue_slot;
+	struct hisi_sas_slot *s, *s1;
+	struct list_head *dq_list;
+	int dlvry_queue = dq->id;
+	int wp, count = 0;
+
+	dq_list = &dq->list;
+	list_for_each_entry_safe(s, s1, &dq->list, delivery) {
+		if (!s->ready)
+			break;
+		count++;
+		wp = (s->dlvry_queue_slot + 1) % HISI_SAS_QUEUE_SLOTS;
+		list_del(&s->delivery);
+	}
+
+	if (!count)
+		return;
 
-	dq->wr_point = ++dlvry_queue_slot % HISI_SAS_QUEUE_SLOTS;
-	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14),
-			 dq->wr_point);
+	hisi_sas_write32(hisi_hba, DLVRY_Q_0_WR_PTR + (dlvry_queue * 0x14), wp);
 }
 
 static void prep_prd_sge_v3_hw(struct hisi_hba *hisi_hba,
-- 
1.9.1

[PATCH 5/6] scsi: hisi_sas: Use device lock to protect slot alloc/free

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

The IPTT of a slot is unique, and we currently use hisi_hba lock
to protect it.

Now slot is managed on hisi_sas_device.list, so use DQ lock to
protect for allocating and freeing the slot.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 56 ++++++++++------------------------
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c |  3 --
 2 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index bf374a7..a451625 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -214,6 +214,8 @@ static void hisi_sas_slot_index_init(struct hisi_hba *hisi_hba)
 void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 			     struct hisi_sas_slot *slot)
 {
+	struct hisi_sas_dq *dq = &hisi_hba->dq[slot->dlvry_queue];
+	unsigned long flags;
 
 	if (task) {
 		struct device *dev = hisi_hba->dev;
@@ -233,11 +235,15 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 	if (slot->buf)
 		dma_pool_free(hisi_hba->buffer_pool, slot->buf, slot->buf_dma);
 
+	spin_lock_irqsave(&dq->lock, flags);
 	list_del_init(&slot->entry);
+	spin_unlock_irqrestore(&dq->lock, flags);
 	slot->buf = NULL;
 	slot->task = NULL;
 	slot->port = NULL;
+	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot->idx);
+	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
 	/* slot memory is fully zeroed when it is reused */
 }
@@ -286,7 +292,6 @@ static void hisi_sas_slot_abort(struct work_struct *work)
 	struct scsi_lun lun;
 	struct device *dev = hisi_hba->dev;
 	int tag = abort_slot->idx;
-	unsigned long flags;
 
 	if (!(task->task_proto & SAS_PROTOCOL_SSP)) {
 		dev_err(dev, "cannot abort slot for non-ssp task\n");
@@ -300,9 +305,7 @@ static void hisi_sas_slot_abort(struct work_struct *work)
 	hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun, &tmf_task);
 out:
 	/* Do cleanup for this task */
-	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_task_free(hisi_hba, task, abort_slot);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 	if (task->task_done)
 		task->task_done(task);
 }
@@ -471,9 +474,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
 		break;
 	}
 
-	spin_lock_irqsave(&hisi_hba->lock, flags);
+	spin_lock_irqsave(&dq->lock, flags);
 	list_add_tail(&slot->entry, &sas_dev->list);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
+	spin_unlock_irqrestore(&dq->lock, flags);
 	spin_lock_irqsave(&task->task_state_lock, flags);
 	task->task_state_flags |= SAS_TASK_AT_INITIATOR;
 	spin_unlock_irqrestore(&task->task_state_lock, flags);
@@ -1047,7 +1050,6 @@ static int hisi_sas_softreset_ata_disk(struct domain_device *device)
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct device *dev = hisi_hba->dev;
 	int s = sizeof(struct host_to_dev_fis);
-	unsigned long flags;
 
 	ata_for_each_link(link, ap, EDGE) {
 		int pmp = sata_srst_pmp(link);
@@ -1072,11 +1074,8 @@ static int hisi_sas_softreset_ata_disk(struct domain_device *device)
 		dev_err(dev, "ata disk reset failed\n");
 	}
 
-	if (rc == TMF_RESP_FUNC_COMPLETE) {
-		spin_lock_irqsave(&hisi_hba->lock, flags);
+	if (rc == TMF_RESP_FUNC_COMPLETE)
 		hisi_sas_release_task(hisi_hba, device);
-		spin_unlock_irqrestore(&hisi_hba->lock, flags);
-	}
 
 	return rc;
 }
@@ -1173,7 +1172,6 @@ static int hisi_sas_controller_reset(struct hisi_hba *hisi_hba)
 	struct device *dev = hisi_hba->dev;
 	struct Scsi_Host *shost = hisi_hba->shost;
 	u32 old_state, state;
-	unsigned long flags;
 	int rc;
 
 	if (!hisi_hba->hw->soft_reset)
@@ -1197,9 +1195,7 @@ static int hisi_sas_controller_reset(struct hisi_hba *hisi_hba)
 		scsi_unblock_requests(shost);
 		goto out;
 	}
-	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_release_tasks(hisi_hba);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
 	clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
 
@@ -1274,11 +1270,8 @@ static int hisi_sas_abort_task(struct sas_task *task)
 		 * will have already been completed
 		 */
 		if (rc == TMF_RESP_FUNC_COMPLETE && rc2 != TMF_RESP_FUNC_SUCC) {
-			if (task->lldd_task) {
-				spin_lock_irqsave(&hisi_hba->lock, flags);
+			if (task->lldd_task)
 				hisi_sas_do_release_task(hisi_hba, task, slot);
-				spin_unlock_irqrestore(&hisi_hba->lock, flags);
-			}
 		}
 	} else if (task->task_proto & SAS_PROTOCOL_SATA ||
 		task->task_proto & SAS_PROTOCOL_STP) {
@@ -1300,11 +1293,8 @@ static int hisi_sas_abort_task(struct sas_task *task)
 		rc = hisi_sas_internal_task_abort(hisi_hba, device,
 			     HISI_SAS_INT_ABT_CMD, tag);
 		if (((rc < 0) || (rc == TMF_RESP_FUNC_FAILED)) &&
-					task->lldd_task) {
-			spin_lock_irqsave(&hisi_hba->lock, flags);
+					task->lldd_task)
 			hisi_sas_do_release_task(hisi_hba, task, slot);
-			spin_unlock_irqrestore(&hisi_hba->lock, flags);
-		}
 	}
 
 out:
@@ -1319,7 +1309,6 @@ static int hisi_sas_abort_task_set(struct domain_device *device, u8 *lun)
 	struct device *dev = hisi_hba->dev;
 	struct hisi_sas_tmf_task tmf_task;
 	int rc = TMF_RESP_FUNC_FAILED;
-	unsigned long flags;
 
 	rc = hisi_sas_internal_task_abort(hisi_hba, device,
 					HISI_SAS_INT_ABT_DEV, 0);
@@ -1332,11 +1321,8 @@ static int hisi_sas_abort_task_set(struct domain_device *device, u8 *lun)
 	tmf_task.tmf = TMF_ABORT_TASK_SET;
 	rc = hisi_sas_debug_issue_ssp_tmf(device, lun, &tmf_task);
 
-	if (rc == TMF_RESP_FUNC_COMPLETE) {
-		spin_lock_irqsave(&hisi_hba->lock, flags);
+	if (rc == TMF_RESP_FUNC_COMPLETE)
 		hisi_sas_release_task(hisi_hba, device);
-		spin_unlock_irqrestore(&hisi_hba->lock, flags);
-	}
 
 	return rc;
 }
@@ -1369,7 +1355,6 @@ static int hisi_sas_I_T_nexus_reset(struct domain_device *device)
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct device *dev = hisi_hba->dev;
 	int rc = TMF_RESP_FUNC_FAILED;
-	unsigned long flags;
 
 	if (sas_dev->dev_status != HISI_SAS_DEV_EH)
 		return TMF_RESP_FUNC_FAILED;
@@ -1385,11 +1370,9 @@ static int hisi_sas_I_T_nexus_reset(struct domain_device *device)
 
 	rc = hisi_sas_debug_I_T_nexus_reset(device);
 
-	if ((rc == TMF_RESP_FUNC_COMPLETE) || (rc == -ENODEV)) {
-		spin_lock_irqsave(&hisi_hba->lock, flags);
+	if ((rc == TMF_RESP_FUNC_COMPLETE) || (rc == -ENODEV))
 		hisi_sas_release_task(hisi_hba, device);
-		spin_unlock_irqrestore(&hisi_hba->lock, flags);
-	}
+
 	return rc;
 }
 
@@ -1398,7 +1381,6 @@ static int hisi_sas_lu_reset(struct domain_device *device, u8 *lun)
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct device *dev = hisi_hba->dev;
-	unsigned long flags;
 	int rc = TMF_RESP_FUNC_FAILED;
 
 	sas_dev->dev_status = HISI_SAS_DEV_EH;
@@ -1418,11 +1400,8 @@ static int hisi_sas_lu_reset(struct domain_device *device, u8 *lun)
 
 		rc = sas_phy_reset(phy, 1);
 
-		if (rc == 0) {
-			spin_lock_irqsave(&hisi_hba->lock, flags);
+		if (rc == 0)
 			hisi_sas_release_task(hisi_hba, device);
-			spin_unlock_irqrestore(&hisi_hba->lock, flags);
-		}
 		sas_put_local_phy(phy);
 	} else {
 		struct hisi_sas_tmf_task tmf_task = { .tmf =  TMF_LU_RESET };
@@ -1436,11 +1415,8 @@ static int hisi_sas_lu_reset(struct domain_device *device, u8 *lun)
 		hisi_sas_dereg_device(hisi_hba, device);
 
 		rc = hisi_sas_debug_issue_ssp_tmf(device, lun, &tmf_task);
-		if (rc == TMF_RESP_FUNC_COMPLETE) {
-			spin_lock_irqsave(&hisi_hba->lock, flags);
+		if (rc == TMF_RESP_FUNC_COMPLETE)
 			hisi_sas_release_task(hisi_hba, device);
-			spin_unlock_irqrestore(&hisi_hba->lock, flags);
-		}
 	}
 out:
 	if (rc != TMF_RESP_FUNC_COMPLETE)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 56f1046..c013673 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2373,7 +2373,6 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, pm_message_t state)
 	u32 device_state, status;
 	int rc;
 	u32 reg_val;
-	unsigned long flags;
 
 	if (!pdev->pm_cap) {
 		dev_err(dev, "PCI PM not supported\n");
@@ -2418,9 +2417,7 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, pm_message_t state)
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, device_state);
 
-	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_release_tasks(hisi_hba);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
 	sas_suspend_ha(sha);
 	return 0;
-- 
1.9.1

[PATCH 5/6] scsi: hisi_sas: Use device lock to protect slot alloc/free

[John Garry]

From: Xiang Chen <chenxiang66@hisilicon.com>

The IPTT of a slot is unique, and we currently use hisi_hba lock
to protect it.

Now slot is managed on hisi_sas_device.list, so use DQ lock to
protect for allocating and freeing the slot.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 56 ++++++++++------------------------
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c |  3 --
 2 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index bf374a7..a451625 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -214,6 +214,8 @@ static void hisi_sas_slot_index_init(struct hisi_hba *hisi_hba)
 void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 			     struct hisi_sas_slot *slot)
 {
+	struct hisi_sas_dq *dq = &hisi_hba->dq[slot->dlvry_queue];
+	unsigned long flags;
 
 	if (task) {
 		struct device *dev = hisi_hba->dev;
@@ -233,11 +235,15 @@ void hisi_sas_slot_task_free(struct hisi_hba *hisi_hba, struct sas_task *task,
 	if (slot->buf)
 		dma_pool_free(hisi_hba->buffer_pool, slot->buf, slot->buf_dma);
 
+	spin_lock_irqsave(&dq->lock, flags);
 	list_del_init(&slot->entry);
+	spin_unlock_irqrestore(&dq->lock, flags);
 	slot->buf = NULL;
 	slot->task = NULL;
 	slot->port = NULL;
+	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_index_free(hisi_hba, slot->idx);
+	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
 	/* slot memory is fully zeroed when it is reused */
 }
@@ -286,7 +292,6 @@ static void hisi_sas_slot_abort(struct work_struct *work)
 	struct scsi_lun lun;
 	struct device *dev = hisi_hba->dev;
 	int tag = abort_slot->idx;
-	unsigned long flags;
 
 	if (!(task->task_proto & SAS_PROTOCOL_SSP)) {
 		dev_err(dev, "cannot abort slot for non-ssp task\n");
@@ -300,9 +305,7 @@ static void hisi_sas_slot_abort(struct work_struct *work)
 	hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun, &tmf_task);
 out:
 	/* Do cleanup for this task */
-	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_slot_task_free(hisi_hba, task, abort_slot);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 	if (task->task_done)
 		task->task_done(task);
 }
@@ -471,9 +474,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
 		break;
 	}
 
-	spin_lock_irqsave(&hisi_hba->lock, flags);
+	spin_lock_irqsave(&dq->lock, flags);
 	list_add_tail(&slot->entry, &sas_dev->list);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
+	spin_unlock_irqrestore(&dq->lock, flags);
 	spin_lock_irqsave(&task->task_state_lock, flags);
 	task->task_state_flags |= SAS_TASK_AT_INITIATOR;
 	spin_unlock_irqrestore(&task->task_state_lock, flags);
@@ -1047,7 +1050,6 @@ static int hisi_sas_softreset_ata_disk(struct domain_device *device)
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct device *dev = hisi_hba->dev;
 	int s = sizeof(struct host_to_dev_fis);
-	unsigned long flags;
 
 	ata_for_each_link(link, ap, EDGE) {
 		int pmp = sata_srst_pmp(link);
@@ -1072,11 +1074,8 @@ static int hisi_sas_softreset_ata_disk(struct domain_device *device)
 		dev_err(dev, "ata disk reset failed\n");
 	}
 
-	if (rc == TMF_RESP_FUNC_COMPLETE) {
-		spin_lock_irqsave(&hisi_hba->lock, flags);
+	if (rc == TMF_RESP_FUNC_COMPLETE)
 		hisi_sas_release_task(hisi_hba, device);
-		spin_unlock_irqrestore(&hisi_hba->lock, flags);
-	}
 
 	return rc;
 }
@@ -1173,7 +1172,6 @@ static int hisi_sas_controller_reset(struct hisi_hba *hisi_hba)
 	struct device *dev = hisi_hba->dev;
 	struct Scsi_Host *shost = hisi_hba->shost;
 	u32 old_state, state;
-	unsigned long flags;
 	int rc;
 
 	if (!hisi_hba->hw->soft_reset)
@@ -1197,9 +1195,7 @@ static int hisi_sas_controller_reset(struct hisi_hba *hisi_hba)
 		scsi_unblock_requests(shost);
 		goto out;
 	}
-	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_release_tasks(hisi_hba);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
 	clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
 
@@ -1274,11 +1270,8 @@ static int hisi_sas_abort_task(struct sas_task *task)
 		 * will have already been completed
 		 */
 		if (rc == TMF_RESP_FUNC_COMPLETE && rc2 != TMF_RESP_FUNC_SUCC) {
-			if (task->lldd_task) {
-				spin_lock_irqsave(&hisi_hba->lock, flags);
+			if (task->lldd_task)
 				hisi_sas_do_release_task(hisi_hba, task, slot);
-				spin_unlock_irqrestore(&hisi_hba->lock, flags);
-			}
 		}
 	} else if (task->task_proto & SAS_PROTOCOL_SATA ||
 		task->task_proto & SAS_PROTOCOL_STP) {
@@ -1300,11 +1293,8 @@ static int hisi_sas_abort_task(struct sas_task *task)
 		rc = hisi_sas_internal_task_abort(hisi_hba, device,
 			     HISI_SAS_INT_ABT_CMD, tag);
 		if (((rc < 0) || (rc == TMF_RESP_FUNC_FAILED)) &&
-					task->lldd_task) {
-			spin_lock_irqsave(&hisi_hba->lock, flags);
+					task->lldd_task)
 			hisi_sas_do_release_task(hisi_hba, task, slot);
-			spin_unlock_irqrestore(&hisi_hba->lock, flags);
-		}
 	}
 
 out:
@@ -1319,7 +1309,6 @@ static int hisi_sas_abort_task_set(struct domain_device *device, u8 *lun)
 	struct device *dev = hisi_hba->dev;
 	struct hisi_sas_tmf_task tmf_task;
 	int rc = TMF_RESP_FUNC_FAILED;
-	unsigned long flags;
 
 	rc = hisi_sas_internal_task_abort(hisi_hba, device,
 					HISI_SAS_INT_ABT_DEV, 0);
@@ -1332,11 +1321,8 @@ static int hisi_sas_abort_task_set(struct domain_device *device, u8 *lun)
 	tmf_task.tmf = TMF_ABORT_TASK_SET;
 	rc = hisi_sas_debug_issue_ssp_tmf(device, lun, &tmf_task);
 
-	if (rc == TMF_RESP_FUNC_COMPLETE) {
-		spin_lock_irqsave(&hisi_hba->lock, flags);
+	if (rc == TMF_RESP_FUNC_COMPLETE)
 		hisi_sas_release_task(hisi_hba, device);
-		spin_unlock_irqrestore(&hisi_hba->lock, flags);
-	}
 
 	return rc;
 }
@@ -1369,7 +1355,6 @@ static int hisi_sas_I_T_nexus_reset(struct domain_device *device)
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct device *dev = hisi_hba->dev;
 	int rc = TMF_RESP_FUNC_FAILED;
-	unsigned long flags;
 
 	if (sas_dev->dev_status != HISI_SAS_DEV_EH)
 		return TMF_RESP_FUNC_FAILED;
@@ -1385,11 +1370,9 @@ static int hisi_sas_I_T_nexus_reset(struct domain_device *device)
 
 	rc = hisi_sas_debug_I_T_nexus_reset(device);
 
-	if ((rc == TMF_RESP_FUNC_COMPLETE) || (rc == -ENODEV)) {
-		spin_lock_irqsave(&hisi_hba->lock, flags);
+	if ((rc == TMF_RESP_FUNC_COMPLETE) || (rc == -ENODEV))
 		hisi_sas_release_task(hisi_hba, device);
-		spin_unlock_irqrestore(&hisi_hba->lock, flags);
-	}
+
 	return rc;
 }
 
@@ -1398,7 +1381,6 @@ static int hisi_sas_lu_reset(struct domain_device *device, u8 *lun)
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct device *dev = hisi_hba->dev;
-	unsigned long flags;
 	int rc = TMF_RESP_FUNC_FAILED;
 
 	sas_dev->dev_status = HISI_SAS_DEV_EH;
@@ -1418,11 +1400,8 @@ static int hisi_sas_lu_reset(struct domain_device *device, u8 *lun)
 
 		rc = sas_phy_reset(phy, 1);
 
-		if (rc == 0) {
-			spin_lock_irqsave(&hisi_hba->lock, flags);
+		if (rc == 0)
 			hisi_sas_release_task(hisi_hba, device);
-			spin_unlock_irqrestore(&hisi_hba->lock, flags);
-		}
 		sas_put_local_phy(phy);
 	} else {
 		struct hisi_sas_tmf_task tmf_task = { .tmf =  TMF_LU_RESET };
@@ -1436,11 +1415,8 @@ static int hisi_sas_lu_reset(struct domain_device *device, u8 *lun)
 		hisi_sas_dereg_device(hisi_hba, device);
 
 		rc = hisi_sas_debug_issue_ssp_tmf(device, lun, &tmf_task);
-		if (rc == TMF_RESP_FUNC_COMPLETE) {
-			spin_lock_irqsave(&hisi_hba->lock, flags);
+		if (rc == TMF_RESP_FUNC_COMPLETE)
 			hisi_sas_release_task(hisi_hba, device);
-			spin_unlock_irqrestore(&hisi_hba->lock, flags);
-		}
 	}
 out:
 	if (rc != TMF_RESP_FUNC_COMPLETE)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 56f1046..c013673 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2373,7 +2373,6 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, pm_message_t state)
 	u32 device_state, status;
 	int rc;
 	u32 reg_val;
-	unsigned long flags;
 
 	if (!pdev->pm_cap) {
 		dev_err(dev, "PCI PM not supported\n");
@@ -2418,9 +2417,7 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, pm_message_t state)
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, device_state);
 
-	spin_lock_irqsave(&hisi_hba->lock, flags);
 	hisi_sas_release_tasks(hisi_hba);
-	spin_unlock_irqrestore(&hisi_hba->lock, flags);
 
 	sas_suspend_ha(sha);
 	return 0;
-- 
1.9.1

[PATCH 6/6] scsi: hisi_sas: add check of device in hisi_sas_task_exec()

[John Garry]

From: Xiaofei Tan <tanxiaofei@huawei.com>

Currently we don't check that device is not gone before
dereferencing it's elements in the function
hisi_sas_task_exec() (specifically, the DQ pointer).

This patch fixes this issue by filling in the DQ
pointer in hisi_sas_task_prep(), after we check
that the device pointer is still safe to
reference.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index a451625..39f694e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -310,12 +310,13 @@ static void hisi_sas_slot_abort(struct work_struct *work)
 		task->task_done(task);
 }
 
-static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
+static int hisi_sas_task_prep(struct sas_task *task,
+			      struct hisi_sas_dq **dq_pointer,
 			      int is_tmf, struct hisi_sas_tmf_task *tmf,
 			      int *pass)
 {
-	struct hisi_hba *hisi_hba = dq->hisi_hba;
 	struct domain_device *device = task->dev;
+	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	struct hisi_sas_port *port;
 	struct hisi_sas_slot *slot;
@@ -323,8 +324,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
 	struct asd_sas_port *sas_port = device->port;
 	struct device *dev = hisi_hba->dev;
 	int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
-	int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
+	int n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
 	unsigned long flags, flags_dq;
+	struct hisi_sas_dq *dq;
 	int wr_q_index;
 
 	if (!sas_port) {
@@ -352,6 +354,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
 		return -ECOMM;
 	}
 
+	*dq_pointer = dq = sas_dev->dq;
+
 	port = to_hisi_sas_port(sas_port);
 	if (port && !port->port_attached) {
 		dev_info(dev, "task prep: %s port%d not attach device\n",
@@ -520,22 +524,21 @@ static int hisi_sas_task_exec(struct sas_task *task, gfp_t gfp_flags,
 	unsigned long flags;
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(task->dev);
 	struct device *dev = hisi_hba->dev;
-	struct domain_device *device = task->dev;
-	struct hisi_sas_device *sas_dev = device->lldd_dev;
-	struct hisi_sas_dq *dq = sas_dev->dq;
+	struct hisi_sas_dq *dq = NULL;
 
 	if (unlikely(test_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags)))
 		return -EINVAL;
 
 	/* protect task_prep and start_delivery sequence */
-	rc = hisi_sas_task_prep(task, dq, is_tmf, tmf, &pass);
+	rc = hisi_sas_task_prep(task, &dq, is_tmf, tmf, &pass);
 	if (rc)
 		dev_err(dev, "task exec: failed[%d]!\n", rc);
 
-	spin_lock_irqsave(&dq->lock, flags);
-	if (likely(pass))
+	if (likely(pass)) {
+		spin_lock_irqsave(&dq->lock, flags);
 		hisi_hba->hw->start_delivery(dq);
-	spin_unlock_irqrestore(&dq->lock, flags);
+		spin_unlock_irqrestore(&dq->lock, flags);
+	}
 
 	return rc;
 }
-- 
1.9.1

[PATCH 6/6] scsi: hisi_sas: add check of device in hisi_sas_task_exec()

[John Garry]

From: Xiaofei Tan <tanxiaofei@huawei.com>

Currently we don't check that device is not gone before
dereferencing it's elements in the function
hisi_sas_task_exec() (specifically, the DQ pointer).

This patch fixes this issue by filling in the DQ
pointer in hisi_sas_task_prep(), after we check
that the device pointer is still safe to
reference.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index a451625..39f694e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -310,12 +310,13 @@ static void hisi_sas_slot_abort(struct work_struct *work)
 		task->task_done(task);
 }
 
-static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
+static int hisi_sas_task_prep(struct sas_task *task,
+			      struct hisi_sas_dq **dq_pointer,
 			      int is_tmf, struct hisi_sas_tmf_task *tmf,
 			      int *pass)
 {
-	struct hisi_hba *hisi_hba = dq->hisi_hba;
 	struct domain_device *device = task->dev;
+	struct hisi_hba *hisi_hba = dev_to_hisi_hba(device);
 	struct hisi_sas_device *sas_dev = device->lldd_dev;
 	struct hisi_sas_port *port;
 	struct hisi_sas_slot *slot;
@@ -323,8 +324,9 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
 	struct asd_sas_port *sas_port = device->port;
 	struct device *dev = hisi_hba->dev;
 	int dlvry_queue_slot, dlvry_queue, rc, slot_idx;
-	int  n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
+	int n_elem = 0, n_elem_req = 0, n_elem_resp = 0;
 	unsigned long flags, flags_dq;
+	struct hisi_sas_dq *dq;
 	int wr_q_index;
 
 	if (!sas_port) {
@@ -352,6 +354,8 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_sas_dq *dq,
 		return -ECOMM;
 	}
 
+	*dq_pointer = dq = sas_dev->dq;
+
 	port = to_hisi_sas_port(sas_port);
 	if (port && !port->port_attached) {
 		dev_info(dev, "task prep: %s port%d not attach device\n",
@@ -520,22 +524,21 @@ static int hisi_sas_task_exec(struct sas_task *task, gfp_t gfp_flags,
 	unsigned long flags;
 	struct hisi_hba *hisi_hba = dev_to_hisi_hba(task->dev);
 	struct device *dev = hisi_hba->dev;
-	struct domain_device *device = task->dev;
-	struct hisi_sas_device *sas_dev = device->lldd_dev;
-	struct hisi_sas_dq *dq = sas_dev->dq;
+	struct hisi_sas_dq *dq = NULL;
 
 	if (unlikely(test_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags)))
 		return -EINVAL;
 
 	/* protect task_prep and start_delivery sequence */
-	rc = hisi_sas_task_prep(task, dq, is_tmf, tmf, &pass);
+	rc = hisi_sas_task_prep(task, &dq, is_tmf, tmf, &pass);
 	if (rc)
 		dev_err(dev, "task exec: failed[%d]!\n", rc);
 
-	spin_lock_irqsave(&dq->lock, flags);
-	if (likely(pass))
+	if (likely(pass)) {
+		spin_lock_irqsave(&dq->lock, flags);
 		hisi_hba->hw->start_delivery(dq);
-	spin_unlock_irqrestore(&dq->lock, flags);
+		spin_unlock_irqrestore(&dq->lock, flags);
+	}
 
 	return rc;
 }
-- 
1.9.1

Re: [PATCH 0/6] hisi_sas: improve DQ locking

[Martin K. Petersen]

John,

> This patchset introduces some patches to much improve DQ lockout for
> sending commands to the HW.

Applied to 4.18/scsi-queue. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/6] hisi_sas: improve DQ locking

[Martin K. Petersen]

John,

> This patchset introduces some patches to much improve DQ lockout for
> sending commands to the HW.

Applied to 4.18/scsi-queue. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering