[Ocfs2-devel] [PATCH 0/13] ocfs2: xattr bucket API

[Ocfs2-devel] [PATCH 0/13] ocfs2: xattr bucket API

[Joel Becker]

When the extended attribute namespace grows to a b-tree, the leaf
clusters are organized by means of 'buckets'.  Each bucket is 4K in
size, regardless of blocksize.  Thus, a bucket may be made of more than
one block.

fs/ocfs2/xattr.c has a nice little abstraction to wrap this, struct
ocfs2_xattr_bucket.  It contains a list of buffer_heads representing
these blocks, and there is even an API to fill it or initialize it.

However, the majority of the code does not use this abstraction.
Instead, it uses raw buffer_heads and jumps through other hoops.
This has two consequences.  First, it's harder to read.  Second, it is
less efficient.  Sometimes it reads the first and last block of a
bucket, when reading and writing all blocks at once is a streaming I/O.

This series expands the bucket API in a fashion similar to
fs/ocfs2/alloc.c's struct ocfs2_path.  The hope is that all bucket
operations can use this API and mostly avoid raw buffer_head work.

This is also needed for checksums of buckets, as the checksum
calculations need to read the entire bucket.  Since a 4K contig I/O is
just as cheap as a single block, this causes no loss of efficiency.

This path series is on top of my xattr-28 fixes branch.  Before going
upstream it will be rebased on top of at least the divide_bucket fix.
It only touches the xattr_get() and xattr_list() paths, becuase I need
Tao's single-transaction xattr_set() before I can do the xattr_set()
path.

All comments and review welcome.  I hope it's mostly self-explanatory.

View:
http://oss.oracle.com/git/?p=jlbec/linux-2.6.git;a=shortlog;h=xattr-buckets
Pull:
git://oss.oracle.com/git/jlbec/linux-2.6.git xattr-buckets

[Ocfs2-devel] [PATCH 01/13] ocfs2: Field prefixes for the xattr_bucket structure

[Joel Becker]

The ocfs2_xattr_bucket structure keeps track of the buffers for one
xattr bucket.  Let's prefix the fields for easier code navigation.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |  100 +++++++++++++++++++++++++++---------------------------
 1 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index a371c01..fb8b401 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -61,8 +61,8 @@ struct ocfs2_xattr_def_value_root {
 };
 
 struct ocfs2_xattr_bucket {
-	struct buffer_head *bhs[OCFS2_XATTR_MAX_BLOCKS_PER_BUCKET];
-	struct ocfs2_xattr_header *xh;
+	struct buffer_head *bu_bhs[OCFS2_XATTR_MAX_BLOCKS_PER_BUCKET];
+	struct ocfs2_xattr_header *bu_xh;
 };
 
 #define OCFS2_XATTR_ROOT_SIZE	(sizeof(struct ocfs2_xattr_def_value_root))
