From owner-freebsd-current@FreeBSD.ORG Wed Feb 11 16:26:26 2009 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4D241106574E for ; Wed, 11 Feb 2009 16:26:26 +0000 (UTC) (envelope-from sean.bruno@dsl-only.net) Received: from iron2.pdx.net (iron2.pdx.net [69.64.224.71]) by mx1.freebsd.org (Postfix) with ESMTP id 2537B8FC2F for ; Wed, 11 Feb 2009 16:26:25 +0000 (UTC) (envelope-from sean.bruno@dsl-only.net) Received: (qmail 16082 invoked from network); 11 Feb 2009 08:25:53 -0800 Received: from 069-064-235-060.pdx.net (HELO ?192.168.1.51?) (69.64.235.60) by iron2.pdx.net with SMTP; 11 Feb 2009 08:25:53 -0800 From: Sean Bruno To: John Baldwin In-Reply-To: <200902110946.46153.jhb@freebsd.org> References: <1234315393.14556.6.camel@localhost.localdomain> <1234332568.14556.11.camel@localhost.localdomain> <200902110946.46153.jhb@freebsd.org> Content-Type: text/plain Date: Wed, 11 Feb 2009 08:26:23 -0800 Message-Id: <1234369583.26300.0.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.24.3 (2.24.3-1.fc10) Content-Transfer-Encoding: 7bit Cc: freebsd-current@freebsd.org Subject: Re: [sysctl] New sysctl LoR today X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Feb 2009 16:26:29 -0000 On Wed, 2009-02-11 at 09:46 -0500, John Baldwin wrote: > On Wednesday 11 February 2009 1:09:28 am Sean Bruno wrote: > > On Tue, 2009-02-10 at 17:23 -0800, Sean Bruno wrote: > > > I'm working on some items in the firewire stack and after a update, I > > > was greeted with a new LoR against the SYSCTL lock. I noted that some > > > things were changing in that space. > > > > > > Did I miss an interface change that I need to pickup in the firewire > > > stack? > > > > > > Sean > > > > > > lock order reversal: (sleepable after non-sleepable) > > > 1st 0xc471bbec sbp (sbp) @ dev/firewire/sbp.c:2253 > > > 2nd 0xc0d3aea4 sysctl lock (sysctl lock) @ kern/kern_sysctl.c:250 > > > KDB: stack backtrace: > > > _sx_xlock(c0d3aea4,0,c0be5d46,fa,c471a000,...) at _sx_xlock+0x85 > > > sysctl_ctx_free(c471a2c0,c0b8f786,c0d0696c,0,c469fa0c,...) at > > > sysctl_ctx_free+0x30 > > > dacleanup(c4c54700,c0b900bb,c480e000,c42aa410,246,...) at dacleanup+0x35 > > > camperiphfree(c4c54700,c4c54700,c42aa694,c047763d,c4c54700,...) at > > No, this is due to CAM calling sysctl_ctx_free() with a lock held. You can > try this change: > > --- //depot/user/jhb/lock/cam/scsi/scsi_cd.c > +++ /home/jhb/work/p4/lock/cam/scsi/scsi_cd.c > @@ -401,11 +401,6 @@ > > xpt_print(periph->path, "removing device entry\n"); > > - if ((softc->flags & CD_FLAG_SCTX_INIT) != 0 > - && sysctl_ctx_free(&softc->sysctl_ctx) != 0) { > - xpt_print(periph->path, "can't remove sysctl context\n"); > - } > - > /* > * In the queued, non-active case, the device in question > * has already been removed from the changer run queue. Since this > @@ -474,9 +469,14 @@ > free(softc->changer, M_DEVBUF); > } > cam_periph_unlock(periph); > + if ((softc->flags & CD_FLAG_SCTX_INIT) != 0 > + && sysctl_ctx_free(&softc->sysctl_ctx) != 0) { > + xpt_print(periph->path, "can't remove sysctl context\n"); > + } > + > disk_destroy(softc->disk); > + free(softc, M_DEVBUF); > cam_periph_lock(periph); > - free(softc, M_DEVBUF); > } > > static void > --- //depot/user/jhb/lock/cam/scsi/scsi_da.c > +++ /home/jhb/work/p4/lock/cam/scsi/scsi_da.c > @@ -995,6 +995,8 @@ > softc = (struct da_softc *)periph->softc; > > xpt_print(periph->path, "removing device entry\n"); > + cam_periph_unlock(periph); > + > /* > * If we can't free the sysctl tree, oh well... > */ > @@ -1003,11 +1005,10 @@ > xpt_print(periph->path, "can't remove sysctl context\n"); > } > > - cam_periph_unlock(periph); > disk_destroy(softc->disk); > callout_drain(&softc->sendordered_c); > + free(softc, M_DEVBUF); > cam_periph_lock(periph); > - free(softc, M_DEVBUF); > } > > static void > > Yup. Thanks for the quick fix! Sean