I'm not sure how this happened but the patch that was intended to fix
abort handling was incomplete. This patch fixes that patch as follows:
- If aborting the SCSI command failed, wait until the SCSI command
  completes.
- Return SUCCESS instead of FAILED if an abort attempt races with SCSI
  command completion.
- Since qla2xxx_eh_abort() increments the sp reference count by calling
  sp_get(), decrement the sp reference count before returning.

Cc: Himanshu Madhani <hmadh...@marvell.com>
Cc: Giridhar Malavali <gmalav...@marvell.com>
Fixes: 219d27d7147e ("scsi: qla2xxx: Fix race conditions in the code for 
aborting SCSI commands")
Signed-off-by: Bart Van Assche <bvanass...@acm.org>
---
 drivers/scsi/qla2xxx/qla_os.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 172ef21827dd..415e12f7f9e7 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1269,6 +1269,7 @@ static int
 qla2xxx_eh_abort(struct scsi_cmnd *cmd)
 {
        scsi_qla_host_t *vha = shost_priv(cmd->device->host);
+       DECLARE_COMPLETION_ONSTACK(comp);
        srb_t *sp;
        int ret;
        unsigned int id;
@@ -1304,6 +1305,7 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
                return SUCCESS;
        }
 
+       /* Get a reference to the sp and drop the lock. */
        if (sp_get(sp)){
                /* ref_count is already 0 */
                spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
@@ -1318,6 +1320,7 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
            "Aborting from RISC nexus=%ld:%d:%llu sp=%p cmd=%p handle=%x\n",
            vha->host_no, id, lun, sp, cmd, sp->handle);
 
+       ret = SUCCESS;
        rval = ha->isp_ops->abort_command(sp);
        ql_dbg(ql_dbg_taskm, vha, 0x8003,
               "Abort command mbx cmd=%p, rval=%x.\n", cmd, rval);
@@ -1329,17 +1332,27 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd)
                 * won't report a completion.
                 */
                sp->done(sp, DID_ABORT << 16);
-               ret = SUCCESS;
                break;
-       default:
-               /*
-                * Either abort failed or abort and completion raced. Let
-                * the SCSI core retry the abort in the former case.
-                */
-               ret = FAILED;
+       case QLA_FUNCTION_PARAMETER_ERROR:
+       default: {
+               /* Wait for the command completion. */
+               uint32_t ratov = ha->r_a_tov/10;
+               uint32_t ratov_j = msecs_to_jiffies(4 * ratov * 1000);
+
+               sp->comp = &comp;
+               if (!wait_for_completion_timeout(&comp, ratov_j)) {
+                       ql_dbg(ql_dbg_taskm, vha, 0xffff,
+                           "%s: Abort wait timer (4 * R_A_TOV[%d]) expired\n",
+                           __func__, ha->r_a_tov);
+                       ret = FAILED;
+                       break;
+               }
                break;
        }
+       }
 
+       sp->comp = NULL;
+       atomic_dec(&sp->ref_count);
        ql_log(ql_log_info, vha, 0x801c,
            "Abort command issued nexus=%ld:%d:%llu -- %x.\n",
            vha->host_no, id, lun, ret);
-- 
2.22.0.rc1

Reply via email to