diff mbox series

[03/15] qla2xxx: Fix abort timeout race condition.

Message ID 20190726160740.25687-4-hmadhani@marvell.com (mailing list archive)
State Accepted
Headers show
Series qla2xxx: Bug fixes for the driver | expand

Commit Message

Himanshu Madhani July 26, 2019, 4:07 p.m. UTC 
From: Quinn Tran <qutran@marvell.com>

If an abort times out, the Abort IOCB completion and Abort
timer can race against each other. This patch provides
unique error code for timer path to allow proper cleanup

Signed-off-by: Quinn Tran <qutran@marvell.com>
Signed-off-by: Himanshu Madhani <hmadhani@marvell.com>
---
 drivers/scsi/qla2xxx/qla_def.h  |  1 +
 drivers/scsi/qla2xxx/qla_init.c | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Bart Van Assche July 26, 2019, 9:26 p.m. UTC | #1 
On 7/26/19 9:07 AM, Himanshu Madhani wrote:
>   static void qla24xx_abort_sp_done(void *ptr, int res)
> @@ -109,7 +122,8 @@ static void qla24xx_abort_sp_done(void *ptr, int res)
>   	srb_t *sp = ptr;
>   	struct srb_iocb *abt = &sp->u.iocb_cmd;
>   
> -	if (del_timer(&sp->u.iocb_cmd.timer)) {
> +	if ((res == QLA_OS_TIMER_EXPIRED) ||
> +	    del_timer(&sp->u.iocb_cmd.timer)) {
>   		if (sp->flags & SRB_WAKEUP_ON_COMP)
>   			complete(&abt->u.abt.comp);
>   		else

A better fix is probably to ignore the return value of del_timer() - see 
also "[PATCH 20/20] qla2xxx: Fix qla24xx_abort_sp_done()" 
(https://www.spinics.net/lists/linux-scsi/msg130664.html).

Anyway, I'm fine with this and the other 14 patches going upstream. I 
can rebase my patches on top of this patch series.

Bart.
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index bad2b12604f1..d3d624d43ec3 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -4628,6 +4628,7 @@  struct secure_flash_update_block_pk {
 #define QLA_SUSPENDED			0x106
 #define QLA_BUSY			0x107
 #define QLA_ALREADY_REGISTERED		0x109
+#define QLA_OS_TIMER_EXPIRED		0x10a
 
 #define NVRAM_DELAY()		udelay(10)
 
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 4059655639d9..68d7496a8aff 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -99,9 +99,22 @@  static void qla24xx_abort_iocb_timeout(void *data)
 {
 	srb_t *sp = data;
 	struct srb_iocb *abt = &sp->u.iocb_cmd;
+	struct qla_qpair *qpair = sp->qpair;
+	u32 handle;
+	unsigned long flags;
+
+	spin_lock_irqsave(qpair->qp_lock_ptr, flags);
+	for (handle = 1; handle < qpair->req->num_outstanding_cmds; handle++) {
+		/* removing the abort */
+		if (qpair->req->outstanding_cmds[handle] == sp) {
+			qpair->req->outstanding_cmds[handle] = NULL;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
 
 	abt->u.abt.comp_status = CS_TIMEOUT;
-	sp->done(sp, QLA_FUNCTION_TIMEOUT);
+	sp->done(sp, QLA_OS_TIMER_EXPIRED);
 }
 
 static void qla24xx_abort_sp_done(void *ptr, int res)
@@ -109,7 +122,8 @@  static void qla24xx_abort_sp_done(void *ptr, int res)
 	srb_t *sp = ptr;
 	struct srb_iocb *abt = &sp->u.iocb_cmd;
 
-	if (del_timer(&sp->u.iocb_cmd.timer)) {
+	if ((res == QLA_OS_TIMER_EXPIRED) ||
+	    del_timer(&sp->u.iocb_cmd.timer)) {
 		if (sp->flags & SRB_WAKEUP_ON_COMP)
 			complete(&abt->u.abt.comp);
 		else