Date: Sun, 18 Jun 2006 15:23:52 GMT From: Scott Long <scottl@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 99528 for review Message-ID: <200606181523.k5IFNqQY023691@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=99528 Change 99528 by scottl@scottl-wv1u on 2006/06/18 15:23:05 Start working out the periph locking API: cam_periph_lock: Take a hard reference on the SIM via an interlock with the XPT. Returns with the SIM lock held. cam_periph_unlock: removes a reference and returns with the SIM unlocked. cam_periph_acquire: Take a reference on the SIM. Requires the SIM lock to already be held. cam_periph_release: Removes a reference a releases the periph if the refcount reaches 0. Requires the SIM lock to already be held. Adapt the da, pass, and ses drivers to this API. Also, this codifies the lock order as xpt_lock first and sim lock second, so rearrange locking in the camisr to honor this. Affected files ... .. //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#11 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#38 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.h#4 edit .. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_da.c#11 edit .. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_pass.c#9 edit .. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_ses.c#7 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#11 (text+ko) ==== @@ -51,6 +51,7 @@ #include <cam/cam_xpt_periph.h> #include <cam/cam_periph.h> #include <cam/cam_debug.h> +#include <cam/cam_sim.h> #include <cam/scsi/scsi_all.h> #include <cam/scsi/scsi_message.h> @@ -277,6 +278,7 @@ return(CAM_REQ_CMP_ERR); s = splsoftcam(); + mtx_assert(periph->sim->mtx, MA_OWNED); periph->refcount++; splx(s); @@ -292,6 +294,7 @@ return; s = splsoftcam(); + mtx_assert(periph->sim->mtx, MA_OWNED); if ((--periph->refcount == 0) && (periph->flags & CAM_PERIPH_INVALID)) { camperiphfree(periph); @@ -500,23 +503,25 @@ int cam_periph_lock(struct cam_periph *periph, int priority) { - int error; + + /* + * Interlock the SIM lock with the XPT lock to protect against the + * SIM going away unexpectedly while we are trying reference it. + */ + xpt_lock_buses(); + + /* + * Now it's safe to take the SIM lock. + */ + mtx_lock(periph->sim->mtx); + xpt_unlock_buses(); /* - * Increment the reference count on the peripheral - * while we wait for our lock attempt to succeed - * to ensure the peripheral doesn't disappear out - * from under us while we sleep. + * Increment the reference count on the peripheral. */ - if (cam_periph_acquire(periph) != CAM_REQ_CMP) + if (cam_periph_acquire(periph) != CAM_REQ_CMP) { + mtx_unlock(periph->sim->mtx); return(ENXIO); - - while ((periph->flags & CAM_PERIPH_LOCKED) != 0) { - periph->flags |= CAM_PERIPH_LOCK_WANTED; - if ((error = tsleep(periph, priority, "caplck", 0)) != 0) { - cam_periph_release(periph); - return error; - } } periph->flags |= CAM_PERIPH_LOCKED; @@ -530,12 +535,9 @@ cam_periph_unlock(struct cam_periph *periph) { periph->flags &= ~CAM_PERIPH_LOCKED; - if ((periph->flags & CAM_PERIPH_LOCK_WANTED) != 0) { - periph->flags &= ~CAM_PERIPH_LOCK_WANTED; - wakeup(periph); - } cam_periph_release(periph); + mtx_unlock(periph->sim->mtx); } /* ==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#38 (text+ko) ==== @@ -7181,6 +7181,18 @@ { } +void +xpt_lock_buses(void) +{ + mtx_lock(&xsoftc.xpt_lock); +} + +void +xpt_unlock_buses(void) +{ + mtx_unlock(&xsoftc.xpt_lock); +} + static void camisr(void *V_queue) { @@ -7205,8 +7217,6 @@ int runq; TAILQ_REMOVE(&queue, ccb_h, sim_links.tqe); - sim = ccb_h->path->bus->sim; - mtx_lock(sim->mtx); ccb_h->pinfo.index = CAM_UNQUEUED_INDEX; splx(s); @@ -7228,7 +7238,6 @@ * Increment the count since this command is done. */ xsoftc.num_highpower++; - mtx_unlock(&xsoftc.xpt_lock); /* * Any high powered commands queued up? @@ -7236,11 +7245,17 @@ if (send_ccb != NULL) { STAILQ_REMOVE_HEAD(hphead, xpt_links.stqe); + mtx_unlock(&xsoftc.xpt_lock); xpt_release_devq(send_ccb->ccb_h.path, /*count*/1, /*runqueue*/TRUE); - } + } else + mtx_unlock(&xsoftc.xpt_lock); } + + sim = ccb_h->path->bus->sim; + mtx_lock(sim->mtx); + if ((ccb_h->func_code & XPT_FC_USER_CCB) == 0) { struct cam_ed *dev; ==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.h#4 (text+ko) ==== @@ -71,6 +71,8 @@ struct cam_periph *xpt_path_periph(struct cam_path *path); void xpt_async(u_int32_t async_code, struct cam_path *path, void *async_arg); +void xpt_lock_buses(void); +void xpt_unlock_buses(void); #endif /* _KERNEL */ #endif /* _CAM_CAM_XPT_H */ ==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_da.c#11 (text+ko) ==== @@ -531,11 +531,6 @@ splx(s); return (ENXIO); } - mtx_lock(periph->sim->mtx); - - unit = periph->unit_number; - - softc = (struct da_softc *)periph->softc; CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("daopen: disk=%s%d (unit %d)\n", dp->d_name, dp->d_unit, @@ -548,6 +543,9 @@ mtx_unlock(periph->sim->mtx); return(ENXIO); } + + unit = periph->unit_number; + softc = (struct da_softc *)periph->softc; softc->flags |= DA_FLAG_OPEN; if ((softc->flags & DA_FLAG_PACK_INVALID) != 0) { @@ -578,7 +576,6 @@ cam_periph_release(periph); } cam_periph_unlock(periph); - mtx_unlock(periph->sim->mtx); return (error); } @@ -593,14 +590,12 @@ if (periph == NULL) return (ENXIO); - mtx_lock(periph->sim->mtx); - softc = (struct da_softc *)periph->softc; - if ((error = cam_periph_lock(periph, PRIBIO)) != 0) { - mtx_unlock(periph->sim->mtx); return (error); /* error code from tsleep */ } + softc = (struct da_softc *)periph->softc; + if ((softc->quirks & DA_Q_NO_SYNC_CACHE) == 0) { union ccb *ccb; @@ -665,7 +660,6 @@ softc->flags &= ~DA_FLAG_OPEN; cam_periph_unlock(periph); cam_periph_release(periph); - mtx_unlock(periph->sim->mtx); return (0); } @@ -1201,7 +1195,7 @@ * Lock this peripheral until we are setup. * This first call can't block */ - (void)cam_periph_lock(periph, PRIBIO); + (void)cam_periph_acquire(periph); xpt_schedule(periph, /*priority*/5); return(CAM_REQ_CMP); @@ -1661,7 +1655,7 @@ * operation. */ xpt_release_ccb(done_ccb); - cam_periph_unlock(periph); + cam_periph_release(periph); return; } case DA_CCB_WAITING: ==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_pass.c#9 (text+ko) ==== @@ -332,13 +332,17 @@ if (periph == NULL) return (ENXIO); + if ((error = cam_periph_lock(periph, PRIBIO | PCATCH)) != 0) { + splx(s); + return (error); + } + softc = (struct pass_softc *)periph->softc; s = splsoftcam(); - mtx_lock(periph->sim->mtx); if (softc->flags & PASS_FLAG_INVALID) { splx(s); - mtx_unlock(periph->sim->mtx); + cam_periph_unlock(periph); return(ENXIO); } @@ -348,7 +352,7 @@ error = securelevel_gt(td->td_ucred, 1); if (error) { splx(s); - mtx_unlock(periph->sim->mtx); + cam_periph_unlock(periph); return(error); } @@ -357,7 +361,7 @@ */ if (((flags & FWRITE) == 0) || ((flags & FREAD) == 0)) { splx(s); - mtx_unlock(periph->sim->mtx); + cam_periph_unlock(periph); return(EPERM); } @@ -368,26 +372,21 @@ xpt_print_path(periph->path); printf("can't do nonblocking accesss\n"); splx(s); - mtx_unlock(periph->sim->mtx); + cam_periph_unlock(periph); return(EINVAL); } - if ((error = cam_periph_lock(periph, PRIBIO | PCATCH)) != 0) { - splx(s); - mtx_unlock(periph->sim->mtx); - return (error); - } - splx(s); if ((softc->flags & PASS_FLAG_OPEN) == 0) { - if (cam_periph_acquire(periph) != CAM_REQ_CMP) + if (cam_periph_acquire(periph) != CAM_REQ_CMP) { + cam_periph_unlock(periph); return(ENXIO); + } softc->flags |= PASS_FLAG_OPEN; } cam_periph_unlock(periph); - mtx_unlock(periph->sim->mtx); return (error); } @@ -403,19 +402,15 @@ if (periph == NULL) return (ENXIO); - softc = (struct pass_softc *)periph->softc; - - mtx_lock(periph->sim->mtx); if ((error = cam_periph_lock(periph, PRIBIO)) != 0) { - mtx_unlock(periph->sim->mtx); return (error); } + softc = (struct pass_softc *)periph->softc; softc->flags &= ~PASS_FLAG_OPEN; cam_periph_unlock(periph); cam_periph_release(periph); - mtx_unlock(periph->sim->mtx); return (0); } ==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_ses.c#7 (text+ko) ==== @@ -420,17 +420,14 @@ splx(s); return (ENXIO); } - mtx_lock(periph->sim->mtx); if ((error = cam_periph_lock(periph, PRIBIO | PCATCH)) != 0) { splx(s); - mtx_unlock(periph->sim->mtx); return (error); } splx(s); if (cam_periph_acquire(periph) != CAM_REQ_CMP) { cam_periph_unlock(periph); - mtx_unlock(periph->sim->mtx); return (ENXIO); } @@ -463,7 +460,6 @@ cam_periph_release(periph); } cam_periph_unlock(periph); - mtx_unlock(periph->sim->mtx); return (error); } @@ -480,19 +476,15 @@ if (periph == NULL) return (ENXIO); - softc = (struct ses_softc *)periph->softc; - - mtx_lock(periph->sim->mtx); if ((error = cam_periph_lock(periph, PRIBIO)) != 0) { - mtx_unlock(periph->sim->mtx); return (error); } + softc = (struct ses_softc *)periph->softc; softc->ses_flags &= ~SES_FLAG_OPEN; cam_periph_unlock(periph); cam_periph_release(periph); - mtx_unlock(periph->sim->mtx); return (0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606181523.k5IFNqQY023691>