Date: Mon, 06 Jan 2003 23:54:16 -0800 From: Terry Lambert <tlambert2@mindspring.com> To: "M. Warner Losh" <imp@bsdimp.com> Cc: nate@root.org, current@FreeBSD.ORG, net@FreeBSD.ORG Subject: Re: Proper -current if_attach locking? Message-ID: <3E1A87A8.F9C079F@mindspring.com> References: <Pine.BSF.4.21.0301031458210.99923-100000@root.org> <20030107.001924.02080410.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
"M. Warner Losh" wrote: > In message: <Pine.BSF.4.21.0301031458210.99923-100000@root.org> > Nate Lawson <nate@root.org> 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. 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 entire idea of lock order reversals (please, don't use the abbreviation "LOR": my mind keeps telling me that Legolas just got his commit bit) is predicated, I think, on the idea that there is an order of operation that has to be adhered to because we are talking not about data object locking, we are in fact talking about (theoretically independent) code path locking, and what happens on a reentry during a sleep. Basically, this means that for code that can't result in a sleep, there's no such thing as a lock order reversal -- there's only locking at the wrong functional layer. In this particular case, we're talking about a interface list, and we're talking about a device instance, and the events that get a device on or off the list are *not* independent and they *are* mutually exclusive, based on state. Given all that, it seems to me that this would be an ideal place for a "critical section" lock -- or, at least, a lock that's asserted on the list of interfaces, and which doesn't have any witness registration (per Bruce's earlier suggestion). My preference (if it's not obvious) would be a critical section lock, but a data lock on a list held over the same operations is functionally the same thing, anyway. Perhaps it's time to step back, and look over the overall locking philosophy that everyone is supposed to be implementing to, and the implementation assumptions that that philosophy makes necessary for things to work? I think that some of the assumptions are themselves mutually exclusive with current implementation, and it would be nice to know, up front, what needs to be rewritten, and what *should* be rewritten, to conform to the new philosophy. 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.. If someone were to "lay down the law", we could at least discuss it, and come to some conclusions that would let people know what to do -- even if there's no change in "the law" as a result, it's a useful exercise, since people can then just "code to the spec.", and not have to have in depth knowledge of the overall design (if there is one) to deal with all the related issues... they can just copy the changes into every other driver/whatever that's in the same niche, ecologically. Parallel progress is a Good Thing(tm). -- Terry To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3E1A87A8.F9C079F>