@@ -790,11 +790,11 @@ static int ocfs2_xattr_block_get(struct inode *inode,
 
 		if (le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED) {
 			ret = ocfs2_xattr_bucket_get_name_value(inode,
-								xs->bucket.xh,
+								xs->bucket.bu_xh,
 								i,
 								&block_off,
 								&name_offset);
-			xs->base = xs->bucket.bhs[block_off]->b_data;
+			xs->base = xs->bucket.bu_bhs[block_off]->b_data;
 		}
 		if (ocfs2_xattr_is_local(xs->here)) {
 			memcpy(buffer, (void *)xs->base +
@@ -813,7 +813,7 @@ static int ocfs2_xattr_block_get(struct inode *inode,
 	ret = size;
 cleanup:
 	for (i = 0; i < OCFS2_XATTR_MAX_BLOCKS_PER_BUCKET; i++)
-		brelse(xs->bucket.bhs[i]);
+		brelse(xs->bucket.bu_bhs[i]);
 	memset(&xs->bucket, 0, sizeof(xs->bucket));
 
 	brelse(xs->xattr_bh);
@@ -2027,7 +2027,7 @@ cleanup:
 	brelse(di_bh);
 	brelse(xbs.xattr_bh);
 	for (i = 0; i < blk_per_bucket; i++)
-		brelse(xbs.bucket.bhs[i]);
+		brelse(xbs.bucket.bu_bhs[i]);
 
 	return ret;
 }
@@ -2271,13 +2271,13 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 		lower_bh = bh;
 		bh = NULL;
 	}
-	xs->bucket.bhs[0] = lower_bh;
-	xs->bucket.xh = (struct ocfs2_xattr_header *)
-					xs->bucket.bhs[0]->b_data;
+	xs->bucket.bu_bhs[0] = lower_bh;
+	xs->bucket.bu_xh = (struct ocfs2_xattr_header *)
+					xs->bucket.bu_bhs[0]->b_data;
 	lower_bh = NULL;
 
-	xs->header = xs->bucket.xh;
-	xs->base = xs->bucket.bhs[0]->b_data;
+	xs->header = xs->bucket.bu_xh;
+	xs->base = xs->bucket.bu_bhs[0]->b_data;
 	xs->end = xs->base + inode->i_sb->s_blocksize;
 
 	if (found) {
@@ -2285,8 +2285,8 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 		 * If we have found the xattr enty, read all the blocks in
 		 * this bucket.
 		 */
-		ret = ocfs2_read_blocks(inode, xs->bucket.bhs[0]->b_blocknr + 1,
-					blk_per_bucket - 1, &xs->bucket.bhs[1],
+		ret = ocfs2_read_blocks(inode, xs->bucket.bu_bhs[0]->b_blocknr + 1,
+					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
 					0);
 		if (ret) {
 			mlog_errno(ret);
@@ -2295,7 +2295,7 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 
 		xs->here = &xs->header->xh_entries[index];
 		mlog(0, "find xattr %s in bucket %llu, entry = %u\n", name,
-		     (unsigned long long)xs->bucket.bhs[0]->b_blocknr, index);
+		     (unsigned long long)xs->bucket.bu_bhs[0]->b_blocknr, index);
 	} else
 		ret = -ENODATA;
 
@@ -2364,22 +2364,22 @@ static int ocfs2_iterate_xattr_buckets(struct inode *inode,
 
 	for (i = 0; i < num_buckets; i++, blkno += blk_per_bucket) {
 		ret = ocfs2_read_blocks(inode, blkno, blk_per_bucket,
-					bucket.bhs, 0);
+					bucket.bu_bhs, 0);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;
 		}
 
-		bucket.xh = (struct ocfs2_xattr_header *)bucket.bhs[0]->b_data;
+		bucket.bu_xh = (struct ocfs2_xattr_header *)bucket.bu_bhs[0]->b_data;
 		/*
 		 * The real bucket num in this series of blocks is stored
 		 * in the 1st bucket.
 		 */
 		if (i == 0)
-			num_buckets = le16_to_cpu(bucket.xh->xh_num_buckets);
+			num_buckets = le16_to_cpu(bucket.bu_xh->xh_num_buckets);
 
 		mlog(0, "iterating xattr bucket %llu, first hash %u\n", blkno,
-		     le32_to_cpu(bucket.xh->xh_entries[0].xe_name_hash));
+		     le32_to_cpu(bucket.bu_xh->xh_entries[0].xe_name_hash));
 		if (func) {
 			ret = func(inode, &bucket, para);
 			if (ret) {
@@ -2389,13 +2389,13 @@ static int ocfs2_iterate_xattr_buckets(struct inode *inode,
 		}
 
 		for (j = 0; j < blk_per_bucket; j++)
-			brelse(bucket.bhs[j]);
+			brelse(bucket.bu_bhs[j]);
 		memset(&bucket, 0, sizeof(bucket));
 	}
 
 out:
 	for (j = 0; j < blk_per_bucket; j++)
-		brelse(bucket.bhs[j]);
+		brelse(bucket.bu_bhs[j]);
 
 	return ret;
 }
@@ -2434,21 +2434,21 @@ static int ocfs2_list_xattr_bucket(struct inode *inode,
 	int i, block_off, new_offset;
 	const char *prefix, *name;
 
-	for (i = 0 ; i < le16_to_cpu(bucket->xh->xh_count); i++) {
-		struct ocfs2_xattr_entry *entry = &bucket->xh->xh_entries[i];
+	for (i = 0 ; i < le16_to_cpu(bucket->bu_xh->xh_count); i++) {
+		struct ocfs2_xattr_entry *entry = &bucket->bu_xh->xh_entries[i];
 		type = ocfs2_xattr_get_type(entry);
 		prefix = ocfs2_xattr_prefix(type);
 
 		if (prefix) {
 			ret = ocfs2_xattr_bucket_get_name_value(inode,
-								bucket->xh,
+								bucket->bu_xh,
 								i,
 								&block_off,
 								&new_offset);
 			if (ret)
 				break;
 
-			name = (const char *)bucket->bhs[block_off]->b_data +
+			name = (const char *)bucket->bu_bhs[block_off]->b_data +
 				new_offset;
 			ret = ocfs2_xattr_list_entry(xl->buffer,
 						     xl->buffer_size,
@@ -2619,10 +2619,10 @@ static int ocfs2_xattr_update_xattr_search(struct inode *inode,
 	int i, blocksize = inode->i_sb->s_blocksize;
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 
-	xs->bucket.bhs[0] = new_bh;
+	xs->bucket.bu_bhs[0] = new_bh;
 	get_bh(new_bh);
-	xs->bucket.xh = (struct ocfs2_xattr_header *)xs->bucket.bhs[0]->b_data;
-	xs->header = xs->bucket.xh;
+	xs->bucket.bu_xh = (struct ocfs2_xattr_header *)xs->bucket.bu_bhs[0]->b_data;
+	xs->header = xs->bucket.bu_xh;
 
 	xs->base = new_bh->b_data;
 	xs->end = xs->base + inode->i_sb->s_blocksize;
@@ -2630,8 +2630,8 @@ static int ocfs2_xattr_update_xattr_search(struct inode *inode,
 	if (!xs->not_found) {
 		if (OCFS2_XATTR_BUCKET_SIZE != blocksize) {
 			ret = ocfs2_read_blocks(inode,
-					xs->bucket.bhs[0]->b_blocknr + 1,
-					blk_per_bucket - 1, &xs->bucket.bhs[1],
+					xs->bucket.bu_bhs[0]->b_blocknr + 1,
+					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
 					0);
 			if (ret) {
 				mlog_errno(ret);
@@ -2827,7 +2827,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 	size_t end, offset, len, value_len;
 	struct ocfs2_xattr_header *xh;
 	char *entries, *buf, *bucket_buf = NULL;
-	u64 blkno = bucket->bhs[0]->b_blocknr;
+	u64 blkno = bucket->bu_bhs[0]->b_blocknr;
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	u16 xh_free_start;
 	size_t blocksize = inode->i_sb->s_blocksize;
@@ -3844,7 +3844,7 @@ static inline char *ocfs2_xattr_bucket_get_val(struct inode *inode,
 	int block_off = offs >> inode->i_sb->s_blocksize_bits;
 
 	offs = offs % inode->i_sb->s_blocksize;
-	return bucket->bhs[block_off]->b_data + offs;
+	return bucket->bu_bhs[block_off]->b_data + offs;
 }
 
 /*
@@ -4039,12 +4039,12 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 
 	mlog(0, "Set xattr entry len = %lu index = %d in bucket %llu\n",
 	     (unsigned long)xi->value_len, xi->name_index,
-	     (unsigned long long)xs->bucket.bhs[0]->b_blocknr);
+	     (unsigned long long)xs->bucket.bu_bhs[0]->b_blocknr);
 
-	if (!xs->bucket.bhs[1]) {
+	if (!xs->bucket.bu_bhs[1]) {
 		ret = ocfs2_read_blocks(inode,
-					xs->bucket.bhs[0]->b_blocknr + 1,
-					blk_per_bucket - 1, &xs->bucket.bhs[1],
+					xs->bucket.bu_bhs[0]->b_blocknr + 1,
+					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
 					0);
 		if (ret) {
 			mlog_errno(ret);
@@ -4061,7 +4061,7 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 	}
 
 	for (i = 0; i < blk_per_bucket; i++) {
-		ret = ocfs2_journal_access(handle, inode, xs->bucket.bhs[i],
+		ret = ocfs2_journal_access(handle, inode, xs->bucket.bu_bhs[i],
 					   OCFS2_JOURNAL_ACCESS_WRITE);
 		if (ret < 0) {
 			mlog_errno(ret);
@@ -4073,7 +4073,7 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 
 	/*Only dirty the blocks we have touched in set xattr. */
 	ret = ocfs2_xattr_bucket_handle_journal(inode, handle, xs,
-						xs->bucket.bhs, blk_per_bucket);
+						xs->bucket.bu_bhs, blk_per_bucket);
 	if (ret)
 		mlog_errno(ret);
 out:
@@ -4187,10 +4187,10 @@ static int ocfs2_xattr_bucket_value_truncate_xs(struct inode *inode,
 	struct ocfs2_xattr_entry *xe = xs->here;
 	struct ocfs2_xattr_header *xh = (struct ocfs2_xattr_header *)xs->base;
 
-	BUG_ON(!xs->bucket.bhs[0] || !xe || ocfs2_xattr_is_local(xe));
+	BUG_ON(!xs->bucket.bu_bhs[0] || !xe || ocfs2_xattr_is_local(xe));
 
 	offset = xe - xh->xh_entries;
-	ret = ocfs2_xattr_bucket_value_truncate(inode, xs->bucket.bhs[0],
+	ret = ocfs2_xattr_bucket_value_truncate(inode, xs->bucket.bu_bhs[0],
 						offset, len);
 	if (ret)
 		mlog_errno(ret);
@@ -4310,7 +4310,7 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 					 struct ocfs2_xattr_search *xs)
 {
 	handle_t *handle = NULL;
-	struct ocfs2_xattr_header *xh = xs->bucket.xh;
+	struct ocfs2_xattr_header *xh = xs->bucket.bu_xh;
 	struct ocfs2_xattr_entry *last = &xh->xh_entries[
 						le16_to_cpu(xh->xh_count) - 1];
 	int ret = 0;
@@ -4322,7 +4322,7 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 		return;
 	}
 
-	ret = ocfs2_journal_access(handle, inode, xs->bucket.bhs[0],
+	ret = ocfs2_journal_access(handle, inode, xs->bucket.bu_bhs[0],
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
@@ -4335,7 +4335,7 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 	memset(last, 0, sizeof(struct ocfs2_xattr_entry));
 	le16_add_cpu(&xh->xh_count, -1);
 
-	ret = ocfs2_journal_dirty(handle, xs->bucket.bhs[0]);
+	ret = ocfs2_journal_dirty(handle, xs->bucket.bu_bhs[0]);
 	if (ret < 0)
 		mlog_errno(ret);
 out_commit:
@@ -4439,13 +4439,13 @@ out:
 static int ocfs2_check_xattr_bucket_collision(struct inode *inode,
 					      struct ocfs2_xattr_bucket *bucket)
 {
-	struct ocfs2_xattr_header *xh = bucket->xh;
+	struct ocfs2_xattr_header *xh = bucket->bu_xh;
 
 	if (xh->xh_entries[le16_to_cpu(xh->xh_count) - 1].xe_name_hash ==
 	    xh->xh_entries[0].xe_name_hash) {
 		mlog(ML_ERROR, "Too much hash collision in xattr bucket %llu, "
 		     "hash = %u\n",
-		     (unsigned long long)bucket->bhs[0]->b_blocknr,
+		     (unsigned long long)bucket->bu_bhs[0]->b_blocknr,
 		     le32_to_cpu(xh->xh_entries[0].xe_name_hash));
 		return -ENOSPC;
 	}
@@ -4479,7 +4479,7 @@ try_again:
 
 	mlog_bug_on_msg(header_size > blocksize, "bucket %llu has header size "
 			"of %u which exceed block size\n",
-			(unsigned long long)xs->bucket.bhs[0]->b_blocknr,
+			(unsigned long long)xs->bucket.bu_bhs[0]->b_blocknr,
 			header_size);
 
 	if (xi->value && xi->value_len > OCFS2_XATTR_INLINE_SIZE)
@@ -4519,7 +4519,7 @@ try_again:
 	mlog(0, "xs->not_found = %d, in xattr bucket %llu: free = %d, "
 	     "need = %d, max_free = %d, xh_free_start = %u, xh_name_value_len ="
 	     " %u\n", xs->not_found,
-	     (unsigned long long)xs->bucket.bhs[0]->b_blocknr,
+	     (unsigned long long)xs->bucket.bu_bhs[0]->b_blocknr,
 	     free, need, max_free, le16_to_cpu(xh->xh_free_start),
 	     le16_to_cpu(xh->xh_name_value_len));
 
@@ -4570,14 +4570,14 @@ try_again:
 
 		ret = ocfs2_add_new_xattr_bucket(inode,
 						 xs->xattr_bh,
-						 xs->bucket.bhs[0]);
+						 xs->bucket.bu_bhs[0]);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;
 		}
 
 		for (i = 0; i < blk_per_bucket; i++)
-			brelse(xs->bucket.bhs[i]);
+			brelse(xs->bucket.bu_bhs[i]);
 
 		memset(&xs->bucket, 0, sizeof(xs->bucket));
 
@@ -4603,7 +4603,7 @@ static int ocfs2_delete_xattr_in_bucket(struct inode *inode,
 					void *para)
 {
 	int ret = 0;
-	struct ocfs2_xattr_header *xh = bucket->xh;
+	struct ocfs2_xattr_header *xh = bucket->bu_xh;
 	u16 i;
 	struct ocfs2_xattr_entry *xe;
 
@@ -4613,7 +4613,7 @@ static int ocfs2_delete_xattr_in_bucket(struct inode *inode,
 			continue;
 
 		ret = ocfs2_xattr_bucket_value_truncate(inode,
-							bucket->bhs[0],
+							bucket->bu_bhs[0],
 							i, 0);
 		if (ret) {
 			mlog_errno(ret);
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 02/13] ocfs2: Convenient access to an xattr bucket's block number.

[Joel Becker]

The xattr code often wants to know the block number of an xattr bucket.
This is usually found by dereferencing the first bh hanging off of the
ocfs2_xattr_bucket structure.  Rather than do this all the time, let's
provide a nice little macro.  The idea is ripped from the ocfs2_path
code.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index fb8b401..7b320b2 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -154,6 +154,8 @@ static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
 	return len / sizeof(struct ocfs2_xattr_entry);
 }
 
+#define bucket_blkno(_b) ((_b)->bu_bhs[0]->b_blocknr)
+
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
@@ -2285,7 +2287,7 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 		 * If we have found the xattr enty, read all the blocks in
 		 * this bucket.
 		 */
-		ret = ocfs2_read_blocks(inode, xs->bucket.bu_bhs[0]->b_blocknr + 1,
+		ret = ocfs2_read_blocks(inode, bucket_blkno(&xs->bucket) + 1,
 					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
 					0);
 		if (ret) {
@@ -2295,7 +2297,7 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 
 		xs->here = &xs->header->xh_entries[index];
 		mlog(0, "find xattr %s in bucket %llu, entry = %u\n", name,
-		     (unsigned long long)xs->bucket.bu_bhs[0]->b_blocknr, index);
+		     (unsigned long long)bucket_blkno(&xs->bucket), index);
 	} else
 		ret = -ENODATA;
 
@@ -2630,7 +2632,7 @@ static int ocfs2_xattr_update_xattr_search(struct inode *inode,
 	if (!xs->not_found) {
 		if (OCFS2_XATTR_BUCKET_SIZE != blocksize) {
 			ret = ocfs2_read_blocks(inode,
-					xs->bucket.bu_bhs[0]->b_blocknr + 1,
+					bucket_blkno(&xs->bucket) + 1,
 					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
 					0);
 			if (ret) {
@@ -2827,7 +2829,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 	size_t end, offset, len, value_len;
 	struct ocfs2_xattr_header *xh;
 	char *entries, *buf, *bucket_buf = NULL;
-	u64 blkno = bucket->bu_bhs[0]->b_blocknr;
+	u64 blkno = bucket_blkno(bucket);
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	u16 xh_free_start;
 	size_t blocksize = inode->i_sb->s_blocksize;
@@ -4039,11 +4041,11 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 
 	mlog(0, "Set xattr entry len = %lu index = %d in bucket %llu\n",
 	     (unsigned long)xi->value_len, xi->name_index,
-	     (unsigned long long)xs->bucket.bu_bhs[0]->b_blocknr);
+	     (unsigned long long)bucket_blkno(&xs->bucket));
 
 	if (!xs->bucket.bu_bhs[1]) {
 		ret = ocfs2_read_blocks(inode,
-					xs->bucket.bu_bhs[0]->b_blocknr + 1,
+					bucket_blkno(&xs->bucket) + 1,
 					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
 					0);
 		if (ret) {
@@ -4445,7 +4447,7 @@ static int ocfs2_check_xattr_bucket_collision(struct inode *inode,
 	    xh->xh_entries[0].xe_name_hash) {
 		mlog(ML_ERROR, "Too much hash collision in xattr bucket %llu, "
 		     "hash = %u\n",
-		     (unsigned long long)bucket->bu_bhs[0]->b_blocknr,
+		     (unsigned long long)bucket_blkno(bucket),
 		     le32_to_cpu(xh->xh_entries[0].xe_name_hash));
 		return -ENOSPC;
 	}
@@ -4479,7 +4481,7 @@ try_again:
 
 	mlog_bug_on_msg(header_size > blocksize, "bucket %llu has header size "
 			"of %u which exceed block size\n",
-			(unsigned long long)xs->bucket.bu_bhs[0]->b_blocknr,
+			(unsigned long long)bucket_blkno(&xs->bucket),
 			header_size);
 
 	if (xi->value && xi->value_len > OCFS2_XATTR_INLINE_SIZE)
@@ -4519,7 +4521,7 @@ try_again:
 	mlog(0, "xs->not_found = %d, in xattr bucket %llu: free = %d, "
 	     "need = %d, max_free = %d, xh_free_start = %u, xh_name_value_len ="
 	     " %u\n", xs->not_found,
-	     (unsigned long long)xs->bucket.bu_bhs[0]->b_blocknr,
+	     (unsigned long long)bucket_blkno(&xs->bucket),
 	     free, need, max_free, le16_to_cpu(xh->xh_free_start),
 	     le16_to_cpu(xh->xh_name_value_len));
 
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 03/13] ocfs2: Convenient access to xattr bucket data blocks.

[Joel Becker]

The xattr code often wants to access the data pointer for blocks in an
xattr bucket.  This is usually found by dereferencing the bh array
hanging off of the ocfs2_xattr_bucket structure.  Rather than do this
all the time, let's provide a nice little macro.  The idea is ripped
from the ocfs2_path code.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 7b320b2..7142554 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -155,6 +155,7 @@ static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
 }
 
 #define bucket_blkno(_b) ((_b)->bu_bhs[0]->b_blocknr)
+#define bucket_block(_b, _n) ((_b)->bu_bhs[(_n)]->b_data)
 
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
@@ -796,7 +797,7 @@ static int ocfs2_xattr_block_get(struct inode *inode,
 								i,
 								&block_off,
 								&name_offset);
-			xs->base = xs->bucket.bu_bhs[block_off]->b_data;
+			xs->base = bucket_block(&xs->bucket, block_off);
 		}
 		if (ocfs2_xattr_is_local(xs->here)) {
 			memcpy(buffer, (void *)xs->base +
@@ -2275,11 +2276,11 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 	}
 	xs->bucket.bu_bhs[0] = lower_bh;
 	xs->bucket.bu_xh = (struct ocfs2_xattr_header *)
-					xs->bucket.bu_bhs[0]->b_data;
+					bucket_block(&xs->bucket, 0);
 	lower_bh = NULL;
 
 	xs->header = xs->bucket.bu_xh;
-	xs->base = xs->bucket.bu_bhs[0]->b_data;
+	xs->base = bucket_block(&xs->bucket, 0);
 	xs->end = xs->base + inode->i_sb->s_blocksize;
 
 	if (found) {
@@ -2372,7 +2373,7 @@ static int ocfs2_iterate_xattr_buckets(struct inode *inode,
 			goto out;
 		}
 
-		bucket.bu_xh = (struct ocfs2_xattr_header *)bucket.bu_bhs[0]->b_data;
+		bucket.bu_xh = (struct ocfs2_xattr_header *)bucket_block(&bucket, 0);
 		/*
 		 * The real bucket num in this series of blocks is stored
 		 * in the 1st bucket.
@@ -2450,7 +2451,7 @@ static int ocfs2_list_xattr_bucket(struct inode *inode,
 			if (ret)
 				break;
 
-			name = (const char *)bucket->bu_bhs[block_off]->b_data +
+			name = (const char *)bucket_block(bucket, block_off) +
 				new_offset;
 			ret = ocfs2_xattr_list_entry(xl->buffer,
 						     xl->buffer_size,
@@ -2623,7 +2624,7 @@ static int ocfs2_xattr_update_xattr_search(struct inode *inode,
 
 	xs->bucket.bu_bhs[0] = new_bh;
 	get_bh(new_bh);
-	xs->bucket.bu_xh = (struct ocfs2_xattr_header *)xs->bucket.bu_bhs[0]->b_data;
+	xs->bucket.bu_xh = (struct ocfs2_xattr_header *)bucket_block(&xs->bucket, 0);
 	xs->header = xs->bucket.bu_xh;
 
 	xs->base = new_bh->b_data;
@@ -3846,7 +3847,7 @@ static inline char *ocfs2_xattr_bucket_get_val(struct inode *inode,
 	int block_off = offs >> inode->i_sb->s_blocksize_bits;
 
 	offs = offs % inode->i_sb->s_blocksize;
-	return bucket->bu_bhs[block_off]->b_data + offs;
+	return bucket_block(bucket, block_off) + offs;
 }
 
 /*
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 04/13] ocfs2: Convenient access to an xattr bucket's header.

[Joel Becker]

The xattr code often wants to access the ocfs2_xattr_header at the start
of an bucket.  Rather than walk the pointer chains, let's just create
another nice macro.  As a side benefit, we can get rid of the mostly
spurious ->bu_xh element on the bucket structure.  The idea is ripped
from the ocfs2_path code.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |   28 ++++++++++++----------------
 1 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 7142554..ff1cd6c 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -62,7 +62,6 @@ struct ocfs2_xattr_def_value_root {
 
 struct ocfs2_xattr_bucket {
 	struct buffer_head *bu_bhs[OCFS2_XATTR_MAX_BLOCKS_PER_BUCKET];
-	struct ocfs2_xattr_header *bu_xh;
 };
 
 #define OCFS2_XATTR_ROOT_SIZE	(sizeof(struct ocfs2_xattr_def_value_root))
@@ -156,6 +155,7 @@ static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
 
 #define bucket_blkno(_b) ((_b)->bu_bhs[0]->b_blocknr)
 #define bucket_block(_b, _n) ((_b)->bu_bhs[(_n)]->b_data)
+#define bucket_xh(_b) ((struct ocfs2_xattr_header *)bucket_block((_b), 0))
 
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
@@ -793,7 +793,7 @@ static int ocfs2_xattr_block_get(struct inode *inode,
 
 		if (le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED) {
 			ret = ocfs2_xattr_bucket_get_name_value(inode,
-								xs->bucket.bu_xh,
+								bucket_xh(&xs->bucket),
 								i,
 								&block_off,
 								&name_offset);
@@ -2275,11 +2275,9 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 		bh = NULL;
 	}
 	xs->bucket.bu_bhs[0] = lower_bh;
-	xs->bucket.bu_xh = (struct ocfs2_xattr_header *)
-					bucket_block(&xs->bucket, 0);
 	lower_bh = NULL;
 
-	xs->header = xs->bucket.bu_xh;
+	xs->header = bucket_xh(&xs->bucket);
 	xs->base = bucket_block(&xs->bucket, 0);
 	xs->end = xs->base + inode->i_sb->s_blocksize;
 
@@ -2373,16 +2371,15 @@ static int ocfs2_iterate_xattr_buckets(struct inode *inode,
 			goto out;
 		}
 
-		bucket.bu_xh = (struct ocfs2_xattr_header *)bucket_block(&bucket, 0);
 		/*
 		 * The real bucket num in this series of blocks is stored
 		 * in the 1st bucket.
 		 */
 		if (i == 0)
-			num_buckets = le16_to_cpu(bucket.bu_xh->xh_num_buckets);
+			num_buckets = le16_to_cpu(bucket_xh(&bucket)->xh_num_buckets);
 
 		mlog(0, "iterating xattr bucket %llu, first hash %u\n", blkno,
-		     le32_to_cpu(bucket.bu_xh->xh_entries[0].xe_name_hash));
+		     le32_to_cpu(bucket_xh(&bucket)->xh_entries[0].xe_name_hash));
 		if (func) {
 			ret = func(inode, &bucket, para);
 			if (ret) {
@@ -2437,14 +2434,14 @@ static int ocfs2_list_xattr_bucket(struct inode *inode,
 	int i, block_off, new_offset;
 	const char *prefix, *name;
 
-	for (i = 0 ; i < le16_to_cpu(bucket->bu_xh->xh_count); i++) {
-		struct ocfs2_xattr_entry *entry = &bucket->bu_xh->xh_entries[i];
+	for (i = 0 ; i < le16_to_cpu(bucket_xh(bucket)->xh_count); i++) {
+		struct ocfs2_xattr_entry *entry = &bucket_xh(bucket)->xh_entries[i];
 		type = ocfs2_xattr_get_type(entry);
 		prefix = ocfs2_xattr_prefix(type);
 
 		if (prefix) {
 			ret = ocfs2_xattr_bucket_get_name_value(inode,
-								bucket->bu_xh,
+								bucket_xh(bucket),
 								i,
 								&block_off,
 								&new_offset);
@@ -2624,8 +2621,7 @@ static int ocfs2_xattr_update_xattr_search(struct inode *inode,
 
 	xs->bucket.bu_bhs[0] = new_bh;
 	get_bh(new_bh);
-	xs->bucket.bu_xh = (struct ocfs2_xattr_header *)bucket_block(&xs->bucket, 0);
-	xs->header = xs->bucket.bu_xh;
+	xs->header = bucket_xh(&xs->bucket);
 
 	xs->base = new_bh->b_data;
 	xs->end = xs->base + inode->i_sb->s_blocksize;
@@ -4313,7 +4309,7 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 					 struct ocfs2_xattr_search *xs)
 {
 	handle_t *handle = NULL;
-	struct ocfs2_xattr_header *xh = xs->bucket.bu_xh;
+	struct ocfs2_xattr_header *xh = bucket_xh(&xs->bucket);
 	struct ocfs2_xattr_entry *last = &xh->xh_entries[
 						le16_to_cpu(xh->xh_count) - 1];
 	int ret = 0;
@@ -4442,7 +4438,7 @@ out:
 static int ocfs2_check_xattr_bucket_collision(struct inode *inode,
 					      struct ocfs2_xattr_bucket *bucket)
 {
-	struct ocfs2_xattr_header *xh = bucket->bu_xh;
+	struct ocfs2_xattr_header *xh = bucket_xh(bucket);
 
 	if (xh->xh_entries[le16_to_cpu(xh->xh_count) - 1].xe_name_hash ==
 	    xh->xh_entries[0].xe_name_hash) {
@@ -4606,7 +4602,7 @@ static int ocfs2_delete_xattr_in_bucket(struct inode *inode,
 					void *para)
 {
 	int ret = 0;
-	struct ocfs2_xattr_header *xh = bucket->bu_xh;
+	struct ocfs2_xattr_header *xh = bucket_xh(bucket);
 	u16 i;
 	struct ocfs2_xattr_entry *xe;
 
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 05/13] ocfs2: Provide a wrapper to brelse() xattr bucket buffers.

[Joel Becker]

A common theme is walking all the buffer heads on an ocfs2_xattr_bucket
and releasing them.  Let's wrap that.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |   33 ++++++++++++++++++---------------
 1 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index ff1cd6c..1f0569f 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -157,6 +157,17 @@ static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
 #define bucket_block(_b, _n) ((_b)->bu_bhs[(_n)]->b_data)
 #define bucket_xh(_b) ((struct ocfs2_xattr_header *)bucket_block((_b), 0))
 
+static void ocfs2_xattr_bucket_relse(struct inode *inode,
+				     struct ocfs2_xattr_bucket *bucket)
+{
+	int i, blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+
+	for (i = 0; i < blks; i++) {
+		brelse(bucket->bu_bhs[i]);
+		bucket->bu_bhs[i] = NULL;
+	}
+}
+
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
@@ -815,8 +826,7 @@ static int ocfs2_xattr_block_get(struct inode *inode,
 	}
 	ret = size;
 cleanup:
-	for (i = 0; i < OCFS2_XATTR_MAX_BLOCKS_PER_BUCKET; i++)
-		brelse(xs->bucket.bu_bhs[i]);
+	ocfs2_xattr_bucket_relse(inode, &xs->bucket);
 	memset(&xs->bucket, 0, sizeof(xs->bucket));
 
 	brelse(xs->xattr_bh);
@@ -1927,7 +1937,6 @@ int ocfs2_xattr_set(struct inode *inode,
 	struct buffer_head *di_bh = NULL;
 	struct ocfs2_dinode *di;
 	int ret;
-	u16 i, blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 
 	struct ocfs2_xattr_info xi = {
 		.name_index = name_index,
@@ -2029,8 +2038,7 @@ cleanup:
 	ocfs2_inode_unlock(inode, 1);
 	brelse(di_bh);
 	brelse(xbs.xattr_bh);
-	for (i = 0; i < blk_per_bucket; i++)
-		brelse(xbs.bucket.bu_bhs[i]);
+	ocfs2_xattr_bucket_relse(inode, &xbs.bucket);
 
 	return ret;
 }
@@ -2352,7 +2360,7 @@ static int ocfs2_iterate_xattr_buckets(struct inode *inode,
 				       xattr_bucket_func *func,
 				       void *para)
 {
-	int i, j, ret = 0;
+	int i, ret = 0;
 	int blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	u32 bpc = ocfs2_xattr_buckets_per_cluster(OCFS2_SB(inode->i_sb));
 	u32 num_buckets = clusters * bpc;
@@ -2388,14 +2396,12 @@ static int ocfs2_iterate_xattr_buckets(struct inode *inode,
 			}
 		}
 
-		for (j = 0; j < blk_per_bucket; j++)
-			brelse(bucket.bu_bhs[j]);
+		ocfs2_xattr_bucket_relse(inode, &bucket);
 		memset(&bucket, 0, sizeof(bucket));
 	}
 
 out:
-	for (j = 0; j < blk_per_bucket; j++)
-		brelse(bucket.bu_bhs[j]);
+	ocfs2_xattr_bucket_relse(inode, &bucket);
 
 	return ret;
 }
@@ -4459,11 +4465,10 @@ static int ocfs2_xattr_set_entry_index_block(struct inode *inode,
 	struct ocfs2_xattr_header *xh;
 	struct ocfs2_xattr_entry *xe;
 	u16 count, header_size, xh_free_start;
-	int i, free, max_free, need, old;
+	int free, max_free, need, old;
 	size_t value_size = 0, name_len = strlen(xi->name);
 	size_t blocksize = inode->i_sb->s_blocksize;
 	int ret, allocation = 0;
-	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 
 	mlog_entry("Set xattr %s in xattr index block\n", xi->name);
 
@@ -4575,9 +4580,7 @@ try_again:
 			goto out;
 		}
 
-		for (i = 0; i < blk_per_bucket; i++)
-			brelse(xs->bucket.bu_bhs[i]);
-
+		ocfs2_xattr_bucket_relse(inode, &xs->bucket);
 		memset(&xs->bucket, 0, sizeof(xs->bucket));
 
 		ret = ocfs2_xattr_index_block_find(inode, xs->xattr_bh,
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 06/13] ocfs2: Improve ocfs2_read_xattr_bucket().

[Joel Becker]

The ocfs2_read_xattr_bucket() function would read an xattr bucket into a
list of buffer heads.  However, we have a nice ocfs2_xattr_bucket
structure.  Let's have it fill that out instead.

In addition, ocfs2_read_xattr_bucket() would initialize buffer heads for
a bucket that's never been on disk before.  That's confusing.  Let's
call that functionality ocfs2_init_xattr_bucket().

The functions ocfs2_cp_xattr_bucket() and ocfs2_half_xattr_bucket() are
updated to use the ocfs2_xattr_bucket structure rather than raw bh
lists.  That way they can use the new read/init calls.  In addition,
they drop the wasted read of an existing target bucket.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |  159 ++++++++++++++++++++++++++----------------------------
 1 files changed, 76 insertions(+), 83 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 1f0569f..61dee99 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -168,6 +168,48 @@ static void ocfs2_xattr_bucket_relse(struct inode *inode,
 	}
 }
 
+/*
+ * A bucket that has never been written to disk doesn't need to be
+ * read.  We just need the buffer_heads.  Don't call this for
+ * buckets that are already on disk.  ocfs2_read_xattr_bucket() initializes
+ * them fully.
+ */
+static int ocfs2_init_xattr_bucket(struct inode *inode,
+				   struct ocfs2_xattr_bucket *bucket,
+				   u64 xb_blkno)
+{
+	int i, rc = 0;
+	int blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+
+	for (i = 0; i < blks; i++) {
+		bucket->bu_bhs[i] = sb_getblk(inode->i_sb, xb_blkno + i);
+		if (!bucket->bu_bhs[i]) {
+			rc = -EIO;
+			mlog_errno(rc);
+			break;
+		}
+
+		ocfs2_set_new_buffer_uptodate(inode, bucket->bu_bhs[i]);
+	}
+
+	if (rc)
+		ocfs2_xattr_bucket_relse(inode, bucket);
+	return rc;
+}
+
+/* Read the xattr bucket at xb_blkno */
+static int ocfs2_read_xattr_bucket(struct inode *inode,
+				   struct ocfs2_xattr_bucket *bucket,
+				   u64 xb_blkno)
+{
+	int rc, blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+
+	rc = ocfs2_read_blocks(inode, xb_blkno, blks, bucket->bu_bhs, 0);
+	if (rc)
+		ocfs2_xattr_bucket_relse(inode, bucket);
+	return rc;
+}
+
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
@@ -3089,31 +3131,6 @@ out:
 	return ret;
 }
 
-static int ocfs2_read_xattr_bucket(struct inode *inode,
-				   u64 blkno,
-				   struct buffer_head **bhs,
-				   int new)
-{
-	int ret = 0;
-	u16 i, blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
-
-	if (!new)
-		return ocfs2_read_blocks(inode, blkno,
-					 blk_per_bucket, bhs, 0);
-
-	for (i = 0; i < blk_per_bucket; i++) {
-		bhs[i] = sb_getblk(inode->i_sb, blkno + i);
-		if (bhs[i] == NULL) {
-			ret = -EIO;
-			mlog_errno(ret);
-			break;
-		}
-		ocfs2_set_new_buffer_uptodate(inode, bhs[i]);
-	}
-
-	return ret;
-}
-
 /*
  * Move half num of the xattrs in old bucket(blk) to new bucket(new_blk).
  * first_hash will record the 1st hash of the new bucket.
@@ -3128,7 +3145,7 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	int ret, i;
 	u16 count, start, len, name_value_len, xe_len, name_offset;
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
-	struct buffer_head **s_bhs, **t_bhs = NULL;
+	struct ocfs2_xattr_bucket s_bucket, t_bucket;
 	struct ocfs2_xattr_header *xh;
 	struct ocfs2_xattr_entry *xe;
 	int blocksize = inode->i_sb->s_blocksize;
@@ -3136,37 +3153,34 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	mlog(0, "move half of xattrs from bucket %llu to %llu\n",
 	     blk, new_blk);
 
-	s_bhs = kcalloc(blk_per_bucket, sizeof(struct buffer_head *), GFP_NOFS);
-	if (!s_bhs)
-		return -ENOMEM;
+	memset(&s_bucket, 0, sizeof(struct ocfs2_xattr_bucket));
+	memset(&t_bucket, 0, sizeof(struct ocfs2_xattr_bucket));
 
-	ret = ocfs2_read_xattr_bucket(inode, blk, s_bhs, 0);
+	ret = ocfs2_read_xattr_bucket(inode, &s_bucket, blk);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;
 	}
 
-	ret = ocfs2_journal_access(handle, inode, s_bhs[0],
+	ret = ocfs2_journal_access(handle, inode, s_bucket.bu_bhs[0],
 				   OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;
 	}
 
-	t_bhs = kcalloc(blk_per_bucket, sizeof(struct buffer_head *), GFP_NOFS);
-	if (!t_bhs) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	ret = ocfs2_read_xattr_bucket(inode, new_blk, t_bhs, new_bucket_head);
+	/*
+	 * Even if !new_bucket_head, we're overwriting t_bucket.  Thus,
+	 * there's no need to read it.
+	 */
+	ret = ocfs2_init_xattr_bucket(inode, &t_bucket, new_blk);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;
 	}
 
 	for (i = 0; i < blk_per_bucket; i++) {
-		ret = ocfs2_journal_access(handle, inode, t_bhs[i],
+		ret = ocfs2_journal_access(handle, inode, t_bucket.bu_bhs[i],
 					   new_bucket_head ?
 					   OCFS2_JOURNAL_ACCESS_CREATE :
 					   OCFS2_JOURNAL_ACCESS_WRITE);
@@ -3178,10 +3192,11 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 
 	/* copy the whole bucket to the new first. */
 	for (i = 0; i < blk_per_bucket; i++)
-		memcpy(t_bhs[i]->b_data, s_bhs[i]->b_data, blocksize);
+		memcpy(bucket_block(&t_bucket, i), bucket_block(&s_bucket, i),
+		       blocksize);
 
 	/* update the new bucket. */
-	xh = (struct ocfs2_xattr_header *)t_bhs[0]->b_data;
+	xh = bucket_xh(&t_bucket);
 	count = le16_to_cpu(xh->xh_count);
 	start = count / 2;
 
@@ -3247,7 +3262,7 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 		xh->xh_num_buckets = 0;
 
 	for (i = 0; i < blk_per_bucket; i++) {
-		ocfs2_journal_dirty(handle, t_bhs[i]);
+		ocfs2_journal_dirty(handle, t_bucket.bu_bhs[i]);
 		if (ret)
 			mlog_errno(ret);
 	}
@@ -3260,29 +3275,20 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	 * Now only update the 1st block of the old bucket.
 	 * Please note that the entry has been sorted already above.
 	 */
-	xh = (struct ocfs2_xattr_header *)s_bhs[0]->b_data;
+	xh = bucket_xh(&s_bucket);
 	memset(&xh->xh_entries[start], 0,
 	       sizeof(struct ocfs2_xattr_entry) * (count - start));
 	xh->xh_count = cpu_to_le16(start);
 	xh->xh_free_start = cpu_to_le16(name_offset);
 	xh->xh_name_value_len = cpu_to_le16(name_value_len);
 
-	ocfs2_journal_dirty(handle, s_bhs[0]);
+	ocfs2_journal_dirty(handle, s_bucket.bu_bhs[0]);
 	if (ret)
 		mlog_errno(ret);
 
 out:
-	if (s_bhs) {
-		for (i = 0; i < blk_per_bucket; i++)
-			brelse(s_bhs[i]);
-	}
-	kfree(s_bhs);
-
-	if (t_bhs) {
-		for (i = 0; i < blk_per_bucket; i++)
-			brelse(t_bhs[i]);
-	}
-	kfree(t_bhs);
+	ocfs2_xattr_bucket_relse(inode, &s_bucket);
+	ocfs2_xattr_bucket_relse(inode, &t_bucket);
 
 	return ret;
 }
@@ -3302,35 +3308,30 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode,
 	int ret, i;
 	int blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	int blocksize = inode->i_sb->s_blocksize;
-	struct buffer_head **s_bhs, **t_bhs = NULL;
+	struct ocfs2_xattr_bucket s_bucket, t_bucket;
 
 	BUG_ON(s_blkno == t_blkno);
 
 	mlog(0, "cp bucket %llu to %llu, target is %d\n",
 	     s_blkno, t_blkno, t_is_new);
 
-	s_bhs = kzalloc(sizeof(struct buffer_head *) * blk_per_bucket,
-			GFP_NOFS);
-	if (!s_bhs)
-		return -ENOMEM;
+	memset(&s_bucket, 0, sizeof(struct ocfs2_xattr_bucket));
+	memset(&t_bucket, 0, sizeof(struct ocfs2_xattr_bucket));
 
-	ret = ocfs2_read_xattr_bucket(inode, s_blkno, s_bhs, 0);
+	ret = ocfs2_read_xattr_bucket(inode, &s_bucket, s_blkno);
 	if (ret)
 		goto out;
 
-	t_bhs = kzalloc(sizeof(struct buffer_head *) * blk_per_bucket,
-			GFP_NOFS);
-	if (!t_bhs) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	ret = ocfs2_read_xattr_bucket(inode, t_blkno, t_bhs, t_is_new);
+	/*
+	 * Even if !t_is_new, we're overwriting t_bucket.  Thus,
+	 * there's no need to read it.
+	 */
+	ret = ocfs2_init_xattr_bucket(inode, &t_bucket, t_blkno);
 	if (ret)
 		goto out;
 
 	for (i = 0; i < blk_per_bucket; i++) {
-		ret = ocfs2_journal_access(handle, inode, t_bhs[i],
+		ret = ocfs2_journal_access(handle, inode, t_bucket.bu_bhs[i],
 					   t_is_new ?
 					   OCFS2_JOURNAL_ACCESS_CREATE :
 					   OCFS2_JOURNAL_ACCESS_WRITE);
@@ -3339,22 +3340,14 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode,
 	}
 
 	for (i = 0; i < blk_per_bucket; i++) {
-		memcpy(t_bhs[i]->b_data, s_bhs[i]->b_data, blocksize);
-		ocfs2_journal_dirty(handle, t_bhs[i]);
+		memcpy(bucket_block(&t_bucket, i), bucket_block(&s_bucket, i),
+		       blocksize);
+		ocfs2_journal_dirty(handle, t_bucket.bu_bhs[i]);
 	}
 
 out:
-	if (s_bhs) {
-		for (i = 0; i < blk_per_bucket; i++)
-			brelse(s_bhs[i]);
-	}
-	kfree(s_bhs);
-
-	if (t_bhs) {
-		for (i = 0; i < blk_per_bucket; i++)
-			brelse(t_bhs[i]);
-	}
-	kfree(t_bhs);
+	ocfs2_xattr_bucket_relse(inode, &s_bucket);
+	ocfs2_xattr_bucket_relse(inode, &t_bucket);
 
 	return ret;
 }
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 07/13] ocfs2: Wrap journal_access/journal_dirty for xattr buckets.

[Joel Becker]

A common action is to call ocfs2_journal_access() and
ocfs2_journal_dirty() on the buffer heads of an xattr bucket.  Let's
create nice wrappers.

While we're there, let's drop the places that try to be smart by writing
only the first and last blocks of a bucket.  A bucket is contiguous, so
writing the whole thing is actually more efficient.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |  140 +++++++++++++++++++++++++-----------------------------
 1 files changed, 64 insertions(+), 76 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 61dee99..55e2c1a 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -210,6 +210,37 @@ static int ocfs2_read_xattr_bucket(struct inode *inode,
 	return rc;
 }
 
+static int ocfs2_xattr_bucket_journal_access(handle_t *handle,
+					     struct inode *inode,
+					     struct ocfs2_xattr_bucket *bucket,
+					     int type)
+{
+	int i, rc = 0;
+	int blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+
+	for (i = 0; i < blks; i++) {
+		rc = ocfs2_journal_access(handle, inode,
+					  bucket->bu_bhs[i], type);
+		if (rc) {
+			mlog_errno(rc);
+			break;
+		}
+	}
+
+	return rc;
+}
+
+static void ocfs2_xattr_bucket_journal_dirty(handle_t *handle,
+					     struct inode *inode,
+					     struct ocfs2_xattr_bucket *bucket)
+{
+	int i, blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+
+	for (i = 0; i < blks; i++)
+		ocfs2_journal_dirty(handle, bucket->bu_bhs[i]);
+}
+
+
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
 	struct xattr_handler *handler = NULL;
@@ -3162,8 +3193,8 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 		goto out;
 	}
 
-	ret = ocfs2_journal_access(handle, inode, s_bucket.bu_bhs[0],
-				   OCFS2_JOURNAL_ACCESS_WRITE);
+	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &s_bucket,
+						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;
@@ -3179,15 +3210,13 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 		goto out;
 	}
 
-	for (i = 0; i < blk_per_bucket; i++) {
-		ret = ocfs2_journal_access(handle, inode, t_bucket.bu_bhs[i],
-					   new_bucket_head ?
-					   OCFS2_JOURNAL_ACCESS_CREATE :
-					   OCFS2_JOURNAL_ACCESS_WRITE);
-		if (ret) {
-			mlog_errno(ret);
-			goto out;
-		}
+	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &t_bucket,
+						new_bucket_head ?
+						OCFS2_JOURNAL_ACCESS_CREATE :
+						OCFS2_JOURNAL_ACCESS_WRITE);
+	if (ret) {
+		mlog_errno(ret);
+		goto out;
 	}
 
 	/* copy the whole bucket to the new first. */
@@ -3261,11 +3290,7 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	else
 		xh->xh_num_buckets = 0;
 
-	for (i = 0; i < blk_per_bucket; i++) {
-		ocfs2_journal_dirty(handle, t_bucket.bu_bhs[i]);
-		if (ret)
-			mlog_errno(ret);
-	}
+	ocfs2_xattr_bucket_journal_dirty(handle, inode, &t_bucket);
 
 	/* store the first_hash of the new bucket. */
 	if (first_hash)
@@ -3282,9 +3307,7 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	xh->xh_free_start = cpu_to_le16(name_offset);
 	xh->xh_name_value_len = cpu_to_le16(name_value_len);
 
-	ocfs2_journal_dirty(handle, s_bucket.bu_bhs[0]);
-	if (ret)
-		mlog_errno(ret);
+	ocfs2_xattr_bucket_journal_dirty(handle, inode, &s_bucket);
 
 out:
 	ocfs2_xattr_bucket_relse(inode, &s_bucket);
@@ -3330,20 +3353,18 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode,
 	if (ret)
 		goto out;
 
-	for (i = 0; i < blk_per_bucket; i++) {
-		ret = ocfs2_journal_access(handle, inode, t_bucket.bu_bhs[i],
-					   t_is_new ?
-					   OCFS2_JOURNAL_ACCESS_CREATE :
-					   OCFS2_JOURNAL_ACCESS_WRITE);
-		if (ret)
-			goto out;
-	}
+	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &t_bucket,
+						t_is_new ?
+						OCFS2_JOURNAL_ACCESS_CREATE :
+						OCFS2_JOURNAL_ACCESS_WRITE);
+	if (ret)
+		goto out;
 
 	for (i = 0; i < blk_per_bucket; i++) {
 		memcpy(bucket_block(&t_bucket, i), bucket_block(&s_bucket, i),
 		       blocksize);
-		ocfs2_journal_dirty(handle, t_bucket.bu_bhs[i]);
 	}
+	ocfs2_xattr_bucket_journal_dirty(handle, inode, &t_bucket);
 
 out:
 	ocfs2_xattr_bucket_relse(inode, &s_bucket);
@@ -3714,9 +3735,9 @@ static int ocfs2_extend_xattr_bucket(struct inode *inode,
 
 	/*
 	 * We will touch all the buckets after the start_bh(include it).
-	 * Add one more bucket and modify the first_bh.
+	 * Then we add one more bucket.
 	 */
-	credits = end_blk - start_blk + 2 * blk_per_bucket + 1;
+	credits = end_blk - start_blk + 3 * blk_per_bucket + 1;
 	handle = ocfs2_start_trans(osb, credits);
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);
@@ -3992,33 +4013,6 @@ set_new_name_value:
 	return;
 }
 
-static int ocfs2_xattr_bucket_handle_journal(struct inode *inode,
-					     handle_t *handle,
-					     struct ocfs2_xattr_search *xs,
-					     struct buffer_head **bhs,
-					     u16 bh_num)
-{
-	int ret = 0, off, block_off;
-	struct ocfs2_xattr_entry *xe = xs->here;
-
-	/*
-	 * First calculate all the blocks we should journal_access
-	 * and journal_dirty. The first block should always be touched.
-	 */
-	ret = ocfs2_journal_dirty(handle, bhs[0]);
-	if (ret)
-		mlog_errno(ret);
-
-	/* calc the data. */
-	off = le16_to_cpu(xe->xe_name_offset);
-	block_off = off >> inode->i_sb->s_blocksize_bits;
-	ret = ocfs2_journal_dirty(handle, bhs[block_off]);
-	if (ret)
-		mlog_errno(ret);
-
-	return ret;
-}
-
 /*
  * Set the xattr entry in the specified bucket.
  * The bucket is indicated by xs->bucket and it should have the enough
@@ -4030,7 +4024,7 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 					   u32 name_hash,
 					   int local)
 {
-	int i, ret;
+	int ret;
 	handle_t *handle = NULL;
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
@@ -4058,22 +4052,16 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 		goto out;
 	}
 
-	for (i = 0; i < blk_per_bucket; i++) {
-		ret = ocfs2_journal_access(handle, inode, xs->bucket.bu_bhs[i],
-					   OCFS2_JOURNAL_ACCESS_WRITE);
-		if (ret < 0) {
-			mlog_errno(ret);
-			goto out;
-		}
+	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &xs->bucket,
+						OCFS2_JOURNAL_ACCESS_WRITE);
+	if (ret < 0) {
+		mlog_errno(ret);
+		goto out;
 	}
 
 	ocfs2_xattr_set_entry_normal(inode, xi, xs, name_hash, local);
+	ocfs2_xattr_bucket_journal_dirty(handle, inode, &xs->bucket);
 
-	/*Only dirty the blocks we have touched in set xattr. */
-	ret = ocfs2_xattr_bucket_handle_journal(inode, handle, xs,
-						xs->bucket.bu_bhs, blk_per_bucket);
-	if (ret)
-		mlog_errno(ret);
 out:
 	ocfs2_commit_trans(osb, handle);
 
@@ -4313,15 +4301,16 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 						le16_to_cpu(xh->xh_count) - 1];
 	int ret = 0;
 
-	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)), 1);
+	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)),
+				   ocfs2_blocks_per_xattr_bucket(inode->i_sb));
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);
 		mlog_errno(ret);
 		return;
 	}
 
-	ret = ocfs2_journal_access(handle, inode, xs->bucket.bu_bhs[0],
-				   OCFS2_JOURNAL_ACCESS_WRITE);
+	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &xs->bucket,
+						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
 		goto out_commit;
@@ -4333,9 +4322,8 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 	memset(last, 0, sizeof(struct ocfs2_xattr_entry));
 	le16_add_cpu(&xh->xh_count, -1);
 
-	ret = ocfs2_journal_dirty(handle, xs->bucket.bu_bhs[0]);
-	if (ret < 0)
-		mlog_errno(ret);
+	ocfs2_xattr_bucket_journal_dirty(handle, inode, &xs->bucket);
+
 out_commit:
 	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
 }
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 08/13] ocfs2: Copy xattr buckets with a dedicated function.

[Joel Becker]

Now that the places that copy whole buckets are using struct
ocfs2_xattr_bucket, we can do the copy in a dedicated function.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 55e2c1a..cf84a67 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -240,6 +240,19 @@ static void ocfs2_xattr_bucket_journal_dirty(handle_t *handle,
 		ocfs2_journal_dirty(handle, bucket->bu_bhs[i]);
 }
 
+static void ocfs2_xattr_bucket_copy_data(struct inode *inode,
+					 struct ocfs2_xattr_bucket *dest,
+					 struct ocfs2_xattr_bucket *src)
+{
+	int i;
+	int blocksize = inode->i_sb->s_blocksize;
+	int blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+
+	for (i = 0; i < blks; i++) {
+		memcpy(bucket_block(dest, i), bucket_block(src, i),
+		       blocksize);
+	}
+}
 
 static inline const char *ocfs2_xattr_prefix(int name_index)
 {
@@ -3175,11 +3188,9 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 {
 	int ret, i;
 	u16 count, start, len, name_value_len, xe_len, name_offset;
-	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	struct ocfs2_xattr_bucket s_bucket, t_bucket;
 	struct ocfs2_xattr_header *xh;
 	struct ocfs2_xattr_entry *xe;
-	int blocksize = inode->i_sb->s_blocksize;
 
 	mlog(0, "move half of xattrs from bucket %llu to %llu\n",
 	     blk, new_blk);
@@ -3220,9 +3231,7 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	}
 
 	/* copy the whole bucket to the new first. */
-	for (i = 0; i < blk_per_bucket; i++)
-		memcpy(bucket_block(&t_bucket, i), bucket_block(&s_bucket, i),
-		       blocksize);
+	ocfs2_xattr_bucket_copy_data(inode, &t_bucket, &s_bucket);
 
 	/* update the new bucket. */
 	xh = bucket_xh(&t_bucket);
@@ -3328,9 +3337,7 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode,
 				 u64 t_blkno,
 				 int t_is_new)
 {
-	int ret, i;
-	int blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
-	int blocksize = inode->i_sb->s_blocksize;
+	int ret;
 	struct ocfs2_xattr_bucket s_bucket, t_bucket;
 
 	BUG_ON(s_blkno == t_blkno);
@@ -3360,10 +3367,7 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode,
 	if (ret)
 		goto out;
 
-	for (i = 0; i < blk_per_bucket; i++) {
-		memcpy(bucket_block(&t_bucket, i), bucket_block(&s_bucket, i),
-		       blocksize);
-	}
+	ocfs2_xattr_bucket_copy_data(inode, &t_bucket, &s_bucket);
 	ocfs2_xattr_bucket_journal_dirty(handle, inode, &t_bucket);
 
 out:
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 09/13] ocfs2: Take ocfs2_xattr_bucket structures off of the stack.

[Joel Becker]

The ocfs2_xattr_bucket structure is a nice abstraction, but it is a bit
large to have on the stack.  Just like ocfs2_path, let's allocate it
with a ocfs2_xattr_bucket_new() function.

We can now store the inode on the bucket, cleaning up all the other
bucket functions.  While we're here, we catch another place or two that
wasn't using ocfs2_read_xattr_bucket().

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |  269 ++++++++++++++++++++++++++++++++----------------------
 1 files changed, 161 insertions(+), 108 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index cf84a67..b922f00 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -61,7 +61,14 @@ struct ocfs2_xattr_def_value_root {
 };
 
 struct ocfs2_xattr_bucket {
+	/* The inode these xattrs are associated with */
+	struct inode *bu_inode;
+
+	/* The actual buffers that make up the bucket */
 	struct buffer_head *bu_bhs[OCFS2_XATTR_MAX_BLOCKS_PER_BUCKET];
+
+	/* How many blocks make up one bucket for this filesystem */
+	int bu_blocks;
 };
 
 #define OCFS2_XATTR_ROOT_SIZE	(sizeof(struct ocfs2_xattr_def_value_root))
@@ -97,7 +104,7 @@ struct ocfs2_xattr_search {
 	 */
 	struct buffer_head *xattr_bh;
 	struct ocfs2_xattr_header *header;
-	struct ocfs2_xattr_bucket bucket;
+	struct ocfs2_xattr_bucket *bucket;
 	void *base;
 	void *end;
 	struct ocfs2_xattr_entry *here;
@@ -157,69 +164,90 @@ static inline u16 ocfs2_xattr_max_xe_in_bucket(struct super_block *sb)
 #define bucket_block(_b, _n) ((_b)->bu_bhs[(_n)]->b_data)
 #define bucket_xh(_b) ((struct ocfs2_xattr_header *)bucket_block((_b), 0))
 
-static void ocfs2_xattr_bucket_relse(struct inode *inode,
-				     struct ocfs2_xattr_bucket *bucket)
+static struct ocfs2_xattr_bucket *ocfs2_xattr_bucket_new(struct inode *inode)
+{
+	struct ocfs2_xattr_bucket *bucket;
+	int blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+
+	BUG_ON(blks > OCFS2_XATTR_MAX_BLOCKS_PER_BUCKET);
+
+	bucket = kzalloc(sizeof(struct ocfs2_xattr_bucket), GFP_NOFS);
+	if (bucket) {
+		bucket->bu_inode = inode;
+		bucket->bu_blocks = blks;
+	}
+
+	return bucket;
+}
+
+static void ocfs2_xattr_bucket_relse(struct ocfs2_xattr_bucket *bucket)
 {
-	int i, blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+	int i;
 
-	for (i = 0; i < blks; i++) {
+	for (i = 0; i < bucket->bu_blocks; i++) {
 		brelse(bucket->bu_bhs[i]);
 		bucket->bu_bhs[i] = NULL;
 	}
 }
 
+static void ocfs2_xattr_bucket_free(struct ocfs2_xattr_bucket *bucket)
+{
+	if (bucket) {
+		ocfs2_xattr_bucket_relse(bucket);
+		bucket->bu_inode = NULL;
+	}
+}
+
 /*
  * A bucket that has never been written to disk doesn't need to be
  * read.  We just need the buffer_heads.  Don't call this for
  * buckets that are already on disk.  ocfs2_read_xattr_bucket() initializes
  * them fully.
  */
-static int ocfs2_init_xattr_bucket(struct inode *inode,
-				   struct ocfs2_xattr_bucket *bucket,
+static int ocfs2_init_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 				   u64 xb_blkno)
 {
 	int i, rc = 0;
-	int blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 
-	for (i = 0; i < blks; i++) {
-		bucket->bu_bhs[i] = sb_getblk(inode->i_sb, xb_blkno + i);
+	for (i = 0; i < bucket->bu_blocks; i++) {
+		bucket->bu_bhs[i] = sb_getblk(bucket->bu_inode->i_sb,
+					      xb_blkno + i);
 		if (!bucket->bu_bhs[i]) {
 			rc = -EIO;
 			mlog_errno(rc);
 			break;
 		}
 
-		ocfs2_set_new_buffer_uptodate(inode, bucket->bu_bhs[i]);
+		ocfs2_set_new_buffer_uptodate(bucket->bu_inode,
+					      bucket->bu_bhs[i]);
 	}
 
 	if (rc)
-		ocfs2_xattr_bucket_relse(inode, bucket);
+		ocfs2_xattr_bucket_relse(bucket);
 	return rc;
 }
 
 /* Read the xattr bucket at xb_blkno */
-static int ocfs2_read_xattr_bucket(struct inode *inode,
-				   struct ocfs2_xattr_bucket *bucket,
+static int ocfs2_read_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 				   u64 xb_blkno)
 {
-	int rc, blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+	int rc;
 
-	rc = ocfs2_read_blocks(inode, xb_blkno, blks, bucket->bu_bhs, 0);
+	rc = ocfs2_read_blocks(bucket->bu_inode, xb_blkno,
+			       bucket->bu_blocks, bucket->bu_bhs, 0);
 	if (rc)
-		ocfs2_xattr_bucket_relse(inode, bucket);
+		ocfs2_xattr_bucket_relse(bucket);
 	return rc;
 }
 
 static int ocfs2_xattr_bucket_journal_access(handle_t *handle,
-					     struct inode *inode,
 					     struct ocfs2_xattr_bucket *bucket,
 					     int type)
 {
 	int i, rc = 0;
-	int blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 
-	for (i = 0; i < blks; i++) {
-		rc = ocfs2_journal_access(handle, inode,
+	for (i = 0; i < bucket->bu_blocks; i++) {
+		rc = ocfs2_journal_access(handle, bucket->bu_inode,
 					  bucket->bu_bhs[i], type);
 		if (rc) {
 			mlog_errno(rc);
@@ -231,24 +259,24 @@ static int ocfs2_xattr_bucket_journal_access(handle_t *handle,
 }
 
 static void ocfs2_xattr_bucket_journal_dirty(handle_t *handle,
-					     struct inode *inode,
 					     struct ocfs2_xattr_bucket *bucket)
 {
-	int i, blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+	int i;
 
-	for (i = 0; i < blks; i++)
+	for (i = 0; i < bucket->bu_blocks; i++)
 		ocfs2_journal_dirty(handle, bucket->bu_bhs[i]);
 }
 
-static void ocfs2_xattr_bucket_copy_data(struct inode *inode,
-					 struct ocfs2_xattr_bucket *dest,
+static void ocfs2_xattr_bucket_copy_data(struct ocfs2_xattr_bucket *dest,
 					 struct ocfs2_xattr_bucket *src)
 {
 	int i;
-	int blocksize = inode->i_sb->s_blocksize;
-	int blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+	int blocksize = src->bu_inode->i_sb->s_blocksize;
+
+	BUG_ON(dest->bu_blocks != src->bu_blocks);
+	BUG_ON(dest->bu_inode != src->bu_inode);
 
-	for (i = 0; i < blks; i++) {
+	for (i = 0; i < src->bu_blocks; i++) {
 		memcpy(bucket_block(dest, i), bucket_block(src, i),
 		       blocksize);
 	}
@@ -869,7 +897,12 @@ static int ocfs2_xattr_block_get(struct inode *inode,
 	size_t size;
 	int ret = -ENODATA, name_offset, name_len, block_off, i;
 
-	memset(&xs->bucket, 0, sizeof(xs->bucket));
+	xs->bucket = ocfs2_xattr_bucket_new(inode);
+	if (!xs->bucket) {
+		ret = -ENOMEM;
+		mlog_errno(ret);
+		goto cleanup;
+	}
 
 	ret = ocfs2_xattr_block_find(inode, name_index, name, xs);
 	if (ret) {
@@ -890,11 +923,11 @@ static int ocfs2_xattr_block_get(struct inode *inode,
 
 		if (le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED) {
 			ret = ocfs2_xattr_bucket_get_name_value(inode,
-								bucket_xh(&xs->bucket),
+								bucket_xh(xs->bucket),
 								i,
 								&block_off,
 								&name_offset);
-			xs->base = bucket_block(&xs->bucket, block_off);
+			xs->base = bucket_block(xs->bucket, block_off);
 		}
 		if (ocfs2_xattr_is_local(xs->here)) {
 			memcpy(buffer, (void *)xs->base +
@@ -912,8 +945,7 @@ static int ocfs2_xattr_block_get(struct inode *inode,
 	}
 	ret = size;
 cleanup:
-	ocfs2_xattr_bucket_relse(inode, &xs->bucket);
-	memset(&xs->bucket, 0, sizeof(xs->bucket));
+	ocfs2_xattr_bucket_free(xs->bucket);
 
 	brelse(xs->xattr_bh);
 	xs->xattr_bh = NULL;
@@ -2042,6 +2074,18 @@ int ocfs2_xattr_set(struct inode *inode,
 	if (!ocfs2_supports_xattr(OCFS2_SB(inode->i_sb)))
 		return -EOPNOTSUPP;
 
+	xis.bucket = ocfs2_xattr_bucket_new(inode);
+	if (!xis.bucket) {
+		mlog_errno(-ENOMEM);
+		return -ENOMEM;
+	}
+	xbs.bucket = ocfs2_xattr_bucket_new(inode);
+	if (!xbs.bucket) {
+		ocfs2_xattr_bucket_free(xbs.bucket);
+		mlog_errno(-ENOMEM);
+		return -ENOMEM;
+	}
+
 	ret = ocfs2_inode_lock(inode, &di_bh, 1);
 	if (ret < 0) {
 		mlog_errno(ret);
@@ -2124,7 +2168,8 @@ cleanup:
 	ocfs2_inode_unlock(inode, 1);
 	brelse(di_bh);
 	brelse(xbs.xattr_bh);
-	ocfs2_xattr_bucket_relse(inode, &xbs.bucket);
+	ocfs2_xattr_bucket_free(xis.bucket);
+	ocfs2_xattr_bucket_free(xbs.bucket);
 
 	return ret;
 }
@@ -2368,11 +2413,11 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 		lower_bh = bh;
 		bh = NULL;
 	}
-	xs->bucket.bu_bhs[0] = lower_bh;
+	xs->bucket->bu_bhs[0] = lower_bh;
 	lower_bh = NULL;
 
-	xs->header = bucket_xh(&xs->bucket);
-	xs->base = bucket_block(&xs->bucket, 0);
+	xs->header = bucket_xh(xs->bucket);
+	xs->base = bucket_block(xs->bucket, 0);
 	xs->end = xs->base + inode->i_sb->s_blocksize;
 
 	if (found) {
@@ -2380,8 +2425,8 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 		 * If we have found the xattr enty, read all the blocks in
 		 * this bucket.
 		 */
-		ret = ocfs2_read_blocks(inode, bucket_blkno(&xs->bucket) + 1,
-					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
+		ret = ocfs2_read_blocks(inode, bucket_blkno(xs->bucket) + 1,
+					blk_per_bucket - 1, &xs->bucket->bu_bhs[1],
 					0);
 		if (ret) {
 			mlog_errno(ret);
@@ -2390,7 +2435,7 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 
 		xs->here = &xs->header->xh_entries[index];
 		mlog(0, "find xattr %s in bucket %llu, entry = %u\n", name,
-		     (unsigned long long)bucket_blkno(&xs->bucket), index);
+		     (unsigned long long)bucket_blkno(xs->bucket), index);
 	} else
 		ret = -ENODATA;
 
@@ -2447,22 +2492,24 @@ static int ocfs2_iterate_xattr_buckets(struct inode *inode,
 				       void *para)
 {
 	int i, ret = 0;
-	int blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	u32 bpc = ocfs2_xattr_buckets_per_cluster(OCFS2_SB(inode->i_sb));
 	u32 num_buckets = clusters * bpc;
-	struct ocfs2_xattr_bucket bucket;
+	struct ocfs2_xattr_bucket *bucket;
 
-	memset(&bucket, 0, sizeof(bucket));
+	bucket = ocfs2_xattr_bucket_new(inode);
+	if (!bucket) {
+		mlog_errno(-ENOMEM);
+		return -ENOMEM;
+	}
 
 	mlog(0, "iterating xattr buckets in %u clusters starting from %llu\n",
 	     clusters, blkno);
 
-	for (i = 0; i < num_buckets; i++, blkno += blk_per_bucket) {
-		ret = ocfs2_read_blocks(inode, blkno, blk_per_bucket,
-					bucket.bu_bhs, 0);
+	for (i = 0; i < num_buckets; i++, blkno += bucket->bu_blocks) {
+		ret = ocfs2_read_xattr_bucket(bucket, blkno);
 		if (ret) {
 			mlog_errno(ret);
-			goto out;
+			break;
 		}
 
 		/*
@@ -2470,25 +2517,23 @@ static int ocfs2_iterate_xattr_buckets(struct inode *inode,
 		 * in the 1st bucket.
 		 */
 		if (i == 0)
-			num_buckets = le16_to_cpu(bucket_xh(&bucket)->xh_num_buckets);
+			num_buckets = le16_to_cpu(bucket_xh(bucket)->xh_num_buckets);
 
 		mlog(0, "iterating xattr bucket %llu, first hash %u\n", blkno,
-		     le32_to_cpu(bucket_xh(&bucket)->xh_entries[0].xe_name_hash));
+		     le32_to_cpu(bucket_xh(bucket)->xh_entries[0].xe_name_hash));
 		if (func) {
-			ret = func(inode, &bucket, para);
-			if (ret) {
+			ret = func(inode, bucket, para);
+			if (ret)
 				mlog_errno(ret);
-				break;
-			}
+			/* Fall through to bucket_relse() */
 		}
 
-		ocfs2_xattr_bucket_relse(inode, &bucket);
-		memset(&bucket, 0, sizeof(bucket));
+		ocfs2_xattr_bucket_relse(bucket);
+		if (ret)
+			break;
 	}
 
-out:
-	ocfs2_xattr_bucket_relse(inode, &bucket);
-
+	ocfs2_xattr_bucket_free(bucket);
 	return ret;
 }
 
@@ -2711,9 +2756,9 @@ static int ocfs2_xattr_update_xattr_search(struct inode *inode,
 	int i, blocksize = inode->i_sb->s_blocksize;
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 
-	xs->bucket.bu_bhs[0] = new_bh;
+	xs->bucket->bu_bhs[0] = new_bh;
 	get_bh(new_bh);
-	xs->header = bucket_xh(&xs->bucket);
+	xs->header = bucket_xh(xs->bucket);
 
 	xs->base = new_bh->b_data;
 	xs->end = xs->base + inode->i_sb->s_blocksize;
@@ -2721,8 +2766,8 @@ static int ocfs2_xattr_update_xattr_search(struct inode *inode,
 	if (!xs->not_found) {
 		if (OCFS2_XATTR_BUCKET_SIZE != blocksize) {
 			ret = ocfs2_read_blocks(inode,
-					bucket_blkno(&xs->bucket) + 1,
-					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
+					bucket_blkno(xs->bucket) + 1,
+					blk_per_bucket - 1, &xs->bucket->bu_bhs[1],
 					0);
 			if (ret) {
 				mlog_errno(ret);
@@ -3188,23 +3233,28 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 {
 	int ret, i;
 	u16 count, start, len, name_value_len, xe_len, name_offset;
-	struct ocfs2_xattr_bucket s_bucket, t_bucket;
+	struct ocfs2_xattr_bucket *s_bucket = NULL, *t_bucket = NULL;
 	struct ocfs2_xattr_header *xh;
 	struct ocfs2_xattr_entry *xe;
 
 	mlog(0, "move half of xattrs from bucket %llu to %llu\n",
 	     blk, new_blk);
 
-	memset(&s_bucket, 0, sizeof(struct ocfs2_xattr_bucket));
-	memset(&t_bucket, 0, sizeof(struct ocfs2_xattr_bucket));
+	s_bucket = ocfs2_xattr_bucket_new(inode);
+	t_bucket = ocfs2_xattr_bucket_new(inode);
+	if (!s_bucket || !t_bucket) {
+		ret = -ENOMEM;
+		mlog_errno(ret);
+		goto out;
+	}
 
-	ret = ocfs2_read_xattr_bucket(inode, &s_bucket, blk);
+	ret = ocfs2_read_xattr_bucket(s_bucket, blk);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;
 	}
 
-	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &s_bucket,
+	ret = ocfs2_xattr_bucket_journal_access(handle, s_bucket,
 						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
@@ -3215,13 +3265,13 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	 * Even if !new_bucket_head, we're overwriting t_bucket.  Thus,
 	 * there's no need to read it.
 	 */
-	ret = ocfs2_init_xattr_bucket(inode, &t_bucket, new_blk);
+	ret = ocfs2_init_xattr_bucket(t_bucket, new_blk);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;
 	}
 
-	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &t_bucket,
+	ret = ocfs2_xattr_bucket_journal_access(handle, t_bucket,
 						new_bucket_head ?
 						OCFS2_JOURNAL_ACCESS_CREATE :
 						OCFS2_JOURNAL_ACCESS_WRITE);
@@ -3231,10 +3281,10 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	}
 
 	/* copy the whole bucket to the new first. */
-	ocfs2_xattr_bucket_copy_data(inode, &t_bucket, &s_bucket);
+	ocfs2_xattr_bucket_copy_data(t_bucket, s_bucket);
 
 	/* update the new bucket. */
-	xh = bucket_xh(&t_bucket);
+	xh = bucket_xh(t_bucket);
 	count = le16_to_cpu(xh->xh_count);
 	start = count / 2;
 
@@ -3299,7 +3349,7 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	else
 		xh->xh_num_buckets = 0;
 
-	ocfs2_xattr_bucket_journal_dirty(handle, inode, &t_bucket);
+	ocfs2_xattr_bucket_journal_dirty(handle, t_bucket);
 
 	/* store the first_hash of the new bucket. */
 	if (first_hash)
@@ -3309,18 +3359,18 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
 	 * Now only update the 1st block of the old bucket.
 	 * Please note that the entry has been sorted already above.
 	 */
-	xh = bucket_xh(&s_bucket);
+	xh = bucket_xh(s_bucket);
 	memset(&xh->xh_entries[start], 0,
 	       sizeof(struct ocfs2_xattr_entry) * (count - start));
 	xh->xh_count = cpu_to_le16(start);
 	xh->xh_free_start = cpu_to_le16(name_offset);
 	xh->xh_name_value_len = cpu_to_le16(name_value_len);
 
-	ocfs2_xattr_bucket_journal_dirty(handle, inode, &s_bucket);
+	ocfs2_xattr_bucket_journal_dirty(handle, s_bucket);
 
 out:
-	ocfs2_xattr_bucket_relse(inode, &s_bucket);
-	ocfs2_xattr_bucket_relse(inode, &t_bucket);
+	ocfs2_xattr_bucket_free(s_bucket);
+	ocfs2_xattr_bucket_free(t_bucket);
 
 	return ret;
 }
@@ -3338,17 +3388,22 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode,
 				 int t_is_new)
 {
 	int ret;
-	struct ocfs2_xattr_bucket s_bucket, t_bucket;
+	struct ocfs2_xattr_bucket *s_bucket = NULL, *t_bucket = NULL;
 
 	BUG_ON(s_blkno == t_blkno);
 
 	mlog(0, "cp bucket %llu to %llu, target is %d\n",
 	     s_blkno, t_blkno, t_is_new);
 
-	memset(&s_bucket, 0, sizeof(struct ocfs2_xattr_bucket));
-	memset(&t_bucket, 0, sizeof(struct ocfs2_xattr_bucket));
+	s_bucket = ocfs2_xattr_bucket_new(inode);
+	t_bucket = ocfs2_xattr_bucket_new(inode);
+	if (!s_bucket || !t_bucket) {
+		ret = -ENOMEM;
+		mlog_errno(ret);
+		goto out;
+	}
 
-	ret = ocfs2_read_xattr_bucket(inode, &s_bucket, s_blkno);
+	ret = ocfs2_read_xattr_bucket(s_bucket, s_blkno);
 	if (ret)
 		goto out;
 
@@ -3356,23 +3411,23 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode,
 	 * Even if !t_is_new, we're overwriting t_bucket.  Thus,
 	 * there's no need to read it.
 	 */
-	ret = ocfs2_init_xattr_bucket(inode, &t_bucket, t_blkno);
+	ret = ocfs2_init_xattr_bucket(t_bucket, t_blkno);
 	if (ret)
 		goto out;
 
-	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &t_bucket,
+	ret = ocfs2_xattr_bucket_journal_access(handle, t_bucket,
 						t_is_new ?
 						OCFS2_JOURNAL_ACCESS_CREATE :
 						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret)
 		goto out;
 
-	ocfs2_xattr_bucket_copy_data(inode, &t_bucket, &s_bucket);
-	ocfs2_xattr_bucket_journal_dirty(handle, inode, &t_bucket);
+	ocfs2_xattr_bucket_copy_data(t_bucket, s_bucket);
+	ocfs2_xattr_bucket_journal_dirty(handle, t_bucket);
 
 out:
-	ocfs2_xattr_bucket_relse(inode, &s_bucket);
-	ocfs2_xattr_bucket_relse(inode, &t_bucket);
+	ocfs2_xattr_bucket_free(t_bucket);
+	ocfs2_xattr_bucket_free(s_bucket);
 
 	return ret;
 }
@@ -3922,7 +3977,7 @@ static void ocfs2_xattr_set_entry_normal(struct inode *inode,
 				xe->xe_value_size = 0;
 
 			val = ocfs2_xattr_bucket_get_val(inode,
-							 &xs->bucket, offs);
+							 xs->bucket, offs);
 			memset(val + OCFS2_XATTR_SIZE(name_len), 0,
 			       size - OCFS2_XATTR_SIZE(name_len));
 			if (OCFS2_XATTR_SIZE(xi->value_len) > 0)
@@ -4000,8 +4055,7 @@ set_new_name_value:
 		xh->xh_free_start = cpu_to_le16(offs);
 	}
 
-	val = ocfs2_xattr_bucket_get_val(inode,
-					 &xs->bucket, offs - size);
+	val = ocfs2_xattr_bucket_get_val(inode, xs->bucket, offs - size);
 	xe->xe_name_offset = cpu_to_le16(offs - size);
 
 	memset(val, 0, size);
@@ -4035,12 +4089,12 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 
 	mlog(0, "Set xattr entry len = %lu index = %d in bucket %llu\n",
 	     (unsigned long)xi->value_len, xi->name_index,
-	     (unsigned long long)bucket_blkno(&xs->bucket));
+	     (unsigned long long)bucket_blkno(xs->bucket));
 
-	if (!xs->bucket.bu_bhs[1]) {
+	if (!xs->bucket->bu_bhs[1]) {
 		ret = ocfs2_read_blocks(inode,
-					bucket_blkno(&xs->bucket) + 1,
-					blk_per_bucket - 1, &xs->bucket.bu_bhs[1],
+					bucket_blkno(xs->bucket) + 1,
+					blk_per_bucket - 1, &xs->bucket->bu_bhs[1],
 					0);
 		if (ret) {
 			mlog_errno(ret);
@@ -4056,7 +4110,7 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 		goto out;
 	}
 
-	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &xs->bucket,
+	ret = ocfs2_xattr_bucket_journal_access(handle, xs->bucket,
 						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret < 0) {
 		mlog_errno(ret);
@@ -4064,7 +4118,7 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 	}
 
 	ocfs2_xattr_set_entry_normal(inode, xi, xs, name_hash, local);
-	ocfs2_xattr_bucket_journal_dirty(handle, inode, &xs->bucket);
+	ocfs2_xattr_bucket_journal_dirty(handle, xs->bucket);
 
 out:
 	ocfs2_commit_trans(osb, handle);
@@ -4177,10 +4231,10 @@ static int ocfs2_xattr_bucket_value_truncate_xs(struct inode *inode,
 	struct ocfs2_xattr_entry *xe = xs->here;
 	struct ocfs2_xattr_header *xh = (struct ocfs2_xattr_header *)xs->base;
 
-	BUG_ON(!xs->bucket.bu_bhs[0] || !xe || ocfs2_xattr_is_local(xe));
+	BUG_ON(!xs->bucket->bu_bhs[0] || !xe || ocfs2_xattr_is_local(xe));
 
 	offset = xe - xh->xh_entries;
-	ret = ocfs2_xattr_bucket_value_truncate(inode, xs->bucket.bu_bhs[0],
+	ret = ocfs2_xattr_bucket_value_truncate(inode, xs->bucket->bu_bhs[0],
 						offset, len);
 	if (ret)
 		mlog_errno(ret);
@@ -4300,7 +4354,7 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 					 struct ocfs2_xattr_search *xs)
 {
 	handle_t *handle = NULL;
-	struct ocfs2_xattr_header *xh = bucket_xh(&xs->bucket);
+	struct ocfs2_xattr_header *xh = bucket_xh(xs->bucket);
 	struct ocfs2_xattr_entry *last = &xh->xh_entries[
 						le16_to_cpu(xh->xh_count) - 1];
 	int ret = 0;
@@ -4313,7 +4367,7 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 		return;
 	}
 
-	ret = ocfs2_xattr_bucket_journal_access(handle, inode, &xs->bucket,
+	ret = ocfs2_xattr_bucket_journal_access(handle, xs->bucket,
 						OCFS2_JOURNAL_ACCESS_WRITE);
 	if (ret) {
 		mlog_errno(ret);
@@ -4326,7 +4380,7 @@ static void ocfs2_xattr_bucket_remove_xs(struct inode *inode,
 	memset(last, 0, sizeof(struct ocfs2_xattr_entry));
 	le16_add_cpu(&xh->xh_count, -1);
 
-	ocfs2_xattr_bucket_journal_dirty(handle, inode, &xs->bucket);
+	ocfs2_xattr_bucket_journal_dirty(handle, xs->bucket);
 
 out_commit:
 	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
@@ -4468,7 +4522,7 @@ try_again:
 
 	mlog_bug_on_msg(header_size > blocksize, "bucket %llu has header size "
 			"of %u which exceed block size\n",
-			(unsigned long long)bucket_blkno(&xs->bucket),
+			(unsigned long long)bucket_blkno(xs->bucket),
 			header_size);
 
 	if (xi->value && xi->value_len > OCFS2_XATTR_INLINE_SIZE)
@@ -4508,7 +4562,7 @@ try_again:
 	mlog(0, "xs->not_found = %d, in xattr bucket %llu: free = %d, "
 	     "need = %d, max_free = %d, xh_free_start = %u, xh_name_value_len ="
 	     " %u\n", xs->not_found,
-	     (unsigned long long)bucket_blkno(&xs->bucket),
+	     (unsigned long long)bucket_blkno(xs->bucket),
 	     free, need, max_free, le16_to_cpu(xh->xh_free_start),
 	     le16_to_cpu(xh->xh_name_value_len));
 
@@ -4520,7 +4574,7 @@ try_again:
 			 * name/value will be moved, the xe shouldn't be changed
 			 * in xs.
 			 */
-			ret = ocfs2_defrag_xattr_bucket(inode, &xs->bucket);
+			ret = ocfs2_defrag_xattr_bucket(inode, xs->bucket);
 			if (ret) {
 				mlog_errno(ret);
 				goto out;
@@ -4551,7 +4605,7 @@ try_again:
 		 * one bucket's worth, so check it here whether we need to
 		 * add a new bucket for the insert.
 		 */
-		ret = ocfs2_check_xattr_bucket_collision(inode, &xs->bucket);
+		ret = ocfs2_check_xattr_bucket_collision(inode, xs->bucket);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;
@@ -4559,14 +4613,13 @@ try_again:
 
 		ret = ocfs2_add_new_xattr_bucket(inode,
 						 xs->xattr_bh,
-						 xs->bucket.bu_bhs[0]);
+						 xs->bucket->bu_bhs[0]);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;
 		}
 
-		ocfs2_xattr_bucket_relse(inode, &xs->bucket);
-		memset(&xs->bucket, 0, sizeof(xs->bucket));
+		ocfs2_xattr_bucket_relse(xs->bucket);
 
 		ret = ocfs2_xattr_index_block_find(inode, xs->xattr_bh,
 						   xi->name_index,
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 10/13] ocfs2: Use buckets in ocfs2_xattr_bucket_find().

[Joel Becker]

Change the ocfs2_xattr_bucket_find() function to use ocfs2_xattr_bucket
as its abstraction.  This makes for more efficient reads, as buckets are
linear blocks, and also has improved caching characteristics.  It also
reads better.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |   89 +++++++++++++++++++-----------------------------------
 1 files changed, 31 insertions(+), 58 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index b922f00..2977c0d 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -2244,7 +2244,7 @@ typedef int (xattr_bucket_func)(struct inode *inode,
 				void *para);
 
 static int ocfs2_find_xe_in_bucket(struct inode *inode,
-				   struct buffer_head *header_bh,
+				   struct ocfs2_xattr_bucket *bucket,
 				   int name_index,
 				   const char *name,
 				   u32 name_hash,
@@ -2252,11 +2252,9 @@ static int ocfs2_find_xe_in_bucket(struct inode *inode,
 				   int *found)
 {
 	int i, ret = 0, cmp = 1, block_off, new_offset;
-	struct ocfs2_xattr_header *xh =
-			(struct ocfs2_xattr_header *)header_bh->b_data;
+	struct ocfs2_xattr_header *xh = bucket_xh(bucket);
 	size_t name_len = strlen(name);
 	struct ocfs2_xattr_entry *xe = NULL;
-	struct buffer_head *name_bh = NULL;
 	char *xe_name;
 
 	/*
@@ -2287,19 +2285,8 @@ static int ocfs2_find_xe_in_bucket(struct inode *inode,
 			break;
 		}
 
-		ret = ocfs2_read_block(inode, header_bh->b_blocknr + block_off,
-				       &name_bh);
-		if (ret) {
-			mlog_errno(ret);
-			break;
-		}
-		xe_name = name_bh->b_data + new_offset;
-
-		cmp = memcmp(name, xe_name, name_len);
-		brelse(name_bh);
-		name_bh = NULL;
-
-		if (cmp == 0) {
+		xe_name = bucket_block(bucket, block_off) + new_offset;
+		if (!memcmp(name, xe_name, name_len)) {
 			*xe_index = i;
 			*found = 1;
 			ret = 0;
@@ -2329,39 +2316,42 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 				   struct ocfs2_xattr_search *xs)
 {
 	int ret, found = 0;
-	struct buffer_head *bh = NULL;
-	struct buffer_head *lower_bh = NULL;
 	struct ocfs2_xattr_header *xh = NULL;
 	struct ocfs2_xattr_entry *xe = NULL;
 	u16 index = 0;
 	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	int low_bucket = 0, bucket, high_bucket;
+	struct ocfs2_xattr_bucket *search;
 	u32 last_hash;
-	u64 blkno;
+	u64 blkno, lower_blkno = 0;
 
-	ret = ocfs2_read_block(inode, p_blkno, &bh);
+	search = ocfs2_xattr_bucket_new(inode);
+	if (!search) {
+		ret = -ENOMEM;
+		mlog_errno(ret);
+		goto out;
+	}
+
+	ret = ocfs2_read_xattr_bucket(search, p_blkno);
 	if (ret) {
 		mlog_errno(ret);
 		goto out;
 	}
 
-	xh = (struct ocfs2_xattr_header *)bh->b_data;
+	xh = bucket_xh(search);
 	high_bucket = le16_to_cpu(xh->xh_num_buckets) - 1;
-
 	while (low_bucket <= high_bucket) {
-		brelse(bh);
-		bh = NULL;
-		bucket = (low_bucket + high_bucket) / 2;
+		ocfs2_xattr_bucket_relse(search);
 
+		bucket = (low_bucket + high_bucket) / 2;
 		blkno = p_blkno + bucket * blk_per_bucket;
-
-		ret = ocfs2_read_block(inode, blkno, &bh);
+		ret = ocfs2_read_xattr_bucket(search, blkno);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;
 		}
 
-		xh = (struct ocfs2_xattr_header *)bh->b_data;
+		xh = bucket_xh(search);
 		xe = &xh->xh_entries[0];
 		if (name_hash < le32_to_cpu(xe->xe_name_hash)) {
 			high_bucket = bucket - 1;
@@ -2378,10 +2368,8 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 
 		last_hash = le32_to_cpu(xe->xe_name_hash);
 
-		/* record lower_bh which may be the insert place. */
-		brelse(lower_bh);
-		lower_bh = bh;
-		bh = NULL;
+		/* record lower_blkno which may be the insert place. */
+		lower_blkno = blkno;
 
 		if (name_hash > le32_to_cpu(xe->xe_name_hash)) {
 			low_bucket = bucket + 1;
@@ -2389,7 +2377,7 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 		}
 
 		/* the searched xattr should reside in this bucket if exists. */
-		ret = ocfs2_find_xe_in_bucket(inode, lower_bh,
+		ret = ocfs2_find_xe_in_bucket(inode, search,
 					      name_index, name, name_hash,
 					      &index, &found);
 		if (ret) {
@@ -2404,35 +2392,21 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 	 * When the xattr's hash value is in the gap of 2 buckets, we will
 	 * always set it to the previous bucket.
 	 */
-	if (!lower_bh) {
-		/*
-		 * We can't find any bucket whose first name_hash is less
-		 * than the find name_hash.
-		 */
-		BUG_ON(bh->b_blocknr != p_blkno);
-		lower_bh = bh;
-		bh = NULL;
+	if (!lower_blkno)
+		lower_blkno = p_blkno;
+
+	/* This should be in cache - we just read it during the search */
+	ret = ocfs2_read_xattr_bucket(xs->bucket, lower_blkno);
+	if (ret) {
+		mlog_errno(ret);
+		goto out;
 	}
-	xs->bucket->bu_bhs[0] = lower_bh;
-	lower_bh = NULL;
 
 	xs->header = bucket_xh(xs->bucket);
 	xs->base = bucket_block(xs->bucket, 0);
 	xs->end = xs->base + inode->i_sb->s_blocksize;
 
 	if (found) {
-		/*
-		 * If we have found the xattr enty, read all the blocks in
-		 * this bucket.
-		 */
-		ret = ocfs2_read_blocks(inode, bucket_blkno(xs->bucket) + 1,
-					blk_per_bucket - 1, &xs->bucket->bu_bhs[1],
-					0);
-		if (ret) {
-			mlog_errno(ret);
-			goto out;
-		}
-
 		xs->here = &xs->header->xh_entries[index];
 		mlog(0, "find xattr %s in bucket %llu, entry = %u\n", name,
 		     (unsigned long long)bucket_blkno(xs->bucket), index);
@@ -2440,8 +2414,7 @@ static int ocfs2_xattr_bucket_find(struct inode *inode,
 		ret = -ENODATA;
 
 out:
-	brelse(bh);
-	brelse(lower_bh);
+	ocfs2_xattr_bucket_free(search);
 	return ret;
 }
 
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 11/13] ocfs2: Use buckets in ocfs2_xattr_create_index_block().

[Joel Becker]

Use the ocfs2_xattr_bucket abstraction in
ocfs2_xattr_create_index_block() and its helpers.  We get more efficient
reads, a lot less buffer_head munging, and nicer code to boot.  While
we're at it, ocfs2_xattr_update_xattr_search() becomes void.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |  118 +++++++++++++++++-------------------------------------
 1 files changed, 37 insertions(+), 81 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 2977c0d..80d4187 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -2643,32 +2643,34 @@ static void swap_xe(void *a, void *b, int size)
 /*
  * When the ocfs2_xattr_block is filled up, new bucket will be created
  * and all the xattr entries will be moved to the new bucket.
+ * The header goes at the start of the bucket, and the names+values are
+ * filled from the end.  This is why *target starts as the last buffer.
  * Note: we need to sort the entries since they are not saved in order
  * in the ocfs2_xattr_block.
  */
 static void ocfs2_cp_xattr_block_to_bucket(struct inode *inode,
 					   struct buffer_head *xb_bh,
-					   struct buffer_head *xh_bh,
-					   struct buffer_head *data_bh)
+					   struct ocfs2_xattr_bucket *bucket)
 {
 	int i, blocksize = inode->i_sb->s_blocksize;
+	int blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	u16 offset, size, off_change;
 	struct ocfs2_xattr_entry *xe;
 	struct ocfs2_xattr_block *xb =
 				(struct ocfs2_xattr_block *)xb_bh->b_data;
 	struct ocfs2_xattr_header *xb_xh = &xb->xb_attrs.xb_header;
-	struct ocfs2_xattr_header *xh =
-				(struct ocfs2_xattr_header *)xh_bh->b_data;
+	struct ocfs2_xattr_header *xh = bucket_xh(bucket);
 	u16 count = le16_to_cpu(xb_xh->xh_count);
-	char *target = xh_bh->b_data, *src = xb_bh->b_data;
+	char *src = xb_bh->b_data;
+	char *target = bucket_block(bucket, blks - 1);
 
 	mlog(0, "cp xattr from block %llu to bucket %llu\n",
 	     (unsigned long long)xb_bh->b_blocknr,
-	     (unsigned long long)xh_bh->b_blocknr);
+	     (unsigned long long)bucket_blkno(bucket));
+
+	for (i = 0; i < blks; i++)
+		memset(bucket_block(bucket, i), 0, blocksize);
 
-	memset(xh_bh->b_data, 0, blocksize);
-	if (data_bh)
-		memset(data_bh->b_data, 0, blocksize);
 	/*
 	 * Since the xe_name_offset is based on ocfs2_xattr_header,
 	 * there is a offset change corresponding to the change of
@@ -2680,8 +2682,6 @@ static void ocfs2_cp_xattr_block_to_bucket(struct inode *inode,
 	size = blocksize - offset;
 
 	/* copy all the names and values. */
-	if (data_bh)
-		target = data_bh->b_data;
 	memcpy(target + offset, src + offset, size);
 
 	/* Init new header now. */
@@ -2691,7 +2691,7 @@ static void ocfs2_cp_xattr_block_to_bucket(struct inode *inode,
 	xh->xh_free_start = cpu_to_le16(OCFS2_XATTR_BUCKET_SIZE - size);
 
 	/* copy all the entries. */
-	target = xh_bh->b_data;
+	target = bucket_block(bucket, 0);
 	offset = offsetof(struct ocfs2_xattr_header, xh_entries);
 	size = count * sizeof(struct ocfs2_xattr_entry);
 	memcpy(target + offset, (char *)xb_xh + offset, size);
@@ -2717,42 +2717,30 @@ static void ocfs2_cp_xattr_block_to_bucket(struct inode *inode,
  * While if the entry is in index b-tree, "bucket" indicates the
  * real place of the xattr.
  */
-static int ocfs2_xattr_update_xattr_search(struct inode *inode,
-					   struct ocfs2_xattr_search *xs,
-					   struct buffer_head *old_bh,
-					   struct buffer_head *new_bh)
+static void ocfs2_xattr_update_xattr_search(struct inode *inode,
+					    struct ocfs2_xattr_search *xs,
+					    struct buffer_head *old_bh)
 {
-	int ret = 0;
 	char *buf = old_bh->b_data;
 	struct ocfs2_xattr_block *old_xb = (struct ocfs2_xattr_block *)buf;
 	struct ocfs2_xattr_header *old_xh = &old_xb->xb_attrs.xb_header;
-	int i, blocksize = inode->i_sb->s_blocksize;
-	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
+	int i;
 
-	xs->bucket->bu_bhs[0] = new_bh;
-	get_bh(new_bh);
 	xs->header = bucket_xh(xs->bucket);
-
-	xs->base = new_bh->b_data;
+	xs->base = bucket_block(xs->bucket, 0);
 	xs->end = xs->base + inode->i_sb->s_blocksize;
 
-	if (!xs->not_found) {
-		if (OCFS2_XATTR_BUCKET_SIZE != blocksize) {
-			ret = ocfs2_read_blocks(inode,
-					bucket_blkno(xs->bucket) + 1,
-					blk_per_bucket - 1, &xs->bucket->bu_bhs[1],
-					0);
-			if (ret) {
-				mlog_errno(ret);
-				return ret;
-			}
+	if (xs->not_found)
+		return;
 
-			i = xs->here - old_xh->xh_entries;
-			xs->here = &xs->header->xh_entries[i];
-		}
+	/*
+	 * If a bucket is more than one block, the name+value moved when
+	 * we went to a bucket.
+	 */
+	if (xs->bucket->bu_blocks > 1) {
+		i = xs->here - old_xh->xh_entries;
+		xs->here = &xs->header->xh_entries[i];
 	}
-
-	return ret;
 }
 
 static int ocfs2_xattr_create_index_block(struct inode *inode,
@@ -2765,18 +2753,17 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
 	struct ocfs2_inode_info *oi = OCFS2_I(inode);
 	struct ocfs2_alloc_context *data_ac;
-	struct buffer_head *xh_bh = NULL, *data_bh = NULL;
 	struct buffer_head *xb_bh = xs->xattr_bh;
 	struct ocfs2_xattr_block *xb =
 			(struct ocfs2_xattr_block *)xb_bh->b_data;
 	struct ocfs2_xattr_tree_root *xr;
 	u16 xb_flags = le16_to_cpu(xb->xb_flags);
-	u16 bpb = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 
 	mlog(0, "create xattr index block for %llu\n",
 	     (unsigned long long)xb_bh->b_blocknr);
 
 	BUG_ON(xb_flags & OCFS2_XATTR_INDEXED);
+	BUG_ON(!xs->bucket);
 
 	ret = ocfs2_reserve_clusters(osb, 1, &data_ac);
 	if (ret) {
@@ -2792,10 +2779,10 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 	down_write(&oi->ip_alloc_sem);
 
 	/*
-	 * 3 more credits, one for xattr block update, one for the 1st block
-	 * of the new xattr bucket and one for the value/data.
+	 * We need more credits.  One for the xattr block update and one
+	 * for each block of the new xattr bucket.
 	 */
-	credits += 3;
+	credits += 1 + ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	handle = ocfs2_start_trans(osb, credits);
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);
@@ -2825,51 +2812,23 @@ static int ocfs2_xattr_create_index_block(struct inode *inode,
 
 	mlog(0, "allocate 1 cluster from %llu to xattr block\n", blkno);
 
-	xh_bh = sb_getblk(inode->i_sb, blkno);
-	if (!xh_bh) {
-		ret = -EIO;
+	ret = ocfs2_init_xattr_bucket(xs->bucket, blkno);
+	if (ret) {
 		mlog_errno(ret);
 		goto out_commit;
 	}
 
-	ocfs2_set_new_buffer_uptodate(inode, xh_bh);
-
-	ret = ocfs2_journal_access(handle, inode, xh_bh,
-				   OCFS2_JOURNAL_ACCESS_CREATE);
+	ret = ocfs2_xattr_bucket_journal_access(handle, xs->bucket,
+						OCFS2_JOURNAL_ACCESS_CREATE);
 	if (ret) {
 		mlog_errno(ret);
 		goto out_commit;
 	}
 
-	if (bpb > 1) {
-		data_bh = sb_getblk(inode->i_sb, blkno + bpb - 1);
-		if (!data_bh) {
-			ret = -EIO;
-			mlog_errno(ret);
-			goto out_commit;
-		}
-
-		ocfs2_set_new_buffer_uptodate(inode, data_bh);
-
-		ret = ocfs2_journal_access(handle, inode, data_bh,
-					   OCFS2_JOURNAL_ACCESS_CREATE);
-		if (ret) {
-			mlog_errno(ret);
-			goto out_commit;
-		}
-	}
-
-	ocfs2_cp_xattr_block_to_bucket(inode, xb_bh, xh_bh, data_bh);
-
-	ocfs2_journal_dirty(handle, xh_bh);
-	if (data_bh)
-		ocfs2_journal_dirty(handle, data_bh);
+	ocfs2_cp_xattr_block_to_bucket(inode, xb_bh, xs->bucket);
+	ocfs2_xattr_bucket_journal_dirty(handle, xs->bucket);
 
-	ret = ocfs2_xattr_update_xattr_search(inode, xs, xb_bh, xh_bh);
-	if (ret) {
-		mlog_errno(ret);
-		goto out_commit;
-	}
+	ocfs2_xattr_update_xattr_search(inode, xs, xb_bh);
 
 	/* Change from ocfs2_xattr_header to ocfs2_xattr_tree_root */
 	memset(&xb->xb_attrs, 0, inode->i_sb->s_blocksize -
@@ -2904,9 +2863,6 @@ out:
 	if (data_ac)
 		ocfs2_free_alloc_context(data_ac);
 
-	brelse(xh_bh);
-	brelse(data_bh);
-
 	return ret;
 }
 
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 12/13] ocfs2: Use buckets in ocfs2_defrag_xattr_bucket().

[Joel Becker]

Use the ocfs2_xattr_bucket abstraction for reading and writing the
bucket in ocfs2_defrag_xattr_bucket().

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |   55 ++++++++++++++++++++++-------------------------------
 1 files changed, 23 insertions(+), 32 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 80d4187..a24a27c 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -2893,21 +2893,11 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 	struct ocfs2_xattr_header *xh;
 	char *entries, *buf, *bucket_buf = NULL;
 	u64 blkno = bucket_blkno(bucket);
-	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	u16 xh_free_start;
 	size_t blocksize = inode->i_sb->s_blocksize;
 	handle_t *handle;
-	struct buffer_head **bhs;
 	struct ocfs2_xattr_entry *xe;
-
-	bhs = kzalloc(sizeof(struct buffer_head *) * blk_per_bucket,
-			GFP_NOFS);
-	if (!bhs)
-		return -ENOMEM;
-
-	ret = ocfs2_read_blocks(inode, blkno, blk_per_bucket, bhs, 0);
-	if (ret)
-		goto out;
+	struct ocfs2_xattr_bucket *wb = NULL;
 
 	/*
 	 * In order to make the operation more efficient and generic,
@@ -2921,11 +2911,21 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 		goto out;
 	}
 
+	wb = ocfs2_xattr_bucket_new(inode);
+	if (!wb) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = ocfs2_read_xattr_bucket(wb, blkno);
+	if (ret)
+		goto out;
+
 	buf = bucket_buf;
-	for (i = 0; i < blk_per_bucket; i++, buf += blocksize)
-		memcpy(buf, bhs[i]->b_data, blocksize);
+	for (i = 0; i < wb->bu_blocks; i++, buf += blocksize)
+		memcpy(buf, bucket_block(wb, i), blocksize);
 
-	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)), blk_per_bucket);
+	handle = ocfs2_start_trans((OCFS2_SB(inode->i_sb)), wb->bu_blocks);
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);
 		handle = NULL;
@@ -2933,13 +2933,11 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 		goto out;
 	}
 
-	for (i = 0; i < blk_per_bucket; i++) {
-		ret = ocfs2_journal_access(handle, inode, bhs[i],
-					   OCFS2_JOURNAL_ACCESS_WRITE);
-		if (ret < 0) {
-			mlog_errno(ret);
-			goto commit;
-		}
+	ret = ocfs2_xattr_bucket_journal_access(handle, wb,
+						OCFS2_JOURNAL_ACCESS_WRITE);
+	if (ret < 0) {
+		mlog_errno(ret);
+		goto commit;
 	}
 
 	xh = (struct ocfs2_xattr_header *)bucket_buf;
@@ -3008,21 +3006,14 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
 	     cmp_xe, swap_xe);
 
 	buf = bucket_buf;
-	for (i = 0; i < blk_per_bucket; i++, buf += blocksize) {
-		memcpy(bhs[i]->b_data, buf, blocksize);
-		ocfs2_journal_dirty(handle, bhs[i]);
-	}
+	for (i = 0; i < wb->bu_blocks; i++, buf += blocksize)
+		memcpy(bucket_block(wb, i), buf, blocksize);
+	ocfs2_xattr_bucket_journal_dirty(handle, wb);
 
 commit:
 	ocfs2_commit_trans(OCFS2_SB(inode->i_sb), handle);
 out:
-
-	if (bhs) {
-		for (i = 0; i < blk_per_bucket; i++)
-			brelse(bhs[i]);
-	}
-	kfree(bhs);
-
+	ocfs2_xattr_bucket_free(wb);
 	kfree(bucket_buf);
 	return ret;
 }
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 13/13] ocfs2: Use buckets in ocfs2_xattr_set_entry_in_bucket().

[Joel Becker]

The ocfs2_xattr_set_entry_in_bucket() function is already working on an
ocfs2_xattr_bucket structure, so let's use the bucket API.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/xattr.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index a24a27c..c8c979a 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -4004,25 +4004,24 @@ static int ocfs2_xattr_set_entry_in_bucket(struct inode *inode,
 {
 	int ret;
 	handle_t *handle = NULL;
-	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
 	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+	u64 blkno;
 
 	mlog(0, "Set xattr entry len = %lu index = %d in bucket %llu\n",
 	     (unsigned long)xi->value_len, xi->name_index,
 	     (unsigned long long)bucket_blkno(xs->bucket));
 
 	if (!xs->bucket->bu_bhs[1]) {
-		ret = ocfs2_read_blocks(inode,
-					bucket_blkno(xs->bucket) + 1,
-					blk_per_bucket - 1, &xs->bucket->bu_bhs[1],
-					0);
+		blkno = bucket_blkno(xs->bucket);
+		ocfs2_xattr_bucket_relse(xs->bucket);
+		ret = ocfs2_read_xattr_bucket(xs->bucket, blkno);
 		if (ret) {
 			mlog_errno(ret);
 			goto out;
 		}
 	}
 
-	handle = ocfs2_start_trans(osb, blk_per_bucket);
+	handle = ocfs2_start_trans(osb, xs->bucket->bu_blocks);
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);
 		handle = NULL;
-- 
1.5.6.5

[Ocfs2-devel] [PATCH 09/13] ocfs2: Take ocfs2_xattr_bucket structures off of the stack.

[Tao Ma]

Joel Becker wrote:
> The ocfs2_xattr_bucket structure is a nice abstraction, but it is a bit
> large to have on the stack.  Just like ocfs2_path, let's allocate it
> with a ocfs2_xattr_bucket_new() function.
> 
> We can now store the inode on the bucket, cleaning up all the other
> bucket functions.  While we're here, we catch another place or two that
> wasn't using ocfs2_read_xattr_bucket().
> 
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
> ---

> +
> +static void ocfs2_xattr_bucket_relse(struct ocfs2_xattr_bucket *bucket)
>  {
> -	int i, blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
> +	int i;
>  
> -	for (i = 0; i < blks; i++) {
> +	for (i = 0; i < bucket->bu_blocks; i++) {
>  		brelse(bucket->bu_bhs[i]);
>  		bucket->bu_bhs[i] = NULL;
>  	}
>  }
>  
> +static void ocfs2_xattr_bucket_free(struct ocfs2_xattr_bucket *bucket)
> +{
> +	if (bucket) {
> +		ocfs2_xattr_bucket_relse(bucket);
> +		bucket->bu_inode = NULL;
we need "kfree(bucket)" here.
> +	}
> +}

Regards,
Tao

[Ocfs2-devel] [PATCH 09/13] ocfs2: Take ocfs2_xattr_bucket structures off of the stack.

[Joel Becker]

On Tue, Oct 28, 2008 at 09:50:12AM +0800, Tao Ma wrote:
>
>
> Joel Becker wrote:
>> The ocfs2_xattr_bucket structure is a nice abstraction, but it is a bit
>> large to have on the stack.  Just like ocfs2_path, let's allocate it
>> with a ocfs2_xattr_bucket_new() function.
>>
>> We can now store the inode on the bucket, cleaning up all the other
>> bucket functions.  While we're here, we catch another place or two that
>> wasn't using ocfs2_read_xattr_bucket().
>>
>> Signed-off-by: Joel Becker <joel.becker@oracle.com>
>> ---
>
>> +
>> +static void ocfs2_xattr_bucket_relse(struct ocfs2_xattr_bucket *bucket)
>>  {
>> -	int i, blks = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
>> +	int i;
>>  -	for (i = 0; i < blks; i++) {
>> +	for (i = 0; i < bucket->bu_blocks; i++) {
>>  		brelse(bucket->bu_bhs[i]);
>>  		bucket->bu_bhs[i] = NULL;
>>  	}
>>  }
>>  +static void ocfs2_xattr_bucket_free(struct ocfs2_xattr_bucket 
>> *bucket)
>> +{
>> +	if (bucket) {
>> +		ocfs2_xattr_bucket_relse(bucket);
>> +		bucket->bu_inode = NULL;
> we need "kfree(bucket)" here.

	Yes we do!  I screwed that up splitting the patches.  Thanks for
catching!

Joel

-- 

Life's Little Instruction Book #267

	"Lie on your back and look at the stars."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 09/13] ocfs2: Take ocfs2_xattr_bucket structures off of the stack.

[Tao Ma]

Joel Becker wrote:
> The ocfs2_xattr_bucket structure is a nice abstraction, but it is a bit
> large to have on the stack.  Just like ocfs2_path, let's allocate it
> with a ocfs2_xattr_bucket_new() function.
> 
> We can now store the inode on the bucket, cleaning up all the other
> bucket functions.  While we're here, we catch another place or two that
> wasn't using ocfs2_read_xattr_bucket().
> 
> Signed-off-by: Joel Becker <joel.becker@oracle.com>


> @@ -2042,6 +2074,18 @@ int ocfs2_xattr_set(struct inode *inode,
>  	if (!ocfs2_supports_xattr(OCFS2_SB(inode->i_sb)))
>  		return -EOPNOTSUPP;
>  
> +	xis.bucket = ocfs2_xattr_bucket_new(inode);
> +	if (!xis.bucket) {
> +		mlog_errno(-ENOMEM);
> +		return -ENOMEM;
> +	}
xis is used for xattr search in inode, so we don't need to allocate bucket.
> +	xbs.bucket = ocfs2_xattr_bucket_new(inode);
> +	if (!xbs.bucket) {
> +		ocfs2_xattr_bucket_free(xbs.bucket);
> +		mlog_errno(-ENOMEM);
> +		return -ENOMEM;
> +	}
can we allocate this only when we really need this? that means we may 
allocate it in ocfs2_create_index_block(when we create a new one) and 
ocfs2_xattr_index_block_find if there is some buckets exist?

Regards,
Tao

[Ocfs2-devel] [PATCH 06/13] ocfs2: Improve ocfs2_read_xattr_bucket().

[Tao Ma]

Joel Becker wrote:
> The ocfs2_read_xattr_bucket() function would read an xattr bucket into a
> list of buffer heads.  However, we have a nice ocfs2_xattr_bucket
> structure.  Let's have it fill that out instead.
> 
> In addition, ocfs2_read_xattr_bucket() would initialize buffer heads for
> a bucket that's never been on disk before.  That's confusing.  Let's
> call that functionality ocfs2_init_xattr_bucket().
> 
> The functions ocfs2_cp_xattr_bucket() and ocfs2_half_xattr_bucket() are
> updated to use the ocfs2_xattr_bucket structure rather than raw bh
> lists.  That way they can use the new read/init calls.  In addition,
> they drop the wasted read of an existing target bucket.
> 
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
> ---
>  /*
>   * Move half num of the xattrs in old bucket(blk) to new bucket(new_blk).
>   * first_hash will record the 1st hash of the new bucket.
> @@ -3128,7 +3145,7 @@ static int ocfs2_half_xattr_bucket(struct inode *inode,
>  	int ret, i;
>  	u16 count, start, len, name_value_len, xe_len, name_offset;
>  	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
> -	struct buffer_head **s_bhs, **t_bhs = NULL;
> +	struct ocfs2_xattr_bucket s_bucket, t_bucket;
>  	struct ocfs2_xattr_header *xh;
>  	struct ocfs2_xattr_entry *xe;
>  	int blocksize = inode->i_sb->s_blocksize;
snip
> -	t_bhs = kcalloc(blk_per_bucket, sizeof(struct buffer_head *), GFP_NOFS);
> -	if (!t_bhs) {
> -		ret = -ENOMEM;
> -		goto out;
> -	}
> -
> -	ret = ocfs2_read_xattr_bucket(inode, new_blk, t_bhs, new_bucket_head);
> +	/*
> +	 * Even if !new_bucket_head, we're overwriting t_bucket.  Thus,
> +	 * there's no need to read it.
> +	 */
> +	ret = ocfs2_init_xattr_bucket(inode, &t_bucket, new_blk);
>  	if (ret) {
>  		mlog_errno(ret);
>  		goto out;
>  	}
>  
>  	for (i = 0; i < blk_per_bucket; i++) {
> -		ret = ocfs2_journal_access(handle, inode, t_bhs[i],
> +		ret = ocfs2_journal_access(handle, inode, t_bucket.bu_bhs[i],
>  					   new_bucket_head ?
>  					   OCFS2_JOURNAL_ACCESS_CREATE :
>  					   OCFS2_JOURNAL_ACCESS_WRITE);
I have read the caller of ocfs2_half_xattr_bucket again. In 
ocfs2_extend_xattr_bucket when we want to half the bucket to a 
never-used new bucket within the same cluster(while we always pass 
new_bucket_head=0), do we need to use OCFS2_JOURNAL_ACCESS_CREATE? If 
yes, maybe we have to modify the caller to be more precisely(I can 
handle this based on your patch set). The same goes for 
ocfs2_cp_xattr_bucket.

Regards,
Tao

[Ocfs2-devel] [PATCH 06/13] ocfs2: Improve ocfs2_read_xattr_bucket().

[Joel Becker]

On Tue, Oct 28, 2008 at 10:44:35AM +0800, Tao Ma wrote:
> Joel Becker wrote:
>> @@ -3128,7 +3145,7 @@ static int ocfs2_half_xattr_bucket(struct inode 
>> *inode,
>>  	int ret, i;
>>  	u16 count, start, len, name_value_len, xe_len, name_offset;
>>  	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
>> -	struct buffer_head **s_bhs, **t_bhs = NULL;
>> +	struct ocfs2_xattr_bucket s_bucket, t_bucket;
>>  	struct ocfs2_xattr_header *xh;
>>  	struct ocfs2_xattr_entry *xe;
>>  	int blocksize = inode->i_sb->s_blocksize;
> snip
>> -	t_bhs = kcalloc(blk_per_bucket, sizeof(struct buffer_head *), GFP_NOFS);
>> -	if (!t_bhs) {
>> -		ret = -ENOMEM;
>> -		goto out;
>> -	}
>> -
>> -	ret = ocfs2_read_xattr_bucket(inode, new_blk, t_bhs, new_bucket_head);
>> +	/*
>> +	 * Even if !new_bucket_head, we're overwriting t_bucket.  Thus,
>> +	 * there's no need to read it.
>> +	 */
>> +	ret = ocfs2_init_xattr_bucket(inode, &t_bucket, new_blk);
>>  	if (ret) {
>>  		mlog_errno(ret);
>>  		goto out;
>>  	}
>>   	for (i = 0; i < blk_per_bucket; i++) {
>> -		ret = ocfs2_journal_access(handle, inode, t_bhs[i],
>> +		ret = ocfs2_journal_access(handle, inode, t_bucket.bu_bhs[i],
>>  					   new_bucket_head ?
>>  					   OCFS2_JOURNAL_ACCESS_CREATE :
>>  					   OCFS2_JOURNAL_ACCESS_WRITE);
> I have read the caller of ocfs2_half_xattr_bucket again. In 
> ocfs2_extend_xattr_bucket when we want to half the bucket to a never-used 
> new bucket within the same cluster(while we always pass new_bucket_head=0), 
> do we need to use OCFS2_JOURNAL_ACCESS_CREATE? If yes, maybe we have to 
> modify the caller to be more precisely(I can handle this based on your 
> patch set). The same goes for ocfs2_cp_xattr_bucket.

	Mark and I were discussing this on #ocfs2 just today.  We were
having a hard time understanding the condition of the bucket - what
new_bucket_head really means.  This comes from my patch to set the
JOURNAL_ACCESS based on the new_bucket_head value (same in
cp_xattr_bucket).
	Is the target bucket (t_bucket) always in the same cluster for
all callers regardless of new_bucket_head?  Has it ever been written
before?  May it have been allocated a long time ago?

Joel

-- 

Life's Little Instruction Book #497

	"Go down swinging."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 09/13] ocfs2: Take ocfs2_xattr_bucket structures off of the stack.

[Joel Becker]

On Tue, Oct 28, 2008 at 10:24:38AM +0800, Tao Ma wrote:
> Joel Becker wrote:
>> The ocfs2_xattr_bucket structure is a nice abstraction, but it is a bit
>> large to have on the stack.  Just like ocfs2_path, let's allocate it
>> with a ocfs2_xattr_bucket_new() function.
>> We can now store the inode on the bucket, cleaning up all the other
>> bucket functions.  While we're here, we catch another place or two that
>> wasn't using ocfs2_read_xattr_bucket().
>> Signed-off-by: Joel Becker <joel.becker@oracle.com>
>
>
>> @@ -2042,6 +2074,18 @@ int ocfs2_xattr_set(struct inode *inode,
>>  	if (!ocfs2_supports_xattr(OCFS2_SB(inode->i_sb)))
>>  		return -EOPNOTSUPP;
>>  +	xis.bucket = ocfs2_xattr_bucket_new(inode);
>> +	if (!xis.bucket) {
>> +		mlog_errno(-ENOMEM);
>> +		return -ENOMEM;
>> +	}
> xis is used for xattr search in inode, so we don't need to allocate bucket.

	There is code lower down that I assume any xattr_search has a
bucket.  If we guarantee that xis will never get into that code, then we
don't need to allocate.

>> +	xbs.bucket = ocfs2_xattr_bucket_new(inode);
>> +	if (!xbs.bucket) {
>> +		ocfs2_xattr_bucket_free(xbs.bucket);
>> +		mlog_errno(-ENOMEM);
>> +		return -ENOMEM;
>> +	}
> can we allocate this only when we really need this? that means we may 
> allocate it in ocfs2_create_index_block(when we create a new one) and 
> ocfs2_xattr_index_block_find if there is some buckets exist?

	Because functions that xbs is passed to seem to assume the
bucket exists, I did it here.  We could allocate it later, but we'd have
to audit the code paths.

Joel

-- 

Life's Little Instruction Book #314

	"Never underestimate the power of forgiveness."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 06/13] ocfs2: Improve ocfs2_read_xattr_bucket().

[Tao Ma]

Joel Becker wrote:
> On Tue, Oct 28, 2008 at 10:44:35AM +0800, Tao Ma wrote:
>> Joel Becker wrote:
>>> @@ -3128,7 +3145,7 @@ static int ocfs2_half_xattr_bucket(struct inode 
>>> *inode,
>>>  	int ret, i;
>>>  	u16 count, start, len, name_value_len, xe_len, name_offset;
>>>  	u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb);
>>> -	struct buffer_head **s_bhs, **t_bhs = NULL;
>>> +	struct ocfs2_xattr_bucket s_bucket, t_bucket;
>>>  	struct ocfs2_xattr_header *xh;
>>>  	struct ocfs2_xattr_entry *xe;
>>>  	int blocksize = inode->i_sb->s_blocksize;
>> snip
>>> -	t_bhs = kcalloc(blk_per_bucket, sizeof(struct buffer_head *), GFP_NOFS);
>>> -	if (!t_bhs) {
>>> -		ret = -ENOMEM;
>>> -		goto out;
>>> -	}
>>> -
>>> -	ret = ocfs2_read_xattr_bucket(inode, new_blk, t_bhs, new_bucket_head);
>>> +	/*
>>> +	 * Even if !new_bucket_head, we're overwriting t_bucket.  Thus,
>>> +	 * there's no need to read it.
>>> +	 */
>>> +	ret = ocfs2_init_xattr_bucket(inode, &t_bucket, new_blk);
>>>  	if (ret) {
>>>  		mlog_errno(ret);
>>>  		goto out;
>>>  	}
>>>   	for (i = 0; i < blk_per_bucket; i++) {
>>> -		ret = ocfs2_journal_access(handle, inode, t_bhs[i],
>>> +		ret = ocfs2_journal_access(handle, inode, t_bucket.bu_bhs[i],
>>>  					   new_bucket_head ?
>>>  					   OCFS2_JOURNAL_ACCESS_CREATE :
>>>  					   OCFS2_JOURNAL_ACCESS_WRITE);
>> I have read the caller of ocfs2_half_xattr_bucket again. In 
>> ocfs2_extend_xattr_bucket when we want to half the bucket to a never-used 
>> new bucket within the same cluster(while we always pass new_bucket_head=0), 
>> do we need to use OCFS2_JOURNAL_ACCESS_CREATE? If yes, maybe we have to 
>> modify the caller to be more precisely(I can handle this based on your 
>> patch set). The same goes for ocfs2_cp_xattr_bucket.
> 
> 	Mark and I were discussing this on #ocfs2 just today.  We were
> having a hard time understanding the condition of the bucket - what
> new_bucket_head really means.  This comes from my patch to set the
> JOURNAL_ACCESS based on the new_bucket_head value (same in
> cp_xattr_bucket).
> 	Is the target bucket (t_bucket) always in the same cluster for
> all callers regardless of new_bucket_head?  Has it ever been written
> before?  May it have been allocated a long time ago?
I have read your discussion about it. :) The situation is a little 
complicated here. Sometimes in ocfs2_half_xattr_bucket t_bucket is never 
used before while sometimes it isn't.
Anyway, I will generate a patch for it later after you push this patch 
set, so that you can see what I mean and sob or NAK It.

Regards,
Tao

> 
> Joel
> 

[Ocfs2-devel] [PATCH 0/13] ocfs2: xattr bucket API

[Mark Fasheh]

On Mon, Oct 27, 2008 at 06:20:15PM -0700, Joel Becker wrote:
> When the extended attribute namespace grows to a b-tree, the leaf
> clusters are organized by means of 'buckets'.  Each bucket is 4K in
> size, regardless of blocksize.  Thus, a bucket may be made of more than
> one block.
> 
> fs/ocfs2/xattr.c has a nice little abstraction to wrap this, struct
> ocfs2_xattr_bucket.  It contains a list of buffer_heads representing
> these blocks, and there is even an API to fill it or initialize it.
> 
> However, the majority of the code does not use this abstraction.
> Instead, it uses raw buffer_heads and jumps through other hoops.
> This has two consequences.  First, it's harder to read.  Second, it is
> less efficient.  Sometimes it reads the first and last block of a
> bucket, when reading and writing all blocks at once is a streaming I/O.
> 
> This series expands the bucket API in a fashion similar to
> fs/ocfs2/alloc.c's struct ocfs2_path.  The hope is that all bucket
> operations can use this API and mostly avoid raw buffer_head work.
> 
> This is also needed for checksums of buckets, as the checksum
> calculations need to read the entire bucket.  Since a 4K contig I/O is
> just as cheap as a single block, this causes no loss of efficiency.
> 
> This path series is on top of my xattr-28 fixes branch.  Before going
> upstream it will be rebased on top of at least the divide_bucket fix.
> It only touches the xattr_get() and xattr_list() paths, becuase I need
> Tao's single-transaction xattr_set() before I can do the xattr_set()
> path.
> 
> All comments and review welcome.  I hope it's mostly self-explanatory.

This looks great.

I rebased your branch and added my signoffs to start the new merge_window
branch. Tao, this includes your latest fixes. Once Tao and Tiger have an
updated set of acl / security attr patches, I'll drop those in right on top.

One thing though - your patches are built on top of our 'fixes' branch,
which is great for merging down the line. Right now, merge_window will just
live alone, until Linus pulls our fixes. Once that happens, I'll rebase
merge_window one more time and start pulling it into linux-next / ALL
branches.

Sound good?
	--Mark

--
Mark Fasheh

[Ocfs2-devel] [PATCH 0/13] ocfs2: xattr bucket API

[Joel Becker]

On Fri, Nov 07, 2008 at 01:21:14PM -0800, Mark Fasheh wrote:
> One thing though - your patches are built on top of our 'fixes' branch,
> which is great for merging down the line. Right now, merge_window will just
> live alone, until Linus pulls our fixes. Once that happens, I'll rebase
> merge_window one more time and start pulling it into linux-next / ALL
> branches.

	I would think that merge_window should live atop our latest at
all times, including 'fixes'.

Joel

-- 

Life's Little Instruction Book #15

	"Own a great stereo system."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127