Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 15 Nov 2012 18:36:04 +0100
From:      Marko Zec <zec@fer.hr>
To:        Adrian Chadd <adrian@freebsd.org>
Cc:        freebsd-net@freebsd.org, Hans Petter Selasky <hselasky@freebsd.org>, freebsd-hackers@freebsd.org
Subject:   Re: VIMAGE crashes on 9.x with hotplug net80211 devices
Message-ID:  <201211151836.04709.zec@fer.hr>
In-Reply-To: <CAJ-Vmom-kuqXwwTOpZLRFQNrRsdYw9aCOE-KndQH9hoWErhZmg@mail.gmail.com>
References:  <CAJ-VmomchJZ7GUKrAjmmyBXDw-6H6O5fAxT_tfAFfhU=HknG1g@mail.gmail.com> <201210291115.23845.zec@fer.hr> <CAJ-Vmom-kuqXwwTOpZLRFQNrRsdYw9aCOE-KndQH9hoWErhZmg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_EgSpQL+nIDcE64N
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On Thursday 15 November 2012 07:18:31 Adrian Chadd wrote:
> Hi,
>
> Here's what I have thus far. Please ignore the device_printf() change.
>
> This works for me, both for hotplug cardbus wireless devices as well
> as (inadvertently!) a USB bluetooth device.
>
> What do you think?

It looks that you've hit the right spot to set curvnet context in 
device_probe_and_attach().

Could you try out a slightly revised verstion (attached) - this one also 
removes now redundant curvnet setting from linker routines (kldload / 
kldunload), and adds a few extra bits which might be necessary for a 
broader range of drivers to work.

Note that I haven't tested this myself as I don't have a -CURRENT machine 
ATM, but a similar patch for 8.3 apparently works fine, though I don't have 
hotplugabble network cards to play with (neither cardbus nor USB)...

Cheers,

Marko

--Boundary-00=_EgSpQL+nIDcE64N
Content-Type: text/x-diff; charset="iso 8859-15";
	name="hotplug_vnet_20121115.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="hotplug_vnet_20121115.diff"

Index: sys/kern/subr_bus.c
===================================================================
--- sys/kern/subr_bus.c	(revision 243091)
+++ sys/kern/subr_bus.c	(working copy)
@@ -53,6 +53,8 @@
 #include <sys/bus.h>
 #include <sys/interrupt.h>
 
+#include <net/vnet.h>
+
 #include <machine/stdarg.h>
 
 #include <vm/uma.h>
@@ -2735,7 +2737,11 @@
 		return (0);
 	else if (error != 0)
 		return (error);
-	return (device_attach(dev));
+
+	CURVNET_SET_QUIET(vnet0);
+	error = device_attach(dev);
+	CURVNET_RESTORE();
+	return (error);
 }
 
 /**
Index: sys/kern/kern_linker.c
===================================================================
--- sys/kern/kern_linker.c	(revision 243091)
+++ sys/kern/kern_linker.c	(working copy)
@@ -53,8 +53,6 @@
 #include <sys/syscallsubr.h>
 #include <sys/sysctl.h>
 
-#include <net/vnet.h>
-
 #include <security/mac/mac_framework.h>
 
 #include "linker_if.h"
@@ -1019,12 +1017,6 @@
 		return (error);
 
 	/*
-	 * It is possible that kldloaded module will attach a new ifnet,
-	 * so vnet context must be set when this ocurs.
-	 */
-	CURVNET_SET(TD_TO_VNET(td));
-
-	/*
 	 * If file does not contain a qualified name or any dot in it
 	 * (kldname.ko, or kldname.ver.ko) treat it as an interface
 	 * name.
@@ -1041,7 +1033,7 @@
 	error = linker_load_module(kldname, modname, NULL, NULL, &lf);
 	if (error) {
 		KLD_UNLOCK();
-		goto done;
+		return (error);
 	}
 	lf->userrefs++;
 	if (fileid != NULL)
@@ -1055,9 +1047,6 @@
 #else
 	KLD_UNLOCK();
 #endif
-
-done:
-	CURVNET_RESTORE();
 	return (error);
 }
 
@@ -1095,7 +1084,6 @@
 	if ((error = priv_check(td, PRIV_KLD_UNLOAD)) != 0)
 		return (error);
 
-	CURVNET_SET(TD_TO_VNET(td));
 	KLD_LOCK();
 	lf = linker_find_file_by_id(fileid);
 	if (lf) {
@@ -1137,7 +1125,6 @@
 #else
 	KLD_UNLOCK();
 #endif
-	CURVNET_RESTORE();
 	return (error);
 }
 
Index: sys/netgraph/bluetooth/socket/ng_btsocket.c
===================================================================
--- sys/netgraph/bluetooth/socket/ng_btsocket.c	(revision 243091)
+++ sys/netgraph/bluetooth/socket/ng_btsocket.c	(working copy)
@@ -46,6 +46,8 @@
 #include <sys/sysctl.h>
 #include <sys/taskqueue.h>
 
+#include <net/vnet.h>
+
 #include <netgraph/ng_message.h>
 #include <netgraph/netgraph.h>
 #include <netgraph/bluetooth/include/ng_bluetooth.h>
@@ -285,4 +287,4 @@
 	return (error);
 } /* ng_btsocket_modevent */
 
-DOMAIN_SET(ng_btsocket_);
+VNET_DOMAIN_SET(ng_btsocket_);
Index: sys/net/if.c
===================================================================
--- sys/net/if.c	(revision 243091)
+++ sys/net/if.c	(working copy)
@@ -504,6 +504,7 @@
 
 	ifp->if_flags |= IFF_DYING;			/* XXX: Locking */
 
+	CURVNET_SET_QUIET(ifp->if_vnet);
 	IFNET_WLOCK();
 	KASSERT(ifp == ifnet_byindex_locked(ifp->if_index),
 	    ("%s: freeing unallocated ifnet", ifp->if_xname));
@@ -511,9 +512,9 @@
 	ifindex_free_locked(ifp->if_index);
 	IFNET_WUNLOCK();
 
-	if (!refcount_release(&ifp->if_refcount))
-		return;
-	if_free_internal(ifp);
+	if (refcount_release(&ifp->if_refcount))
+		if_free_internal(ifp);
+	CURVNET_RESTORE();
 }
 
 /*
@@ -793,7 +794,9 @@
 if_detach(struct ifnet *ifp)
 {
 
+	CURVNET_SET_QUIET(ifp->if_vnet);
 	if_detach_internal(ifp, 0);
+	CURVNET_RESTORE();
 }
 
 static void

--Boundary-00=_EgSpQL+nIDcE64N--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201211151836.04709.zec>