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: [EMAIL PROTECTED]
Nonlinear Systems Ltd. Phone: +44 181 442 9037
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
