From owner-p4-projects@FreeBSD.ORG Sun Jun 18 15:23:52 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id C326A16A47B; Sun, 18 Jun 2006 15:23:52 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9FF5B16A479 for ; Sun, 18 Jun 2006 15:23:52 +0000 (UTC) (envelope-from scottl@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 597F543D46 for ; Sun, 18 Jun 2006 15:23:52 +0000 (GMT) (envelope-from scottl@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id k5IFNqeO023695 for ; Sun, 18 Jun 2006 15:23:52 GMT (envelope-from scottl@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id k5IFNqQY023691 for perforce@freebsd.org; Sun, 18 Jun 2006 15:23:52 GMT (envelope-from scottl@freebsd.org) Date: Sun, 18 Jun 2006 15:23:52 GMT Message-Id: <200606181523.k5IFNqQY023691@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to scottl@freebsd.org using -f From: Scott Long To: Perforce Change Reviews Cc: Subject: PERFORCE change 99528 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Jun 2006 15:23:53 -0000 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 #include #include +#include #include #include @@ -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); }