lkml.org 
[lkml]   [2012]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH 02/16 v2] f2fs: add on-disk layout
    Date
    > -----Original Message-----
    > From: NeilBrown [mailto:neilb@suse.de]
    > Sent: Tuesday, October 23, 2012 12:47 PM
    > To: Jaegeuk Kim
    > Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; gregkh@linuxfoundation.org;
    > viro@zeniv.linux.org.uk; arnd@arndb.de; tytso@mit.edu; chur.lee@samsung.com; cm224.lee@samsung.com;
    > jooyoung.hwang@samsung.com
    > Subject: Re: [PATCH 02/16 v2] f2fs: add on-disk layout
    > Importance: High
    >
    > On Tue, 23 Oct 2012 11:26:00 +0900 Jaegeuk Kim <jaegeuk.kim@samsung.com>
    > wrote:
    >
    > > This adds a header file describing the on-disk layout of f2fs.
    > >
    >
    >
    > > +struct f2fs_inode {
    > > + __le16 i_mode; /* File mode */
    > > + __u8 i_advise; /* File hints */
    > > + __u8 i_reserved; /* Reserved */
    > > + __le32 i_uid; /* User ID */
    > > + __le32 i_gid; /* Group ID */
    > > + __le32 i_links; /* Links count */
    > > + __le64 i_size; /* File size in bytes */
    > > + __le64 i_blocks; /* File size in blocks */
    > > + __le64 i_ctime; /* Inode change time */
    > > + __le64 i_mtime; /* Modification time */
    > > + __le32 i_ctime_nsec;
    > > + __le32 i_mtime_nsec;
    > > + __le32 current_depth;
    > > + __le32 i_xattr_nid; /* nid to save xattr */
    > > + __le32 i_flags; /* file attributes */
    > > + __le32 i_pino; /* parent inode number */
    > > + __le32 i_namelen; /* file name length */
    > > + __u8 i_name[F2FS_MAX_NAME_LEN]; /* file name for SPOR */
    > > +
    > > + struct f2fs_extent i_ext; /* caching a largest extent */
    > > +
    > > + __le32 i_addr[ADDRS_PER_INODE]; /* Pointers to data blocks */
    > > +
    > > + __le32 i_nid[5]; /* direct(2), indirect(2),
    > > + double_indirect(1) node id */
    > > +} __packed;
    > > +
    >
    >
    > You appear to have dropped i_btime - no big deal, you weren't using it anyway.
    > However if you ever want to support NFS export you will need some value which
    > is assigned when the inode is allocated and never changed until it is
    > de-allocated. This is used to detect when an NFS file-handle refers to a
    > previous incarnation of an inode and so should be rejected as STALE.
    > i_btime could have possibly provided this, but not any more. You might want
    > to add something back.
    > ext3 uses "i_generation" and has an 's_next_generation' in the superblock to
    > ensure that each new inode gets a new generation number.

    Agreed. I'll check that.

    >
    > You've also dropped i_atime. I can certainly understand the desire to do
    > that, but I wonder if it is entirely wise. There are some use-cases where
    > i_mtime is a poor substitute.

    Got it.

    >
    > Also 'current_depth' looks a little odd without a 'i_' prefix. It wouldn't
    > hurt to have a comment noting that it is for directories.

    Agreed.
    Thank you for comments. :)

    >
    > Thanks,
    > NeilBrown


    ---
    Jaegeuk Kim
    Samsung




    \
     
     \ /
      Last update: 2012-10-23 09:21    [W:2.401 / U:0.708 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site