Ocfs2 devel - Feb 2009 - [PATCH 0/2] ocfs2: two fixes for xattr -v2

Hi,

I have fixed the problems in version 1 patches. These two patches based 
on the latest main line kernel.

Thanks,
tiger
> For EAs data structure in inode/block are little different from them in
> bucket. These two patches try to make them same for the most part.
> 
> The first patch set xh_free_start and xh_name_value_len when EAs in
> inode/block. xh_free_start is useful to keep the minimum offset of the
> xattr name/value. But xh_name_value_len is not very useful because we 
> don't have "hole" when EAs in inode/block, we just calculate
and set it
> here, maybe it's useful for fsck.
> 
> The second patch reserve a blank space between xattr entry and 
> name/value when EAs in bucket. This is same as EAs in inode/block and is 
> useful in fsck.

This patch update fields about xh_free_start and
xh_name_value_len when xattr header in inode/block.
Those fields only be used for bucket before.
With xh_free_start, we are free to calculate minimum
offset of xattr name/value.

Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
---
 fs/ocfs2/ocfs2_fs.h |    2 +-
 fs/ocfs2/xattr.c    |  150 ++++++++++++++++++++++++--------------------------
 2 files changed, 73 insertions(+), 79 deletions(-)

diff --git a/fs/ocfs2/ocfs2_fs.h b/fs/ocfs2/ocfs2_fs.h
index c7ae45a..597d047 100644
--- a/fs/ocfs2/ocfs2_fs.h
+++ b/fs/ocfs2/ocfs2_fs.h
@@ -839,7 +839,7 @@ struct ocfs2_xattr_header {
 	__le16	xh_free_start;                  /* current offset for storing
 						   xattr. */
 	__le16	xh_name_value_len;              /* total length of name/value
-						   length in this bucket. */
+						   length in this object. */
 	__le16	xh_num_buckets;                 /* Number of xattr buckets
 						   in this extent record,
 						   only valid in the first
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 915039f..41c9c5d 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -89,6 +89,9 @@ struct ocfs2_xattr_set_ctxt {
 					 - sizeof(struct ocfs2_xattr_block) \
 					 - sizeof(struct ocfs2_xattr_header) \
 					 - sizeof(__u32))
+#define OCFS2_XATTR_HEADER_SIZE(ptr)	(le16_to_cpu((ptr)->xh_count) \
+					 * sizeof(struct ocfs2_xattr_entry) \
+					 + sizeof(struct ocfs2_xattr_header))
 
 static struct ocfs2_xattr_def_value_root def_xv = {
 	.xv.xr_list.l_count = cpu_to_le16(1),
@@ -1365,8 +1368,7 @@ static int ocfs2_xattr_set_value_outside(struct inode
*inode,
 static void ocfs2_xattr_set_entry_local(struct inode *inode,
 					struct ocfs2_xattr_info *xi,
 					struct ocfs2_xattr_search *xs,
-					struct ocfs2_xattr_entry *last,
-					size_t min_offs)
+					struct ocfs2_xattr_entry *last)
 {
 	size_t name_len = strlen(xi->name);
 	int i;
@@ -1382,7 +1384,7 @@ static void ocfs2_xattr_set_entry_local(struct inode
*inode,
 		void *val;
 		size_t offs, size;
 
-		first_val = xs->base + min_offs;
+		first_val = xs->base + le16_to_cpu(xs->header->xh_free_start);
 		offs = le16_to_cpu(xs->here->xe_name_offset);
 		val = xs->base + offs;
 
@@ -1417,7 +1419,8 @@ static void ocfs2_xattr_set_entry_local(struct inode
*inode,
 		ocfs2_xattr_set_local(xs->here, 1);
 		xs->here->xe_value_size = 0;
 
-		min_offs += size;
+		le16_add_cpu(&xs->header->xh_free_start, size);
+		le16_add_cpu(&xs->header->xh_name_value_len, -size);
 
 		/* Adjust all value offsets. */
 		last = xs->header->xh_entries;
@@ -1440,11 +1443,14 @@ static void ocfs2_xattr_set_entry_local(struct inode
*inode,
 	}
 	if (xi->value) {
 		/* Insert the new name+value. */
+		void *val;
 		size_t size = OCFS2_XATTR_SIZE(name_len) +
 				OCFS2_XATTR_SIZE(xi->value_len);
-		void *val = xs->base + min_offs - size;
 
-		xs->here->xe_name_offset = cpu_to_le16(min_offs - size);
+		le16_add_cpu(&xs->header->xh_free_start, -size);
+		le16_add_cpu(&xs->header->xh_name_value_len, size);
+		val = xs->base + le16_to_cpu(xs->header->xh_free_start);
+		xs->here->xe_name_offset = xs->header->xh_free_start;
 		memset(val, 0, size);
 		memcpy(val, xi->name, name_len);
 		memcpy(val + OCFS2_XATTR_SIZE(name_len),
@@ -1476,10 +1482,10 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 	struct ocfs2_xattr_entry *last;
 	struct ocfs2_inode_info *oi = OCFS2_I(inode);
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)xs->inode_bh->b_data;
-	size_t min_offs = xs->end - xs->base, name_len = strlen(xi->name);
+	size_t name_len = strlen(xi->name);
 	size_t size_l = 0;
 	handle_t *handle = ctxt->handle;
-	int free, i, ret;
+	int free, ret;
 	struct ocfs2_xattr_info xi_l = {
 		.name_index = xi->name_index,
 		.name = xi->name,
@@ -1497,17 +1503,10 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 	} else
 		BUG_ON(xs->xattr_bh != xs->inode_bh);
 
-	/* Compute min_offs, last and free space. */
-	last = xs->header->xh_entries;
-
-	for (i = 0 ; i < le16_to_cpu(xs->header->xh_count); i++) {
-		size_t offs = le16_to_cpu(last->xe_name_offset);
-		if (offs < min_offs)
-			min_offs = offs;
-		last += 1;
-	}
-
-	free = min_offs - ((void *)last - xs->base) - sizeof(__u32);
+	/* Compute last entry and free space. */
+	last =
&xs->header->xh_entries[le16_to_cpu(xs->header->xh_count)];
+	free = le16_to_cpu(xs->header->xh_free_start) -
+		OCFS2_XATTR_HEADER_SIZE(xs->header) - sizeof(__u32);
 	if (free < 0)
 		return -EIO;
 
@@ -1522,22 +1521,17 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 		free += (size + sizeof(struct ocfs2_xattr_entry));
 	}
 	/* Check free space in inode or block */
-	if (xi->value && xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
-		if (free < sizeof(struct ocfs2_xattr_entry) +
-			   OCFS2_XATTR_SIZE(name_len) +
-			   OCFS2_XATTR_ROOT_SIZE) {
+	if (xi->value) {
+		if (ocfs2_xattr_entry_real_size(name_len,
+						xi->value_len) > free) {
 			ret = -ENOSPC;
 			goto out;
 		}
-		size_l = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
-		xi_l.value = (void *)&def_xv;
-		xi_l.value_len = OCFS2_XATTR_ROOT_SIZE;
-	} else if (xi->value) {
-		if (free < sizeof(struct ocfs2_xattr_entry) +
-			   OCFS2_XATTR_SIZE(name_len) +
-			   OCFS2_XATTR_SIZE(xi->value_len)) {
-			ret = -ENOSPC;
-			goto out;
+		if (xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
+			size_l = OCFS2_XATTR_SIZE(name_len) +
+				 OCFS2_XATTR_ROOT_SIZE;
+			xi_l.value = (void *)&def_xv;
+			xi_l.value_len = OCFS2_XATTR_ROOT_SIZE;
 		}
 	}
 
@@ -1629,7 +1623,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 	 * Set value in local, include set tree root in local.
 	 * This is the first step for value size >INLINE_SIZE.
 	 */
-	ocfs2_xattr_set_entry_local(inode, &xi_l, xs, last, min_offs);
+	ocfs2_xattr_set_entry_local(inode, &xi_l, xs, last);
 
 	if (!(flag & OCFS2_INLINE_XATTR_FL)) {
 		ret = ocfs2_journal_dirty(handle, xs->xattr_bh);
@@ -1941,6 +1935,22 @@ static int ocfs2_xattr_has_space_inline(struct inode
*inode,
 	return 0;
 }
 
+static size_t ocfs2_xattr_min_offset(struct ocfs2_xattr_header *xh,
+				     size_t size)
+{
+	int i;
+	size_t min_offs = size;
+
+	for (i = 0 ; i < le16_to_cpu(xh->xh_count); i++) {
+		struct ocfs2_xattr_entry *xe = &xh->xh_entries[i];
+		size_t offs = le16_to_cpu(xe->xe_name_offset);
+
+		if (offs < min_offs)
+			min_offs = offs;
+	}
+	return min_offs;
+}
+
 /*
  * ocfs2_xattr_ibody_find()
  *
@@ -1970,12 +1980,22 @@ static int ocfs2_xattr_ibody_find(struct inode *inode,
 
 	xs->xattr_bh = xs->inode_bh;
 	xs->end = (void *)di + inode->i_sb->s_blocksize;
-	if (oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)
+	if (oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL) {
 		xs->header = (struct ocfs2_xattr_header *)
 			(xs->end - le16_to_cpu(di->i_xattr_inline_size));
-	else
+		if (!xs->header->xh_free_start) {
+			size_t min_offs = ocfs2_xattr_min_offset(xs->header,
+					le16_to_cpu(di->i_xattr_inline_size));
+			xs->header->xh_free_start = cpu_to_le16(min_offs);
+			xs->header->xh_name_value_len =	cpu_to_le16(
+			le16_to_cpu(di->i_xattr_inline_size) - min_offs);
+		}
+	} else {
 		xs->header = (struct ocfs2_xattr_header *)
 			(xs->end - OCFS2_SB(inode->i_sb)->s_xattr_inline_size);
+		xs->header->xh_free_start = cpu_to_le16(
+				OCFS2_SB(inode->i_sb)->s_xattr_inline_size);
+	}
 	xs->base = (void *)xs->header;
 	xs->here = xs->header->xh_entries;
 
@@ -2058,6 +2078,13 @@ static int ocfs2_xattr_block_find(struct inode *inode,
 		xs->base = (void *)xs->header;
 		xs->end = (void *)(blk_bh->b_data) + blk_bh->b_size;
 		xs->here = xs->header->xh_entries;
+		if (!xs->header->xh_free_start) {
+			size_t min_offs = ocfs2_xattr_min_offset(xs->header,
+							xs->end - xs->base);
+			xs->header->xh_free_start = cpu_to_le16(min_offs);
+			xs->header->xh_name_value_len +				cpu_to_le16(xs->end -
xs->base - min_offs);
+		}
 
 		ret = ocfs2_xattr_find_entry(name_index, name, xs);
 	} else
@@ -2138,6 +2165,8 @@ static int ocfs2_xattr_block_set(struct inode *inode,
 		xs->base = (void *)xs->header;
 		xs->end = (void *)xblk + inode->i_sb->s_blocksize;
 		xs->here = xs->header->xh_entries;
+		xblk->xb_attrs.xb_header.xh_free_start +					cpu_to_le16(xs->end -
xs->base);
 
 		ret = ocfs2_journal_dirty(handle, new_bh);
 		if (ret < 0) {
@@ -2168,46 +2197,6 @@ end:
 	return ret;
 }
 
-/* Check whether the new xattr can be inserted into the inode. */
-static int ocfs2_xattr_can_be_in_inode(struct inode *inode,
-				       struct ocfs2_xattr_info *xi,
-				       struct ocfs2_xattr_search *xs)
-{
-	u64 value_size;
-	struct ocfs2_xattr_entry *last;
-	int free, i;
-	size_t min_offs = xs->end - xs->base;
-
-	if (!xs->header)
-		return 0;
-
-	last = xs->header->xh_entries;
-
-	for (i = 0; i < le16_to_cpu(xs->header->xh_count); i++) {
-		size_t offs = le16_to_cpu(last->xe_name_offset);
-		if (offs < min_offs)
-			min_offs = offs;
-		last += 1;
-	}
-
-	free = min_offs - ((void *)last - xs->base) - sizeof(__u32);
-	if (free < 0)
-		return 0;
-
-	BUG_ON(!xs->not_found);
-
-	if (xi->value_len > OCFS2_XATTR_INLINE_SIZE)
-		value_size = OCFS2_XATTR_ROOT_SIZE;
-	else
-		value_size = OCFS2_XATTR_SIZE(xi->value_len);
-
-	if (free >= sizeof(struct ocfs2_xattr_entry) +
-		   OCFS2_XATTR_SIZE(strlen(xi->name)) + value_size)
-		return 1;
-
-	return 0;
-}
-
 static int ocfs2_calc_xattr_set_need(struct inode *inode,
 				     struct ocfs2_dinode *di,
 				     struct ocfs2_xattr_info *xi,
@@ -2303,7 +2292,13 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
 		 * one will be removed from the xattr block, and this xattr
 		 * will be inserted into inode as a new xattr in inode.
 		 */
-		if (ocfs2_xattr_can_be_in_inode(inode, xi, xis)) {
+		int free = le16_to_cpu(xis->header->xh_free_start) -
+			   OCFS2_XATTR_HEADER_SIZE(xis->header) -
+			   sizeof(__u32);
+		int size = ocfs2_xattr_entry_real_size(strlen(xi->name),
+						       xi->value_len);
+
+		if (size <= free) {
 			clusters_add += new_clusters;
 			credits += ocfs2_remove_extent_credits(inode->i_sb) +
 				    OCFS2_INODE_UPDATE_CREDITS;
@@ -5058,8 +5053,7 @@ try_again:
 	xh = xs->header;
 	count = le16_to_cpu(xh->xh_count);
 	xh_free_start = le16_to_cpu(xh->xh_free_start);
-	header_size = sizeof(struct ocfs2_xattr_header) +
-			count * sizeof(struct ocfs2_xattr_entry);
+	header_size = OCFS2_XATTR_HEADER_SIZE(xh);
 	max_free = OCFS2_XATTR_BUCKET_SIZE -
 		le16_to_cpu(xh->xh_name_value_len) - header_size;
 
-- 
1.5.4.1

This patch set a gap (4 bytes) between xattr entry and
name/value when xattr in bucket. This gap use to seperate
entry and name/value when a bucket is full. It had already
been set when xattr in inode/block.

Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
---
 fs/ocfs2/xattr.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 41c9c5d..880bcb3 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -82,6 +82,7 @@ struct ocfs2_xattr_set_ctxt {
 
 #define OCFS2_XATTR_ROOT_SIZE	(sizeof(struct ocfs2_xattr_def_value_root))
 #define OCFS2_XATTR_INLINE_SIZE	80
+#define OCFS2_XATTR_HEADER_GAP	4
 #define OCFS2_XATTR_FREE_IN_IBODY	(OCFS2_MIN_XATTR_INLINE_SIZE \
 					 - sizeof(struct ocfs2_xattr_header) \
 					 - sizeof(__u32))
@@ -1506,7 +1507,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
 	/* Compute last entry and free space. */
 	last =
&xs->header->xh_entries[le16_to_cpu(xs->header->xh_count)];
 	free = le16_to_cpu(xs->header->xh_free_start) -
-		OCFS2_XATTR_HEADER_SIZE(xs->header) - sizeof(__u32);
+		OCFS2_XATTR_HEADER_SIZE(xs->header) - OCFS2_XATTR_HEADER_GAP;
 	if (free < 0)
 		return -EIO;
 
@@ -2294,7 +2295,7 @@ static int ocfs2_calc_xattr_set_need(struct inode *inode,
 		 */
 		int free = le16_to_cpu(xis->header->xh_free_start) -
 			   OCFS2_XATTR_HEADER_SIZE(xis->header) -
-			   sizeof(__u32);
+			   OCFS2_XATTR_HEADER_GAP;
 		int size = ocfs2_xattr_entry_real_size(strlen(xi->name),
 						       xi->value_len);
 
@@ -5054,8 +5055,8 @@ try_again:
 	count = le16_to_cpu(xh->xh_count);
 	xh_free_start = le16_to_cpu(xh->xh_free_start);
 	header_size = OCFS2_XATTR_HEADER_SIZE(xh);
-	max_free = OCFS2_XATTR_BUCKET_SIZE -
-		le16_to_cpu(xh->xh_name_value_len) - header_size;
+	max_free = OCFS2_XATTR_BUCKET_SIZE - header_size -
+		le16_to_cpu(xh->xh_name_value_len) - OCFS2_XATTR_HEADER_GAP;
 
 	mlog_bug_on_msg(header_size > blocksize, "bucket %llu has header size
"
 			"of %u which exceed block size\n",
@@ -5088,7 +5089,7 @@ try_again:
 			need = 0;
 	}
 
-	free = xh_free_start - header_size;
+	free = xh_free_start - header_size - OCFS2_XATTR_HEADER_GAP;
 	/*
 	 * We need to make sure the new name/value pair
 	 * can exist in the same block.
@@ -5121,7 +5122,8 @@ try_again:
 			}
 
 			xh_free_start = le16_to_cpu(xh->xh_free_start);
-			free = xh_free_start - header_size;
+			free = xh_free_start - header_size
+				- OCFS2_XATTR_HEADER_GAP;
 			if (xh_free_start % blocksize < need)
 				free -= xh_free_start % blocksize;
 
-- 
1.5.4.1

Hi, Joel and Mark,

I suddenly found there might be a potential problem in this patch.

If user setting EAs sometimes with .28 kernel and sometimes with .29 
kernel, xh_free_start will go stale. And fsck will make it worse.

If this really a problem I withdraw this patch but I think the second 
patch is OK.

Thanks,
tiger

Tiger Yang wrote:
> This patch update fields about xh_free_start and
> xh_name_value_len when xattr header in inode/block.
> Those fields only be used for bucket before.
> With xh_free_start, we are free to calculate minimum
> offset of xattr name/value.
> 
> Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
> ---
>  fs/ocfs2/ocfs2_fs.h |    2 +-
>  fs/ocfs2/xattr.c    |  150
++++++++++++++++++++++++--------------------------
>  2 files changed, 73 insertions(+), 79 deletions(-)
> 
> diff --git a/fs/ocfs2/ocfs2_fs.h b/fs/ocfs2/ocfs2_fs.h
> index c7ae45a..597d047 100644
> --- a/fs/ocfs2/ocfs2_fs.h
> +++ b/fs/ocfs2/ocfs2_fs.h
> @@ -839,7 +839,7 @@ struct ocfs2_xattr_header {
>  	__le16	xh_free_start;                  /* current offset for storing
>  						   xattr. */
>  	__le16	xh_name_value_len;              /* total length of name/value
> -						   length in this bucket. */
> +						   length in this object. */
>  	__le16	xh_num_buckets;                 /* Number of xattr buckets
>  						   in this extent record,
>  						   only valid in the first
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 915039f..41c9c5d 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -89,6 +89,9 @@ struct ocfs2_xattr_set_ctxt {
>  					 - sizeof(struct ocfs2_xattr_block) \
>  					 - sizeof(struct ocfs2_xattr_header) \
>  					 - sizeof(__u32))
> +#define OCFS2_XATTR_HEADER_SIZE(ptr)	(le16_to_cpu((ptr)->xh_count) \
> +					 * sizeof(struct ocfs2_xattr_entry) \
> +					 + sizeof(struct ocfs2_xattr_header))
>  
>  static struct ocfs2_xattr_def_value_root def_xv = {
>  	.xv.xr_list.l_count = cpu_to_le16(1),
> @@ -1365,8 +1368,7 @@ static int ocfs2_xattr_set_value_outside(struct inode
*inode,
>  static void ocfs2_xattr_set_entry_local(struct inode *inode,
>  					struct ocfs2_xattr_info *xi,
>  					struct ocfs2_xattr_search *xs,
> -					struct ocfs2_xattr_entry *last,
> -					size_t min_offs)
> +					struct ocfs2_xattr_entry *last)
>  {
>  	size_t name_len = strlen(xi->name);
>  	int i;
> @@ -1382,7 +1384,7 @@ static void ocfs2_xattr_set_entry_local(struct inode
*inode,
>  		void *val;
>  		size_t offs, size;
>  
> -		first_val = xs->base + min_offs;
> +		first_val = xs->base + le16_to_cpu(xs->header->xh_free_start);
>  		offs = le16_to_cpu(xs->here->xe_name_offset);
>  		val = xs->base + offs;
>  
> @@ -1417,7 +1419,8 @@ static void ocfs2_xattr_set_entry_local(struct inode
*inode,
>  		ocfs2_xattr_set_local(xs->here, 1);
>  		xs->here->xe_value_size = 0;
>  
> -		min_offs += size;
> +		le16_add_cpu(&xs->header->xh_free_start, size);
> +		le16_add_cpu(&xs->header->xh_name_value_len, -size);
>  
>  		/* Adjust all value offsets. */
>  		last = xs->header->xh_entries;
> @@ -1440,11 +1443,14 @@ static void ocfs2_xattr_set_entry_local(struct
inode *inode,
>  	}
>  	if (xi->value) {
>  		/* Insert the new name+value. */
> +		void *val;
>  		size_t size = OCFS2_XATTR_SIZE(name_len) +
>  				OCFS2_XATTR_SIZE(xi->value_len);
> -		void *val = xs->base + min_offs - size;
>  
> -		xs->here->xe_name_offset = cpu_to_le16(min_offs - size);
> +		le16_add_cpu(&xs->header->xh_free_start, -size);
> +		le16_add_cpu(&xs->header->xh_name_value_len, size);
> +		val = xs->base + le16_to_cpu(xs->header->xh_free_start);
> +		xs->here->xe_name_offset = xs->header->xh_free_start;
>  		memset(val, 0, size);
>  		memcpy(val, xi->name, name_len);
>  		memcpy(val + OCFS2_XATTR_SIZE(name_len),
> @@ -1476,10 +1482,10 @@ static int ocfs2_xattr_set_entry(struct inode
*inode,
>  	struct ocfs2_xattr_entry *last;
>  	struct ocfs2_inode_info *oi = OCFS2_I(inode);
>  	struct ocfs2_dinode *di = (struct ocfs2_dinode
*)xs->inode_bh->b_data;
> -	size_t min_offs = xs->end - xs->base, name_len =
strlen(xi->name);
> +	size_t name_len = strlen(xi->name);
>  	size_t size_l = 0;
>  	handle_t *handle = ctxt->handle;
> -	int free, i, ret;
> +	int free, ret;
>  	struct ocfs2_xattr_info xi_l = {
>  		.name_index = xi->name_index,
>  		.name = xi->name,
> @@ -1497,17 +1503,10 @@ static int ocfs2_xattr_set_entry(struct inode
*inode,
>  	} else
>  		BUG_ON(xs->xattr_bh != xs->inode_bh);
>  
> -	/* Compute min_offs, last and free space. */
> -	last = xs->header->xh_entries;
> -
> -	for (i = 0 ; i < le16_to_cpu(xs->header->xh_count); i++) {
> -		size_t offs = le16_to_cpu(last->xe_name_offset);
> -		if (offs < min_offs)
> -			min_offs = offs;
> -		last += 1;
> -	}
> -
> -	free = min_offs - ((void *)last - xs->base) - sizeof(__u32);
> +	/* Compute last entry and free space. */
> +	last =
&xs->header->xh_entries[le16_to_cpu(xs->header->xh_count)];
> +	free = le16_to_cpu(xs->header->xh_free_start) -
> +		OCFS2_XATTR_HEADER_SIZE(xs->header) - sizeof(__u32);
>  	if (free < 0)
>  		return -EIO;
>  
> @@ -1522,22 +1521,17 @@ static int ocfs2_xattr_set_entry(struct inode
*inode,
>  		free += (size + sizeof(struct ocfs2_xattr_entry));
>  	}
>  	/* Check free space in inode or block */
> -	if (xi->value && xi->value_len >
OCFS2_XATTR_INLINE_SIZE) {
> -		if (free < sizeof(struct ocfs2_xattr_entry) +
> -			   OCFS2_XATTR_SIZE(name_len) +
> -			   OCFS2_XATTR_ROOT_SIZE) {
> +	if (xi->value) {
> +		if (ocfs2_xattr_entry_real_size(name_len,
> +						xi->value_len) > free) {
>  			ret = -ENOSPC;
>  			goto out;
>  		}
> -		size_l = OCFS2_XATTR_SIZE(name_len) + OCFS2_XATTR_ROOT_SIZE;
> -		xi_l.value = (void *)&def_xv;
> -		xi_l.value_len = OCFS2_XATTR_ROOT_SIZE;
> -	} else if (xi->value) {
> -		if (free < sizeof(struct ocfs2_xattr_entry) +
> -			   OCFS2_XATTR_SIZE(name_len) +
> -			   OCFS2_XATTR_SIZE(xi->value_len)) {
> -			ret = -ENOSPC;
> -			goto out;
> +		if (xi->value_len > OCFS2_XATTR_INLINE_SIZE) {
> +			size_l = OCFS2_XATTR_SIZE(name_len) +
> +				 OCFS2_XATTR_ROOT_SIZE;
> +			xi_l.value = (void *)&def_xv;
> +			xi_l.value_len = OCFS2_XATTR_ROOT_SIZE;
>  		}
>  	}
>  
> @@ -1629,7 +1623,7 @@ static int ocfs2_xattr_set_entry(struct inode *inode,
>  	 * Set value in local, include set tree root in local.
>  	 * This is the first step for value size >INLINE_SIZE.
>  	 */
> -	ocfs2_xattr_set_entry_local(inode, &xi_l, xs, last, min_offs);
> +	ocfs2_xattr_set_entry_local(inode, &xi_l, xs, last);
>  
>  	if (!(flag & OCFS2_INLINE_XATTR_FL)) {
>  		ret = ocfs2_journal_dirty(handle, xs->xattr_bh);
> @@ -1941,6 +1935,22 @@ static int ocfs2_xattr_has_space_inline(struct inode
*inode,
>  	return 0;
>  }
>  
> +static size_t ocfs2_xattr_min_offset(struct ocfs2_xattr_header *xh,
> +				     size_t size)
> +{
> +	int i;
> +	size_t min_offs = size;
> +
> +	for (i = 0 ; i < le16_to_cpu(xh->xh_count); i++) {
> +		struct ocfs2_xattr_entry *xe = &xh->xh_entries[i];
> +		size_t offs = le16_to_cpu(xe->xe_name_offset);
> +
> +		if (offs < min_offs)
> +			min_offs = offs;
> +	}
> +	return min_offs;
> +}
> +
>  /*
>   * ocfs2_xattr_ibody_find()
>   *
> @@ -1970,12 +1980,22 @@ static int ocfs2_xattr_ibody_find(struct inode
*inode,
>  
>  	xs->xattr_bh = xs->inode_bh;
>  	xs->end = (void *)di + inode->i_sb->s_blocksize;
> -	if (oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)
> +	if (oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL) {
>  		xs->header = (struct ocfs2_xattr_header *)
>  			(xs->end - le16_to_cpu(di->i_xattr_inline_size));
> -	else
> +		if (!xs->header->xh_free_start) {
> +			size_t min_offs = ocfs2_xattr_min_offset(xs->header,
> +					le16_to_cpu(di->i_xattr_inline_size));
> +			xs->header->xh_free_start = cpu_to_le16(min_offs);
> +			xs->header->xh_name_value_len =	cpu_to_le16(
> +			le16_to_cpu(di->i_xattr_inline_size) - min_offs);
> +		}
> +	} else {
>  		xs->header = (struct ocfs2_xattr_header *)
>  			(xs->end - OCFS2_SB(inode->i_sb)->s_xattr_inline_size);
> +		xs->header->xh_free_start = cpu_to_le16(
> +				OCFS2_SB(inode->i_sb)->s_xattr_inline_size);
> +	}
>  	xs->base = (void *)xs->header;
>  	xs->here = xs->header->xh_entries;
>  
> @@ -2058,6 +2078,13 @@ static int ocfs2_xattr_block_find(struct inode
*inode,
>  		xs->base = (void *)xs->header;
>  		xs->end = (void *)(blk_bh->b_data) + blk_bh->b_size;
>  		xs->here = xs->header->xh_entries;
> +		if (!xs->header->xh_free_start) {
> +			size_t min_offs = ocfs2_xattr_min_offset(xs->header,
> +							xs->end - xs->base);
> +			xs->header->xh_free_start = cpu_to_le16(min_offs);
> +			xs->header->xh_name_value_len > +				cpu_to_le16(xs->end -
xs->base - min_offs);
> +		}
>  
>  		ret = ocfs2_xattr_find_entry(name_index, name, xs);
>  	} else
> @@ -2138,6 +2165,8 @@ static int ocfs2_xattr_block_set(struct inode *inode,
>  		xs->base = (void *)xs->header;
>  		xs->end = (void *)xblk + inode->i_sb->s_blocksize;
>  		xs->here = xs->header->xh_entries;
> +		xblk->xb_attrs.xb_header.xh_free_start > +				
cpu_to_le16(xs->end - xs->base);
>  
>  		ret = ocfs2_journal_dirty(handle, new_bh);
>  		if (ret < 0) {
> @@ -2168,46 +2197,6 @@ end:
>  	return ret;
>  }
>  
> -/* Check whether the new xattr can be inserted into the inode. */
> -static int ocfs2_xattr_can_be_in_inode(struct inode *inode,
> -				       struct ocfs2_xattr_info *xi,
> -				       struct ocfs2_xattr_search *xs)
> -{
> -	u64 value_size;
> -	struct ocfs2_xattr_entry *last;
> -	int free, i;
> -	size_t min_offs = xs->end - xs->base;
> -
> -	if (!xs->header)
> -		return 0;
> -
> -	last = xs->header->xh_entries;
> -
> -	for (i = 0; i < le16_to_cpu(xs->header->xh_count); i++) {
> -		size_t offs = le16_to_cpu(last->xe_name_offset);
> -		if (offs < min_offs)
> -			min_offs = offs;
> -		last += 1;
> -	}
> -
> -	free = min_offs - ((void *)last - xs->base) - sizeof(__u32);
> -	if (free < 0)
> -		return 0;
> -
> -	BUG_ON(!xs->not_found);
> -
> -	if (xi->value_len > OCFS2_XATTR_INLINE_SIZE)
> -		value_size = OCFS2_XATTR_ROOT_SIZE;
> -	else
> -		value_size = OCFS2_XATTR_SIZE(xi->value_len);
> -
> -	if (free >= sizeof(struct ocfs2_xattr_entry) +
> -		   OCFS2_XATTR_SIZE(strlen(xi->name)) + value_size)
> -		return 1;
> -
> -	return 0;
> -}
> -
>  static int ocfs2_calc_xattr_set_need(struct inode *inode,
>  				     struct ocfs2_dinode *di,
>  				     struct ocfs2_xattr_info *xi,
> @@ -2303,7 +2292,13 @@ static int ocfs2_calc_xattr_set_need(struct inode
*inode,
>  		 * one will be removed from the xattr block, and this xattr
>  		 * will be inserted into inode as a new xattr in inode.
>  		 */
> -		if (ocfs2_xattr_can_be_in_inode(inode, xi, xis)) {
> +		int free = le16_to_cpu(xis->header->xh_free_start) -
> +			   OCFS2_XATTR_HEADER_SIZE(xis->header) -
> +			   sizeof(__u32);
> +		int size = ocfs2_xattr_entry_real_size(strlen(xi->name),
> +						       xi->value_len);
> +
> +		if (size <= free) {
>  			clusters_add += new_clusters;
>  			credits += ocfs2_remove_extent_credits(inode->i_sb) +
>  				    OCFS2_INODE_UPDATE_CREDITS;
> @@ -5058,8 +5053,7 @@ try_again:
>  	xh = xs->header;
>  	count = le16_to_cpu(xh->xh_count);
>  	xh_free_start = le16_to_cpu(xh->xh_free_start);
> -	header_size = sizeof(struct ocfs2_xattr_header) +
> -			count * sizeof(struct ocfs2_xattr_entry);
> +	header_size = OCFS2_XATTR_HEADER_SIZE(xh);
>  	max_free = OCFS2_XATTR_BUCKET_SIZE -
>  		le16_to_cpu(xh->xh_name_value_len) - header_size;
>