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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 01/13] ocfs2: Field prefixes for the xattr_bucket structure
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 02/13] ocfs2: Convenient access to an xattr bucket's block number.
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 03/13] ocfs2: Convenient access to xattr bucket data blocks.
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 04/13] ocfs2: Convenient access to an xattr bucket's header.
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 05/13] ocfs2: Provide a wrapper to brelse() xattr bucket buffers.
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 06/13] ocfs2: Improve ocfs2_read_xattr_bucket().
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 07/13] ocfs2: Wrap journal_access/journal_dirty for xattr buckets.
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 08/13] ocfs2: Copy xattr buckets with a dedicated function.
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 09/13] ocfs2: Take ocfs2_xattr_bucket structures off of the stack.
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 10/13] ocfs2: Use buckets in ocfs2_xattr_bucket_find().
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 11/13] ocfs2: Use buckets in ocfs2_xattr_create_index_block().
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 at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 12/13] ocfs2: Use buckets in ocfs2_defrag_xattr_bucket().
Use the ocfs2_xattr_bucket abstraction for reading and writing the bucket in ocfs2_defrag_xattr_bucket(). Signed-off-by: Joel Becker <joel.becker at 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
Joel Becker
2008-Oct-28 01:20 UTC
[Ocfs2-devel] [PATCH 13/13] ocfs2: Use buckets in ocfs2_xattr_set_entry_in_bucket().
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 at 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
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
Apparently Analagous Threads
- [PATCH] ocfs2: Use xs->bucket to set xattr value outside.
- [PATCH 1/1] ocfs2/xattr: Proper hash collision handle in bucket division.v3
- [git patches] Ocfs2 patches for merge window, batch 2/3
- [PATCH 00/42] ocfs2: Add reflink file support. V1
- [PATCH 00/42] ocfs2: Add reflink file support. V2