> -----Original Message-----
> From: NeilBrown [mailto:ne...@suse.de]
> Sent: Tuesday, October 23, 2012 12:47 PM
> To: Jaegeuk Kim
> Cc: linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org; 
> gre...@linuxfoundation.org;
> v...@zeniv.linux.org.uk; a...@arndb.de; ty...@mit.edu; chur....@samsung.com; 
> cm224....@samsung.com;
> jooyoung.hw...@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....@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


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

Reply via email to