> -----Original Message----- > From: NeilBrown [mailto:neilb@xxxxxxx] > Sent: Tuesday, October 23, 2012 12:47 PM > To: Jaegeuk Kim > Cc: linux-fsdevel@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; > viro@xxxxxxxxxxxxxxxxxx; arnd@xxxxxxxx; tytso@xxxxxxx; chur.lee@xxxxxxxxxxx; cm224.lee@xxxxxxxxxxx; > jooyoung.hwang@xxxxxxxxxxx > 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@xxxxxxxxxxx> > 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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
