Re: netgraph(4) initialization order

Dear Hackers,

any objections to the attached patch?

thanks,
max


>>>> Dear Hackers,
>>>> 
>>>> would anyone object if i change SI_ORDER_MIDDLE in the 
>>>> /sys/netgraph/ng_base.c:2994 to say SI_ORDER_THIRD, i.e.
>>>> 
>>>> change
>>>> 
>>>> DECLARE_MODULE(netgraph, netgraph_mod, SI_SUB_DRIVERS, 
>>>> SI_ORDER_MIDDLE);
>>>> 
>>>> to
>>>> 
>>>> DECLARE_MODULE(netgraph, netgraph_mod, SI_SUB_DRIVERS,
>>>> SI_ORDER_THIRD);
>>>> 
>>>> the reason for this change is that bluetooth device drivers
>>>> depend on netgraph(4) and when both netgraph(4) and bluetooth
>>>> device driver (such as ng_ubt(4)) compiled in the kernel you
>>>> get a crash. basically ng_ubt(4) mod_load callback is called
>>>> before netgraph(4) mod_load callback and ng_findtype() crashes
>>>> on uninitialized mutex (DEVICE_MODULE macro passes
>>>> SI_SUB_DRIVERS, SI_ORDER_THIRD to the
>>>                  ^^^^^^^^^^^^^^ this should be SI_ORDER_MIDDLE :)
>> 
>>>> DECLARE_MODULE).
>>> 
>>> I thought this is the task of MODULE_DEPEND.
>> 
>> i thought so too :) but it appears to work only when module is 
>> _loaded_ (by hand or from /boot/loader.conf), i.e. it does not work
>> if module was compiled in the kernel.
> 
> maybe the config stuff could be extended to integrate the module 
> dependency stuff along with the suggested order by moving things
> backwards in the list if their dependencies suggest it.  (after the
> bubble sort).
> 
>>>> option #2 would be to have DEVICE_MODULE_ORDERED macro which
>>>> accepts two extra parameters.
>>>> 
>>>> 
>>>> and finally option #3 would be to duplicate entire content of
>>>> the DEVICE_MODULE macro in all bluetooth device drivers and
>>>> specify order in DECLARE_MODULE macro.
>>>> 
>>>> 
>>>> any thoughts?
>>>> 
>>>> thanks, max

--- ng_base.c.orig	Wed Jan  5 12:04:36 2005
+++ ng_base.c	Wed Jan  5 12:23:39 2005
_at__at_ -46,6 +46,7 _at__at_
 
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/conf.h>
 #include <sys/ctype.h>
 #include <sys/errno.h>
 #include <sys/kdb.h>
_at__at_ -2953,27 +2954,40 _at__at_
  * Handle loading and unloading for this code.
  * The only thing we need to link into is the NETISR strucure.
  */
+
+static void
+ngb_sysinit(void)
+{
+	mtx_init(&ng_worklist_mtx, "ng_worklist", NULL, MTX_SPIN);
+	mtx_init(&ng_typelist_mtx, "netgraph types mutex", NULL, MTX_DEF);
+	mtx_init(&ng_nodelist_mtx, "netgraph nodelist mutex", NULL, MTX_DEF);
+	mtx_init(&ng_idhash_mtx, "netgraph idhash mutex", NULL, MTX_DEF);
+	mtx_init(&ngq_mtx, "netgraph free item list mutex", NULL, MTX_DEF);
+
+	/* XXX could use NETISR_MPSAFE but need to verify code */
+	netisr_register(NETISR_NETGRAPH, (netisr_t *)ngintr, NULL, 0);
+}
+
+static void
+ngb_sysuninit(void)
+{
+	netisr_unregister(NETISR_NETGRAPH);
+
+	mtx_destroy(&ngq_mtx);
+	mtx_destroy(&ng_idhash_mtx);
+	mtx_destroy(&ng_nodelist_mtx);
+	mtx_destroy(&ng_typelist_mtx);
+	mtx_destroy(&ng_worklist_mtx);
+}
+
 static int
 ngb_mod_event(module_t mod, int event, void *data)
 {
-	int s, error = 0;
+	int error = 0;
 
 	switch (event) {
 	case MOD_LOAD:
-		/* Register line discipline */
-		mtx_init(&ng_worklist_mtx, "ng_worklist", NULL, MTX_SPIN);
-		mtx_init(&ng_typelist_mtx, "netgraph types mutex", NULL,
-		    MTX_DEF);
-		mtx_init(&ng_nodelist_mtx, "netgraph nodelist mutex", NULL,
-		    MTX_DEF);
-		mtx_init(&ng_idhash_mtx, "netgraph idhash mutex", NULL,
-		    MTX_DEF);
-		mtx_init(&ngq_mtx, "netgraph free item list mutex", NULL,
-		    MTX_DEF);
-		s = splimp();
-		/* XXX could use NETISR_MPSAFE but need to verify code */
-		netisr_register(NETISR_NETGRAPH, (netisr_t *)ngintr, NULL, 0);
-		splx(s);
+		error = 0;
 		break;
 	case MOD_UNLOAD:
 		/* You cant unload it because an interface may be using it.  */
_at__at_ -2986,12 +3000,9 _at__at_
 	return (error);
 }
 
-static moduledata_t netgraph_mod = {
-	"netgraph",
-	ngb_mod_event,
-	(NULL)
-};
-DECLARE_MODULE(netgraph, netgraph_mod, SI_SUB_DRIVERS, SI_ORDER_MIDDLE);
+SYSINIT(netgraph, SI_SUB_DRIVERS, SI_ORDER_FIRST, ngb_sysinit, NULL);
+SYSUNINIT(netgraph, SI_SUB_DRIVERS, SI_ORDER_ANY, ngb_sysuninit, NULL);
+DEV_MODULE(netgraph, ngb_mod_event, NULL);
 SYSCTL_NODE(_net, OID_AUTO, graph, CTLFLAG_RW, 0, "netgraph Family");
 SYSCTL_INT(_net_graph, OID_AUTO, abi_version, CTLFLAG_RD, 0, NG_ABI_VERSION,"");
 SYSCTL_INT(_net_graph, OID_AUTO, msg_version, CTLFLAG_RD, 0, NG_VERSION, "");