On Thu, Aug 07, 2008 at 03:11:13PM +0800, Tiger Yang wrote:
> This patch reserve some space in inode block for extended attribute.
> That space used to store extent list or inline data.
This patch looks much better, thanks. One small note below.
> diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
> index 3169237..32631a5 100644
> --- a/fs/ocfs2/ocfs2.h
> +++ b/fs/ocfs2/ocfs2.h
> @@ -227,6 +227,7 @@ struct ocfs2_super
> int s_sectsize_bits;
> int s_clustersize;
> int s_clustersize_bits;
> + unsigned int s_xattr_inline_size;
>
> atomic_t vol_state;
> struct mutex recovery_lock;
> diff --git a/fs/ocfs2/ocfs2_fs.h b/fs/ocfs2/ocfs2_fs.h
> index af91f9d..13bf3e5 100644
> --- a/fs/ocfs2/ocfs2_fs.h
> +++ b/fs/ocfs2/ocfs2_fs.h
> @@ -300,6 +300,9 @@ struct ocfs2_new_group_input {
> */
> #define OCFS2_DEFAULT_LOCAL_ALLOC_SIZE 8
>
> +/* Inline extended attribute size (in bytes) */
> +#define OCFS2_MIN_XATTR_INLINE_SIZE 256
> +
> struct ocfs2_system_inode_info {
> char *si_name;
> int si_iflags;
> @@ -622,7 +625,8 @@ struct ocfs2_dinode {
> belongs to */
> __le16 i_suballoc_bit; /* Bit offset in suballocator
> block group */
> -/*10*/ __le32 i_reserved0;
> +/*10*/ __le16 i_reserved0;
> + __le16 i_xattr_inline_size;
> __le32 i_clusters; /* Cluster count */
> __le32 i_uid; /* Owner UID */
> __le32 i_gid; /* Owning GID */
> @@ -641,11 +645,12 @@ struct ocfs2_dinode {
> __le32 i_atime_nsec;
> __le32 i_ctime_nsec;
> __le32 i_mtime_nsec;
> - __le32 i_attr;
> +/*70*/ __le32 i_attr;
> __le16 i_orphaned_slot; /* Only valid when OCFS2_ORPHANED_FL
> was set in i_flags */
> __le16 i_dyn_features;
> -/*70*/ __le64 i_reserved2[8];
> + __le64 i_xattr_loc;
> +/*80*/ __le64 i_reserved2[7];
> /*B8*/ union {
> __le64 i_pad1; /* Generic way to refer to this
> 64bit union */
> @@ -843,6 +848,20 @@ static inline int ocfs2_max_inline_data(struct
> super_block *sb)
> offsetof(struct ocfs2_dinode, id2.i_data.id_data);
> }
>
> +static inline int ocfs2_max_inline_data_with_xattr(struct super_block *sb,
> + struct ocfs2_dinode *di)
> +{
> + unsigned int xattrsize = le16_to_cpu(di->i_xattr_inline_size);
> +
> + if (le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_XATTR_FL)
> + return sb->s_blocksize -
> + offsetof(struct ocfs2_dinode, id2.i_data.id_data) -
> + xattrsize;
> + else
> + return sb->s_blocksize -
> + offsetof(struct ocfs2_dinode, id2.i_data.id_data);
> +}
> +
> static inline int ocfs2_extent_recs_per_inode(struct super_block *sb)
> {
> int size;
> @@ -853,6 +872,24 @@ static inline int ocfs2_extent_recs_per_inode(struct
> super_block *sb)
> return size / sizeof(struct ocfs2_extent_rec);
> }
>
> +static inline int ocfs2_extent_recs_per_inode_with_xattr(
> + struct super_block *sb,
> + struct ocfs2_dinode *di)
> +{
> + int size;
> + unsigned int xattrsize = le16_to_cpu(di->i_xattr_inline_size);
> +
> + if (le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_XATTR_FL)
> + size = sb->s_blocksize -
> + offsetof(struct ocfs2_dinode, id2.i_list.l_recs) -
> + xattrsize;
> + else
> + size = sb->s_blocksize -
> + offsetof(struct ocfs2_dinode, id2.i_list.l_recs);
> +
> + return size / sizeof(struct ocfs2_extent_rec);
> +}
> +
> static inline int ocfs2_chain_recs_per_inode(struct super_block *sb)
> {
> int size;
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index df63ba2..c0a1cdf 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -1421,6 +1421,8 @@ static int ocfs2_initialize_super(struct super_block
> *sb,
>
> osb->slot_num = OCFS2_INVALID_SLOT;
>
> + osb->s_xattr_inline_size = OCFS2_MIN_XATTR_INLINE_SIZE;
Why not use the value of i_xattr_inline_size off the super-block inode to
tell the module how large xattrs should be? That way we could allow users to
change it in the future.
By the way, regarding sizes. I just realized that the sizes chosen for
xattrs need to be aligned to 16 byte boundaries (sizeof(struct
ocfs2_extent_rec)) so we don't waste space in the extent list above them.
Does that make sense to you? I think we're safe - you chose the value of
OCFS2_MIN_XATTR_INLINE_SIZE well. A comment above it noting this might be nice
though
so that we don't make a mistake in the future.
--Mark
--
Mark Fasheh
_______________________________________________
Ocfs2-devel mailing list
[email protected]
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
