Date: Sun, 2 Jul 2000 20:34:28 -0400 (EDT) From: Brian Fundakowski Feldman <green@FreeBSD.org> To: Andrzej Bialecki <abial@webgiro.com> Cc: dfr@freebsd.org, jlemon@freebsd.org, freebsd-arch@freebsd.org Subject: Re: UPDATE: Re: Dynamic sysctls, next round (please review) Message-ID: <Pine.BSF.4.21.0007022000500.38342-100000@green.dyndns.org> In-Reply-To: <Pine.BSF.4.20.0007030059510.55501-100000@mx.webgiro.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 3 Jul 2000, Andrzej Bialecki wrote: > On Sun, 2 Jul 2000, Brian Fundakowski Feldman wrote: > > > On Sun, 2 Jul 2000, Andrzej Bialecki wrote: > > > * module B is left with subtree hanging in void: > > > -static_root > > > NOTHING! > > > -node2(1) > > > -leaf2(1) > > > > > > I don't have any good solution for it right now, except to warn users that > > > they should always hang their subtrees off of static nodes. :-( > > > > There's already a good solution for this: you must always MODULE_DEPEND() > > upon the owner of a node which you hang things from :) Now, we just need > > to make sure each node is created by a module which you can MODULE_DEPEND() > > on, and that's not too hard! > > I don't think this is relevant here. See the example (which is BTW a > module, but doesn't need to be) provided with the patches. The granularity > of dependancy is *per context*, not per module - in fact, the code that > creates dynamic sysctls doesn't have to be a module, and it can create > multiple contexts. I'm not understanding something here. Why would you want to add nodes to a dynamically created node when you don't know who created the node? If you don't know who created the node, you don't know whether it's appropriate to add your oids to it; if you do know who created it, you should be doing a MODULE_DEPEND() on them anyway. If you're the one creating that node, then you should be depended on, so why would you want two modules which both create the same node? I really can't see why you'd realistically want to do that and therefore need the context. > The latest patches provide a working solution to this - perhaps not > beautiful, but still something... Overall, it's quite good :) but I still have some issues with it. Other than indentation style, which I'm quite willing to fix for you if you'd like, there are some strange things. Why M_SYSCTL1? If anything, I'd name it M_SYSCTLOID. What's the point of all of the KASSERT()s against NULL? The system will crash in either case. Why was const removed from const char *oid_name? You still don't want anything modifying the contents of *oid_name. int ref should really be unsigned int oid_refcnt. Why do you want SYSCTL_CTX_* macros exactly? The SYSCTL_CTX_CREATE() one is broken, but I don't see the need for it anyway, and it encourages the MALLOC()-like lvalue-in-parameter list idiom which only leads to confusion. As far as the algorithms themselves go and the code, it looks good to me :) I would not be the one to give the final review for sysctl-related things, it does look alright to me. I'd like to have the functionality before 5.0. > BTW. I could limit support for dyn_sysctls to disjoint subtrees > exclusively - but I think the ability to have partially overlapping trees > is attractive enough to justify small overhead of traversing the tailq's > several times... I wouldn't worry about performance issues that are only applicable to the actual addition and deletion of nodes, and not the lookup :) > Andrzej Bialecki > > // <abial@webgiro.com> WebGiro AB, Sweden (http://www.webgiro.com) > // ------------------------------------------------------------------- > // ------ FreeBSD: The Power to Serve. http://www.freebsd.org -------- > // --- Small & Embedded FreeBSD: http://www.freebsd.org/~picobsd/ ---- > > > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-arch" in the body of the message > -- Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! / green@FreeBSD.org `------------------------------' To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0007022000500.38342-100000>