[Ocfs2-devel] [PATCH 3/3] Ocfs2: Treat ocfs2 truncate as a special case of punching holes.

[TaoMa <tao.ma@oracle.com>]

Tristan Ye wrote:
> As we known, truncate is just a special case of punching holes(from new i_size
> to end), we therefore could take advantage of existing ocfs2_remove_btree_range()
> codes to reduce the comlexity and redundancy in alloc.c, the goal here is to make
> truncate codes more generic and straightforward.
>
> Several former functions only used by ocfs2_commit_truncate() will be simply wiped off.
> New logic for truncating will remove extents from truncate_size to file end one by one,
> just like punching holes did.
>
> v2 patch uses former sequence of records truncating(from tail to new_i_size) which keeps
> a high efficiency in performance, also, it has the refcount support as well.
>
> Signed-off-by: Tristan Ye <tristan.ye@oracle.com>
> ---
>  fs/ocfs2/alloc.c |  556 +++++------------------------------------------------
>  fs/ocfs2/alloc.h |    3 +-
>  fs/ocfs2/file.c  |    9 +-
>  fs/ocfs2/inode.c |    9 +-
>  4 files changed, 56 insertions(+), 521 deletions(-)
>
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 6c5f1de..0c42003 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
>
> @@ -7422,21 +6999,25 @@ out:
>   */
>  int ocfs2_commit_truncate(struct ocfs2_super *osb,
>  			  struct inode *inode,
> -			  struct buffer_head *fe_bh,
> -			  struct ocfs2_truncate_context *tc)
> +			  struct buffer_head *fe_bh)
>  {
> -	int status, i, credits, tl_sem = 0;
> -	u32 clusters_to_del, new_highest_cpos, range;
> +	int status = 0, i, credits = 0, ref_blocks = 0, flags = 0;
> +	u32 new_highest_cpos, range, trunc_cpos, trunc_len, phys_cpos, coff;
>  	u64 blkno = 0;
>  	struct ocfs2_extent_list *el;
> -	handle_t *handle = NULL;
> -	struct inode *tl_inode = osb->osb_tl_inode;
> +	struct ocfs2_extent_rec *rec;
>  	struct ocfs2_path *path = NULL;
>  	struct ocfs2_dinode *di = (struct ocfs2_dinode *)fe_bh->b_data;
> -	struct ocfs2_alloc_context *meta_ac = NULL;
> +
> +	struct ocfs2_extent_tree et;
> +	struct ocfs2_cached_dealloc_ctxt dealloc;
> +
>  	struct ocfs2_refcount_tree *ref_tree = NULL;
>  
>  	mlog_entry_void();
> +	
> +	ocfs2_init_dinode_extent_tree(&et, INODE_CACHE(inode), fe_bh);
> +	ocfs2_init_dealloc_ctxt(&dealloc);
>  
>  	new_highest_cpos = ocfs2_clusters_for_bytes(osb->sb,
>  						     i_size_read(inode));
> @@ -7449,8 +7030,6 @@ int ocfs2_commit_truncate(struct ocfs2_super *osb,
>  		goto bail;
>  	}
>  
> -	ocfs2_extent_map_trunc(inode, new_highest_cpos);
> -
>   
Please keep this line. It will truncate the extent map.
>  start:
>  	/*
>  	 * Check that we still have allocation to delete.
> @@ -7461,6 +7040,7 @@ start:
>  	}
>  
>  	credits = 0;
> +	ref_blocks = 0;
>  
>  	/*
>  	 * Truncate always works against the rightmost tree branch.
> @@ -7496,30 +7076,47 @@ start:
>  	}
>  
>  	i = le16_to_cpu(el->l_next_free_rec) - 1;
> -	range = le32_to_cpu(el->l_recs[i].e_cpos) +
> -		ocfs2_rec_clusters(el, &el->l_recs[i]);
> -	if (i == 0 && ocfs2_is_empty_extent(&el->l_recs[i])) {
> -		clusters_to_del = 0;
> -	} else if (le32_to_cpu(el->l_recs[i].e_cpos) >= new_highest_cpos) {
> -		clusters_to_del = ocfs2_rec_clusters(el, &el->l_recs[i]);
> -		blkno = le64_to_cpu(el->l_recs[i].e_blkno);
> +	rec = &el->l_recs[i];
> +	flags = rec->e_flags;
> +	range = le32_to_cpu(rec->e_cpos) + ocfs2_rec_clusters(el, rec);
> +
> +	if (i == 0 && ocfs2_is_empty_extent(rec)) {
> +		/*
> +		 * Should remove this extent block.
> +		 */
> +		trunc_cpos = le32_to_cpu(rec->e_cpos);
> +		trunc_len = 0;
> +		coff = 0;
> +		blkno = 0;
> +	} else if (le32_to_cpu(rec->e_cpos) >= new_highest_cpos) {
> +		/* 
> +		 * Truncate entire record.
> +		 */
> +		trunc_cpos = le32_to_cpu(rec->e_cpos);
> +		trunc_len = ocfs2_rec_clusters(el, rec);
> +		coff = 0;
> +		blkno = le64_to_cpu(rec->e_blkno);
>  	} else if (range > new_highest_cpos) {
> -		clusters_to_del = (ocfs2_rec_clusters(el, &el->l_recs[i]) +
> -				   le32_to_cpu(el->l_recs[i].e_cpos)) -
> -				  new_highest_cpos;
> -		blkno = le64_to_cpu(el->l_recs[i].e_blkno) +
> -			ocfs2_clusters_to_blocks(inode->i_sb,
> -				ocfs2_rec_clusters(el, &el->l_recs[i]) -
> -				clusters_to_del);
> +		/*
> +		 * Partial truncate. it also should be
> +		 * the last truncate we're doing.
> +		 */
> +		trunc_cpos = new_highest_cpos;
> +		trunc_len = range - new_highest_cpos;
> +		coff = new_highest_cpos - le32_to_cpu(rec->e_cpos);
> +		blkno = le64_to_cpu(rec->e_blkno) +
> +				ocfs2_clusters_to_blocks(inode->i_sb, coff);
>   
another reason you want to change clusters_to_del to trunc_len? The 
calcuation is the same.
>  	} else {
> +		/*
> +		 * Truncate completed, leave happily.
> +		 */
>  		status = 0;
>  		goto bail;
>  	}
>  
> -	mlog(0, "clusters_to_del = %u in this pass, tail blk=%llu\n",
> -	     clusters_to_del, (unsigned long long)path_leaf_bh(path)->b_blocknr);
> +	phys_cpos = ocfs2_blocks_to_clusters(inode->i_sb, blkno);
>  
> -	if (el->l_recs[i].e_flags & OCFS2_EXT_REFCOUNTED && clusters_to_del) {
> +	if (el->l_recs[i].e_flags & OCFS2_EXT_REFCOUNTED && trunc_len) {
>  		BUG_ON(!(OCFS2_I(inode)->ip_dyn_features &
>  			 OCFS2_HAS_REFCOUNT_FL));
>  
> @@ -7533,59 +7130,25 @@ start:
>  
>  		status = ocfs2_prepare_refcount_change_for_del(inode, fe_bh,
>  							       blkno,
> -							       clusters_to_del,
> +							       trunc_len,
>  							       &credits,
> -							       &meta_ac);
>   
As I said in the previous e-mail, we may add this function to the 
ocfs2_remove_btree_range.
And what's more, we don't need to fixing punching hole any more since 
this function can handle refcount tree by itself.
> -		if (status < 0) {
> -			mlog_errno(status);
> -			goto bail;
> -		}
> -	}
> -
> -	mutex_lock(&tl_inode->i_mutex);
> -	tl_sem = 1;
> -	/* ocfs2_truncate_log_needs_flush guarantees us at least one
> -	 * record is free for use. If there isn't any, we flush to get
> -	 * an empty truncate log.  */
> -	if (ocfs2_truncate_log_needs_flush(osb)) {
> -		status = __ocfs2_flush_truncate_log(osb);
> +							       &ref_blocks);
>  		if (status < 0) {
>  			mlog_errno(status);
>  			goto bail;
>  		}
>  	}
>  
> -	credits += ocfs2_calc_tree_trunc_credits(osb->sb, clusters_to_del,
> -						(struct ocfs2_dinode *)fe_bh->b_data,
> -						el);
> -	handle = ocfs2_start_trans(osb, credits);
> -	if (IS_ERR(handle)) {
> -		status = PTR_ERR(handle);
> -		handle = NULL;
> -		mlog_errno(status);
> -		goto bail;
> -	}
> -
> -	status = ocfs2_do_truncate(osb, clusters_to_del, inode, fe_bh, handle,
> -				   tc, path, meta_ac);
> +	status = ocfs2_remove_btree_range(inode, &et, trunc_cpos, phys_cpos,
> +					  trunc_len, &dealloc, credits,
> +					  ref_blocks, flags);
>  	if (status < 0) {
>  		mlog_errno(status);
>  		goto bail;
>  	}
> -
> -	mutex_unlock(&tl_inode->i_mutex);
> -	tl_sem = 0;
> -
> -	ocfs2_commit_trans(osb, handle);
> -	handle = NULL;
> -
> +	
>  	ocfs2_reinit_path(path, 1);
>  
> -	if (meta_ac) {
> -		ocfs2_free_alloc_context(meta_ac);
> -		meta_ac = NULL;
> -	}
> -
>  	if (ref_tree) {
>  		ocfs2_unlock_refcount_tree(osb, ref_tree, 1);
>  		ref_tree = NULL;
> @@ -7598,28 +7161,15 @@ start:
>  	goto start;
>  
>  bail:
> -
>  	ocfs2_schedule_truncate_log_flush(osb, 1);
>  
> -	if (tl_sem)
> -		mutex_unlock(&tl_inode->i_mutex);
> -
> -	if (handle)
> -		ocfs2_commit_trans(osb, handle);
> -
> -	if (meta_ac)
> -		ocfs2_free_alloc_context(meta_ac);
> -
>  	if (ref_tree)
>  		ocfs2_unlock_refcount_tree(osb, ref_tree, 1);
>  
> -	ocfs2_run_deallocs(osb, &tc->tc_dealloc);
> +	ocfs2_run_deallocs(osb, &dealloc);
>  
>  	ocfs2_free_path(path);
>  
> -	/* This will drop the ext_alloc cluster lock for us */
> -	ocfs2_free_truncate_context(tc);
> -
>   
I would suggest you to separate the removal of tc and 
ocfs2_prepare_truncate to another function since it has nothing to do 
with this patch.

Regards,
Tao