Re: Dynamic sysctls - patches for review

2000-03-27 Thread Doug Rabson

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

2000-03-26 Thread Doug Rabson

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

2000-03-26 Thread Andrzej Bialecki

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

2000-03-26 Thread Doug Rabson

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

2000-03-26 Thread Louis A. Mamakos


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

2000-03-26 Thread Andrzej Bialecki

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



Dynamic sysctls - patches for review

2000-03-24 Thread Andrzej Bialecki

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



Re: Dynamic sysctls - patches for review

2000-03-24 Thread Brian Fundakowski Feldman

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