lkml.org 
[lkml]   [2002]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: PATCH 2.5.2.9: ext2 unbork fs.h (part 1/7)
    Date
    On January 7, 2002 02:21 pm, Jeff Garzik wrote:
    > Here's my idea for the solution. Each patch in the series has been
    > tested individually and can be applied individually, as long as all
    > preceding patches are applied. (ie. to apply and testing patch N,
    > patches 1 through N-1 must also be applied) The light testing consisted
    > of unpacking, catting, and removing kernel trees, along with a fillmem
    > runs to ensure that slab caches are properly purged. An fsck was forced
    > after each run of tests.
    >
    > This is the first of seven steps in the Make Fs.h Happy program.
    > It borrows direction from Daniel and Linus as well as my own.
    >
    > patch1 (this patch): use accessor function ext2_i to access inode->u.ext2_i
    > The rest of the patches borrows ideas but no code. This patch
    > is the only exception: it borrows substantially Daniel's ext2_i
    > patch.
    > patch2: use accessor function ext2_sb to access sb->u.ext2_sb
    > patch3: dynamically allocate sb->u.ext2_sbp
    > patch4: dynamically allocate inode->u.ext2_ip
    > patch5: move include/linux/ext2*.h to fs/ext2
    >
    > at this point we've reached the limits of how far the current
    > VFS API will go. inode and superblock fs-level private info
    > is dynamically allocated.
    >
    > patch6: add sb->s_op->{alloc,destroy}_inode to VFS API
    > patch7: implement ext2 use of s_op->{alloc,destroy}

    The two main problems I see with this are:

    - If a filesystem doesn't want to use genericp_ip/sbp then fs.h has
    to know about it. Why should fs.h know about every filesystem in
    the world?

    - You are dreferencing a pointer, and have two allocations for every
    inode instead of one.

    It's not horrible, it's just not optimal.

    Moving the ext2 headers from include/linux to fs/ext2 is an interesting
    feature of your patch, though it isn't essential to the idea you're
    presenting. But is there a good reason why ext2_fs_i.h and ext2_fs_sb.h
    should remain separate from ext2_fs.h? It looks like gratuitous
    modularity to me.

    Minor nit:

    if (!inode->u.ext2_ip)
    BUG();

    You don't have to do this, if the pointer is null you will get a perfectly
    fine oops.

    --
    Daniel
    -
    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/

    \
     
     \ /
      Last update: 2005-03-22 13:15    [W:3.944 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site