Subject: Re: kernfs bug fixes, PR 24786
To: Christian Limpach <chris@pin.lu>
From: Bill Studenmund <wrstuden@netbsd.org>
List: tech-kern
Date: 04/27/2004 13:48:22
--GID0FwUMdk1T2AWN
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Apr 27, 2004 at 05:07:30PM +0200, Christian Limpach wrote:
> Hello,
>=20
> I'd like to add some additional nodes in /kern/xen for the Xen port.
> While doing this, I've fixed PR 24786 and discovered and then fixed
> the problem that stat and readdir return different inode numbers.

[snip]

> inode mismatch between stat and readdir:
> This is fixed by patch kernfs_inodes.patch
> Before patch logfile: kernfs_inodes.txt
> After patch logfile: kernfs_inodes-fixed.txt
>=20
> The cause of the problem is that the mapping from entry to fileno is
> done in kernfs_subr.c:kernfs_allocvp and in kernfs_vnops.c:kernfs_readdir
> and the mapping doesn't always give the same result.  I've changed
> kernfs_readdir to use kernfs_allocvp to do the mapping.  This needed all
> the kernfs_nodes to have a valid kfs_kt member to avoid a deadlock (see
> check in kernfs_setdirentfileno).  It also required a change to the
> KERNFS_FILENO mapping since it returned random results for kern_targets
> not from the kern_targets array.
> I've also changed the mapping such that only entries from the kern_targets
> array have inodes with all bits higher than 6 clear.  I know this change
> is not required but needed later when dynamically adding additional
> entries to the root directory.

I have a few concerns about this change. First off, though, thank you for=
=20
working on this. Even with my concerns, things are better with your=20
changes than without.

First off, I don't think we should be looking at a device's fileno to set=
=20
the kernfs's fileno. fileno's are per-mount point, so there's no reason to=
=20
have them line up. Also, by pulling them from a different mount point, we=
=20
open up the possibility that we'll end up with a duplicate. Just make them=
=20
14 and 15, and we'll be fine.

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

Part of where my thoughts are going is I'd like to make a sysctl directory=
=20
off of kernfs root, and make it so that the whole sysctl hierarchy is=20
under there. That'd mean a hierarchy of directories, for instance. While I=
=20
haven't started that, I'm trying to think of things that will make it all=
=20
the easier.

My other concern is that we create vnodes while we list directories. I'm=20
not sure what else we can do, as we need to store the fileno somewhere,=20
and we are starting with otherwise static struct kern_target's. Also, we=20
aren't listing every entry in the kern_target's for the ipsec directories.

Also, some day I'd love to make fhtovp work with kernfs. :-)

Take care,

Bill

--GID0FwUMdk1T2AWN
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (NetBSD)

iD8DBQFAjscWWz+3JHUci9cRAt72AJ9t2X4v3vXpaZZBW6Lc1kG+gqL/egCdGlkF
Ii/NX12cGSlB2VEDMVmpSpQ=
=az3b
-----END PGP SIGNATURE-----

--GID0FwUMdk1T2AWN--