--- a/sys/dev/disk/iscsi/initiator/iscsivar.h +++ b/sys/dev/disk/iscsi/initiator/iscsivar.h @@ -137,6 +137,12 @@ int targetid; // int cid; // Connection ID // int tsih; // target session identifier handle + /* + | in-flight users of this session (ic_action, per-session sysctl + | handlers); guarded by isc_softc.lock. ism_stop() drains this + | to zero before destroying the session. + */ + int refs; sn_t sn; // sequence number stuff; int cws; // current window size @@ -249,6 +255,12 @@ int ism_start(isc_session_t *sp); void ism_stop(isc_session_t *sp); +/* the driver softc (was static in iscsi.c) */ +extern struct isc_softc isc; + +isc_session_t *isc_sess_lookup(int sid); /* takes a reference */ +void isc_sess_rele(isc_session_t *sp); /* drops a reference */ + int scsi_encap(struct cam_sim *sim, union ccb *ccb); int scsi_decap(isc_session_t *sp, pduq_t *opq, pduq_t *pq); void iscsi_r2t(isc_session_t *sp, pduq_t *opq, pduq_t *pq); --- a/sys/dev/disk/iscsi/initiator/iscsi.c +++ b/sys/dev/disk/iscsi/initiator/iscsi.c @@ -63,7 +63,7 @@ static char *iscsi_driver_version = "2.1.0"; -static struct isc_softc isc; +struct isc_softc isc; MALLOC_DEFINE(M_ISCSI, "iSCSI", "iSCSI driver"); --- a/sys/dev/disk/iscsi/initiator/isc_sm.c +++ b/sys/dev/disk/iscsi/initiator/isc_sm.c @@ -626,6 +626,35 @@ } #endif +/* + | Look up a session by sid and take a reference, so the caller may + | use sp while ism_stop() runs concurrently: ism_stop() clears + | sc->sessions[sid] under sc->lock before tearing the session down, + | and drains sp->refs to zero before freeing it. + */ +isc_session_t * +isc_sess_lookup(int sid) +{ + isc_session_t *sp; + + if(sid < 0 || sid >= MAX_SESSIONS) + return NULL; + lockmgr(&isc.lock, LK_EXCLUSIVE); + if((sp = isc.sessions[sid]) != NULL) + sp->refs++; + lockmgr(&isc.lock, LK_RELEASE); + return sp; +} + +void +isc_sess_rele(isc_session_t *sp) +{ + lockmgr(&sp->isc->lock, LK_EXCLUSIVE); + if(--sp->refs == 0) + wakeup(sp); + lockmgr(&sp->isc->lock, LK_RELEASE); +} + static int isc_dump_stats(SYSCTL_HANDLER_ARGS) { @@ -634,7 +663,13 @@ char buf[1024], *bp; int error, n; - sp = (isc_session_t *)arg1; + /* + | arg1 is the sid, NOT a raw session pointer: the sysctl OID can + | race (or outlive) the session, so resolve it under the lock. + */ + sp = isc_sess_lookup((int)(intptr_t)arg1); + if(sp == NULL) + return ENODEV; sc = sp->isc; bp = buf; @@ -649,29 +684,40 @@ ksnprintf(bp, n, " cws=%d cmd=%x exp=%x max=%x stat=%x itt=%x", sp->cws, sp->sn.cmd, sp->sn.expCmd, sp->sn.maxCmd, sp->sn.stat, sp->sn.itt); error = SYSCTL_OUT(req, buf, strlen(buf)); + isc_sess_rele(sp); return error; } static int isc_sysctl_targetName(SYSCTL_HANDLER_ARGS) { + isc_session_t *sp; char buf[128], **cp; int error; - cp = (char **)arg1; + sp = isc_sess_lookup((int)(intptr_t)arg1); + if(sp == NULL) + return ENODEV; + cp = (char **)&sp->opt.targetName; ksnprintf(buf, sizeof(buf), "%s", *cp); error = SYSCTL_OUT(req, buf, strlen(buf)); + isc_sess_rele(sp); return error; } static int isc_sysctl_targetAddress(SYSCTL_HANDLER_ARGS) { + isc_session_t *sp; char buf[128], **cp; int error; - cp = (char **)arg1; + sp = isc_sess_lookup((int)(intptr_t)arg1); + if(sp == NULL) + return ENODEV; + cp = (char **)&sp->opt.targetAddress; ksnprintf(buf, sizeof(buf), "%s", *cp); error = SYSCTL_OUT(req, buf, strlen(buf)); + isc_sess_rele(sp); return error; } static void @@ -693,7 +739,7 @@ OID_AUTO, "targetname", CTLFLAG_RD, - (void *)&sp->opt.targetName, 0, + (void *)(intptr_t)sp->sid, 0, isc_sysctl_targetName, "A", "target name"); SYSCTL_ADD_PROC(&sp->clist, @@ -701,7 +747,7 @@ OID_AUTO, "targeaddress", CTLFLAG_RD, - (void *)&sp->opt.targetAddress, 0, + (void *)(intptr_t)sp->sid, 0, isc_sysctl_targetAddress, "A", "target address"); SYSCTL_ADD_PROC(&sp->clist, @@ -709,7 +755,7 @@ OID_AUTO, "stats", CTLFLAG_RD, - (void *)sp, 0, + (void *)(intptr_t)sp->sid, 0, isc_dump_stats, "A", "statistics"); SYSCTL_ADD_INT(&sp->clist, @@ -727,6 +773,16 @@ cdev_t dev; debug_called(8); + /* + | Detach from the lookup table FIRST, under sc->lock, so no new + | ic_action() or per-session sysctl reader can find the session + | while it is torn down (they look it up under the same lock and + | take a reference, or see NULL and fail cleanly). + */ + lockmgr(&sc->lock, LK_EXCLUSIVE); + sc->sessions[sp->sid] = NULL; + lockmgr(&sc->lock, LK_RELEASE); + sdebug(2, "terminating"); /* | first stop the receiver @@ -756,6 +812,18 @@ sc->nsess--; lockmgr(&sc->lock, LK_RELEASE); + /* + | Wait for in-flight ic_action()/sysctl holders to drop their + | references before the session is destroyed. + */ + lockmgr(&sc->lock, LK_EXCLUSIVE); + while(sp->refs != 0) { + lockmgr(&sc->lock, LK_RELEASE); + tsleep(sp, 0, "isc-drn", 1); + lockmgr(&sc->lock, LK_EXCLUSIVE); + } + lockmgr(&sc->lock, LK_RELEASE); + dev = sp->dev; sp->dev = NULL; @@ -768,12 +836,16 @@ mtx_uninit(&sp->snd_mtx); mtx_uninit(&sp->io_mtx); - i_freeopt(&sp->opt); - sc->sessions[sp->sid] = NULL; - + /* + | Deregister the sysctl OIDs BEFORE freeing what they resolve + | through; with the drain above no handler can still be running + | and no new one can start. + */ if(sysctl_ctx_free(&sp->clist)) xdebug("sysctl_ctx_free failed"); + i_freeopt(&sp->opt); + kfree(sp, M_ISCSI); } --- a/sys/dev/disk/iscsi/initiator/isc_cam.c +++ b/sys/dev/disk/iscsi/initiator/isc_cam.c @@ -207,8 +207,20 @@ debug_called(8); - if((ccb_h->target_id != CAM_TARGET_WILDCARD) && (ccb_h->target_id < MAX_SESSIONS)) + if((ccb_h->target_id != CAM_TARGET_WILDCARD) && (ccb_h->target_id < MAX_SESSIONS)) { + /* + | Take a reference under the lock so ism_stop() cannot free + | the session while we use it; released before returning. + | (The XPT_SCSI_IO queue path keeps the pre-existing pdu/CCB + | lifetime, which ism_stop's xmitter stop + i_pdu_flush + | already bound.) + */ + lockmgr(&isp->lock, LK_EXCLUSIVE); sp = isp->sessions[ccb_h->target_id]; + if(sp != NULL) + sp->refs++; + lockmgr(&isp->lock, LK_RELEASE); + } else sp = NULL; @@ -304,6 +316,8 @@ ccb_h->status = CAM_REQ_INVALID; break; } + if(sp != NULL) + isc_sess_rele(sp); xpt_done(ccb); return; }