Subject: Re: kernfs bug fixes, PR 24786
To: Bill Studenmund <wrstuden@netbsd.org>
From: Christian Limpach <chris@pin.lu>
List: tech-kern
Date: 04/28/2004 01:09:51
Hi,

On Tue, Apr 27, 2004 at 01:48:22PM -0700, Bill Studenmund wrote:
> First off, I don't think we should be looking at a device's fileno to set 
> the kernfs's fileno. fileno's are per-mount point, so there's no reason to 
> have them line up. Also, by pulling them from a different mount point, we 
> open up the possibility that we'll end up with a duplicate. Just make them 
> 14 and 15, and we'll be fine.

Yes, I noticed that and also wondered about the possibility/likelihood of
a conflict.  I agree that not special handling those entries is better but
I'd prefer to only make that change after the current set of changes.

> Also, how we come up with fileno's for things outside of the root 
> directory seems very weird to me. We end up using the lower 6 bits, the 
> "Type", to differentiate between different classes of nodes. Then the 
> upper 26 bits indicate which item we're looking at. I realize it was that 
> way before you got to it, but that's just, uhm, strange. I'd have put the 
> type in the upper 6 and cookie in the lower 26. Not sure if I think that 
> should change now...

The current numbering system has the only "feature" that root directory
entries get really low numbers and all other entries also get reasonably
low numbers.  The only uses of this "feature" I can think of, are really
geeky ;-)
 
> Part of where my thoughts are going is I'd like to make a sysctl directory 
> off of kernfs root, and make it so that the whole sysctl hierarchy is 
> under there. That'd mean a hierarchy of directories, for instance. While I 
> haven't started that, I'm trying to think of things that will make it all 
> the easier.

I think the extensions I posted seperately don't quite allow this but
there's not a lot missing:  you'd add hooks for read and/or xread and
create a sysctl-node type which returns the data from the node and you'd
add hooks for lookup and readdir (there's already a hook for getattr)
and implement those functions for the sysctl-dir type.  Maybe a more
generic solution with a KFSdynsubdir type which has a callback function
would be better since the ipsec dirs could then also be changed to use
it.  The only lose point is probably possible races when nodes get
added/removed, but those also exist for the ipsec dirs...

> My other concern is that we create vnodes while we list directories. I'm 
> not sure what else we can do, as we need to store the fileno somewhere, 
> and we are starting with otherwise static struct kern_target's.

Yes, it must be possible to do the mapping differently per directory
kfstype.  For KFSkern we can still use the index in kern_targets and
store the fileno in the dynamic entrie's dyn_kern_target, same for
KFSsubdir type directories.  And for directory types where the
directory contents are dynamic (ipsec, sysctl) you'd have to have a
hook which would return a fileno.

Anyway, once/if we have an alternative to store/generate the fileno,
we'll only have to change kernfs_setdirentfileno_kt to use it instead
of calling allocvp and also make allocvp use it.

> Also, we 
> aren't listing every entry in the kern_target's for the ipsec directories.

We're not?  I noticed now one check i >= 2 + n which is/was wrong in the
readdir/KFSipsecspdir case but other than that?

Thank you for your comments.

    christian