Hi,
On Sat, 3 Nov 2001, Richard Gooch wrote:
> Hi, all. Version 196 of my devfs patch is now available from:
Some small comments:
- You should consider to integrate devfs into the dcache (check e.g.
ramfs), currently you duplicate lots of infrastructure, which you get
for free (and faster) from the dcache. It would save you also lots of
locking.
- you should delay the path generation in try_modload to the read
function, then you're not limited here in the pathname length and don't
have to abuse the stack.
- you should use "%.*s" if you want to print a dentry name (no need to
copy it).
- you should do something about the recursive calls, it's an invitation to
abuse them.
- symlink/slave handling of tapes/disk/cdroms is maybe better done in
userspace.
- uid/gid in devfsd_buf_entry is 16 bit
- devfs is not a database! E.g. devfs has no business to store the
char/block device ops table. So devfs_get_ops is wrong here, the same
for devfs_[gs]et_info. devfs has to stay optional and storing/retrieving
certain device data should not be done in different ways, this is only
asking for trouble because of subtle differences. The problem so far is
that we have no [bc]dev_t, where this info should be stored, but this
hopefully changes early 2.5. So the path to get e.g. to the ops table
should be "kdev_t -> [bc]dev_t -> ops" and not "kdev_t -> search whole
devfs tree -> no ops" (because you missed manual dev nodes).
- the simple event mechanism looks prone to DOS attacks (even if all races
are gone). Events are too easily delayed or even dropped. This makes
the events unreliable and unsuitable for any serious use.
- in _devfs_make_parent_for_leaf you shouldn't simply return if
_devfs_append_entry fails, because someone else might have created the
directory since _devfs_descend.
Especially the first point is very important, this was even suggested by
Linus already more than 3 years ago. devfs could be a very thin layer, but
right now it's far bigger than it had to be.
bye, Roman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Nov 23 2001 - 21:00:14 EST