Subject: Re: CVS commit: src/sys/miscfs/kernfs
To: Christos Zoulas <christos@zoulas.com>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: source-changes
Date: 06/23/2006 21:07:37
On Fri, Jun 23, 2006 at 02:54:12PM -0400, Christos Zoulas wrote:
> On Jun 23,  8:16pm, bouyer@antioche.eu.org (Manuel Bouyer) wrote:
> -- Subject: Re: CVS commit: src/sys/miscfs/kernfs
> 
> | > 1. If you don't kernfs_alloctype (which obviously xen does so it works)
> | >    the splay tree will be empty so the read method will not be found.
> | 
> | Yes, that's what I found. All the ops using kernfs_try_fileop() are broken.
> | I can confirm that write are broken too, and the write code has been there
> | for years.
> 
> I also don't understand the assignment in the first loop:
> 
> 	dkf[i].kf_type = nextfreetype;

This is kernfs_alloctype(). Each time we allocate a new kernfs type,
a new fileop structure is added to the splay tree for this type.

> | 
> | Agreed for read(). write in this state for years, so I don't know if
> | write silently failing was intended or not.
> 
> I think it is an accident. In fact, I think that the whole splay tree
> code is overkill; we have ~ 10 fileops and for that a sorted array is
> good enough. Or even a non-sorted one!

One array per (user-defined) type, so it's a dual-dimenention array.
It would be enouth, the issue is that it's dynamically created when
user-defined types are registered.


> | This code has been there for more than 3 months. Either kernfs is not
> | used much outside of Xen and leaving it in this state a few more days while
> | a real fix is found is not a big deal, it broke recently and my code is 
> | not to blame. 
> 
> Well, most of the nodes are not writable, and you broke the read path.
> 
> | Anyway I'm looking at it.
> 
> I am fine with that. I don't agree though that you put back the broken
> code till you found a fix. Unless you believe that the # of Xen users
> is the majority.

All Xen users are affected by the backout, and i386 users too because this
break release builds for the i386 port. Non-Xen users probably don't care
much as this has been brocken for more than 3 months and nobody noticed
until today.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--