From owner-p4-projects@FreeBSD.ORG Wed Nov 1 16:14:17 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 57C4C16A415; Wed, 1 Nov 2006 16:14:17 +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 ED82716A53F; Wed, 1 Nov 2006 16:14:16 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by mx1.FreeBSD.org (Postfix) with ESMTP id 57E7543D81; Wed, 1 Nov 2006 16:13:59 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.6/8.13.6) with ESMTP id kA1GDjwD001931; Wed, 1 Nov 2006 11:13:50 -0500 (EST) (envelope-from jhb@freebsd.org) From: John Baldwin To: Scott Long Date: Wed, 1 Nov 2006 10:47:07 -0500 User-Agent: KMail/1.9.1 References: <200611010112.kA11C1Jt066210@repoman.freebsd.org> In-Reply-To: <200611010112.kA11C1Jt066210@repoman.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200611011047.07627.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Wed, 01 Nov 2006 11:13:50 -0500 (EST) X-Virus-Scanned: ClamAV 0.88.3/2138/Wed Nov 1 06:55:22 2006 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Perforce Change Reviews Subject: Re: PERFORCE change 108878 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: Wed, 01 Nov 2006 16:14:17 -0000 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