Date: Wed, 05 Jan 2005 13:52:40 -0700 From: Scott Long <scottl@freebsd.org> To: Maksim Yevmenkin <maksim.yevmenkin@savvis.net> Cc: Roman Kurakin <rik@cronyx.ru> Subject: Re: netgraph(4) initialization order Message-ID: <41DC5398.8020508@freebsd.org> In-Reply-To: <41DC4FA2.8070609@savvis.net> References: <41DB08B9.6090801@savvis.net> <41DB1310.4060807@cronyx.ru> <41DB1700.7060708@savvis.net> <41DB1839.9080104@elischer.org> <41DC4FA2.8070609@savvis.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Maksim Yevmenkin wrote:
> Dear Hackers,
>
> any objections to the attached patch?
>
> thanks,
> max
>
Yes, as I stated in another email, I think that the core netgraph
module should be initialized before the SI_SUB_DRIVERS step. I
propose creating a new sysinit called SI_SUB_NETGRAPH with a value
of 0x30100000. That way it comes after SI_SUB_IF and before
SI_SUB_DRIVERS. This make fiddling with SI_ORDER_* unneccesary.
Scott
>
>>>>> 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
> @@ -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, "");
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41DC5398.8020508>