Date: Tue, 07 Jan 2003 02:14:28 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: tlambert2@mindspring.com Cc: nate@root.org, current@FreeBSD.ORG, net@FreeBSD.ORG Subject: Re: Proper -current if_attach locking? Message-ID: <20030107.021428.126452776.imp@bsdimp.com> In-Reply-To: <3E1A87A8.F9C079F@mindspring.com> References: <Pine.BSF.4.21.0301031458210.99923-100000@root.org> <20030107.001924.02080410.imp@bsdimp.com> <3E1A87A8.F9C079F@mindspring.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <3E1A87A8.F9C079F@mindspring.com>
Terry Lambert <tlambert2@mindspring.com> writes:
: "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.
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030107.021428.126452776.imp>