summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Garry <john.garry@huawei.com>2022-10-17 17:20:32 +0800
committerMartin K. Petersen <martin.petersen@oracle.com>2022-10-18 02:37:45 +0000
commit0b639decf65160b1afd9993019be37d7869c0340 (patch)
tree6186dc2cbcba6fbaafbc23528cf18b6804a5ccac
parent4ef4f1a6155571d3d53583a4e8e7ccbbec220b8a (diff)
scsi: pm8001: Modify task abort handling for SATA task
When we try to abort a SATA task, the CCB of the task which we are trying to avoid may still complete. In this case, we should not touch the task associated with that CCB as we can race with libsas freeing the last later in sas_eh_handle_sas_errors() -> sas_eh_finish_cmd() for when TASK_IS_ABORTED is returned from sas_scsi_find_task() Signed-off-by: John Garry <john.garry@huawei.com> Link: https://lore.kernel.org/r/1665998435-199946-6-git-send-email-john.garry@huawei.com Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Tested-by: Niklas Cassel <niklas.cassel@wdc.com> # pm80xx Acked-by: Jack Wang <jinpu.wang@ionos.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
-rw-r--r--drivers/scsi/pm8001/pm8001_hwi.c15
-rw-r--r--drivers/scsi/pm8001/pm8001_sas.c8
-rw-r--r--drivers/scsi/pm8001/pm80xx_hwi.c14
3 files changed, 31 insertions, 6 deletions
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 628b08ba6770..c0adc3a9d196 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -2295,7 +2295,9 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
if (t->dev && (t->dev->lldd_dev))
pm8001_dev = t->dev->lldd_dev;
} else {
- pm8001_dbg(pm8001_ha, FAIL, "task null\n");
+ pm8001_dbg(pm8001_ha, FAIL, "task null, freeing CCB tag %d\n",
+ ccb->ccb_tag);
+ pm8001_ccb_free(pm8001_ha, ccb);
return;
}
@@ -2675,8 +2677,17 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
pm8001_dev = ccb->device;
if (event)
pm8001_dbg(pm8001_ha, FAIL, "sata IO status 0x%x\n", event);
- if (unlikely(!t || !t->lldd_task || !t->dev))
+
+ if (unlikely(!t)) {
+ pm8001_dbg(pm8001_ha, FAIL, "task null, freeing CCB tag %d\n",
+ ccb->ccb_tag);
+ pm8001_ccb_free(pm8001_ha, ccb);
return;
+ }
+
+ if (unlikely(!t->lldd_task || !t->dev))
+ return;
+
ts = &t->task_status;
pm8001_dbg(pm8001_ha, DEVIO,
"port_id:0x%x, device_id:0x%x, tag:0x%x, event:0x%x\n",
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 8e3f2f9ddaac..d5ec29f69be3 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -983,6 +983,7 @@ int pm8001_query_task(struct sas_task *task)
/* mandatory SAM-3, still need free task/ccb info, abort the specified task */
int pm8001_abort_task(struct sas_task *task)
{
+ struct pm8001_ccb_info *ccb = task->lldd_task;
unsigned long flags;
u32 tag;
struct domain_device *dev ;
@@ -1113,6 +1114,13 @@ int pm8001_abort_task(struct sas_task *task)
pm8001_dev, DS_OPERATIONAL);
wait_for_completion(&completion);
} else {
+ /*
+ * Ensure that if we see a completion for the ccb
+ * associated with the task which we are trying to
+ * abort then we should not touch the sas_task as it
+ * may race with libsas freeing it when return here.
+ */
+ ccb->task = NULL;
ret = sas_execute_internal_abort_single(dev, tag, 0, NULL);
}
rc = TMF_RESP_FUNC_COMPLETE;
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index f8b8624458f7..dd0e06983cd3 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2396,7 +2396,9 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
if (t->dev && (t->dev->lldd_dev))
pm8001_dev = t->dev->lldd_dev;
} else {
- pm8001_dbg(pm8001_ha, FAIL, "task null\n");
+ pm8001_dbg(pm8001_ha, FAIL, "task null, freeing CCB tag %d\n",
+ ccb->ccb_tag);
+ pm8001_ccb_free(pm8001_ha, ccb);
return;
}
@@ -2813,12 +2815,16 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha,
ccb = &pm8001_ha->ccb_info[tag];
t = ccb->task;
pm8001_dev = ccb->device;
-
- if (unlikely(!t || !t->lldd_task || !t->dev)) {
- pm8001_dbg(pm8001_ha, FAIL, "task or dev null\n");
+ if (unlikely(!t)) {
+ pm8001_dbg(pm8001_ha, FAIL, "task null, freeing CCB tag %d\n",
+ ccb->ccb_tag);
+ pm8001_ccb_free(pm8001_ha, ccb);
return;
}
+ if (unlikely(!t->lldd_task || !t->dev))
+ return;
+
ts = &t->task_status;
pm8001_dbg(pm8001_ha, IOERR, "port_id:0x%x, tag:0x%x, event:0x%x\n",
port_id, tag, event);