scsi: pm80xx: Fix double completion for SATA devices
authorAjish Koshy <Ajish.Koshy@microchip.com>
Mon, 24 Jan 2022 08:22:55 +0000 (13:52 +0530)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 23 Feb 2022 11:03:06 +0000 (12:03 +0100)
[ Upstream commit c26b85ea16365079be8d206b20556a60a0c69ad4 ]

Current code handles completions for SATA devices in mpi_sata_completion()
and mpi_sata_event().

However, at the time when any SATA event happens, for almost all the event
types, the command is still in the target. It is therefore incorrect to
complete the task in sata_event().

There are some events for which we get sata_completions, some need recovery
procedure and others abort. All the tasks must be completed via
sata_completion() path.

Removed the task done related code from sata_events().  For tasks where we
don't get completions, let top layer call abort() to abort the command post
timeout.

Link: https://lore.kernel.org/r/20220124082255.86223-1-Ajish.Koshy@microchip.com
Acked-by: Jack Wang <jinpu.wang@ionos.com>
Co-developed-by: Viswas G <Viswas.G@microchip.com>
Signed-off-by: Viswas G <Viswas.G@microchip.com>
Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
drivers/scsi/pm8001/pm8001_hwi.c
drivers/scsi/pm8001/pm80xx_hwi.c

index 880e1f356defc11324a185ec08528abb0f8133f2..5e6b23da4157c615ca4c764040843732572ccd26 100644 (file)
@@ -2695,7 +2695,6 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
        u32 tag = le32_to_cpu(psataPayload->tag);
        u32 port_id = le32_to_cpu(psataPayload->port_id);
        u32 dev_id = le32_to_cpu(psataPayload->device_id);
-       unsigned long flags;
 
        ccb = &pm8001_ha->ccb_info[tag];
 
@@ -2735,8 +2734,6 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
                ts->resp = SAS_TASK_COMPLETE;
                ts->stat = SAS_DATA_OVERRUN;
                ts->residual = 0;
-               if (pm8001_dev)
-                       atomic_dec(&pm8001_dev->running_req);
                break;
        case IO_XFER_ERROR_BREAK:
                pm8001_dbg(pm8001_ha, IO, "IO_XFER_ERROR_BREAK\n");
@@ -2778,7 +2775,6 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
                                IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
                        ts->resp = SAS_TASK_COMPLETE;
                        ts->stat = SAS_QUEUE_FULL;
-                       pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
                        return;
                }
                break;
@@ -2864,20 +2860,6 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
                ts->stat = SAS_OPEN_TO;
                break;
        }
-       spin_lock_irqsave(&t->task_state_lock, flags);
-       t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
-       t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
-       t->task_state_flags |= SAS_TASK_STATE_DONE;
-       if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
-               spin_unlock_irqrestore(&t->task_state_lock, flags);
-               pm8001_dbg(pm8001_ha, FAIL,
-                          "task 0x%p done with io_status 0x%x resp 0x%x stat 0x%x but aborted by upper layer!\n",
-                          t, event, ts->resp, ts->stat);
-               pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
-       } else {
-               spin_unlock_irqrestore(&t->task_state_lock, flags);
-               pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
-       }
 }
 
 /*See the comments for mpi_ssp_completion */
index ed13e0e044b74c5ad7b9f1d7574c3fe414856517..2cd100c062aba9833a12725f7ec09ecd85de7a2e 100644 (file)
@@ -2828,7 +2828,6 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha,
        u32 tag = le32_to_cpu(psataPayload->tag);
        u32 port_id = le32_to_cpu(psataPayload->port_id);
        u32 dev_id = le32_to_cpu(psataPayload->device_id);
-       unsigned long flags;
 
        ccb = &pm8001_ha->ccb_info[tag];
 
@@ -2866,8 +2865,6 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha,
                ts->resp = SAS_TASK_COMPLETE;
                ts->stat = SAS_DATA_OVERRUN;
                ts->residual = 0;
-               if (pm8001_dev)
-                       atomic_dec(&pm8001_dev->running_req);
                break;
        case IO_XFER_ERROR_BREAK:
                pm8001_dbg(pm8001_ha, IO, "IO_XFER_ERROR_BREAK\n");
@@ -2916,11 +2913,6 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha,
                                IO_OPEN_CNX_ERROR_IT_NEXUS_LOSS);
                        ts->resp = SAS_TASK_COMPLETE;
                        ts->stat = SAS_QUEUE_FULL;
-                       spin_unlock_irqrestore(&circularQ->oq_lock,
-                                       circularQ->lock_flags);
-                       pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
-                       spin_lock_irqsave(&circularQ->oq_lock,
-                                       circularQ->lock_flags);
                        return;
                }
                break;
@@ -3020,24 +3012,6 @@ static void mpi_sata_event(struct pm8001_hba_info *pm8001_ha,
                ts->stat = SAS_OPEN_TO;
                break;
        }
-       spin_lock_irqsave(&t->task_state_lock, flags);
-       t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
-       t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
-       t->task_state_flags |= SAS_TASK_STATE_DONE;
-       if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
-               spin_unlock_irqrestore(&t->task_state_lock, flags);
-               pm8001_dbg(pm8001_ha, FAIL,
-                          "task 0x%p done with io_status 0x%x resp 0x%x stat 0x%x but aborted by upper layer!\n",
-                          t, event, ts->resp, ts->stat);
-               pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
-       } else {
-               spin_unlock_irqrestore(&t->task_state_lock, flags);
-               spin_unlock_irqrestore(&circularQ->oq_lock,
-                               circularQ->lock_flags);
-               pm8001_ccb_task_free_done(pm8001_ha, t, ccb, tag);
-               spin_lock_irqsave(&circularQ->oq_lock,
-                               circularQ->lock_flags);
-       }
 }
 
 /*See the comments for mpi_ssp_completion */