Re: VFS not completely factored

From: Daniel Phillips (phillips@bonn-fries.net)
Date: Tue Feb 15 2000 - 16:23:00 EST


On Tue, 15 Feb 2000, James Manning wrote:
> [ Monday, February 14, 2000 ] breed@almaden.ibm.com wrote:
> > On a related issue, if there was a char generic_data[] member in the inode
> > union that had a defined size (probably the size of the largest structure),
> > people (like me) writing filesystems that aren't ready to be put into the
> > sources yet could put data right in the union and know that it will fit
> > without having to resort to using generic_ip and allocating an info block
> > to store inode specific info. (Narly sentence huh?)
>
> That would seem to place a restriction on the size of your inode data,
> something generic_ip frees you from (at not a high cost imho). I'd
> go ahead and advocate everyone having to go through the indirection
> and rip out the entire union for a void *inode_info first (helping a
> good bit towards freeing vfs from specific fs's). Then do the same
> for the super_block struct and you seem to be well on your way to the
> factored vfs, as you'll be rid of fs-specific includes in linux/fs.h
> (attached). Whether the indirection and sacrificed cache line is worth
> it is the call of another.

The problem comes in the first place from trying to avoid the extra
de-reference to get to the filesystem-specific data. This was successfully
done, but at great expense in increased source code complexity and also
by sacrificing the idea that the vfs should be properly separated from the
filesystems it supports. I'd like to suggest a relatively small change
that would still save the extra dereference, but without tangling the fs
headers in knots.

What I was actually thinking of is adding a couple of extra integer fields
onto the end of the file_system_type struct, giving the size of the
filesystem-specific data required for the superblocks and inodes:

        struct file_system_type {
                const char *name;
                int fs_flags;
                struct super_block *(*read_super) (struct super_block *, void *, int);
                struct file_system_type * next;
+ int fs_specific_size_super;
+ int fs_specific_size_inode;
        };

For ext2, the file_system_type initializor would be:

        static struct file_system_type ext2_fs_type = {
                "ext2",
                FS_REQUIRES_DEV /* | FS_IBASKET */, /* ibaskets have unresolved bugs */
                ext2_read_super,
                NULL,
                sizeof(ext2_sb_info),
                sizeof(ext2_inode_info)
        };

Macro symbols would be substituted for the unions in fs.h:

        struct super_block {
                struct list_head s_list;
                kdev_t s_dev;
                ...
                struct list_head s_dirty; /* dirty inodes */
                struct semaphore s_vfs_rename_sem; /* Kludge */
                FS_SPECIFIC_SUPERBLOCK_INFO
        };

To compile ext2 fs you use the following defines before including fs.h:

        #define FS_SPECIFIC_SUPER_INFO union {struct ext2_sb_info ext2_sb;} u;
        #define FS_SPECIFIC_INODE_INFO union {struct ext2_inode_info ext2_i;} u;

All the changes required are trivial, both in the vfs code and in the specific
filesystems. In fact, it would be easy to avoid changing the specific
filesystems at all if that were desired, though I don't see why that would be
good.

This strategy would allow all the file systems to compile properly, with only
trivial changes, and it would allow the file system headers to be untangled a
great deal. Furthermore, each filesystem maintainer the choice of continuing
to use the (awkward) sb->u.fs_specific_struct.actual_field syntax, or getting
rid of it completely in the interest of improving the readability of the code.

Not only readability, but generality as well is being held back by the current
arrangement.

-- 
Daniel

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Wed Feb 23 2000 - 21:00:16 EST