From owner-freebsd-net Tue Jan 7 1:15:23 2003 Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A540137B401; Tue, 7 Jan 2003 01:15:21 -0800 (PST) Received: from harmony.village.org (rover.bsdimp.com [204.144.255.66]) by mx1.FreeBSD.org (Postfix) with ESMTP id B6B1943ED8; Tue, 7 Jan 2003 01:15:20 -0800 (PST) (envelope-from imp@bsdimp.com) Received: from localhost (warner@rover2.village.org [10.0.0.1]) by harmony.village.org (8.12.6/8.12.3) with ESMTP id h079FD1e066855; Tue, 7 Jan 2003 02:15:13 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Tue, 07 Jan 2003 02:14:28 -0700 (MST) Message-Id: <20030107.021428.126452776.imp@bsdimp.com> To: tlambert2@mindspring.com Cc: nate@root.org, current@FreeBSD.ORG, net@FreeBSD.ORG Subject: Re: Proper -current if_attach locking? From: "M. Warner Losh" In-Reply-To: <3E1A87A8.F9C079F@mindspring.com> References: <20030107.001924.02080410.imp@bsdimp.com> <3E1A87A8.F9C079F@mindspring.com> X-Mailer: Mew version 2.1 on Emacs 21.2 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org In message: <3E1A87A8.F9C079F@mindspring.com> Terry Lambert writes: : "M. Warner Losh" wrote: : > In message: : > Nate Lawson writes: : > : I was looking into some "could sleep messages" and found some bogus : > : locking in the attach routine of many drivers. Several init a mtx in : > : their softc and then lock/unlock it in their attach routine. This, of : > : course, does nothing to provide exclusive access to a device. I assume : > : there is going to be a global IF_LOCK or something to be used in attach : > : routines. Can someone fill me in on the intended design? : > : > The locking in the attach routines is generally bogus. Locking is : > only needed when you have more than one thread of execution. You : > don't have more than one thread of execution until after you establish : > the ISR and turn on interrupts. We should likely not be enabling : > interrupts until very late in the attach routine so that we don't need : > any locking in them. : : I looked at this. It seems to me that it's not quite that : simple (sorry). I think that there are issues with locking : because you don't know if this is a driver that's getting : loaded well after the system has booted, or if this is a : PCCARD or other "hot plug" device that has just arrived in : the system. That doesn't mattar at all. If it is a new device that's just arrived, the attach still won't be interrupted *by other code in the driver* until after it has setup its ISR and told the hardware to start generating interrupts. No device locking is needed in the attach routine until after interrupts are enabled in the hardware. : It also seems to me that the "reversal" problems that would : result by simply inserting locking have to do with the fact : that the code is relatively schitzophrenic on deciding whether : it's locking data, or it's locking a critical path. The reversal is because of the bogus locking. The first time through it locks the device then the interface. However, after that it locks the interface and then the device, which can be bad. It does point to a problem, however, in that sometimes we'll take out the locks in one order and other times other orders depending on the code path if we aren't careful. I should go look at the new code more closely. I worry that in the non interrupt case we get things in the IF, DEV order (because the IF locks, then calls the callback routines, which then call the DEV lock). But in the interrupt case we get the DEV lock first, then try to queue data and that somehow causes the IF locks to be grabbed. But you are right, I do need to go look at the code to see what, exactly, is happening and how the new interface locking code is interacting with the semi-bogus locking than many of the wpaul drivers have in them now. : I can't be the only one who finds FreeBSD 5.x to be in such a state : of flux that it's almost impossible to know what a correct : implementation is supposed to look like, for a given subsystem : and/or device driver, list, etc.. There we agree. It takes a lot to keep up, and even then I fall behind when something new happens behind my back. Warner To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message