Date: Wed, 1 Nov 2006 10:47:07 -0500 From: John Baldwin <jhb@freebsd.org> To: Scott Long <scottl@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 108878 for review Message-ID: <200611011047.07627.jhb@freebsd.org> In-Reply-To: <200611010112.kA11C1Jt066210@repoman.freebsd.org> References: <200611010112.kA11C1Jt066210@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday 31 October 2006 20:12, Scott Long wrote: > http://perforce.freebsd.org/chv.cgi?CH=108878 > > Change 108878 by scottl@scottl-x64 on 2006/11/01 01:11:30 > > For some wonderful reason, you cannot pass &Giant to msleep. Work > around that in a crude fashion. Also add some more assertions. Ah, yes, that would be most unhappy. I guess mostly the idea is that Giant should be rather implicit and explicitly using Giant for an object lock is discouraged. I'm not sure that is what you are doing though. I think maybe you are borrowing Giant that's already held? > Affected files ... > > .. //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#14 edit > > Differences ... > > ==== //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#14 (text+ko) ==== > > @@ -515,14 +515,16 @@ > /* > * 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(); > */ > - 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. > @@ -757,9 +759,11 @@ > cam_periph_getccb(struct cam_periph *periph, u_int32_t priority) > { > struct ccb_hdr *ccb_h; > + struct mtx *mtx; > int s; > > CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("entering cdgetccb\n")); > + mtx_assert(periph->sim->mtx, MA_OWNED); > > s = splsoftcam(); > > @@ -770,7 +774,12 @@ > if ((SLIST_FIRST(&periph->ccb_list) != NULL) > && (SLIST_FIRST(&periph->ccb_list)->pinfo.priority == priority)) > break; > - msleep(&periph->ccb_list, periph->sim->mtx, PRIBIO, "cgticb", 0); > + mtx_assert(periph->sim->mtx, MA_OWNED); > + if (periph->sim->mtx == &Giant) > + mtx = NULL; > + else > + mtx = periph->sim->mtx; > + msleep(&periph->ccb_list, mtx, PRIBIO, "cgticb", 0); > } > > ccb_h = SLIST_FIRST(&periph->ccb_list); > @@ -782,14 +791,19 @@ > void > cam_periph_ccbwait(union ccb *ccb) > { > + struct mtx *mtx; > struct cam_sim *sim; > int s; > > s = splsoftcam(); > sim = xpt_path_sim(ccb->ccb_h.path); > + if (sim->mtx == &Giant) > + mtx = NULL; > + else > + mtx = sim->mtx; > if ((ccb->ccb_h.pinfo.index != CAM_UNQUEUED_INDEX) > || ((ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_INPROG)) > - msleep(&ccb->ccb_h.cbfcnp, sim->mtx, PRIBIO, "cbwait", 0); > + msleep(&ccb->ccb_h.cbfcnp, mtx, PRIBIO, "cbwait", 0); > > splx(s); > } > @@ -864,10 +878,13 @@ > cam_flags camflags, u_int32_t sense_flags, > struct devstat *ds) > { > + struct cam_sim *sim; > int error; > > error = 0; > - > + sim = xpt_path_sim(ccb->ccb_h.path); > + > + mtx_assert(sim->mtx, MA_OWNED); > /* > * If the user has supplied a stats structure, and if we understand > * this particular type of ccb, record the transaction start. > -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200611011047.07627.jhb>