From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
To: linux-scsi@vger.kernel.org,
"Martin K . Petersen" <martin.petersen@oracle.com>,
Jack Wang <jinpu.wang@ionos.com>,
John Garry <john.garry@huawei.com>
Cc: Xiang Chen <chenxiang66@hisilicon.com>,
Jason Yan <yanaijie@huawei.com>,
Luo Jiaxing <luojiaxing@huawei.com>
Subject: [PATCH v6 19/31] scsi: pm8001: Fix tag values handling
Date: Sun, 20 Feb 2022 12:17:58 +0900 [thread overview]
Message-ID: <20220220031810.738362-20-damien.lemoal@opensource.wdc.com> (raw)
In-Reply-To: <20220220031810.738362-1-damien.lemoal@opensource.wdc.com>
The function pm8001_tag_alloc() determines free tags using the function
find_first_zero_bit() which can return 0 when the first bit of the
bitmap being inspected is 0. As such, tag 0 is a valid tag value that
should not be dismissed as invalid. Fix the functions pm8001_work_fn(),
mpi_sata_completion(), pm8001_mpi_task_abort_resp() and
pm8001_open_reject_retry() to not dismiss 0 tags as invalid.
The value 0xffffffff is used for invalid tags for unused ccb
information structures. Add the macro definition PM8001_INVALID_TAG to
define this value.
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/scsi/pm8001/pm8001_hwi.c | 52 +++++++++++--------------------
drivers/scsi/pm8001/pm8001_init.c | 3 +-
drivers/scsi/pm8001/pm8001_sas.c | 13 ++++----
drivers/scsi/pm8001/pm8001_sas.h | 2 ++
drivers/scsi/pm8001/pm80xx_hwi.c | 5 ---
5 files changed, 28 insertions(+), 47 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index be57b9ae5486..6f9ee77cc576 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1522,7 +1522,6 @@ void pm8001_work_fn(struct work_struct *work)
case IO_XFER_ERROR_BREAK:
{ /* This one stashes the sas_task instead */
struct sas_task *t = (struct sas_task *)pm8001_dev;
- u32 tag;
struct pm8001_ccb_info *ccb;
struct pm8001_hba_info *pm8001_ha = pw->pm8001_ha;
unsigned long flags, flags1;
@@ -1544,8 +1543,8 @@ void pm8001_work_fn(struct work_struct *work)
/* Search for a possible ccb that matches the task */
for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
ccb = &pm8001_ha->ccb_info[i];
- tag = ccb->ccb_tag;
- if ((tag != 0xFFFFFFFF) && (ccb->task == t))
+ if ((ccb->ccb_tag != PM8001_INVALID_TAG) &&
+ (ccb->task == t))
break;
}
if (!ccb) {
@@ -1566,11 +1565,11 @@ void pm8001_work_fn(struct work_struct *work)
spin_unlock_irqrestore(&t->task_state_lock, flags1);
pm8001_dbg(pm8001_ha, FAIL, "task 0x%p done with event 0x%x resp 0x%x stat 0x%x but aborted by upper layer!\n",
t, pw->handler, ts->resp, ts->stat);
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, t, ccb, ccb->ccb_tag);
spin_unlock_irqrestore(&pm8001_ha->lock, flags);
} else {
spin_unlock_irqrestore(&t->task_state_lock, flags1);
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, t, ccb, ccb->ccb_tag);
mb();/* in order to force CPU ordering */
spin_unlock_irqrestore(&pm8001_ha->lock, flags);
t->task_done(t);
@@ -1579,7 +1578,6 @@ void pm8001_work_fn(struct work_struct *work)
case IO_XFER_OPEN_RETRY_TIMEOUT:
{ /* This one stashes the sas_task instead */
struct sas_task *t = (struct sas_task *)pm8001_dev;
- u32 tag;
struct pm8001_ccb_info *ccb;
struct pm8001_hba_info *pm8001_ha = pw->pm8001_ha;
unsigned long flags, flags1;
@@ -1613,8 +1611,8 @@ void pm8001_work_fn(struct work_struct *work)
/* Search for a possible ccb that matches the task */
for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
ccb = &pm8001_ha->ccb_info[i];
- tag = ccb->ccb_tag;
- if ((tag != 0xFFFFFFFF) && (ccb->task == t))
+ if ((ccb->ccb_tag != PM8001_INVALID_TAG) &&
+ (ccb->task == t))
break;
}
if (!ccb) {
@@ -1685,19 +1683,13 @@ void pm8001_work_fn(struct work_struct *work)
struct task_status_struct *ts;
struct sas_task *task;
int i;
- u32 tag, device_id;
+ u32 device_id;
for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
ccb = &pm8001_ha->ccb_info[i];
task = ccb->task;
ts = &task->task_status;
- tag = ccb->ccb_tag;
- /* check if tag is NULL */
- if (!tag) {
- pm8001_dbg(pm8001_ha, FAIL,
- "tag Null\n");
- continue;
- }
+
if (task != NULL) {
dev = task->dev;
if (!dev) {
@@ -1706,10 +1698,11 @@ void pm8001_work_fn(struct work_struct *work)
continue;
}
/*complete sas task and update to top layer */
- pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, task, ccb,
+ ccb->ccb_tag);
ts->resp = SAS_TASK_COMPLETE;
task->task_done(task);
- } else if (tag != 0xFFFFFFFF) {
+ } else if (ccb->ccb_tag != PM8001_INVALID_TAG) {
/* complete the internal commands/non-sas task */
pm8001_dev = ccb->device;
if (pm8001_dev->dcompletion) {
@@ -1717,7 +1710,7 @@ void pm8001_work_fn(struct work_struct *work)
pm8001_dev->dcompletion = NULL;
}
complete(pm8001_ha->nvmd_completion);
- pm8001_tag_free(pm8001_ha, tag);
+ pm8001_tag_free(pm8001_ha, ccb->ccb_tag);
}
}
/* Deregister all the device ids */
@@ -2313,11 +2306,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
param = le32_to_cpu(psataPayload->param);
tag = le32_to_cpu(psataPayload->tag);
- if (!tag) {
- pm8001_dbg(pm8001_ha, FAIL, "tag null\n");
- return;
- }
-
ccb = &pm8001_ha->ccb_info[tag];
t = ccb->task;
pm8001_dev = ccb->device;
@@ -3051,7 +3039,7 @@ void pm8001_mpi_set_dev_state_resp(struct pm8001_hba_info *pm8001_ha,
device_id, pds, nds, status);
complete(pm8001_dev->setds_completion);
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
}
@@ -3069,7 +3057,7 @@ void pm8001_mpi_set_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
dlen_status);
}
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
}
@@ -3096,7 +3084,7 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
* freed by requesting path anywhere.
*/
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
return;
}
@@ -3142,7 +3130,7 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
complete(pm8001_ha->nvmd_completion);
pm8001_dbg(pm8001_ha, MSG, "Get nvmd data complete!\n");
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
}
@@ -3555,7 +3543,7 @@ int pm8001_mpi_reg_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
complete(pm8001_dev->dcompletion);
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, htag);
return 0;
}
@@ -3627,7 +3615,7 @@ int pm8001_mpi_fw_flash_update_resp(struct pm8001_hba_info *pm8001_ha,
}
kfree(ccb->fw_control_context);
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
complete(pm8001_ha->nvmd_completion);
return 0;
@@ -3663,10 +3651,6 @@ int pm8001_mpi_task_abort_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
status = le32_to_cpu(pPayload->status);
tag = le32_to_cpu(pPayload->tag);
- if (!tag) {
- pm8001_dbg(pm8001_ha, FAIL, " TAG NULL. RETURNING !!!\n");
- return -1;
- }
scp = le32_to_cpu(pPayload->scp);
ccb = &pm8001_ha->ccb_info[tag];
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 381105286953..9b04f1a6a67d 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1217,10 +1217,11 @@ pm8001_init_ccb_tag(struct pm8001_hba_info *pm8001_ha, struct Scsi_Host *shost,
goto err_out;
}
pm8001_ha->ccb_info[i].task = NULL;
- pm8001_ha->ccb_info[i].ccb_tag = 0xffffffff;
+ pm8001_ha->ccb_info[i].ccb_tag = PM8001_INVALID_TAG;
pm8001_ha->ccb_info[i].device = NULL;
++pm8001_ha->tags_num;
}
+
return 0;
err_out_noccb:
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index f5678be4c17b..f409dfa049c0 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -553,7 +553,7 @@ void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha,
task->lldd_task = NULL;
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
ccb->open_retry = 0;
pm8001_tag_free(pm8001_ha, ccb_idx);
}
@@ -831,9 +831,11 @@ void pm8001_open_reject_retry(
struct task_status_struct *ts;
struct pm8001_device *pm8001_dev;
unsigned long flags1;
- u32 tag;
struct pm8001_ccb_info *ccb = &pm8001_ha->ccb_info[i];
+ if (ccb->ccb_tag == PM8001_INVALID_TAG)
+ continue;
+
pm8001_dev = ccb->device;
if (!pm8001_dev || (pm8001_dev->dev_type == SAS_PHY_UNUSED))
continue;
@@ -845,9 +847,6 @@ void pm8001_open_reject_retry(
continue;
} else if (pm8001_dev != device_to_close)
continue;
- tag = ccb->ccb_tag;
- if (!tag || (tag == 0xFFFFFFFF))
- continue;
task = ccb->task;
if (!task || !task->task_done)
continue;
@@ -867,11 +866,11 @@ void pm8001_open_reject_retry(
& SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&task->task_state_lock,
flags1);
- pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb->ccb_tag);
} else {
spin_unlock_irqrestore(&task->task_state_lock,
flags1);
- pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb->ccb_tag);
mb();/* in order to force CPU ordering */
spin_unlock_irqrestore(&pm8001_ha->lock, flags);
task->task_done(task);
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index d26f25186779..5082c7dc07ee 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -732,6 +732,8 @@ void pm8001_free_dev(struct pm8001_device *pm8001_dev);
/* ctl shared API */
extern const struct attribute_group *pm8001_host_groups[];
+#define PM8001_INVALID_TAG ((u32)-1)
+
static inline void
pm8001_ccb_task_free_done(struct pm8001_hba_info *pm8001_ha,
struct sas_task *task, struct pm8001_ccb_info *ccb,
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 76260d06b6be..cc46e2013eeb 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2402,11 +2402,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
param = le32_to_cpu(psataPayload->param);
tag = le32_to_cpu(psataPayload->tag);
- if (!tag) {
- pm8001_dbg(pm8001_ha, FAIL, "tag null\n");
- return;
- }
-
ccb = &pm8001_ha->ccb_info[tag];
t = ccb->task;
pm8001_dev = ccb->device;
--
2.34.1
next prev parent reply other threads:[~2022-02-20 3:18 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-20 3:17 [PATCH v6 00/31] libsas and pm8001 fixes Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 01/31] scsi: libsas: Fix sas_ata_qc_issue() handling of NCQ NON DATA commands Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 02/31] scsi: pm8001: Fix __iomem pointer use in pm8001_phy_control() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 03/31] scsi: pm8001: Fix pm8001_update_flash() local variable type Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 04/31] scsi: pm8001: Fix command initialization in pm80XX_send_read_log() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 05/31] scsi: pm8001: Fix pm80xx_pci_mem_copy() interface Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 06/31] scsi: pm8001: Fix command initialization in pm8001_chip_ssp_tm_req() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 07/31] scsi: pm8001: Fix payload initialization in pm80xx_set_thermal_config() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 08/31] scsi: pm8001: Fix le32 values handling in pm80xx_set_sas_protocol_timer_config() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 09/31] scsi: pm8001: Fix payload initialization in pm80xx_encrypt_update() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 10/31] scsi: pm8001: Fix le32 values handling in pm80xx_chip_ssp_io_req() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 11/31] scsi: pm8001: Fix le32 values handling in pm80xx_chip_sata_req() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 12/31] scsi: pm8001: Fix use of struct set_phy_profile_req fields Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 13/31] scsi: pm8001: Remove local variable in pm8001_pci_resume() Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 14/31] scsi: pm8001: Fix NCQ NON DATA command task initialization Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 15/31] scsi: pm8001: Fix NCQ NON DATA command completion handling Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 16/31] scsi: pm8001: Fix abort all task initialization Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 17/31] scsi: pm8001: Fix pm8001_tag_alloc() failures handling Damien Le Moal
2022-02-20 3:17 ` [PATCH v6 18/31] scsi: pm8001: Fix pm8001_mpi_task_abort_resp() Damien Le Moal
2022-02-20 3:17 ` Damien Le Moal [this message]
2022-02-20 3:17 ` [PATCH v6 20/31] scsi: pm8001: Fix task leak in pm8001_send_abort_all() Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 21/31] scsi: pm8001: Fix tag leaks on error Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 22/31] scsi: pm8001: fix memory leak in pm8001_chip_fw_flash_update_req() Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 23/31] scsi: libsas: Simplify sas_ata_qc_issue() detection of NCQ commands Damien Le Moal
2022-02-20 9:49 ` John Garry
2022-02-20 3:18 ` [PATCH v6 24/31] scsi: pm8001: Cleanup pm8001_exec_internal_task_abort() Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 25/31] scsi: pm8001: Simplify pm8001_get_ncq_tag() Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 26/31] scsi: pm8001: Introduce ccb alloc/free helpers Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 27/31] scsi: pm8001: Simplify pm8001_mpi_build_cmd() interface Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 28/31] scsi: pm8001: Simplify pm8001_task_exec() Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 29/31] scsi: pm8001: Simplify pm8001_ccb_task_free() Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 30/31] scsi: pm8001: improve pm80XX_send_abort_all() Damien Le Moal
2022-02-20 3:18 ` [PATCH v6 31/31] scsi: pm8001: Fix pm8001_info() message format Damien Le Moal
2022-02-23 2:33 ` [PATCH v6 00/31] libsas and pm8001 fixes Martin K. Petersen
2022-02-28 3:43 ` Martin K. Petersen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220220031810.738362-20-damien.lemoal@opensource.wdc.com \
--to=damien.lemoal@opensource.wdc.com \
--cc=chenxiang66@hisilicon.com \
--cc=jinpu.wang@ionos.com \
--cc=john.garry@huawei.com \
--cc=linux-scsi@vger.kernel.org \
--cc=luojiaxing@huawei.com \
--cc=martin.petersen@oracle.com \
--cc=yanaijie@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.