@@ -483,6 +483,7 @@ struct srb_iocb {
struct timer_list timer;
void (*timeout)(void *);
+ refcount_t ref;
};
/* Values for srb_ctx type */
@@ -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);
}
@@ -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);
}
@@ -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)
{
@@ -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
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@codethink.co.uk> --- 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(-)