Re: [PATCH] devfs v196 available

[Roman Zippel]

Hi,

Richard Gooch wrote:

> That's something that I've already said I'm planning, but that's a 2.5
> thing, and I'm waiting for the dcache to be split from the icache.
> Even if the current, horribly bloated, struct inode is trimmed down by
> removing that union, it will still be too big for my liking. Hence I
> want the dcache/icache split.

Why not do it the other way around? Such a change would require changes
to all filesystems. If devfs would already make better use of existing
infrastructure, it would be far easier to take the needs of devfs into
account, instead of trying to fit it in later. A nice side effect would
be people could help with the conversion.

> > - you should do something about the recursive calls, it's an
> >   invitation to abuse them.
> 
> That's why I've tagged them as such. I've tried to keep their stack
> usage low. Switching to a non-recursive algorithm has it's own
> problems: it's a lot harder to understand, and it's much harder to get
> right in the first place. Non-recursive algorithms are more fragile.

_devfs_walk_path: There is already nicely working code in fs/namei.c.
unregister: you can easily go back with the parent pointer.
find_by_dev: this is just wrong (see below)

> > - symlink/slave handling of tapes/disk/cdroms is maybe better done
> >   in userspace.
> 
> No, because you want to be able to mount your root FS using
> "root=/dev/cdroms/cdrom0", for example.

That's about the only use of it at boot time and can be achieved easier.

> Again, you're talking about 2.5 stuff. If and when we get a decent
> block device structure, I'll look at using that.

The correct interface in 2.4 is get_blkfops. devfs has to use it like
everyone else.

> It's questionable
> whether we'll ever get a generic char device structure, since they're
> all so different. So for char devices, the ops pointer should be kept
> inside the devfs entry.

No, find_by_dev is just wrong, scanning a whole tree for a specific
information is broken. Timing is absolutely unpredictable and prone to
abuse. The current implementation can even return wrong information.
The correct place to store char device info is fs/char_dev.c.

> As for devfs_get_ops(), that has to stay until you can change over all
> the SGI IA64 drivers, which (ab)use it heavily. API change-> 2.5.

If it depends on devfs, it's simply broken and must be fixed, that has
nothing to do with the API.

> Yeah, on more than one occasion I've stated that I'd love to rip out
> all the tree management code in devfs. I'm waiting for VFS changes to
> make that palatable.

Do the changes now and you can push the responsibility to VFS, but the
last you should do is to maintain bloat.

bye, Roman