2022-10-04 11:07:54

by John Garry

[permalink] [raw]
Subject: [PATCH v3 0/7] scsi: libsas: Use request tag in more drivers

Currently hisi_sas is the only libsas driver which uses the request tag
for per-HW IO tag.

All other libsas drivers manage the tags internally. Tag management in
pm8001 and mvsas is currently using a simple bitmap, so use the request
tag when available there. With this change we still need to manage tags
for libsas "internal" commands, like SMP commands, and any other
private commands so reserve some tags for this:
- For pm8001 I went with pre-existing and unused PM8001_RESERVE_SLOT size.
The value is 8, which should be enough. It is greater than mvsas, below,
but this driver sends a lot of other private commands to HW.
- For mvsas I went with 4, which still should be enough.

isci and aic9xx have elaborate tag alloc schemes, so I'm not going to
bother changing them, especially since I have no HW to test with.

Helper sas_task_find_rq() is added to get the request and associated tag
per sas_task when it is available.

This series looks not to conflict with
https://lore.kernel.org/linux-scsi/[email protected]/T/#mefdcb7b63b4e6ebc8b77a689b3923571ab3d33ab

Based on https://lore.kernel.org/linux-scsi/[email protected]/T/#t

Differences to v2:
- Put private tags at bottom of tagset for each driver (Hannes)
- Add RB tags from Jason, Jack, and Hannes (thanks!)

Differences to v1:
- Rework sas_task_find_rq()
- Rename tags->rsvd and remove tag size struct arg for both mvsas and
pm8001
- Decrement can_queue for mvsas
- Use sas_task_find_rq() in pm80xx_chip_get_q_index()
- Add Damien's tags (thanks)

Igor Pylypiv (1):
scsi: pm8001: Remove pm8001_tag_init()

John Garry (6):
scsi: libsas: Add sas_task_find_rq()
scsi: hisi_sas: Use sas_task_find_rq()
scsi: hisi_sas: Put reserved tags in lower region of tagset
scsi: pm8001: Use sas_task_find_rq() for tagging
scsi: mvsas: Delete mvs_tag_init()
scsi: mvsas: Use sas_task_find_rq() for tagging

drivers/scsi/hisi_sas/hisi_sas_main.c | 38 +++++++++---------------
drivers/scsi/mvsas/mv_defs.h | 1 +
drivers/scsi/mvsas/mv_init.c | 11 ++++---
drivers/scsi/mvsas/mv_sas.c | 42 ++++++++++++++-------------
drivers/scsi/mvsas/mv_sas.h | 8 +----
drivers/scsi/pm8001/pm8001_init.c | 14 +++------
drivers/scsi/pm8001/pm8001_sas.c | 20 ++++++-------
drivers/scsi/pm8001/pm8001_sas.h | 12 +++++---
drivers/scsi/pm8001/pm80xx_hwi.c | 17 ++---------
include/scsi/libsas.h | 18 ++++++++++++
10 files changed, 85 insertions(+), 96 deletions(-)

--
2.25.1


2022-10-04 11:08:09

by John Garry

[permalink] [raw]
Subject: [PATCH v3 4/7] scsi: pm8001: Remove pm8001_tag_init()

From: Igor Pylypiv <[email protected]>

