Re: Dynamic sysctls - patches for review
On Sun, 26 Mar 2000, Andrzej Bialecki wrote: > On Sun, 26 Mar 2000, Doug Rabson wrote: > > > On Sun, 26 Mar 2000, Andrzej Bialecki wrote: > > > > 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. > > Could you explain it further? Do you mean something like a transaction > log? But this wouldn't work - the operations on the tree can be > interdependent between users. I just mean creating an object to hold pointers to all the sysctl nodes and leaves which were created (or which had their refcounts incremented). To back out the module's edits, it would just run through the list and destroy each node/leaf. The destroy procedure would decrement the refcount and use that to decide when to free the memory. -- 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
Re: Dynamic sysctls - patches for review
On Sun, 26 Mar 2000, Doug Rabson wrote: > On Sun, 26 Mar 2000, Andrzej Bialecki wrote: > > 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. Could you explain it further? Do you mean something like a transaction log? But this wouldn't work - the operations on the tree can be interdependent between users. Andrzej Bialecki // <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: Dynamic sysctls - patches for review
I think that if the sysctl data was reorganized, so that the per module or instance data was at the leaves of the tree, you could avoid the problem entirely. This is the general approach used on MIB definitions used for SNMP; each variable is an instance (usually the 0th) at the leaf. You don't get the opportunity to clean them all up at once by deleting a whole subtree, but you don't get the hair, either. louie To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: Dynamic sysctls - patches for review
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
Re: Dynamic sysctls - patches for review
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...
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.
Any comments on that?
Andrzej Bialecki
// <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: Dynamic sysctls - patches for review
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.
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
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.
--
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
Re: Dynamic sysctls - patches for review
On Fri, 24 Mar 2000, Andrzej Bialecki wrote: > I'd appreciate some feedback. Thanks! > Note this is not an actual peer review (yet), but... Good job! This is another big step in the right direction, and the code looks good to me :) The only problems I can see right now are just all style bugs (^_^) > Andrzej Bialecki > > // <[EMAIL PROTECTED]> WebGiro AB, Sweden (http://www.webgiro.com) > // --- > // -- FreeBSD: The Power to Serve. http://www.freebsd.org > // --- Small & Embedded FreeBSD: http://www.freebsd.org/~picobsd/ -- Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! / [EMAIL PROTECTED]`--' To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Dynamic sysctls - patches for review
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! Andrzej Bialecki // <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
