On Mon, Jan 24, 2022 at 9:20 AM Ajish Koshy <Ajish.Koshy@xxxxxxxxxxxxx> wrote:
>
> For SATA devices, correct the double
> completion issue.
>
> Current code handles completions for sata
> devices in mpi_sata_completion() and
> mpi_sata_event().
>
> But at the time when any sata event happens,
> for almost all the event types, the command
> is still in the target. It is wrong to
> complete the task in sata_event().
Is it also an issue for SSP? what is the side effect, the current code
will lead to (w/o this patch)?
why SATA is different?
Thanks!
>
> There are some events for which we get
> sata_completions, for some needs recovery
> procedure and for others abort.
>
> All the tasks must be completed via
> sata_completion() path.
>
> Have just 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.
>
> Signed-off-by: Ajish Koshy <Ajish.Koshy@xxxxxxxxxxxxx>
> Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx>
> ---
> drivers/scsi/pm8001/pm8001_hwi.c | 18 ------------------
> drivers/scsi/pm8001/pm80xx_hwi.c | 26 --------------------------
> 2 files changed, 44 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index c814e5071712..9ec310b795c3 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -2692,7 +2692,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;
>
> if (event)
> pm8001_dbg(pm8001_ha, FAIL, "SATA EVENT 0x%x\n", event);
> @@ -2724,8 +2723,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");
> @@ -2767,7 +2764,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;
> @@ -2853,20 +2849,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 */
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 0849ecc913c7..1e573be04407 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -2821,7 +2821,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;
>
> if (event)
> pm8001_dbg(pm8001_ha, FAIL, "SATA EVENT 0x%x\n", event);
> @@ -2854,8 +2853,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");
> @@ -2904,11 +2901,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;
> @@ -3008,24 +3000,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 */
> --
> 2.27.0
>
