From owner-freebsd-net  Tue Jan  7  1:27: 8 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 0686F37B401; Tue,  7 Jan 2003 01:27:07 -0800 (PST)
Received: from harmony.village.org (rover.bsdimp.com [204.144.255.66])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id 5055C43EB2; Tue,  7 Jan 2003 01:27:06 -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 h079R11e066930;
	Tue, 7 Jan 2003 02:27:01 -0700 (MST)
	(envelope-from imp@bsdimp.com)
Date: Tue, 07 Jan 2003 02:26:17 -0700 (MST)
Message-Id: <20030107.022617.23700606.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" <imp@bsdimp.com>
In-Reply-To: <20030107.021428.126452776.imp@bsdimp.com>
References: <20030107.001924.02080410.imp@bsdimp.com>
	<3E1A87A8.F9C079F@mindspring.com>
	<20030107.021428.126452776.imp@bsdimp.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: <freebsd-net.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20freebsd-net>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20freebsd-net>
X-Loop: FreeBSD.org

I was right (and I think you are too).  We do have lock issues.

dc_attach does approximately:

DC_LOCK
ether_attach()	(which does a IFNET_WLOCK/UNLOCK pair)
DC_UNLOCK

(this sets the lock order to be DC_LOCK, IFNET_WLOCK).

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
:-(

Warner

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message