* [PATCH 1/6] scsi: hisi_sas: relocate smp sg map
2018-05-09 15:10 [PATCH 0/6] hisi_sas: improve DQ locking John Garry
@ 2018-05-09 15:10 ` John Garry
2018-05-09 15:10 ` [PATCH 2/6] scsi: hisi_sas: make return type of prep functions void John Garry
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2018-05-09 15:10 UTC (permalink / raw)
To: jejb, martin.petersen
Cc: linux-scsi, linuxarm, linux-kernel, Xiang Chen, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 2/6] scsi: hisi_sas: make return type of prep functions void
2018-05-09 15:10 [PATCH 0/6] hisi_sas: improve DQ locking John Garry
2018-05-09 15:10 ` [PATCH 1/6] scsi: hisi_sas: relocate smp sg map John Garry
@ 2018-05-09 15:10 ` John Garry
2018-05-09 15:10 ` [PATCH 3/6] scsi: hisi_sas: allocate slot buffer earlier John Garry
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2018-05-09 15:10 UTC (permalink / raw)
To: jejb, martin.petersen
Cc: linux-scsi, linuxarm, linux-kernel, Xiang Chen, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 3/6] scsi: hisi_sas: allocate slot buffer earlier
2018-05-09 15:10 [PATCH 0/6] hisi_sas: improve DQ locking John Garry
2018-05-09 15:10 ` [PATCH 1/6] scsi: hisi_sas: relocate smp sg map John Garry
2018-05-09 15:10 ` [PATCH 2/6] scsi: hisi_sas: make return type of prep functions void John Garry
@ 2018-05-09 15:10 ` John Garry
2018-05-09 15:10 ` [PATCH 4/6] scsi: hisi_sas: Don't lock DQ for complete task sending John Garry
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2018-05-09 15:10 UTC (permalink / raw)
To: jejb, martin.petersen
Cc: linux-scsi, linuxarm, linux-kernel, Xiang Chen, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 4/6] scsi: hisi_sas: Don't lock DQ for complete task sending
2018-05-09 15:10 [PATCH 0/6] hisi_sas: improve DQ locking John Garry
` (2 preceding siblings ...)
2018-05-09 15:10 ` [PATCH 3/6] scsi: hisi_sas: allocate slot buffer earlier John Garry
@ 2018-05-09 15:10 ` John Garry
2018-05-09 15:10 ` [PATCH 5/6] scsi: hisi_sas: Use device lock to protect slot alloc/free John Garry
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2018-05-09 15:10 UTC (permalink / raw)
To: jejb, martin.petersen
Cc: linux-scsi, linuxarm, linux-kernel, Xiang Chen, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 5/6] scsi: hisi_sas: Use device lock to protect slot alloc/free
2018-05-09 15:10 [PATCH 0/6] hisi_sas: improve DQ locking John Garry
` (3 preceding siblings ...)
2018-05-09 15:10 ` [PATCH 4/6] scsi: hisi_sas: Don't lock DQ for complete task sending John Garry
@ 2018-05-09 15:10 ` John Garry
2018-05-09 15:10 ` [PATCH 6/6] scsi: hisi_sas: add check of device in hisi_sas_task_exec() John Garry
2018-05-18 15:23 ` [PATCH 0/6] hisi_sas: improve DQ locking Martin K. Petersen
6 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2018-05-09 15:10 UTC (permalink / raw)
To: jejb, martin.petersen
Cc: linux-scsi, linuxarm, linux-kernel, Xiang Chen, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 6/6] scsi: hisi_sas: add check of device in hisi_sas_task_exec()
2018-05-09 15:10 [PATCH 0/6] hisi_sas: improve DQ locking John Garry
` (4 preceding siblings ...)
2018-05-09 15:10 ` [PATCH 5/6] scsi: hisi_sas: Use device lock to protect slot alloc/free John Garry
@ 2018-05-09 15:10 ` John Garry
2018-05-18 15:23 ` [PATCH 0/6] hisi_sas: improve DQ locking Martin K. Petersen
6 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2018-05-09 15:10 UTC (permalink / raw)
To: jejb, martin.petersen
Cc: linux-scsi, linuxarm, linux-kernel, Xiaofei Tan, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 0/6] hisi_sas: improve DQ locking
2018-05-09 15:10 [PATCH 0/6] hisi_sas: improve DQ locking John Garry
` (5 preceding siblings ...)
2018-05-09 15:10 ` [PATCH 6/6] scsi: hisi_sas: add check of device in hisi_sas_task_exec() John Garry
@ 2018-05-18 15:23 ` Martin K. Petersen
6 siblings, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2018-05-18 15:23 UTC (permalink / raw)
To: John Garry; +Cc: jejb, martin.petersen, linux-scsi, linuxarm, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread