Date: Sun, 26 Mar 2000 14:42:39 +0100 (BST) From: Doug Rabson <dfr@nlsystems.com> To: Andrzej Bialecki <abial@webgiro.com> Cc: freebsd-current@freebsd.org Subject: Re: Dynamic sysctls - patches for review Message-ID: <Pine.BSF.4.21.0003261437300.89245-100000@salmon.nlsystems.com> In-Reply-To: <Pine.BSF.4.20.0003261440170.41971-100000@mx.webgiro.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 26 Mar 2000, Andrzej Bialecki wrote: > On Sun, 26 Mar 2000, Doug Rabson wrote: > > > On Fri, 24 Mar 2000, Andrzej Bialecki wrote: > > > > > Hi, > > > > > > Inspired by PR kern/16928 I implemented completely dynamic > > > creation/deletion of sysctl trees at runtime. The patches (relative to > > > -current) can be found at: > > > > > > http://www.freebsd.org/~abial/dyn_sysctl.tgz > > > > > > Included is an example of KLD that creates some subtrees when loaded, and > > > deletes them before unloading. > > > > > > I'd appreciate some feedback. Thanks! > > > > This stuff looks very useful. I have done this kind of thing 'by hand' in > > the past but this should make life quite a bit easier. I think the only > > thing in the patch which I would want to change is to rename > > sysctl_deltree() to sysctl_delete_tree() to be more consistent with the > > naming of other functions. > > No problem with me. > > > How much has this been tested? I wonder if the code in > > sysctl_deltree() which iterates over the children is correct. Surely the > > SLIST_REMOVE called by the child will screw up the SLIST_FOREACH iterator > > Hmmm. Strange - it should be, since it dereferences just freed > pointer... but it worked for me. (8-* > > > of the parent. In this kind of situation, I often write things > > differently: > > > > while ((p = SLIST_FIRST(SYSCTL_CHILDREN(oidp)) != NULL) { > > sysctl_deltree(p); > > } > > > > This will make sure that the parent does not access memory after it has > > been freed. > > Yes, it looks much better to do that. Well, I tested the code creating a > couple of subtrees, either from root or from one of existing > categories. The code "worked for me", but it's not a proof that it does > the correct thing, obviously... This is because the kernel malloc doesn't destroy the contents of the freed memory block (it does change the first few bytes though). I guess the oid_link field survived but this should not be relied on. > > Also, Jonathan Lemon suggested that the dynamic oids should have a > reference number, so that multiple modules could create partially > overlapping trees, like: > > kern.one.two.module1 > kern.one.two.module2 > kern.one.three.module3 > > The problem with that approach, however, is that you no longer can delete > a tree - you would need to have a way to delete a path in the tree. When I > started adding the reference count, I faced a problem when module1 deleted > not only kern.one.two.module1, but kern.one.two.module2 as well, because > it kept a reference to the root of custom tree (one....), and then called > sysctl_deltree, which of course decremented refcnt in one.two, but deleted > both module1 and module2, as they both had a refcnt=1 :-( This left a > dangling kern.one.two node without any children, and with refcnt=1 (that > of module2). > > Another problem is when a module3 just checks for existence of kern.one, > and if it exists, it will not try to create it (thus incrementing refcnt), > but proceed to creating *.three.module3. The refcnt in kern.one will not > be incremented, and when the other two modules will start deleting the > tree, the kern.one will be removed, although the module3 still uses it. > > Well, somehow the idea of overlapping subtrees sounds nice and useful > IMHO. Any suggestions how to solve these issues? > > One possible way to do it would be to keep some ID of the oid's > creator. Then, for nodes they would be deleted when the refcnt goes to 0 > (no matter who created them), but for terminals the deletion would succeed > only for the owners. Also, if you create a subtree not from the root of > dynamic tree, the refcnt++ would propagate up the tree as well, and > similarly refcnt-- would propagate on deletion. This is a reasonable solution. Another would be for dynamic sysctl users to use a 'context' object to record all their edits to the tree which would allow the edits to be backed out without relying on a tree-delete operation. -- Doug Rabson Mail: dfr@nlsystems.com Nonlinear Systems Ltd. Phone: +44 181 442 9037 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" 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.0003261437300.89245-100000>