Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 0/13] More xattr bucket consolidation
There are still places in xattr.c that refer to individual buffer_heads rather than the ocfs2_xattr_bucket structure. This series tries to close all of those. It also reduces some of the buffer_head games we play. While doing this work, I think I've learned some more about the xattr code. So I've tried to add some breadcrumbs behind me. Tao, can we review this quickly? I'd like to get it into merge_window ASAP, so Tristan can test it and make sure I didn't break anything. The patches are on top of merge_window as of today, and are available in my repo at git://oss.oracle.com/git/jlbec/linux-2.6.git xattr-buckets-2 Joel
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 01/13] ocfs2: Dirty the entire bucket in ocfs2_bucket_value_truncate()
ocfs2_bucket_value_truncate() currently takes the first bh of the bucket, and magically plays around with the value bh - even though the bucket structure in the calling function already has it. In addition, future code wants to always dirty the entire bucket when it is changed. So let's pass the entire bucket into this function, skip any block reads (we have them), and add the access/dirty logic Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 44 ++++++++++++++++++++++++++------------------ 1 files changed, 26 insertions(+), 18 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 7e0d62a..4571df8 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -4619,7 +4619,7 @@ out: * Copy the new updated xe and xe_value_root to new_xe and new_xv if needed. */ static int ocfs2_xattr_bucket_value_truncate(struct inode *inode, - struct buffer_head *header_bh, + struct ocfs2_xattr_bucket *bucket, int xe_off, int len, struct ocfs2_xattr_set_ctxt *ctxt) @@ -4629,8 +4629,7 @@ static int ocfs2_xattr_bucket_value_truncate(struct inode *inode, struct buffer_head *value_bh = NULL; struct ocfs2_xattr_value_root *xv; struct ocfs2_xattr_entry *xe; - struct ocfs2_xattr_header *xh - (struct ocfs2_xattr_header *)header_bh->b_data; + struct ocfs2_xattr_header *xh = bucket_xh(bucket); size_t blocksize = inode->i_sb->s_blocksize; xe = &xh->xh_entries[xe_off]; @@ -4644,34 +4643,44 @@ static int ocfs2_xattr_bucket_value_truncate(struct inode *inode, /* We don't allow ocfs2_xattr_value to be stored in different block. */ BUG_ON(value_blk != (offset + OCFS2_XATTR_ROOT_SIZE - 1) / blocksize); - value_blk += header_bh->b_blocknr; - ret = ocfs2_read_block(inode, value_blk, &value_bh, NULL); + value_bh = bucket->bu_bhs[value_blk]; + BUG_ON(!value_bh); + + xv = (struct ocfs2_xattr_value_root *) + (value_bh->b_data + offset % blocksize); + + ret = ocfs2_xattr_bucket_journal_access(ctxt->handle, bucket, + OCFS2_JOURNAL_ACCESS_WRITE); if (ret) { mlog_errno(ret); goto out; } - xv = (struct ocfs2_xattr_value_root *) - (value_bh->b_data + offset % blocksize); - + /* + * From here on out we have to dirty the bucket. The generic + * value calls only modify one of the bucket's bhs, but we need + * to send the bucket at once. So if they error, they *could* have + * modified something. We have to assume they did, and dirty + * the whole bucket. This leaves us in a consistent state. + */ mlog(0, "truncate %u in xattr bucket %llu to %d bytes.\n", - xe_off, (unsigned long long)header_bh->b_blocknr, len); + xe_off, (unsigned long long)bucket_blkno(bucket), len); ret = ocfs2_xattr_value_truncate(inode, value_bh, xv, len, ctxt); if (ret) { mlog_errno(ret); - goto out; + goto out_dirty; } ret = ocfs2_xattr_value_update_size(inode, ctxt->handle, - header_bh, xe, len); - if (ret) { + bucket->bu_bhs[0], xe, len); + if (ret) mlog_errno(ret); - goto out; - } + +out_dirty: + ocfs2_xattr_bucket_journal_dirty(ctxt->handle, bucket); out: - brelse(value_bh); return ret; } @@ -4687,7 +4696,7 @@ static int ocfs2_xattr_bucket_value_truncate_xs(struct inode *inode, 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, offset, len, ctxt); if (ret) mlog_errno(ret); @@ -5129,8 +5138,7 @@ static int ocfs2_delete_xattr_in_bucket(struct inode *inode, if (ocfs2_xattr_is_local(xe)) continue; - ret = ocfs2_xattr_bucket_value_truncate(inode, - bucket->bu_bhs[0], + ret = ocfs2_xattr_bucket_value_truncate(inode, bucket, i, 0, &ctxt); if (ret) { mlog_errno(ret); -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 02/13] ocfs2: Dirty the entire first bucket in ocfs2_extend_xattr_bucket()
ocfs2_extend_xattr_bucket() takes an extent of buckets and shifts some of them down to make room for a new xattr. It is passed the first bh of the first bucket, because that is where we store the number of buckets in the extent. However, future code wants to always dirty the entire bucket when it is changed. So let's pass the entire bucket into this function, skip any block reads (we have them), and add the access/dirty logic. We also can skip passing in the target bucket bh - we only need its block number. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 85 +++++++++++++++++++++++++++++++++++------------------- 1 files changed, 55 insertions(+), 30 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 4571df8..9f2e70b 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -3911,7 +3911,7 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode, mlog_errno(ret); goto out; } - + ret = ocfs2_read_xattr_bucket(s_bucket, s_blkno); if (ret) goto out; @@ -4238,37 +4238,45 @@ leave: } /* - * Extend a new xattr bucket and move xattrs to the end one by one until - * We meet with start_bh. Only move half of the xattrs to the bucket after it. + * We are given an extent. 'first' is the bucket at the very front of + * the extent. The extent has space for an additional bucket past + * bucket_xh(first)->xh_num_buckets. 'target_blkno' is the block number + * of the target bucket. We wish to shift every bucket past the target + * down one, filling in that additional space. When we get back to the + * target, we split the target between itself and the now-empty bucket + * at target+1 (aka, target_blkno + blks_per_bucket). */ static int ocfs2_extend_xattr_bucket(struct inode *inode, handle_t *handle, - struct buffer_head *first_bh, - struct buffer_head *start_bh, + struct ocfs2_xattr_bucket *first, + u64 target_blk, u32 num_clusters) { int ret, credits; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); u16 blk_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb); - u64 start_blk = start_bh->b_blocknr, end_blk; - u32 num_buckets = num_clusters * ocfs2_xattr_buckets_per_cluster(osb); - struct ocfs2_xattr_header *first_xh - (struct ocfs2_xattr_header *)first_bh->b_data; - u16 bucket = le16_to_cpu(first_xh->xh_num_buckets); + u64 end_blk; + u16 new_bucket = le16_to_cpu(bucket_xh(first)->xh_num_buckets); mlog(0, "extend xattr bucket in %llu, xattr extend rec starting " - "from %llu, len = %u\n", (unsigned long long)start_blk, - (unsigned long long)first_bh->b_blocknr, num_clusters); + "from %llu, len = %u\n", (unsigned long long)target_blk, + (unsigned long long)bucket_blkno(first), num_clusters); - BUG_ON(bucket >= num_buckets); + /* The extent must have room for an additional bucket */ + BUG_ON(new_bucket >+ (num_clusters * ocfs2_xattr_buckets_per_cluster(osb))); - end_blk = first_bh->b_blocknr + (bucket - 1) * blk_per_bucket; + /* end_blk points to the last existing bucket */ + end_blk = bucket_blkno(first) + ((new_bucket - 1) * blk_per_bucket); /* - * We will touch all the buckets after the start_bh(include it). - * Then we add one more bucket. + * end_blk is the start of the last existing bucket. + * Thus, (end_blk - target_blk) covers the target bucket and + * every bucket after it up to, but not including, the last + * existing bucket. Then we add the last existing bucket, the + * new bucket, and the first bucket (3 * blk_per_bucket). */ - credits = end_blk - start_blk + 3 * blk_per_bucket + 1 + + credits = (end_blk - target_blk) + (3 * blk_per_bucket) + handle->h_buffer_credits; ret = ocfs2_extend_trans(handle, credits); if (ret) { @@ -4276,14 +4284,14 @@ static int ocfs2_extend_xattr_bucket(struct inode *inode, goto out; } - ret = ocfs2_journal_access(handle, inode, first_bh, - OCFS2_JOURNAL_ACCESS_WRITE); + ret = ocfs2_xattr_bucket_journal_access(handle, first, + OCFS2_JOURNAL_ACCESS_WRITE); if (ret) { mlog_errno(ret); goto out; } - while (end_blk != start_blk) { + while (end_blk != target_blk) { ret = ocfs2_cp_xattr_bucket(inode, handle, end_blk, end_blk + blk_per_bucket, 0); if (ret) @@ -4291,12 +4299,12 @@ static int ocfs2_extend_xattr_bucket(struct inode *inode, end_blk -= blk_per_bucket; } - /* Move half of the xattr in start_blk to the next bucket. */ - ret = ocfs2_divide_xattr_bucket(inode, handle, start_blk, - start_blk + blk_per_bucket, NULL, 0); + /* Move half of the xattr in target_blkno to the next bucket. */ + ret = ocfs2_divide_xattr_bucket(inode, handle, target_blk, + target_blk + blk_per_bucket, NULL, 0); - le16_add_cpu(&first_xh->xh_num_buckets, 1); - ocfs2_journal_dirty(handle, first_bh); + le16_add_cpu(&bucket_xh(first)->xh_num_buckets, 1); + ocfs2_xattr_bucket_journal_dirty(handle, first); out: return ret; @@ -4330,10 +4338,19 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode, int ret, num_buckets, extend = 1; u64 p_blkno; u32 e_cpos, num_clusters; + /* The bucket at the front of the extent */ + struct ocfs2_xattr_bucket *first; mlog(0, "Add new xattr bucket starting form %llu\n", (unsigned long long)header_bh->b_blocknr); + first = ocfs2_xattr_bucket_new(inode); + if (!first) { + ret = -ENOMEM; + mlog_errno(ret); + goto out; + } + /* * Add refrence for header_bh here because it may be * changed in ocfs2_add_new_xattr_cluster and we need @@ -4373,17 +4390,25 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode, } } - if (extend) + if (extend) { + /* These bucket reads should be cached */ + ret = ocfs2_read_xattr_bucket(first, first_bh->b_blocknr); + if (ret) { + mlog_errno(ret); + goto out; + } ret = ocfs2_extend_xattr_bucket(inode, ctxt->handle, - first_bh, - header_bh, + first, header_bh->b_blocknr, num_clusters); - if (ret) - mlog_errno(ret); + if (ret) + mlog_errno(ret); + } + out: brelse(first_bh); brelse(header_bh); + ocfs2_xattr_bucket_free(first); return ret; } -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 03/13] ocfs2: Dirty the entire first bucket in ocfs2_cp_xattr_cluster().
ocfs2_cp_xattr_cluster() takes the last bucket of a full extent and copies it over to a new extent. It then updates the headers of both extents to reflect the new state. It is passed the first bh of the first bucket in order to update that first extent's bucket count. It reads and dirties the first bh of the new extent for the same reason. However, future code wants to always dirty the entire bucket when it is changed. So it is changed to read the entire bucket it is updating for both extents. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 80 ++++++++++++++++++++++++++++++++--------------------- 1 files changed, 48 insertions(+), 32 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 9f2e70b..11b518a 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -3942,9 +3942,10 @@ out: } /* - * Copy one xattr cluster from src_blk to to_blk. - * The to_blk will become the first bucket header of the cluster, so its - * xh_num_buckets will be initialized as the bucket num in the cluster. + * src_blk points to the last cluster of an existing extent. to_blk + * points to a newly allocated extent. We copy the cluster over to the + * new extent, initializing its xh_num_buckets. The old extent's + * xh_num_buckets shrinks by the same amount. */ static int ocfs2_cp_xattr_cluster(struct inode *inode, handle_t *handle, @@ -3956,27 +3957,42 @@ static int ocfs2_cp_xattr_cluster(struct inode *inode, int i, ret, credits; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); int bpc = ocfs2_clusters_to_blocks(inode->i_sb, 1); + int blks_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb); int num_buckets = ocfs2_xattr_buckets_per_cluster(osb); - struct buffer_head *bh = NULL; - struct ocfs2_xattr_header *xh; - u64 to_blk_start = to_blk; + struct ocfs2_xattr_bucket *old_first, *new_first; mlog(0, "cp xattrs from cluster %llu to %llu\n", (unsigned long long)src_blk, (unsigned long long)to_blk); + /* The first bucket of the original extent */ + old_first = ocfs2_xattr_bucket_new(inode); + /* The first bucket of the new extent */ + new_first = ocfs2_xattr_bucket_new(inode); + if (!old_first || !new_first) { + ret = -ENOMEM; + mlog_errno(ret); + goto out; + } + + ret = ocfs2_read_xattr_bucket(old_first, first_bh->b_blocknr); + if (ret) { + mlog_errno(ret); + goto out; + } + /* - * We need to update the new cluster and 1 more for the update of - * the 1st bucket of the previous extent rec. + * We need to update the first bucket of the old extent and the + * entire first cluster of the new extent. */ - credits = bpc + 1 + handle->h_buffer_credits; + credits = blks_per_bucket + bpc + handle->h_buffer_credits; ret = ocfs2_extend_trans(handle, credits); if (ret) { mlog_errno(ret); goto out; } - ret = ocfs2_journal_access(handle, inode, first_bh, - OCFS2_JOURNAL_ACCESS_WRITE); + ret = ocfs2_xattr_bucket_journal_access(handle, old_first, + OCFS2_JOURNAL_ACCESS_WRITE); if (ret) { mlog_errno(ret); goto out; @@ -3984,45 +4000,45 @@ static int ocfs2_cp_xattr_cluster(struct inode *inode, for (i = 0; i < num_buckets; i++) { ret = ocfs2_cp_xattr_bucket(inode, handle, - src_blk, to_blk, 1); + src_blk + (i * blks_per_bucket), + to_blk + (i * blks_per_bucket), + 1); if (ret) { mlog_errno(ret); goto out; } - - src_blk += ocfs2_blocks_per_xattr_bucket(inode->i_sb); - to_blk += ocfs2_blocks_per_xattr_bucket(inode->i_sb); } - /* update the old bucket header. */ - xh = (struct ocfs2_xattr_header *)first_bh->b_data; - le16_add_cpu(&xh->xh_num_buckets, -num_buckets); - - ocfs2_journal_dirty(handle, first_bh); - - /* update the new bucket header. */ - ret = ocfs2_read_block(inode, to_blk_start, &bh, NULL); - if (ret < 0) { + /* + * Get the new bucket ready before we dirty anything + * (This actually shouldn't fail, because we already dirtied + * it once in ocfs2_cp_xattr_bucket()). + */ + ret = ocfs2_read_xattr_bucket(new_first, to_blk); + if (ret) { mlog_errno(ret); goto out; } - - ret = ocfs2_journal_access(handle, inode, bh, - OCFS2_JOURNAL_ACCESS_WRITE); + ret = ocfs2_xattr_bucket_journal_access(handle, new_first, + OCFS2_JOURNAL_ACCESS_WRITE); if (ret) { mlog_errno(ret); goto out; } - xh = (struct ocfs2_xattr_header *)bh->b_data; - xh->xh_num_buckets = cpu_to_le16(num_buckets); + /* Now update the headers */ + le16_add_cpu(&bucket_xh(old_first)->xh_num_buckets, -num_buckets); + ocfs2_xattr_bucket_journal_dirty(handle, old_first); - ocfs2_journal_dirty(handle, bh); + bucket_xh(new_first)->xh_num_buckets = cpu_to_le16(num_buckets); + ocfs2_xattr_bucket_journal_dirty(handle, new_first); if (first_hash) - *first_hash = le32_to_cpu(xh->xh_entries[0].xe_name_hash); + *first_hash = le32_to_cpu(bucket_xh(new_first)->xh_entries[0].xe_name_hash); + out: - brelse(bh); + ocfs2_xattr_bucket_free(new_first); + ocfs2_xattr_bucket_free(old_first); return ret; } -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 04/13] ocfs2: Explain t_is_new in ocfs2_cp_xattr_cluster().
I was unsure of the JOURNAL_ACCESS parameters in ocfs2_cp_xattr_cluster(). They're based on the function argument 't_is_new', but I couldn't quite figure out how t_is_new mapped to allocation. ocfs2_cp_xattr_cluster() actually overwrites the target, regardless of t_is_new. Well, I just figured it out. So I'm adding a big fat comment for those who come after me. ocfs2_divide_xattr_cluster() has the same behavior. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 11b518a..cb71dcc 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -3753,6 +3753,11 @@ static int ocfs2_divide_xattr_bucket(struct inode *inode, goto out; } + /* + * Hey, if we're overwriting t_bucket, what difference does + * ACCESS_CREATE vs ACCESS_WRITE make? See the comment in the + * same part of ocfs2_cp_xattr_bucket(). + */ ret = ocfs2_xattr_bucket_journal_access(handle, t_bucket, new_bucket_head ? OCFS2_JOURNAL_ACCESS_CREATE : @@ -3924,6 +3929,18 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode, if (ret) goto out; + /* + * Hey, if we're overwriting t_bucket, what difference does + * ACCESS_CREATE vs ACCESS_WRITE make? Well, if we allocated a new + * cluster to fill, we came here from ocfs2_cp_xattr_cluster(), and + * it is really new - ACCESS_CREATE is required. But we also + * might have moved data out of t_bucket before extending back + * into it. ocfs2_add_new_xattr_bucket() can do this - its call + * to ocfs2_add_new_xattr_cluster() may have created a new extent + * and copied out the end of the old extent. Then it re-extends + * the old extent back to create space for new xattrs. That's + * how we get here, and the bucket isn't really new. + */ ret = ocfs2_xattr_bucket_journal_access(handle, t_bucket, t_is_new ? OCFS2_JOURNAL_ACCESS_CREATE : -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 05/13] ocfs2: Use ocfs2_cp_xattr_bucket() in ocfs2_mv_xattr_bucket_cross_cluster().
The buffer copy loop of ocfs2_mv_xattr_bucket_cross_cluster() actually looks a lot like ocfs2_cp_xattr_bucket(). Let's just use that instead. We also use bucket operations to update the buckets at the start of each extent. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 169 +++++++++++++++++++++++++++++++++--------------------- 1 files changed, 104 insertions(+), 65 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index cb71dcc..c9e2f77 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -170,6 +170,11 @@ static int ocfs2_xattr_set_entry_index_block(struct inode *inode, static int ocfs2_delete_xattr_index_block(struct inode *inode, struct buffer_head *xb_bh); +static int ocfs2_cp_xattr_bucket(struct inode *inode, + handle_t *handle, + u64 s_blkno, + u64 t_blkno, + int t_is_new); static inline u16 ocfs2_xattr_buckets_per_cluster(struct ocfs2_super *osb) { @@ -3532,13 +3537,21 @@ out: } /* - * Move half nums of the xattr bucket in the previous cluster to this new - * cluster. We only touch the last cluster of the previous extend record. + * prev_blkno points to the start of an existing extent. new_blkno + * points to a newly allocated extent. Because we know each of our + * clusters contains more than bucket, we can easily split one cluster + * at a bucket boundary. So we take the last cluster of the existing + * extent and split it down the middle. We move the last half of the + * buckets in the last cluster of the existing extent over to the new + * extent. + * + * first_bh is the buffer at prev_blkno so we can update the existing + * extent's bucket count. header_bh is the bucket were we were hoping + * to insert our xattr. If the bucket move places the target in the new + * extent, we'll update first_bh and header_bh after modifying the old + * extent. * - * first_bh is the first buffer_head of a series of bucket in the same - * extent rec and header_bh is the header of one bucket in this cluster. - * They will be updated if we move the data header_bh contains to the new - * cluster. first_hash will be set as the 1st xe's name_hash of the new cluster. + * first_hash will be set as the 1st xe's name_hash in the new extent. */ static int ocfs2_mv_xattr_bucket_cross_cluster(struct inode *inode, handle_t *handle, @@ -3551,105 +3564,131 @@ static int ocfs2_mv_xattr_bucket_cross_cluster(struct inode *inode, { int i, ret, credits; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); + int blks_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb); int bpc = ocfs2_clusters_to_blocks(inode->i_sb, 1); int num_buckets = ocfs2_xattr_buckets_per_cluster(osb); - int blocksize = inode->i_sb->s_blocksize; - struct buffer_head *old_bh, *new_bh, *prev_bh, *new_first_bh = NULL; - struct ocfs2_xattr_header *new_xh; + int to_move = num_buckets / 2; + u64 last_cluster_blkno, src_blkno; struct ocfs2_xattr_header *xh (struct ocfs2_xattr_header *)((*first_bh)->b_data); + struct ocfs2_xattr_bucket *old_first, *new_first; BUG_ON(le16_to_cpu(xh->xh_num_buckets) < num_buckets); BUG_ON(OCFS2_XATTR_BUCKET_SIZE == osb->s_clustersize); - prev_bh = *first_bh; - get_bh(prev_bh); - xh = (struct ocfs2_xattr_header *)prev_bh->b_data; - - prev_blkno += (num_clusters - 1) * bpc + bpc / 2; + last_cluster_blkno = prev_blkno + ((num_clusters - 1) * bpc); + src_blkno = last_cluster_blkno + (to_move * blks_per_bucket); mlog(0, "move half of xattrs in cluster %llu to %llu\n", (unsigned long long)prev_blkno, (unsigned long long)new_blkno); + /* The first bucket of the original extent */ + old_first = ocfs2_xattr_bucket_new(inode); + /* The first bucket of the new extent */ + new_first = ocfs2_xattr_bucket_new(inode); + if (!old_first || !new_first) { + ret = -ENOMEM; + mlog_errno(ret); + goto out; + } + + ret = ocfs2_read_xattr_bucket(old_first, prev_blkno); + if (ret) { + mlog_errno(ret); + goto out; + } + /* - * We need to update the 1st half of the new cluster and - * 1 more for the update of the 1st bucket of the previous - * extent record. + * We need to update the 1st half of the new extent, and we + * need to update the first bucket of the old extent. */ - credits = bpc / 2 + 1 + handle->h_buffer_credits; + credits = ((to_move + 1) * blks_per_bucket) + handle->h_buffer_credits; ret = ocfs2_extend_trans(handle, credits); if (ret) { mlog_errno(ret); goto out; } - ret = ocfs2_journal_access(handle, inode, prev_bh, - OCFS2_JOURNAL_ACCESS_WRITE); + ret = ocfs2_xattr_bucket_journal_access(handle, old_first, + OCFS2_JOURNAL_ACCESS_WRITE); if (ret) { mlog_errno(ret); goto out; } - for (i = 0; i < bpc / 2; i++, prev_blkno++, new_blkno++) { - old_bh = new_bh = NULL; - new_bh = sb_getblk(inode->i_sb, new_blkno); - if (!new_bh) { - ret = -EIO; + for (i = 0; i < to_move; i++) { + ret = ocfs2_cp_xattr_bucket(inode, handle, + src_blkno + (i * blks_per_bucket), + new_blkno + (i * blks_per_bucket), + 1); + if (ret) { mlog_errno(ret); goto out; } + } - ocfs2_set_new_buffer_uptodate(inode, new_bh); + /* + * Get the new bucket ready before we dirty anything + * (This actually shouldn't fail, because we already dirtied + * it once in ocfs2_cp_xattr_bucket()). + */ + ret = ocfs2_read_xattr_bucket(new_first, new_blkno); + if (ret) { + mlog_errno(ret); + goto out; + } + ret = ocfs2_xattr_bucket_journal_access(handle, new_first, + OCFS2_JOURNAL_ACCESS_WRITE); + if (ret) { + mlog_errno(ret); + goto out; + } - ret = ocfs2_journal_access(handle, inode, new_bh, - OCFS2_JOURNAL_ACCESS_CREATE); - if (ret < 0) { - mlog_errno(ret); - brelse(new_bh); - goto out; - } + /* Now update the headers */ + le16_add_cpu(&bucket_xh(old_first)->xh_num_buckets, -to_move); + ocfs2_xattr_bucket_journal_dirty(handle, old_first); - ret = ocfs2_read_block(inode, prev_blkno, &old_bh, NULL); - if (ret < 0) { - mlog_errno(ret); - brelse(new_bh); - goto out; - } + bucket_xh(new_first)->xh_num_buckets = cpu_to_le16(to_move); + ocfs2_xattr_bucket_journal_dirty(handle, new_first); - memcpy(new_bh->b_data, old_bh->b_data, blocksize); + if (first_hash) + *first_hash = le32_to_cpu(bucket_xh(new_first)->xh_entries[0].xe_name_hash); - if (i == 0) { - new_xh = (struct ocfs2_xattr_header *)new_bh->b_data; - new_xh->xh_num_buckets = cpu_to_le16(num_buckets / 2); + /* + * If the target bucket is anywhere past src_blkno, we moved + * it to the new extent. We need to update first_bh and header_bh. + */ + if ((*header_bh)->b_blocknr >= src_blkno) { + /* We're done with old_first, so we can re-use it. */ + ocfs2_xattr_bucket_relse(old_first); - if (first_hash) - *first_hash = le32_to_cpu( - new_xh->xh_entries[0].xe_name_hash); - new_first_bh = new_bh; - get_bh(new_first_bh); - } + /* Find the block for the new target bucket */ + src_blkno = new_blkno + + ((*header_bh)->b_blocknr - src_blkno); - ocfs2_journal_dirty(handle, new_bh); + /* + * This shouldn't fail - the buffers are in the + * journal from ocfs2_cp_xattr_bucket(). + */ + ret = ocfs2_read_xattr_bucket(old_first, src_blkno); + if (ret) { + mlog_errno(ret); + goto out; + } - if (*header_bh == old_bh) { - brelse(*header_bh); - *header_bh = new_bh; - get_bh(*header_bh); + brelse(*first_bh); + *first_bh = new_first->bu_bhs[0]; + get_bh(*first_bh); - brelse(*first_bh); - *first_bh = new_first_bh; - get_bh(*first_bh); - } - brelse(new_bh); - brelse(old_bh); + brelse(*header_bh); + *header_bh = old_first->bu_bhs[0]; + get_bh(*header_bh); } - le16_add_cpu(&xh->xh_num_buckets, -(num_buckets / 2)); - - ocfs2_journal_dirty(handle, prev_bh); out: - brelse(prev_bh); - brelse(new_first_bh); + ocfs2_xattr_bucket_free(new_first); + ocfs2_xattr_bucket_free(old_first); + return ret; } -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 06/13] ocfs2: Rename ocfs2_cp_xattr_cluster() to ocfs2_mv_xattr_buckets().
ocfs2_cp_xattr_cluster() takes the last cluster of an xattr extent, copies its buckets to the front of a new extent, and then shrinks the bucket count of the original extent. So it's really moving the data, not copying it. While we're here, the function doesn't need a buffer_head for the old extent, just the block number. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 42 ++++++++++++++++++++++-------------------- 1 files changed, 22 insertions(+), 20 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index c9e2f77..8c0e24b 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -3971,11 +3971,12 @@ static int ocfs2_cp_xattr_bucket(struct inode *inode, /* * Hey, if we're overwriting t_bucket, what difference does * ACCESS_CREATE vs ACCESS_WRITE make? Well, if we allocated a new - * cluster to fill, we came here from ocfs2_cp_xattr_cluster(), and - * it is really new - ACCESS_CREATE is required. But we also - * might have moved data out of t_bucket before extending back - * into it. ocfs2_add_new_xattr_bucket() can do this - its call - * to ocfs2_add_new_xattr_cluster() may have created a new extent + * cluster to fill, we came here from + * ocfs2_mv_xattr_buckets(), and it is really new - + * ACCESS_CREATE is required. But we also might have moved data + * out of t_bucket before extending back into it. + * ocfs2_add_new_xattr_bucket() can do this - its call to + * ocfs2_add_new_xattr_cluster() may have created a new extent * and copied out the end of the old extent. Then it re-extends * the old extent back to create space for new xattrs. That's * how we get here, and the bucket isn't really new. @@ -3998,17 +3999,16 @@ out: } /* - * src_blk points to the last cluster of an existing extent. to_blk - * points to a newly allocated extent. We copy the cluster over to the - * new extent, initializing its xh_num_buckets. The old extent's - * xh_num_buckets shrinks by the same amount. + * src_blk points to the start of an existing extent. last_blk points to + * last cluster in that extent. to_blk points to a newly allocated + * extent. We copy the buckets from cluster at last_blk to the new extent, + * initializing its xh_num_buckets. The old extent's xh_num_buckets + * shrinks by the same amount. */ -static int ocfs2_cp_xattr_cluster(struct inode *inode, +static int ocfs2_mv_xattr_buckets(struct inode *inode, handle_t *handle, - struct buffer_head *first_bh, - u64 src_blk, - u64 to_blk, - u32 *first_hash) + u64 src_blk, u64 last_blk, + u64 to_blk, u32 *first_hash) { int i, ret, credits; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); @@ -4017,8 +4017,8 @@ static int ocfs2_cp_xattr_cluster(struct inode *inode, int num_buckets = ocfs2_xattr_buckets_per_cluster(osb); struct ocfs2_xattr_bucket *old_first, *new_first; - mlog(0, "cp xattrs from cluster %llu to %llu\n", - (unsigned long long)src_blk, (unsigned long long)to_blk); + mlog(0, "mv xattrs from cluster %llu to %llu\n", + (unsigned long long)last_blk, (unsigned long long)to_blk); /* The first bucket of the original extent */ old_first = ocfs2_xattr_bucket_new(inode); @@ -4030,7 +4030,7 @@ static int ocfs2_cp_xattr_cluster(struct inode *inode, goto out; } - ret = ocfs2_read_xattr_bucket(old_first, first_bh->b_blocknr); + ret = ocfs2_read_xattr_bucket(old_first, src_blk); if (ret) { mlog_errno(ret); goto out; @@ -4056,7 +4056,7 @@ static int ocfs2_cp_xattr_cluster(struct inode *inode, for (i = 0; i < num_buckets; i++) { ret = ocfs2_cp_xattr_bucket(inode, handle, - src_blk + (i * blks_per_bucket), + last_blk + (i * blks_per_bucket), to_blk + (i * blks_per_bucket), 1); if (ret) { @@ -4181,8 +4181,10 @@ static int ocfs2_adjust_xattr_cross_cluster(struct inode *inode, u64 last_blk = prev_blk + bpc * (prev_clusters - 1); if (prev_clusters > 1 && (*header_bh)->b_blocknr != last_blk) - ret = ocfs2_cp_xattr_cluster(inode, handle, *first_bh, - last_blk, new_blk, + ret = ocfs2_mv_xattr_buckets(inode, handle, + (*first_bh)->b_blocknr, + last_blk, + new_blk, v_start); else { ret = ocfs2_divide_xattr_cluster(inode, handle, -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 07/13] ocfs2: ocfs2_mv_xattr_buckets() can handle a partial cluster now.
If you look at ocfs2_mv_xattr_bucket_cross_cluster(), you'll notice that two-thirds of the code is almost identical to ocfs2_mv_xattr_buckets(). The only difference is that ocfs2_mv_xattr_buckets() moves a whole cluster's worth, while ocfs2_mv_xattr_bucket_cross_cluster() moves half the cluster. We change ocfs2_mv_xattr_buckets() to allow moving partial clusters. The original caller of ocfs2_mv_xattr_buckets() still moves the whole cluster's worth - it just passes a start_bucket of 0. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 33 ++++++++++++++++++++------------- 1 files changed, 20 insertions(+), 13 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 8c0e24b..652edaf 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -4001,18 +4001,19 @@ out: /* * src_blk points to the start of an existing extent. last_blk points to * last cluster in that extent. to_blk points to a newly allocated - * extent. We copy the buckets from cluster at last_blk to the new extent, - * initializing its xh_num_buckets. The old extent's xh_num_buckets - * shrinks by the same amount. + * extent. We copy the buckets from the cluster at last_blk to the new + * extent. If start_bucket is non-zero, we skip that many buckets before + * we start copying. The new extent's xh_num_buckets gets set to the + * number of buckets we copied. The old extent's xh_num_buckets shrinks + * by the same amount. */ -static int ocfs2_mv_xattr_buckets(struct inode *inode, - handle_t *handle, - u64 src_blk, u64 last_blk, - u64 to_blk, u32 *first_hash) +static int ocfs2_mv_xattr_buckets(struct inode *inode, handle_t *handle, + u64 src_blk, u64 last_blk, u64 to_blk, + unsigned int start_bucket, + u32 *first_hash) { int i, ret, credits; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); - int bpc = ocfs2_clusters_to_blocks(inode->i_sb, 1); int blks_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb); int num_buckets = ocfs2_xattr_buckets_per_cluster(osb); struct ocfs2_xattr_bucket *old_first, *new_first; @@ -4020,6 +4021,12 @@ static int ocfs2_mv_xattr_buckets(struct inode *inode, mlog(0, "mv xattrs from cluster %llu to %llu\n", (unsigned long long)last_blk, (unsigned long long)to_blk); + BUG_ON(start_bucket >= num_buckets); + if (start_bucket) { + num_buckets -= start_bucket; + last_blk += (start_bucket * blks_per_bucket); + } + /* The first bucket of the original extent */ old_first = ocfs2_xattr_bucket_new(inode); /* The first bucket of the new extent */ @@ -4037,10 +4044,11 @@ static int ocfs2_mv_xattr_buckets(struct inode *inode, } /* - * We need to update the first bucket of the old extent and the - * entire first cluster of the new extent. + * We need to update the first bucket of the old extent and all + * the buckets going to the new extent. */ - credits = blks_per_bucket + bpc + handle->h_buffer_credits; + credits = ((num_buckets + 1) * blks_per_bucket) + + handle->h_buffer_credits; ret = ocfs2_extend_trans(handle, credits); if (ret) { mlog_errno(ret); @@ -4183,8 +4191,7 @@ static int ocfs2_adjust_xattr_cross_cluster(struct inode *inode, if (prev_clusters > 1 && (*header_bh)->b_blocknr != last_blk) ret = ocfs2_mv_xattr_buckets(inode, handle, (*first_bh)->b_blocknr, - last_blk, - new_blk, + last_blk, new_blk, 0, v_start); else { ret = ocfs2_divide_xattr_cluster(inode, handle, -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 08/13] ocfs2: Use ocfs2_mv_xattr_buckets() in ocfs2_mv_xattr_bucket_cross_cluster().
Now that ocfs2_mv_xattr_buckets() can move a partial cluster's worth of buckets, ocfs2_mv_xattr_bucket_cross_cluster() can use it. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 110 ++++++++++++++--------------------------------------- 1 files changed, 29 insertions(+), 81 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 652edaf..2c9b673 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -170,11 +170,10 @@ static int ocfs2_xattr_set_entry_index_block(struct inode *inode, static int ocfs2_delete_xattr_index_block(struct inode *inode, struct buffer_head *xb_bh); -static int ocfs2_cp_xattr_bucket(struct inode *inode, - handle_t *handle, - u64 s_blkno, - u64 t_blkno, - int t_is_new); +static int ocfs2_mv_xattr_buckets(struct inode *inode, handle_t *handle, + u64 src_blk, u64 last_blk, u64 to_blk, + unsigned int start_bucket, + u32 *first_hash); static inline u16 ocfs2_xattr_buckets_per_cluster(struct ocfs2_super *osb) { @@ -3562,115 +3561,64 @@ static int ocfs2_mv_xattr_bucket_cross_cluster(struct inode *inode, u32 num_clusters, u32 *first_hash) { - int i, ret, credits; + int ret; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); int blks_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb); - int bpc = ocfs2_clusters_to_blocks(inode->i_sb, 1); int num_buckets = ocfs2_xattr_buckets_per_cluster(osb); int to_move = num_buckets / 2; - u64 last_cluster_blkno, src_blkno; + u64 src_blkno; + u64 last_cluster_blkno = prev_blkno + + ((num_clusters - 1) * ocfs2_clusters_to_blocks(inode->i_sb, 1)); struct ocfs2_xattr_header *xh (struct ocfs2_xattr_header *)((*first_bh)->b_data); - struct ocfs2_xattr_bucket *old_first, *new_first; + struct ocfs2_xattr_bucket *new_target, *new_first; BUG_ON(le16_to_cpu(xh->xh_num_buckets) < num_buckets); BUG_ON(OCFS2_XATTR_BUCKET_SIZE == osb->s_clustersize); - last_cluster_blkno = prev_blkno + ((num_clusters - 1) * bpc); - src_blkno = last_cluster_blkno + (to_move * blks_per_bucket); - mlog(0, "move half of xattrs in cluster %llu to %llu\n", - (unsigned long long)prev_blkno, (unsigned long long)new_blkno); + (unsigned long long)last_cluster_blkno, (unsigned long long)new_blkno); - /* The first bucket of the original extent */ - old_first = ocfs2_xattr_bucket_new(inode); /* The first bucket of the new extent */ new_first = ocfs2_xattr_bucket_new(inode); - if (!old_first || !new_first) { + /* The target bucket if it was moved to the new extent */ + new_target = ocfs2_xattr_bucket_new(inode); + if (!new_target || !new_first) { ret = -ENOMEM; mlog_errno(ret); goto out; } - ret = ocfs2_read_xattr_bucket(old_first, prev_blkno); + ret = ocfs2_mv_xattr_buckets(inode, handle, prev_blkno, + last_cluster_blkno, new_blkno, + to_move, first_hash); if (ret) { mlog_errno(ret); goto out; } - /* - * We need to update the 1st half of the new extent, and we - * need to update the first bucket of the old extent. - */ - credits = ((to_move + 1) * blks_per_bucket) + handle->h_buffer_credits; - ret = ocfs2_extend_trans(handle, credits); - if (ret) { - mlog_errno(ret); - goto out; - } - - ret = ocfs2_xattr_bucket_journal_access(handle, old_first, - OCFS2_JOURNAL_ACCESS_WRITE); - if (ret) { - mlog_errno(ret); - goto out; - } - - for (i = 0; i < to_move; i++) { - ret = ocfs2_cp_xattr_bucket(inode, handle, - src_blkno + (i * blks_per_bucket), - new_blkno + (i * blks_per_bucket), - 1); - if (ret) { - mlog_errno(ret); - goto out; - } - } - - /* - * Get the new bucket ready before we dirty anything - * (This actually shouldn't fail, because we already dirtied - * it once in ocfs2_cp_xattr_bucket()). - */ - ret = ocfs2_read_xattr_bucket(new_first, new_blkno); - if (ret) { - mlog_errno(ret); - goto out; - } - ret = ocfs2_xattr_bucket_journal_access(handle, new_first, - OCFS2_JOURNAL_ACCESS_WRITE); - if (ret) { - mlog_errno(ret); - goto out; - } - - /* Now update the headers */ - le16_add_cpu(&bucket_xh(old_first)->xh_num_buckets, -to_move); - ocfs2_xattr_bucket_journal_dirty(handle, old_first); - - bucket_xh(new_first)->xh_num_buckets = cpu_to_le16(to_move); - ocfs2_xattr_bucket_journal_dirty(handle, new_first); - - if (first_hash) - *first_hash = le32_to_cpu(bucket_xh(new_first)->xh_entries[0].xe_name_hash); + /* This is the first bucket that got moved */ + src_blkno = last_cluster_blkno + (to_move * blks_per_bucket); /* - * If the target bucket is anywhere past src_blkno, we moved - * it to the new extent. We need to update first_bh and header_bh. + * If the target bucket was part of the moved buckets, we need to + * update first_bh and header_bh. */ if ((*header_bh)->b_blocknr >= src_blkno) { - /* We're done with old_first, so we can re-use it. */ - ocfs2_xattr_bucket_relse(old_first); - /* Find the block for the new target bucket */ src_blkno = new_blkno + ((*header_bh)->b_blocknr - src_blkno); /* - * This shouldn't fail - the buffers are in the + * These shouldn't fail - the buffers are in the * journal from ocfs2_cp_xattr_bucket(). */ - ret = ocfs2_read_xattr_bucket(old_first, src_blkno); + ret = ocfs2_read_xattr_bucket(new_first, src_blkno); + if (ret) { + mlog_errno(ret); + goto out; + } + ret = ocfs2_read_xattr_bucket(new_target, src_blkno); if (ret) { mlog_errno(ret); goto out; @@ -3681,13 +3629,13 @@ static int ocfs2_mv_xattr_bucket_cross_cluster(struct inode *inode, get_bh(*first_bh); brelse(*header_bh); - *header_bh = old_first->bu_bhs[0]; + *header_bh = new_target->bu_bhs[0]; get_bh(*header_bh); } out: ocfs2_xattr_bucket_free(new_first); - ocfs2_xattr_bucket_free(old_first); + ocfs2_xattr_bucket_free(new_target); return ret; } -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 09/13] ocfs2: Start using buckets in ocfs2_adjust_xattr_cross_cluster().
We want to be passing around buckets instead of buffer_heads. Let's get them into ocfs2_adjust_xattr_cross_cluster. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 46 ++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 38 insertions(+), 8 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 2c9b673..4dcbc21 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -3613,7 +3613,7 @@ static int ocfs2_mv_xattr_bucket_cross_cluster(struct inode *inode, * These shouldn't fail - the buffers are in the * journal from ocfs2_cp_xattr_bucket(). */ - ret = ocfs2_read_xattr_bucket(new_first, src_blkno); + ret = ocfs2_read_xattr_bucket(new_first, new_blkno); if (ret) { mlog_errno(ret); goto out; @@ -4117,28 +4117,54 @@ static int ocfs2_adjust_xattr_cross_cluster(struct inode *inode, u32 *v_start, int *extend) { - int ret = 0; - int bpc = ocfs2_clusters_to_blocks(inode->i_sb, 1); + int ret; + struct ocfs2_xattr_bucket *first, *target; mlog(0, "adjust xattrs from cluster %llu len %u to %llu\n", (unsigned long long)prev_blk, prev_clusters, (unsigned long long)new_blk); + /* The first bucket of the original extent */ + first = ocfs2_xattr_bucket_new(inode); + /* The target bucket for insert */ + target = ocfs2_xattr_bucket_new(inode); + if (!first || !target) { + ret = -ENOMEM; + mlog_errno(ret); + goto out; + } + + BUG_ON(prev_blk != (*first_bh)->b_blocknr); + ret = ocfs2_read_xattr_bucket(first, prev_blk); + if (ret) { + mlog_errno(ret); + goto out; + } + + ret = ocfs2_read_xattr_bucket(target, (*header_bh)->b_blocknr); + if (ret) { + mlog_errno(ret); + goto out; + } + if (ocfs2_xattr_buckets_per_cluster(OCFS2_SB(inode->i_sb)) > 1) ret = ocfs2_mv_xattr_bucket_cross_cluster(inode, handle, first_bh, header_bh, new_blk, - prev_blk, + bucket_blkno(first), prev_clusters, v_start); else { - u64 last_blk = prev_blk + bpc * (prev_clusters - 1); + /* The start of the last cluster in the first extent */ + u64 last_blk = bucket_blkno(first) + + ((prev_clusters - 1) * + ocfs2_clusters_to_blocks(inode->i_sb, 1)); - if (prev_clusters > 1 && (*header_bh)->b_blocknr != last_blk) + if (prev_clusters > 1 && bucket_blkno(target) != last_blk) ret = ocfs2_mv_xattr_buckets(inode, handle, - (*first_bh)->b_blocknr, + bucket_blkno(first), last_blk, new_blk, 0, v_start); else { @@ -4146,11 +4172,15 @@ static int ocfs2_adjust_xattr_cross_cluster(struct inode *inode, last_blk, new_blk, v_start); - if ((*header_bh)->b_blocknr == last_blk && extend) + if ((bucket_blkno(target) == last_blk) && extend) *extend = 0; } } +out: + ocfs2_xattr_bucket_free(first); + ocfs2_xattr_bucket_free(target); + return ret; } -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 10/13] ocfs2: Pass buckets into ocfs2_mv_xattr_bucket_cross_cluster().
Now that ocfs2_adjust_xattr_cross_cluster() has buckets, it can pass them into ocfs2_mv_xattr_bucket_cross_cluster(). It no longer has to care about buffer_heads. The manipulation of first_bh and header_bh moves up to ocfs2_adjust_xattr_cross_cluster(). Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 84 +++++++++++++++++++++++------------------------------ 1 files changed, 37 insertions(+), 47 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 4dcbc21..8ea4032 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -3554,42 +3554,28 @@ out: */ static int ocfs2_mv_xattr_bucket_cross_cluster(struct inode *inode, handle_t *handle, - struct buffer_head **first_bh, - struct buffer_head **header_bh, + struct ocfs2_xattr_bucket *first, + struct ocfs2_xattr_bucket *target, u64 new_blkno, - u64 prev_blkno, u32 num_clusters, u32 *first_hash) { int ret; - struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); - int blks_per_bucket = ocfs2_blocks_per_xattr_bucket(inode->i_sb); - int num_buckets = ocfs2_xattr_buckets_per_cluster(osb); + struct super_block *sb = inode->i_sb; + int blks_per_bucket = ocfs2_blocks_per_xattr_bucket(sb); + int num_buckets = ocfs2_xattr_buckets_per_cluster(OCFS2_SB(sb)); int to_move = num_buckets / 2; u64 src_blkno; - u64 last_cluster_blkno = prev_blkno + - ((num_clusters - 1) * ocfs2_clusters_to_blocks(inode->i_sb, 1)); - struct ocfs2_xattr_header *xh - (struct ocfs2_xattr_header *)((*first_bh)->b_data); - struct ocfs2_xattr_bucket *new_target, *new_first; + u64 last_cluster_blkno = bucket_blkno(first) + + ((num_clusters - 1) * ocfs2_clusters_to_blocks(sb, 1)); - BUG_ON(le16_to_cpu(xh->xh_num_buckets) < num_buckets); - BUG_ON(OCFS2_XATTR_BUCKET_SIZE == osb->s_clustersize); + BUG_ON(le16_to_cpu(bucket_xh(first)->xh_num_buckets) < num_buckets); + BUG_ON(OCFS2_XATTR_BUCKET_SIZE == OCFS2_SB(sb)->s_clustersize); mlog(0, "move half of xattrs in cluster %llu to %llu\n", (unsigned long long)last_cluster_blkno, (unsigned long long)new_blkno); - /* The first bucket of the new extent */ - new_first = ocfs2_xattr_bucket_new(inode); - /* The target bucket if it was moved to the new extent */ - new_target = ocfs2_xattr_bucket_new(inode); - if (!new_target || !new_first) { - ret = -ENOMEM; - mlog_errno(ret); - goto out; - } - - ret = ocfs2_mv_xattr_buckets(inode, handle, prev_blkno, + ret = ocfs2_mv_xattr_buckets(inode, handle, bucket_blkno(first), last_cluster_blkno, new_blkno, to_move, first_hash); if (ret) { @@ -3602,41 +3588,32 @@ static int ocfs2_mv_xattr_bucket_cross_cluster(struct inode *inode, /* * If the target bucket was part of the moved buckets, we need to - * update first_bh and header_bh. + * update first and target. */ - if ((*header_bh)->b_blocknr >= src_blkno) { + if (bucket_blkno(target) >= src_blkno) { /* Find the block for the new target bucket */ src_blkno = new_blkno + - ((*header_bh)->b_blocknr - src_blkno); + (bucket_blkno(target) - src_blkno); + + ocfs2_xattr_bucket_relse(first); + ocfs2_xattr_bucket_relse(target); /* * These shouldn't fail - the buffers are in the * journal from ocfs2_cp_xattr_bucket(). */ - ret = ocfs2_read_xattr_bucket(new_first, new_blkno); + ret = ocfs2_read_xattr_bucket(first, new_blkno); if (ret) { mlog_errno(ret); goto out; } - ret = ocfs2_read_xattr_bucket(new_target, src_blkno); - if (ret) { + ret = ocfs2_read_xattr_bucket(target, src_blkno); + if (ret) mlog_errno(ret); - goto out; - } - brelse(*first_bh); - *first_bh = new_first->bu_bhs[0]; - get_bh(*first_bh); - - brelse(*header_bh); - *header_bh = new_target->bu_bhs[0]; - get_bh(*header_bh); } out: - ocfs2_xattr_bucket_free(new_first); - ocfs2_xattr_bucket_free(new_target); - return ret; } @@ -4147,16 +4124,29 @@ static int ocfs2_adjust_xattr_cross_cluster(struct inode *inode, goto out; } - if (ocfs2_xattr_buckets_per_cluster(OCFS2_SB(inode->i_sb)) > 1) + if (ocfs2_xattr_buckets_per_cluster(OCFS2_SB(inode->i_sb)) > 1) { ret = ocfs2_mv_xattr_bucket_cross_cluster(inode, handle, - first_bh, - header_bh, + first, target, new_blk, - bucket_blkno(first), prev_clusters, v_start); - else { + if (ret) { + mlog_errno(ret); + goto out; + } + + /* Did first+target get moved? */ + if (prev_blk != bucket_blkno(first)) { + brelse(*first_bh); + *first_bh = first->bu_bhs[0]; + get_bh(*first_bh); + + brelse(*header_bh); + *header_bh = target->bu_bhs[0]; + get_bh(*header_bh); + } + } else { /* The start of the last cluster in the first extent */ u64 last_blk = bucket_blkno(first) + ((prev_clusters - 1) * -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 11/13] ocfs2: Move buckets up into ocfs2_add_new_xattr_cluster().
Lift the buckets from ocfs2_adjust_xattr_cross_cluster() up into ocfs2_add_new_xattr_cluster(). Now ocfs2_adjust_xattr_cross_cluster() doesn't deal with buffer_heads. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 100 ++++++++++++++++++++++++++--------------------------- 1 files changed, 49 insertions(+), 51 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 8ea4032..b9128aa 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -4086,44 +4086,19 @@ static int ocfs2_divide_xattr_cluster(struct inode *inode, */ static int ocfs2_adjust_xattr_cross_cluster(struct inode *inode, handle_t *handle, - struct buffer_head **first_bh, - struct buffer_head **header_bh, + struct ocfs2_xattr_bucket *first, + struct ocfs2_xattr_bucket *target, u64 new_blk, - u64 prev_blk, u32 prev_clusters, u32 *v_start, int *extend) { int ret; - struct ocfs2_xattr_bucket *first, *target; mlog(0, "adjust xattrs from cluster %llu len %u to %llu\n", - (unsigned long long)prev_blk, prev_clusters, + (unsigned long long)bucket_blkno(first), prev_clusters, (unsigned long long)new_blk); - /* The first bucket of the original extent */ - first = ocfs2_xattr_bucket_new(inode); - /* The target bucket for insert */ - target = ocfs2_xattr_bucket_new(inode); - if (!first || !target) { - ret = -ENOMEM; - mlog_errno(ret); - goto out; - } - - BUG_ON(prev_blk != (*first_bh)->b_blocknr); - ret = ocfs2_read_xattr_bucket(first, prev_blk); - if (ret) { - mlog_errno(ret); - goto out; - } - - ret = ocfs2_read_xattr_bucket(target, (*header_bh)->b_blocknr); - if (ret) { - mlog_errno(ret); - goto out; - } - if (ocfs2_xattr_buckets_per_cluster(OCFS2_SB(inode->i_sb)) > 1) { ret = ocfs2_mv_xattr_bucket_cross_cluster(inode, handle, @@ -4131,46 +4106,33 @@ static int ocfs2_adjust_xattr_cross_cluster(struct inode *inode, new_blk, prev_clusters, v_start); - if (ret) { + if (ret) mlog_errno(ret); - goto out; - } - - /* Did first+target get moved? */ - if (prev_blk != bucket_blkno(first)) { - brelse(*first_bh); - *first_bh = first->bu_bhs[0]; - get_bh(*first_bh); - - brelse(*header_bh); - *header_bh = target->bu_bhs[0]; - get_bh(*header_bh); - } } else { /* The start of the last cluster in the first extent */ u64 last_blk = bucket_blkno(first) + ((prev_clusters - 1) * ocfs2_clusters_to_blocks(inode->i_sb, 1)); - if (prev_clusters > 1 && bucket_blkno(target) != last_blk) + if (prev_clusters > 1 && bucket_blkno(target) != last_blk) { ret = ocfs2_mv_xattr_buckets(inode, handle, bucket_blkno(first), last_blk, new_blk, 0, v_start); - else { + if (ret) + mlog_errno(ret); + } else { ret = ocfs2_divide_xattr_cluster(inode, handle, last_blk, new_blk, v_start); + if (ret) + mlog_errno(ret); if ((bucket_blkno(target) == last_blk) && extend) *extend = 0; } } -out: - ocfs2_xattr_bucket_free(first); - ocfs2_xattr_bucket_free(target); - return ret; } @@ -4208,6 +4170,7 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, handle_t *handle = ctxt->handle; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_extent_tree et; + struct ocfs2_xattr_bucket *first, *target; mlog(0, "Add new xattr cluster for %llu, previous xattr hash = %u, " "previous xattr blkno = %llu\n", @@ -4216,6 +4179,29 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, ocfs2_init_xattr_tree_extent_tree(&et, inode, root_bh); + /* The first bucket of the original extent */ + first = ocfs2_xattr_bucket_new(inode); + /* The target bucket for insert */ + target = ocfs2_xattr_bucket_new(inode); + if (!first || !target) { + ret = -ENOMEM; + mlog_errno(ret); + goto leave; + } + + BUG_ON(prev_blkno != (*first_bh)->b_blocknr); + ret = ocfs2_read_xattr_bucket(first, prev_blkno); + if (ret) { + mlog_errno(ret); + goto leave; + } + + ret = ocfs2_read_xattr_bucket(target, (*header_bh)->b_blocknr); + if (ret) { + mlog_errno(ret); + goto leave; + } + ret = ocfs2_journal_access(handle, inode, root_bh, OCFS2_JOURNAL_ACCESS_WRITE); if (ret < 0) { @@ -4256,10 +4242,9 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, } else { ret = ocfs2_adjust_xattr_cross_cluster(inode, handle, - first_bh, - header_bh, + first, + target, block, - prev_blkno, prev_clusters, &v_start, extend); @@ -4267,6 +4252,17 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, mlog_errno(ret); goto leave; } + + /* Did first+target get moved? */ + if (prev_blkno != bucket_blkno(first)) { + brelse(*first_bh); + *first_bh = first->bu_bhs[0]; + get_bh(*first_bh); + + brelse(*header_bh); + *header_bh = target->bu_bhs[0]; + get_bh(*header_bh); + } } mlog(0, "Insert %u clusters at block %llu for xattr at %u\n", @@ -4283,6 +4279,8 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, mlog_errno(ret); leave: + ocfs2_xattr_bucket_free(first); + ocfs2_xattr_bucket_free(target); return ret; } -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 12/13] ocfs2: Move buckets up into ocfs2_add_new_xattr_bucket().
Lift the buckets from ocfs2_add_new_xattr_cluster() up into ocfs2_add_new_xattr_bucket(). Now ocfs2_add_new_xattr_cluster() doesn't deal with buffer_heads. In fact, we no longer have to play get_bh() tricks at all. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 103 +++++++++++++++++------------------------------------- 1 files changed, 32 insertions(+), 71 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index b9128aa..57171d9 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -4154,11 +4154,10 @@ static int ocfs2_adjust_xattr_cross_cluster(struct inode *inode, */ static int ocfs2_add_new_xattr_cluster(struct inode *inode, struct buffer_head *root_bh, - struct buffer_head **first_bh, - struct buffer_head **header_bh, + struct ocfs2_xattr_bucket *first, + struct ocfs2_xattr_bucket *target, u32 *num_clusters, u32 prev_cpos, - u64 prev_blkno, int *extend, struct ocfs2_xattr_set_ctxt *ctxt) { @@ -4170,38 +4169,14 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, handle_t *handle = ctxt->handle; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_extent_tree et; - struct ocfs2_xattr_bucket *first, *target; mlog(0, "Add new xattr cluster for %llu, previous xattr hash = %u, " "previous xattr blkno = %llu\n", (unsigned long long)OCFS2_I(inode)->ip_blkno, - prev_cpos, (unsigned long long)prev_blkno); + prev_cpos, (unsigned long long)bucket_blkno(first)); ocfs2_init_xattr_tree_extent_tree(&et, inode, root_bh); - /* The first bucket of the original extent */ - first = ocfs2_xattr_bucket_new(inode); - /* The target bucket for insert */ - target = ocfs2_xattr_bucket_new(inode); - if (!first || !target) { - ret = -ENOMEM; - mlog_errno(ret); - goto leave; - } - - BUG_ON(prev_blkno != (*first_bh)->b_blocknr); - ret = ocfs2_read_xattr_bucket(first, prev_blkno); - if (ret) { - mlog_errno(ret); - goto leave; - } - - ret = ocfs2_read_xattr_bucket(target, (*header_bh)->b_blocknr); - if (ret) { - mlog_errno(ret); - goto leave; - } - ret = ocfs2_journal_access(handle, inode, root_bh, OCFS2_JOURNAL_ACCESS_WRITE); if (ret < 0) { @@ -4223,7 +4198,7 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, mlog(0, "Allocating %u clusters at block %u for xattr in inode %llu\n", num_bits, bit_off, (unsigned long long)OCFS2_I(inode)->ip_blkno); - if (prev_blkno + prev_clusters * bpc == block && + if (bucket_blkno(first) + (prev_clusters * bpc) == block && (prev_clusters + num_bits) << osb->s_clustersize_bits < OCFS2_MAX_XATTR_TREE_LEAF_SIZE) { /* @@ -4252,17 +4227,6 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, mlog_errno(ret); goto leave; } - - /* Did first+target get moved? */ - if (prev_blkno != bucket_blkno(first)) { - brelse(*first_bh); - *first_bh = first->bu_bhs[0]; - get_bh(*first_bh); - - brelse(*header_bh); - *header_bh = target->bu_bhs[0]; - get_bh(*header_bh); - } } mlog(0, "Insert %u clusters at block %llu for xattr at %u\n", @@ -4363,16 +4327,16 @@ out: * We will move all the buckets starting from header_bh to the next place. As * for this one, half num of its xattrs will be moved to the next one. * - * We will allocate a new cluster if current cluster is full and adjust - * header_bh and first_bh if the insert place is moved to the new cluster. + * We will allocate a new cluster if current cluster is full. The + * underlying calls will make sure that there is space at the target + * bucket, shifting buckets around if necessary. 'target' may be updated + * by those calls. */ static int ocfs2_add_new_xattr_bucket(struct inode *inode, struct buffer_head *xb_bh, struct buffer_head *header_bh, struct ocfs2_xattr_set_ctxt *ctxt) { - struct ocfs2_xattr_header *first_xh = NULL; - struct buffer_head *first_bh = NULL; struct ocfs2_xattr_block *xb (struct ocfs2_xattr_block *)xb_bh->b_data; struct ocfs2_xattr_tree_root *xb_root = &xb->xb_attrs.xb_root; @@ -4380,31 +4344,26 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode, struct ocfs2_xattr_header *xh (struct ocfs2_xattr_header *)header_bh->b_data; u32 name_hash = le32_to_cpu(xh->xh_entries[0].xe_name_hash); - struct super_block *sb = inode->i_sb; - struct ocfs2_super *osb = OCFS2_SB(sb); + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); int ret, num_buckets, extend = 1; u64 p_blkno; u32 e_cpos, num_clusters; /* The bucket at the front of the extent */ - struct ocfs2_xattr_bucket *first; + struct ocfs2_xattr_bucket *first, *target; mlog(0, "Add new xattr bucket starting form %llu\n", (unsigned long long)header_bh->b_blocknr); + /* The first bucket of the original extent */ first = ocfs2_xattr_bucket_new(inode); - if (!first) { + /* The target bucket for insert */ + target = ocfs2_xattr_bucket_new(inode); + if (!first || !target) { ret = -ENOMEM; mlog_errno(ret); goto out; } - /* - * Add refrence for header_bh here because it may be - * changed in ocfs2_add_new_xattr_cluster and we need - * to free it in the end. - */ - get_bh(header_bh); - ret = ocfs2_xattr_get_rec(inode, name_hash, &p_blkno, &e_cpos, &num_clusters, el); if (ret) { @@ -4412,23 +4371,30 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode, goto out; } - ret = ocfs2_read_block(inode, p_blkno, &first_bh, NULL); + ret = ocfs2_read_xattr_bucket(first, p_blkno); if (ret) { mlog_errno(ret); goto out; } - num_buckets = ocfs2_xattr_buckets_per_cluster(osb) * num_clusters; - first_xh = (struct ocfs2_xattr_header *)first_bh->b_data; + ret = ocfs2_read_xattr_bucket(target, header_bh->b_blocknr); + if (ret) { + mlog_errno(ret); + goto out; + } - if (num_buckets == le16_to_cpu(first_xh->xh_num_buckets)) { + num_buckets = ocfs2_xattr_buckets_per_cluster(osb) * num_clusters; + if (num_buckets == le16_to_cpu(bucket_xh(first)->xh_num_buckets)) { + /* + * This can move first+target if the target bucket moves + * to the new extent. + */ ret = ocfs2_add_new_xattr_cluster(inode, xb_bh, - &first_bh, - &header_bh, + first, + target, &num_clusters, e_cpos, - p_blkno, &extend, ctxt); if (ret) { @@ -4438,24 +4404,19 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode, } if (extend) { - /* These bucket reads should be cached */ - ret = ocfs2_read_xattr_bucket(first, first_bh->b_blocknr); - if (ret) { - mlog_errno(ret); - goto out; - } ret = ocfs2_extend_xattr_bucket(inode, ctxt->handle, - first, header_bh->b_blocknr, + first, + bucket_blkno(target), num_clusters); if (ret) mlog_errno(ret); } out: - brelse(first_bh); - brelse(header_bh); ocfs2_xattr_bucket_free(first); + ocfs2_xattr_bucket_free(target); + return ret; } -- 1.5.6.5
Joel Becker
2008-Nov-27 00:06 UTC
[Ocfs2-devel] [PATCH 13/13] ocfs2: Pass xs->bucket into ocfs2_add_new_xattr_bucket().
Pass the actual target bucket for insert through to ocfs2_add_new_xattr_bucket(). Now growing a bucket has no buffer_head knowledge. ocfs2_add_new_xattr_bucket() leavs xs->bucket in the proper state for insert. However, it doesn't update the rest of the search fields in xs, so we still have to relse() and re-find. That's OK, because everything is cached. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 52 +++++++++++++++++++++++++--------------------------- 1 files changed, 25 insertions(+), 27 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 57171d9..cb9617d 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -4322,43 +4322,42 @@ out: } /* - * Add new xattr bucket in an extent record and adjust the buckets accordingly. - * xb_bh is the ocfs2_xattr_block. - * We will move all the buckets starting from header_bh to the next place. As - * for this one, half num of its xattrs will be moved to the next one. + * Add new xattr bucket in an extent record and adjust the buckets + * accordingly. xb_bh is the ocfs2_xattr_block, and target is the + * bucket we want to insert into. * - * We will allocate a new cluster if current cluster is full. The - * underlying calls will make sure that there is space at the target - * bucket, shifting buckets around if necessary. 'target' may be updated - * by those calls. + * In the easy case, we will move all the buckets after target down by + * one. Half of target's xattrs will be moved to the next bucket. + * + * If current cluster is full, we'll allocate a new one. This may not + * be contiguous. The underlying calls will make sure that there is + * space for the insert, shifting buckets around if necessary. + * 'target' may be moved by those calls. */ static int ocfs2_add_new_xattr_bucket(struct inode *inode, struct buffer_head *xb_bh, - struct buffer_head *header_bh, + struct ocfs2_xattr_bucket *target, struct ocfs2_xattr_set_ctxt *ctxt) { struct ocfs2_xattr_block *xb (struct ocfs2_xattr_block *)xb_bh->b_data; struct ocfs2_xattr_tree_root *xb_root = &xb->xb_attrs.xb_root; struct ocfs2_extent_list *el = &xb_root->xt_list; - struct ocfs2_xattr_header *xh - (struct ocfs2_xattr_header *)header_bh->b_data; - u32 name_hash = le32_to_cpu(xh->xh_entries[0].xe_name_hash); + u32 name_hash + le32_to_cpu(bucket_xh(target)->xh_entries[0].xe_name_hash); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); int ret, num_buckets, extend = 1; u64 p_blkno; u32 e_cpos, num_clusters; /* The bucket at the front of the extent */ - struct ocfs2_xattr_bucket *first, *target; + struct ocfs2_xattr_bucket *first; - mlog(0, "Add new xattr bucket starting form %llu\n", - (unsigned long long)header_bh->b_blocknr); + mlog(0, "Add new xattr bucket starting from %llu\n", + (unsigned long long)bucket_blkno(target)); /* The first bucket of the original extent */ first = ocfs2_xattr_bucket_new(inode); - /* The target bucket for insert */ - target = ocfs2_xattr_bucket_new(inode); - if (!first || !target) { + if (!first) { ret = -ENOMEM; mlog_errno(ret); goto out; @@ -4377,12 +4376,6 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode, goto out; } - ret = ocfs2_read_xattr_bucket(target, header_bh->b_blocknr); - if (ret) { - mlog_errno(ret); - goto out; - } - num_buckets = ocfs2_xattr_buckets_per_cluster(osb) * num_clusters; if (num_buckets == le16_to_cpu(bucket_xh(first)->xh_num_buckets)) { /* @@ -4415,7 +4408,6 @@ static int ocfs2_add_new_xattr_bucket(struct inode *inode, out: ocfs2_xattr_bucket_free(first); - ocfs2_xattr_bucket_free(target); return ret; } @@ -5119,15 +5111,21 @@ try_again: ret = ocfs2_add_new_xattr_bucket(inode, xs->xattr_bh, - xs->bucket->bu_bhs[0], + xs->bucket, ctxt); if (ret) { mlog_errno(ret); goto out; } + /* + * ocfs2_add_new_xattr_bucket() will have updated + * xs->bucket if it moved, but it will not have updated + * any of the other search fields. Thus, we drop it and + * re-search. Everything should be cached, so it'll be + * quick. + */ ocfs2_xattr_bucket_relse(xs->bucket); - ret = ocfs2_xattr_index_block_find(inode, xs->xattr_bh, xi->name_index, xi->name, xs); -- 1.5.6.5
Joel Becker
2008-Dec-11 02:13 UTC
[Ocfs2-devel] [PATCH 12/13] ocfs2: Move buckets up into ocfs2_add_new_xattr_bucket().
On Wed, Nov 26, 2008 at 04:06:25PM -0800, Joel Becker wrote:> Lift the buckets from ocfs2_add_new_xattr_cluster() up into > ocfs2_add_new_xattr_bucket(). Now ocfs2_add_new_xattr_cluster() > doesn't deal with buffer_heads. In fact, we no longer have to play > get_bh() tricks at all.Tristan found a bug here, filed as http://oss.oracle.com/bugzilla/show_bug.cgi?id=1057. Here's the fix, attached to the bugzilla entry. I will be rebasing my tree to include this in the original patch (the xattr-buckets-2 branch).>From 61d88e85c741430c15e4176abc99a5cf11435ff2 Mon Sep 17 00:00:00 2001From: Joel Becker <joel.becker at oracle.com> Date: Wed, 10 Dec 2008 14:11:34 -0800 Subject: [PATCH] ocfs2: Don't free buckets in ocfs2_add_new_xattr_cluster() When I moved the buckets to the parent of ocfs2_add_new_xattr_cluster(), I forgot to remove the ocfs2_xattr_bucket_free() calls at the bottom. This causes garbage. Signed-off-by: Joel Becker <joel.becker at oracle.com> --- fs/ocfs2/xattr.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index b75522e..f732e62 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -4288,8 +4288,6 @@ static int ocfs2_add_new_xattr_cluster(struct inode *inode, mlog_errno(ret); leave: - ocfs2_xattr_bucket_free(first); - ocfs2_xattr_bucket_free(target); return ret; } -- 1.5.6.5 -- "I always thought the hardest questions were those I could not answer. Now I know they are the ones I can never ask." - Charlie Watkins Joel Becker Principal Software Developer Oracle E-mail: joel.becker at oracle.com Phone: (650) 506-8127