In commit 5a141315ed7c ("scsi: pm80xx: Increase the number of outstanding
I/O supported to 1024") the pm8001_ha->tags allocation was moved into
pm8001_init_ccb_tag(). This changed the execution order of allocation.
pm8001_tag_init() used to be called after the pm8001_ha->tags allocation
and now it is called before the allocation.

Before:

pm8001_pci_probe()
`--> pm8001_pci_alloc()
`--> pm8001_alloc()
`--> pm8001_ha->tags = kzalloc(...)
`--> pm8001_tag_init(pm8001_ha); // OK: tags are allocated

After:

pm8001_pci_probe()
`--> pm8001_pci_alloc()
| `--> pm8001_alloc()
| `--> pm8001_tag_init(pm8001_ha); // NOK: tags are not allocated
|
`--> pm8001_init_ccb_tag()
`--> pm8001_ha->tags = kzalloc(...) // today it is bitmap_zalloc()

Since pm8001_ha->tags_num is zero when pm8001_tag_init() is called it does
nothing. Tags memory is allocated with bitmap_zalloc() so there is no need
to manually clear each bit with pm8001_tag_free().

Reviewed-by: Changyuan Lyu <[email protected]>
Signed-off-by: Igor Pylypiv <[email protected]>
Signed-off-by: John Garry <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Reviewed-by: Jack Wang <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
---
drivers/scsi/pm8001/pm8001_init.c | 2 --
drivers/scsi/pm8001/pm8001_sas.c | 7 -------
drivers/scsi/pm8001/pm8001_sas.h | 1 -
3 files changed, 10 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index a0028e130a7e..0edc9857a8bd 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -436,8 +436,6 @@ static int pm8001_alloc(struct pm8001_hba_info *pm8001_ha,
atomic_set(&pm8001_ha->devices[i].running_req, 0);
}
pm8001_ha->flags = PM8001F_INIT_TIME;
- /* Initialize tags */
- pm8001_tag_init(pm8001_ha);
return 0;

err_out_nodev:
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index d5ec29f69be3..066dfa9f4683 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -96,13 +96,6 @@ int pm8001_tag_alloc(struct pm8001_hba_info *pm8001_ha, u32 *tag_out)
return 0;
}

-void pm8001_tag_init(struct pm8001_hba_info *pm8001_ha)
-{
- int i;
- for (i = 0; i < pm8001_ha->tags_num; ++i)
- pm8001_tag_free(pm8001_ha, i);
-}
-
/**
* pm8001_mem_alloc - allocate memory for pm8001.
* @pdev: pci device.
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 8ab0654327f9..9acaadf02150 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -632,7 +632,6 @@ extern struct workqueue_struct *pm8001_wq;

/******************** function prototype *********************/
int pm8001_tag_alloc(struct pm8001_hba_info *pm8001_ha, u32 *tag_out);
-void pm8001_tag_init(struct pm8001_hba_info *pm8001_ha);
u32 pm8001_get_ncq_tag(struct sas_task *task, u32 *tag);
void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha,
struct pm8001_ccb_info *ccb);
--
2.25.1

2022-10-04 11:08:09

by John Garry

[permalink] [raw]
Subject: [PATCH v3 6/7] scsi: mvsas: Delete mvs_tag_init()

All mvs_tag_init() does is zero the tag bitmap, but this is already done
with the kzalloc() call to alloc the tags, so delete this unneeded
function.

Signed-off-by: John Garry <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
---
drivers/scsi/mvsas/mv_init.c | 2 --
drivers/scsi/mvsas/mv_sas.c | 7 -------
drivers/scsi/mvsas/mv_sas.h | 1 -
3 files changed, 10 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 2fde496fff5f..c85fb812ad43 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -286,8 +286,6 @@ static int mvs_alloc(struct mvs_info *mvi, struct Scsi_Host *shost)
}
mvi->tags_num = slot_nr;

- /* Initialize tags */
- mvs_tag_init(mvi);
return 0;
err_out:
return 1;
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index a6867dae0e7c..0810e6c930e1 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -51,13 +51,6 @@ inline int mvs_tag_alloc(struct mvs_info *mvi, u32 *tag_out)
return 0;
}

-void mvs_tag_init(struct mvs_info *mvi)
-{
- int i;
- for (i = 0; i < mvi->tags_num; ++i)
- mvs_tag_clear(mvi, i);
-}
-
static struct mvs_info *mvs_find_dev_mvi(struct domain_device *dev)
{
unsigned long i = 0, j = 0, hi = 0;
diff --git a/drivers/scsi/mvsas/mv_sas.h b/drivers/scsi/mvsas/mv_sas.h
index 509d8f32a04f..fe57665bdb50 100644
--- a/drivers/scsi/mvsas/mv_sas.h
+++ b/drivers/scsi/mvsas/mv_sas.h
@@ -428,7 +428,6 @@ void mvs_tag_clear(struct mvs_info *mvi, u32 tag);
void mvs_tag_free(struct mvs_info *mvi, u32 tag);
void mvs_tag_set(struct mvs_info *mvi, unsigned int tag);
int mvs_tag_alloc(struct mvs_info *mvi, u32 *tag_out);
-void mvs_tag_init(struct mvs_info *mvi);
void mvs_iounmap(void __iomem *regs);
int mvs_ioremap(struct mvs_info *mvi, int bar, int bar_ex);
void mvs_phys_reset(struct mvs_info *mvi, u32 phy_mask, int hard);
--
2.25.1

2022-10-04 11:08:41

by John Garry

[permalink] [raw]
Subject: [PATCH v3 2/7] scsi: hisi_sas: Use sas_task_find_rq()

Use sas_task_find_rq() to lookup the request per task for its driver tag.

Signed-off-by: John Garry <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 4c37ae9eb6b6..1011dffed51f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -177,13 +177,13 @@ static void hisi_sas_slot_index_set(struct hisi_hba *hisi_hba, int slot_idx)
}

static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
- struct scsi_cmnd *scsi_cmnd)
+ struct request *rq)
{
int index;
void *bitmap = hisi_hba->slot_index_tags;

- if (scsi_cmnd)
- return scsi_cmd_to_rq(scsi_cmnd)->tag;
+ if (rq)
+ return rq->tag;

spin_lock(&hisi_hba->lock);
index = find_next_zero_bit(bitmap, hisi_hba->slot_index_count,
@@ -461,11 +461,11 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags)
struct asd_sas_port *sas_port = device->port;
struct hisi_sas_device *sas_dev = device->lldd_dev;
bool internal_abort = sas_is_internal_abort(task);
- struct scsi_cmnd *scmd = NULL;
struct hisi_sas_dq *dq = NULL;
struct hisi_sas_port *port;
struct hisi_hba *hisi_hba;
struct hisi_sas_slot *slot;
+ struct request *rq = NULL;
struct device *dev;
int rc;

@@ -520,22 +520,12 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags)
return -ECOMM;
}

- if (task->uldd_task) {
- struct ata_queued_cmd *qc;
-
- if (dev_is_sata(device)) {
- qc = task->uldd_task;
- scmd = qc->scsicmd;
- } else {
- scmd = task->uldd_task;
- }
- }
-
- if (scmd) {
+ rq = sas_task_find_rq(task);
+ if (rq) {
unsigned int dq_index;
u32 blk_tag;

- blk_tag = blk_mq_unique_tag(scsi_cmd_to_rq(scmd));
+ blk_tag = blk_mq_unique_tag(rq);
dq_index = blk_mq_unique_tag_to_hwq(blk_tag);
dq = &hisi_hba->dq[dq_index];
} else {
@@ -580,7 +570,7 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags)
if (!internal_abort && hisi_hba->hw->slot_index_alloc)
rc = hisi_hba->hw->slot_index_alloc(hisi_hba, device);
else
- rc = hisi_sas_slot_index_alloc(hisi_hba, scmd);
+ rc = hisi_sas_slot_index_alloc(hisi_hba, rq);

if (rc < 0)
goto err_out_dif_dma_unmap;
--
2.25.1

2022-10-04 11:48:01

by John Garry

[permalink] [raw]
Subject: [PATCH v3 1/7] scsi: libsas: Add sas_task_find_rq()

blk-mq already provides a unique tag per request. Some libsas LLDDs - like
hisi_sas - already use this tag as the unique per-IO HW tag.

Add a common function to provide the request associated with a sas_task
for all libsas LLDDs.

Signed-off-by: John Garry <[email protected]>
Reviewed-by: Jack Wang <[email protected]>
Reviewed-by: Jason Yan <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
---
include/scsi/libsas.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index f86b56bf7833..f498217961db 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -644,6 +644,24 @@ static inline bool sas_is_internal_abort(struct sas_task *task)
return task->task_proto == SAS_PROTOCOL_INTERNAL_ABORT;
}

+static inline struct request *sas_task_find_rq(struct sas_task *task)
+{
+ struct scsi_cmnd *scmd;
+
+ if (task->task_proto & SAS_PROTOCOL_STP_ALL) {
+ struct ata_queued_cmd *qc = task->uldd_task;
+
+ scmd = qc ? qc->scsicmd : NULL;
+ } else {
+ scmd = task->uldd_task;
+ }
+
+ if (!scmd)
+ return NULL;
+
+ return scsi_cmd_to_rq(scmd);
+}
+
struct sas_domain_function_template {
/* The class calls these to notify the LLDD of an event. */
void (*lldd_port_formed)(struct asd_sas_phy *);
--
2.25.1