From owner-freebsd-current Sun Mar 26 5:35:58 2000 Delivered-To: freebsd-current@freebsd.org Received: from anchor-post-33.mail.demon.net (anchor-post-33.mail.demon.net [194.217.242.91]) by hub.freebsd.org (Postfix) with ESMTP id D3B1137B762 for ; Sun, 26 Mar 2000 05:35:52 -0800 (PST) (envelope-from dfr@nlsystems.com) Received: from nlsys.demon.co.uk ([158.152.125.33] helo=herring.nlsystems.com) by anchor-post-33.mail.demon.net with esmtp (Exim 2.12 #1) id 12ZDCp-0007ao-0X; Sun, 26 Mar 2000 14:35:51 +0100 Received: from salmon.nlsystems.com (salmon.nlsystems.com [10.0.0.3]) by herring.nlsystems.com (8.9.3/8.8.8) with ESMTP id OAA99296; Sun, 26 Mar 2000 14:38:47 +0100 (BST) (envelope-from dfr@nlsystems.com) Date: Sun, 26 Mar 2000 14:42:39 +0100 (BST) From: Doug Rabson To: Andrzej Bialecki Cc: freebsd-current@freebsd.org Subject: Re: Dynamic sysctls - patches for review In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG 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