Date: Tue, 7 Jan 2003 10:03:15 -0500 (EST) From: Andrew Gallatin <gallatin@cs.duke.edu> To: "M. Warner Losh" <imp@bsdimp.com> Cc: tlambert2@mindspring.com, nate@root.org, current@FreeBSD.ORG, net@FreeBSD.ORG Subject: Re: Proper -current if_attach locking? Message-ID: <15898.60467.681434.927797@grasshopper.cs.duke.edu> In-Reply-To: <20030107.022617.23700606.imp@bsdimp.com> References: <20030107.001924.02080410.imp@bsdimp.com> <3E1A87A8.F9C079F@mindspring.com> <20030107.021428.126452776.imp@bsdimp.com> <20030107.022617.23700606.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
M. Warner Losh writes:
<..>
> However in if_slowtimo we have:
>
> if_slowtimo(arg)
> {
> ... IFNET_RLOCK();
> ... if (ifp->if_watchdog)
> (*ifp->if_watchdog)(ifp);
> ... IFNET_RUNLOCK();
> }
>
> and dc_watchdog does a DC_LOCK/UNLOCK pair). This is a Lock Order
> Reversal, and not a LotR :-)
>
> What's worse is that dc_intr does:
>
> DC_LOCK
> ...dc_start (which calls IF_PREPEND which does the IFNET_LOCK/UNLOCK thing)
> DC_UNLOCK
>
> So even if we remove the one from attach, it looks like we have others
> lurking in the code.
>
> Either that, or it is too late for me to be looking at code like this
> :-(
>
I think its too late at night ;)
The IFNET_RLOCK() called in if_slowtimo() is a global lock for the
list of ifnet structs to ensure that no devices are removed or added
while something may be using it. There is one ifnet list in the system.
The lock in IF_PREPEND() (and more commonly used in drivers,
IF_DEQUE()) is per-ifq, to protect against multiple accesses
to a single ifq. There are many ifqs in the system.
FWIW, I've been running my 3rd party Myrinet driver Giant-free and
have had no problems, and no lock order reversals. I don't do bogus
locking in my attach routine, though :)
FWIW2: Running Giant-free brings -current TCP performance up to nearly
63% of -stable performance (from 39%), and udp xmit perf up to 87% of
-stable. (testing w/o WITNESS).
Drew
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?15898.60467.681434.927797>