Date: Wed, 05 Jan 2005 12:35:46 -0800 From: Maksim Yevmenkin <maksim.yevmenkin@savvis.net> To: Julian Elischer <julian@elischer.org>, Roman Kurakin <rik@cronyx.ru>, Scott Long <scottl@freebsd.org> Cc: current@freebsd.org Subject: Re: netgraph(4) initialization order Message-ID: <41DC4FA2.8070609@savvis.net> In-Reply-To: <41DB1839.9080104@elischer.org> References: <41DB08B9.6090801@savvis.net> <41DB1310.4060807@cronyx.ru> <41DB1700.7060708@savvis.net> <41DB1839.9080104@elischer.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------070007020502050005000302
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
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
--------------070007020502050005000302
Content-Type: text/plain;
name="ng_base.c.diff.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="ng_base.c.diff.txt"
--- ng_base.c.orig Wed Jan 5 12:04:36 2005
+++ ng_base.c Wed Jan 5 12:23:39 2005
@@ -46,6 +46,7 @@
#include <sys/param.h>
#include <sys/systm.h>
+#include <sys/conf.h>
#include <sys/ctype.h>
#include <sys/errno.h>
#include <sys/kdb.h>
@@ -2953,27 +2954,40 @@
* 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. */
@@ -2986,12 +3000,9 @@
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, "");
--------------070007020502050005000302--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41DC4FA2.8070609>