qla2x00_sp_free() cancels the iocb timeout timer if it is still
pending, but doesn't (and can't) wait for the timer function to
complete if it is already running. Add a reference count to async
iocb commands to ensure that they aren't freed too early:
- One reference is held by the timer, and dropped either at the end
of the timer function or after the timer is cancelled
- One reference is held by the completion path, and dropped by
qla2x00_sp_free()
Signed-off-by: Ben Hutchings <ben.hutchings@xxxxxxxxxxxxxxx>
---
This probably isn't quite right, since it's based on only a brief code
review and is untested. And maybe there's some reason that this race
condition is somehow avoided.
This depends on the previous two fixes I sent for qla2xxx.
Ben.
drivers/scsi/qla2xxx/qla_def.h | 1 +
drivers/scsi/qla2xxx/qla_gs.c | 4 ++--
drivers/scsi/qla2xxx/qla_init.c | 10 +++++++---
drivers/scsi/qla2xxx/qla_inline.h | 12 ++++++++++++
drivers/scsi/qla2xxx/qla_iocb.c | 6 ++----
5 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index c9689f97c307..0337bacd0dc7 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -483,6 +483,7 @@ struct srb_iocb {
struct timer_list timer;
void (*timeout)(void *);
+ refcount_t ref;
};
/* Values for srb_ctx type */
diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c
index fcde5ea203c0..e98ba70b7cbe 100644
--- a/drivers/scsi/qla2xxx/qla_gs.c
+++ b/drivers/scsi/qla2xxx/qla_gs.c
@@ -545,7 +545,7 @@ static void qla2x00_async_sns_sp_done(void *s, int rc)
if (!e)
goto err2;
- del_timer(&sp->u.iocb_cmd.timer);
+ qla2x00_del_timer(sp);
e->u.iosb.sp = sp;
qla2x00_post_work(vha, e);
return;
@@ -4175,7 +4175,7 @@ void qla24xx_async_gpnft_done(scsi_qla_host_t *vha, srb_t *sp)
{
ql_dbg(ql_dbg_disc, vha, 0xffff,
"%s enter\n", __func__);
- del_timer(&sp->u.iocb_cmd.timer);
+ qla2x00_del_timer(sp);
qla24xx_async_gnnft(vha, sp, sp->gen2);
}
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 45319119606a..ecdb78924ca8 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -60,6 +60,9 @@ qla2x00_sp_timeout(struct timer_list *t)
iocb = &sp->u.iocb_cmd;
iocb->timeout(sp);
spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);
+
+ if (refcount_dec_and_test(&iocb->ref))
+ qla2x00_rel_sp(sp);
}
void
@@ -68,8 +71,9 @@ qla2x00_sp_free(void *ptr)
srb_t *sp = ptr;
struct srb_iocb *iocb = &sp->u.iocb_cmd;
- del_timer(&iocb->timer);
- qla2x00_rel_sp(sp);
+ qla2x00_del_timer(sp);
+ if (refcount_dec_and_test(&iocb->ref))
+ qla2x00_rel_sp(sp);
}
/* Asynchronous Login/Logout Routines -------------------------------------- */
@@ -1553,7 +1557,7 @@ 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 (qla2x00_del_timer(sp))
complete(&abt->u.abt.comp);
}
diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h
index 06c4a843e2ad..75af2c4bb92f 100644
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -277,9 +277,21 @@ qla2x00_init_timer(srb_t *sp, unsigned long tmo)
init_completion(&sp->u.iocb_cmd.u.fxiocb.fxiocb_comp);
if (sp->type == SRB_ELS_DCMD)
init_completion(&sp->u.iocb_cmd.u.els_logo.comp);
+ refcount_set(&sp->u.iocb_cmd.ref, 2);
add_timer(&sp->u.iocb_cmd.timer);
}
+static inline int
+qla2x00_del_timer(srb_t *sp)
+{
+ int rval;
+
+ rval = del_timer(&sp->u.iocb_cmd.timer);
+ if (rval)
+ refcount_dec(&sp->u.iocb_cmd.ref);
+ return rval;
+}
+
static inline int
qla2x00_gid_list_size(struct qla_hw_data *ha)
{
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 6a719c1f8af5..bfde6ebc30d3 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -2384,8 +2384,7 @@ qla2x00_els_dcmd_sp_free(void *data)
elsio->u.els_logo.els_logo_pyld,
elsio->u.els_logo.els_logo_pyld_dma);
- del_timer(&elsio->timer);
- qla2x00_rel_sp(sp);
+ qla2x00_sp_free(sp);
}
static void
@@ -2581,8 +2580,7 @@ qla2x00_els_dcmd2_sp_free(void *data)
elsio->u.els_plogi.els_resp_pyld,
elsio->u.els_plogi.els_resp_pyld_dma);
- del_timer(&elsio->timer);
- qla2x00_rel_sp(sp);
+ qla2x00_sp_free(sp);
}
static void
--
2.15.0.rc0
