From owner-freebsd-current@FreeBSD.ORG Wed Jan 5 20:52:52 2005 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3567F16A4CE for ; Wed, 5 Jan 2005 20:52:52 +0000 (GMT) Received: from pooker.samsco.org (pooker.samsco.org [168.103.85.57]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7F4F343D1D for ; Wed, 5 Jan 2005 20:52:51 +0000 (GMT) (envelope-from scottl@freebsd.org) Received: from [192.168.254.12] (g4.samsco.home [192.168.254.12]) (authenticated bits=0) by pooker.samsco.org (8.12.11/8.12.10) with ESMTP id j05KtcGj018775; Wed, 5 Jan 2005 13:55:39 -0700 (MST) (envelope-from scottl@freebsd.org) Message-ID: <41DC5398.8020508@freebsd.org> Date: Wed, 05 Jan 2005 13:52:40 -0700 From: Scott Long User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040514 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Maksim Yevmenkin References: <41DB08B9.6090801@savvis.net> <41DB1310.4060807@cronyx.ru> <41DB1700.7060708@savvis.net> <41DB1839.9080104@elischer.org> <41DC4FA2.8070609@savvis.net> In-Reply-To: <41DC4FA2.8070609@savvis.net> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, hits=0.0 required=3.8 tests=none autolearn=no version=2.63 X-Spam-Checker-Version: SpamAssassin 2.63 (2004-01-11) on pooker.samsco.org cc: current@freebsd.org cc: Julian Elischer cc: Roman Kurakin Subject: Re: netgraph(4) initialization order X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Jan 2005 20:52:52 -0000 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 > #include > +#include > #include > #include > #include > @@ -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"