>On Tue, Dec 31, 2002 at 06:24:22PM -0800, Adam J. Richter wrote:
>> The following patch replaces devfs with a ramfs-derived
>> implementation which is under one quarter the size although it
>> eliminates certain functionality.
>>
>> wc -l *.c Makefile size (text + data + bss)
>>
>> devfs 3614 lines 25,863 bytes
>> mini-devfs 629 lines 5,367 bytes
>> Reduction 5.7x 4.8x
>Wow, that looks really cool! I just wonder where viro is hiding the last
>weeks, he promised more devfs API cleanups that likely clash with
>your changes.
I don't know what devfs API changes he wants to make, but I
imagine that I can probably follow them.
The one devfs quirk that I really like that one might be
tempted to eliminate is that you can pass a file_operations pointer to
devfs_register, which enables the possibility in future of being able
to build systems that just use devfs names for identifying devices.
>> 3. devfs_handle_t is now a synonym for struct dentry*.
>I wonder whether some code uses struct devfs_entry * directly, at least
>I was tempted to do so in the scsi midlayer.
Thankfully, struct devfs_entry* is an opaque pointer. The
struct is only defined in fs/devfs/base.c. Searching with
"find . -name '*.[ch]' | xargs grep -w devfs_entry" indicates
that everyone declares devfs_handle_t instead of "struct devfs_entry*",
so that's not a problem either.
Your question prompted me to do a little bit of research.
I believe the list of routines that my reduced devfs does not
implement is as follows:
devfs_get_handle
devfs_get_handle_from_inode
devfs_set_file_size
devfs_get_info
devfs_set_info
devfs_get_parent
devfs_get_first_child
devfs_get_next_sibling
devfs_get_name
devfs_register_tape
devfs_unregister_tape
devfs_alloc_major
devfs_dealloc_major
devfs_alloc_devnum
devfs_dealloc_devnum
Storing this list in /tmp/names and grepping for these
identifiers shows only a small number of hits:
% find . -name '*.[ch]' | xargs fgrep -l -w -f /tmp/names
./arch/i386/kernel/cpu/mtrr/if.c
./arch/i386/kernel/microcode.c
./arch/ia64/sn/io/sn1/hubcounters.c
./arch/ia64/sn/io/hcl.c
./arch/ia64/sn/io/labelcl.c
./arch/ia64/sn/kernel/sn1/synergy.c
./arch/x86_64/kernel/mtrr.c
./drivers/ide/ide-tape.c
./drivers/media/radio/miropcm20-rds.c
./drivers/scsi/osst.c
./drivers/scsi/st.c
./fs/devfs/base.c
./fs/devfs/util.c
./fs/mini-devfs/inode.c
./include/linux/devfs_fs_kernel.h
Also, drivers/media/dvb/dvb-core/dvbdev.c has an option
to use DEVFS_F_AUTO_DEVNUM, which my devfs code does not yet support,
but, still it looks like the compatability issues may be pretty small.
>> 4. A lot of the devfs routines are unimplemented. I haven't
>> noticed much code that uses them, and I'm not sure that any code
>> really should. I think arch/ia64/sn uses devfs_get_first_child,
>> devfs_get_next_sibling. I need to understand what if any of the
>> other routines are really necessary and why (for example, why can't
>> we use struct dentry). My computer seems to run fine without them.
>The hcl code in arch/ia64/sn/ is supposed to get replaced by a filesystem
>on it's own once the sn port is properly updated for 2.5/2.6.
Great.
>> First of all, I'd like to debug this code and I'd welcome any
>> help.
>Is it supposed to work out of the box on previously (and for 2.4 use)
>non-devfs systems? I still don't plan to use devfs, but such an effort
>is really worth some debugging help..
Thanks for the encouragement.
>> I think I'd like to change fs/super.c slightly to make it
>> easier to statically allocate the struct super_block for filesystems
>> that can have only one instance even if they are mounted in multiple
>> locations (devfs, procfs, sysfs, usbdevfs, etc.).
>Why do you want to allocate it statically?
A few fields could be initialized statically. A few bytes
would be saved from memory allocation overhead. Cache locality would
improve infinitesemally. If all one-instance filesystems are changed
to do this, it will eliminate one memory allocation failure branch in
fs/super.c. Perhaps the same could be done with the root inode. I
know this is pretty marginal and might end up adding more complexity
than it would save. It's at the bottom of my TODO (or "to try") list.
>> @@ -24,7 +24,11 @@
>> #define DEVFS_SPECIAL_CHR 0
>> #define DEVFS_SPECIAL_BLK 1
>>
>> +#ifdef CONFIG_DEVFS_SMALL
>> +typedef struct dentry * devfs_handle_t;
>> +#else
>> typedef struct devfs_entry * devfs_handle_t;
>> +#endif
>Do you really need to keep both around?
I am starting by posting this as an addition in case it turns
out that there is a camp that needs the existing devfs. If that does
not appear to be the case, then I'd be happy to make a patch to remove
the current devfs.
>> +/* On success, returns with (*parent_inode)->i_sem taken. */
>> +static int devfs_decode(devfs_handle_t dir, const char *name, int is_dir,
>> + struct inode **parent_inode, struct dentry **dentry)
>Do we really have to support this?
It's a static routine to support the existing devfs interface.
Later, the devfs interface can be simplified to eliminate the need
for this routine and perhaps walk_parents_mkdir.
>> +++ linux/fs/devfs2/numspace.c 2002-12-25 17:44:14.000000000 -0800
[...]
>> +int devfs_alloc_unique_number (struct unique_numspace *space)
>Remove the devfs_ prefix here (it's not devfs-specific at all) and
>convert to sane indentation?
I'm trying to first land a patch that involves no changes to
users of devfs. After that, I expect this code to be completely
replaced soon anyhow as a result of efforts independent of devfs
(probably by an enhanced number allocation facility for
device_interface in drivers/base/). Currently
devfs_alloc_unique_number is used only to allocate disk numbers in
/dev/discs/discNNN/. If my shruken devfs gets integrated and
devfs_alloc_unique_number is still in use, I'd be happy to make the
change you suggest. I'd just rather do integration as a separate
phase from API changes to keep the patches somewhat comprehensible.
Anyhow, thanks for all of the feedback.
Adam J. Richter __ ______________ 575 Oroville Road
adam@yggdrasil.com \ / Milpitas, California 95035
+1 408 309-6081 | g g d r a s i l United States of America
"Free Software For The Rest Of Us."
jobs
-
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 : Tue Jan 07 2003 - 22:00:15 EST