Re: CVS commit: src/sys/miscfs/kernfs

[christos%zoulas.com@localhost (Christos Zoulas)]

On Jun 23,  8:16pm, bouyer%antioche.eu.org@localhost (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;

| 
| > 2. kernfs_try_fileop() is being passed an errno of 0 in the read or write
| >    case, so if the fileop is not found the operation will silently succeed.
| >    I don't think that this is how the read case was handled before. I think
| >    that they should all return EOPNOTSUPP or something in the case the
| >    fileop is not found.
| 
| 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!

| > So all the ports are broken except of xen. Either back it out again,
| > or fix it ASAP.
| 
| 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.

christos