[Ocfs2-devel] [PATCH 03/15] Abstract ocfs2_extent_tree in b-tree operations.

[Tao Ma]

Joel Becker wrote:
>
>> +struct ocfs2_extent_tree_operations {
>> +	void (*set_last_eb_blk) (struct ocfs2_extent_tree *et, u64 blkno);
>> +	u64 (*get_last_eb_blk) (struct ocfs2_extent_tree *et);
>> +	void (*update_clusters) (struct inode *inode,
>> +				 struct ocfs2_extent_tree *et,
>> +				 u32 new_clusters);
>> +	int (*sanity_check) (struct inode *inode, struct ocfs2_extent_tree *et);
>> +};
> 
> 	Can we please prefix these?  et_set_last_eb_blk,
> et_update_clusters, etc.
ok.
> 
>> +struct ocfs2_extent_tree {
>> +	enum ocfs2_extent_tree_type type;
>> +	struct ocfs2_extent_tree_operations *eops;
>> +	struct buffer_head *root_bh;
>> +	struct ocfs2_extent_list *root_el;
>> +};
> 
> 	And these too?  et_type, et_ops, et_root_bh, et_root_el.
ok.
> 
>>  int ocfs2_num_free_extents(struct ocfs2_super *osb,
>>  			   struct inode *inode,
>> -			   struct buffer_head *bh)
>> +			   struct buffer_head *root_bh,
>> +			   enum ocfs2_extent_tree_type type)
>>  {
>>  	int retval;
>> -	struct ocfs2_extent_list *el;
>> +	struct ocfs2_extent_list *el = NULL;
>>  	struct ocfs2_extent_block *eb;
>>  	struct buffer_head *eb_bh = NULL;
>> -	struct ocfs2_dinode *fe = (struct ocfs2_dinode *)bh->b_data;
>> +	u64 last_eb_blk = 0;
>>  
>>  	mlog_entry_void();
>>  
>> -	if (!OCFS2_IS_VALID_DINODE(fe)) {
>> -		OCFS2_RO_ON_INVALID_DINODE(inode->i_sb, fe);
>> -		retval = -EIO;
>> -		goto bail;
>> +	if (type == OCFS2_DINODE_EXTENT) {
>> +		struct ocfs2_dinode *fe =
>> +				(struct ocfs2_dinode *)root_bh->b_data;
>> +		if (!OCFS2_IS_VALID_DINODE(fe)) {
>> +			OCFS2_RO_ON_INVALID_DINODE(inode->i_sb, fe);
>> +			retval = -EIO;
>> +			goto bail;
>> +		}
>> +
>> +		if (fe->i_last_eb_blk)
>> +			last_eb_blk = le64_to_cpu(fe->i_last_eb_blk);
>> +		el = &fe->id2.i_list;
> 
> 	Why isn't this using ocfs2_new_extent_tree() and then
> ocfs2_get_last_eb_lock()?  You do that every other place, and it keeps
> you from having to special case each type here.
yes, you are right. Will modify it. Thanks.

Regards,
Tao