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>

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 628b08ba6770bff66f861aae9dc09c02a1a48b96..c0adc3a9d19679701b5c18db444ebf553f68e29c 100644 (file)
--- 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 8e3f2f9ddaacd037cb34b5181179fccd7bbdaaff..d5ec29f69be317d7736879318af6a7dbcf21e0a2 100644 (file)
--- 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 f8b8624458f73f9b56bb17aafaa67a0aa1558ff6..dd0e06983cd389f3e4ebda54dc22af34acd05d58 100644 (file)
--- 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);