From owner-freebsd-bugs@FreeBSD.ORG Sat Jan 10 19:10:06 2009 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 39FE4106564A for ; Sat, 10 Jan 2009 19:10:06 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 277AF8FC0A for ; Sat, 10 Jan 2009 19:10:06 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id n0AJA6nJ096441 for ; Sat, 10 Jan 2009 19:10:06 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id n0AJA5cE096440; Sat, 10 Jan 2009 19:10:05 GMT (envelope-from gnats) Date: Sat, 10 Jan 2009 19:10:05 GMT Message-Id: <200901101910.n0AJA5cE096440@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Sean Bruno Cc: Subject: Re: kern/118093: [firewire] firewire bus reset hogs CPU, causing data to be lost X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Sean Bruno List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 10 Jan 2009 19:10:06 -0000 The following reply was made to PR kern/118093; it has been noted by GNATS. From: Sean Bruno To: Dieter Cc: bug-followup@FreeBSD.org, freebsd-firewire@FreeBSD.org Subject: Re: kern/118093: [firewire] firewire bus reset hogs CPU, causing data to be lost Date: Sat, 10 Jan 2009 10:54:37 -0800 On Tue, 2009-01-06 at 09:33 +0000, Dieter wrote: > In message <1231222803.21260.17.camel@localhost.localdomain>, Sean Bruno writes: > > > > > uint32_t fun; > > > > > > > > + FW_GLOCK(fc); > > > > device_printf(fc->dev, "Initiate bus reset\n"); > > > > sc = (struct fwohci_softc *)fc; > > > > > > > > @@ -2487,6 +2495,7 @@ > > > > fun |= FW_PHY_ISBR | FW_PHY_RHB; > > > > fun = fwphy_wrdata(sc, FW_PHY_ISBR_REG, fun); > > > > #endif > > > > + FW_GUNLOCK(fc); > > > > } > > > > > > Does the lock need to protect the printf? > > > > These are gross, overpowered, way to heavy handed locks that I'm playing > > with. I need to prevent pre-emption of certain events while they are in > > progress. One of these events is the firewire's assertion of "bus > > reset" on the firewire device. I see the h/w interrupt firing before > > this code can actually complete, causing the driver to be confused on > > occasion. > > I understand the basic concept of locking, or at least I did many many years > ago when spls really were levels. Since then they changed spls to not be levels > (at least that's what the man page says) and later replaced with mutex. I assume > these changes are at least in part to better support SMP. > > What I don't understand is things like getting a lock and immediately releasing > it, which appears to me to protect nothing. Or why the printf needs to be inside > the locked section of code. I thought the goal was to hold a lock for as short a > time as possible, and it is not clear to me why the printf needs to be protected. The printf DOES NOT need protection at all. You are absolutely correct. The lock should be moved lower in the function for sure. This is just my attempt to see if I am looking at the same problem you are reporting. Conceptually, I am trying to keep the h/w interrupt which fires during a bus reset from preempting this function and executing before this function is finished. The acquisition of the lock and release keeps the interrupt handler from executing before the setting of bus reset is finished. I hope to fine tune the locks after I have confirmed operational goodness. Sean