[Ocfs2-devel] [PATCH 13/15] Enable xattr set in index btree. v3

[Tao Ma]

Mark Fasheh wrote:
> On Thu, Aug 07, 2008 at 06:31:33AM +0800, Tao Ma wrote:
>> +/*
>> + * Add a new cluster for xattr storage.
>> + *
>> + * If the new cluster is contiguous with the previous one, it will be
>> + * appended to the same extent record, and num_clusters will be updated.
>> + * If not, we will insert a new extent for it and move some xattrs in
>> + * the last cluster into the new allocated one.
>> + * We also need to limit the maximum size of a btree leaf, otherwise we'll
>> + * lose the benefits of hashing because we'll have to search large leaves.
>> + * So now the maximum size is OCFS2_MAX_XATTR_TREE_LEAF_SIZE(or clustersize,
>> + * if it's bigger).
>> + *
>> + * first_bh is the first block of the previous extent rec and header_bh
>> + * indicates the bucket we will insert the new xattrs. They will be updated
>> + * when the header_bh is moved into the new cluster.
>> + */
>> +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,
>> +				       u32 *num_clusters,
>> +				       u32 prev_cpos,
>> +				       u64 prev_blkno,
>> +				       int *extend)
>> +{
>> +	int ret, credits;
>> +	u16 bpc = ocfs2_clusters_to_blocks(inode->i_sb, 1);
>> +	u32 prev_clusters = *num_clusters;
>> +	u32 clusters_to_add = 1, bit_off, num_bits, v_start = 0;
>> +	u64 block;
>> +	handle_t *handle = NULL;
>> +	struct ocfs2_alloc_context *data_ac = NULL;
>> +	struct ocfs2_alloc_context *meta_ac = NULL;
>> +	struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +	struct ocfs2_xattr_block *xb =
>> +			(struct ocfs2_xattr_block *)root_bh->b_data;
>> +	struct ocfs2_xattr_tree_root *xb_root = &xb->xb_attrs.xb_root;
>> +	struct ocfs2_extent_list *root_el = &xb_root->xt_list;
>> +	enum ocfs2_extent_tree_type type = OCFS2_XATTR_TREE_EXTENT;
>> +
>> +	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, prev_blkno);
>> +
>> +	ret = ocfs2_lock_allocators(inode, root_bh, root_el,
>> +				    clusters_to_add, 0, &data_ac,
>> +				    &meta_ac, type, NULL);
>> +	if (ret) {
>> +		mlog_errno(ret);
>> +		goto leave;
>> +	}
>> +
>> +	credits = ocfs2_calc_extend_credits(osb->sb, root_el, clusters_to_add);
>> +	handle = ocfs2_start_trans(osb, credits);
>> +	if (IS_ERR(handle)) {
>> +		ret = PTR_ERR(handle);
>> +		handle = NULL;
>> +		mlog_errno(ret);
>> +		goto leave;
>> +	}
>> +
>> +	ret = ocfs2_journal_access(handle, inode, root_bh,
>> +				   OCFS2_JOURNAL_ACCESS_WRITE);
>> +	if (ret < 0) {
>> +		mlog_errno(ret);
>> +		goto leave;
>> +	}
>> +
>> +	ret = __ocfs2_claim_clusters(osb, handle, data_ac, 1,
>> +				     clusters_to_add, &bit_off, &num_bits);
>> +	if (ret < 0) {
>> +		if (ret != -ENOSPC)
>> +			mlog_errno(ret);
>> +		goto leave;
>> +	}
>> +
>> +	BUG_ON(num_bits > clusters_to_add);
>> +
>> +	block = ocfs2_clusters_to_blocks(osb->sb, bit_off);
>> +	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 &&
>> +	    (prev_clusters + num_bits) << osb->s_clustersize_bits <=
>> +	     OCFS2_MAX_XATTR_TREE_LEAF_SIZE) {
>> +		/*
>> +		 * If this cluster is contiguous with the old one and
>> +		 * adding this new cluster, we don't surpass the limit of
>> +		 * OCFS2_MAX_XATTR_TREE_LEAF_SIZE, cool. We will let it be
>> +		 * initialized and used like other buckets in the previous
>> +		 * cluster.
>> +		 * So add it as a contiguous one. The caller will handle
>> +		 * its init process.
>> +		 */
>> +		v_start = prev_cpos + prev_clusters;
>> +		*num_clusters = prev_clusters + num_bits;
>> +		mlog(0, "Add contiguous %u clusters to previous extent rec.\n",
>> +		     num_bits);
>> +	} else {
>> +		ret = ocfs2_adjust_xattr_cross_cluster(inode,
>> +						       handle,
>> +						       first_bh,
>> +						       header_bh,
>> +						       block,
>> +						       prev_blkno,
>> +						       prev_clusters,
>> +						       &v_start,
>> +						       extend);
>> +		if (ret) {
>> +			mlog_errno(ret);
>> +			goto leave;
>> +		}
>> +	}
>> +
>> +	mlog(0, "Insert %u clusters at block %llu for xattr at %u\n",
>> +	     num_bits, block, v_start);
>> +	ret = ocfs2_xattr_tree_insert_extent(osb, handle, inode, root_bh,
>> +					     v_start, block, num_bits,
>> +					     0, meta_ac);
> 
> I think we need to do something down in the extent insertion code to prevent
> merging in the OCFS2_MAX_XATTR_TREE_LEAF_SIZE cases - what's preventing it
> from appending that allocation to the end of an extent (any extent) in the
> btree?
We don't need to do that. You see, in ocfs2_adjust_xattr_corss_cluster I 
have adjusted v_start(v_start now indicates the first name hash of the 
xattrs in the new cluster), so now they will not be contiguous now. ;)

We only allow xattr hashes to be the same in one bucket(4K) size, so 
with 64K as our leaf size, even if the cluster is contiguous, we will 
not have contiguous name hash. so it is safe to say here v_start can 
never be old_v_start+1.

Regards,
Tao