Re: [PATCH v3] ext4: remove code duplication in update_ind/bind/tind_extent_range

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mar 12, 2019, at 7:09 AM, Vasily Averin <vvs@xxxxxxxxxxxxx> wrote:
> 
> update_ind/bind/tind_extent_page() differs by one variable and can be
> replaced by unified function. These functions are called by similar way
> and their caller function can be simplified too.
> 
> v2: minor style changes, thanks to Andreas Dilger
> v3: rebase to v5.0
> 
> Signed-off-by: Vasily Averin <vvs@xxxxxxxxxxxxx>

Always good to see patches that remove more lines than they add.

Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>

> ---
> fs/ext4/migrate.c | 112 ++++++++++------------------------------------
> 1 file changed, 24 insertions(+), 88 deletions(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index b1e4d359f73b..fde2f1bc96b0 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -109,12 +109,13 @@ static int update_extent_range(handle_t *handle, struct inode *inode,
> 
> static int update_ind_extent_range(handle_t *handle, struct inode *inode,
> 				   ext4_fsblk_t pblock,
> -				   struct migrate_struct *lb)
> +				   struct migrate_struct *lb,
> +				   ext4_lblk_t inc)
> {
> 	struct buffer_head *bh;
> 	__le32 *i_data;
> 	int i, retval = 0;
> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
> +	ext4_lblk_t max_entries = inode->i_sb->s_blocksize >> 2;
> 
> 	bh = ext4_sb_bread(inode->i_sb, pblock, 0);
> 	if (IS_ERR(bh))
> @@ -128,67 +129,7 @@ static int update_ind_extent_range(handle_t *handle, struct inode *inode,
> 			if (retval)
> 				break;
> 		} else {
> -			lb->curr_block++;
> -		}
> -	}
> -	put_bh(bh);
> -	return retval;
> -
> -}
> -
> -static int update_dind_extent_range(handle_t *handle, struct inode *inode,
> -				    ext4_fsblk_t pblock,
> -				    struct migrate_struct *lb)
> -{
> -	struct buffer_head *bh;
> -	__le32 *i_data;
> -	int i, retval = 0;
> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
> -
> -	bh = ext4_sb_bread(inode->i_sb, pblock, 0);
> -	if (IS_ERR(bh))
> -		return PTR_ERR(bh);
> -
> -	i_data = (__le32 *)bh->b_data;
> -	for (i = 0; i < max_entries; i++) {
> -		if (i_data[i]) {
> -			retval = update_ind_extent_range(handle, inode,
> -						le32_to_cpu(i_data[i]), lb);
> -			if (retval)
> -				break;
> -		} else {
> -			/* Only update the file block number */
> -			lb->curr_block += max_entries;
> -		}
> -	}
> -	put_bh(bh);
> -	return retval;
> -
> -}
> -
> -static int update_tind_extent_range(handle_t *handle, struct inode *inode,
> -				    ext4_fsblk_t pblock,
> -				    struct migrate_struct *lb)
> -{
> -	struct buffer_head *bh;
> -	__le32 *i_data;
> -	int i, retval = 0;
> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
> -
> -	bh = ext4_sb_bread(inode->i_sb, pblock, 0);
> -	if (IS_ERR(bh))
> -		return PTR_ERR(bh);
> -
> -	i_data = (__le32 *)bh->b_data;
> -	for (i = 0; i < max_entries; i++) {
> -		if (i_data[i]) {
> -			retval = update_dind_extent_range(handle, inode,
> -						le32_to_cpu(i_data[i]), lb);
> -			if (retval)
> -				break;
> -		} else {
> -			/* Only update the file block number */
> -			lb->curr_block += max_entries * max_entries;
> +			lb->curr_block += inc;
> 		}
> 	}
> 	put_bh(bh);
> @@ -433,7 +374,7 @@ int ext4_ext_migrate(struct inode *inode)
> 	struct ext4_inode_info *ei;
> 	struct inode *tmp_inode = NULL;
> 	struct migrate_struct lb;
> -	unsigned long max_entries;
> +	ext4_lblk_t max_entries, inc, mult;
> 	__u32 goal;
> 	uid_t owner[2];
> 
> @@ -523,34 +464,29 @@ int ext4_ext_migrate(struct inode *inode)
> 
> 	/* 32 bit block address 4 bytes */
> 	max_entries = inode->i_sb->s_blocksize >> 2;
> -	for (i = 0; i < EXT4_NDIR_BLOCKS; i++) {
> +
> +	inc = 1; mult = 1;
> +	for (i = 0; i < EXT4_N_BLOCKS; i++) {
> +		inc *= mult;
> +		if (i == EXT4_IND_BLOCK)
> +			mult = max_entries;
> +
> 		if (i_data[i]) {
> -			retval = update_extent_range(handle, tmp_inode,
> +			if (i < EXT4_IND_BLOCK)
> +				retval = update_extent_range(handle, tmp_inode,
> 						le32_to_cpu(i_data[i]), &lb);
> +			else
> +				retval = update_ind_extent_range(handle,
> +						tmp_inode,
> +						le32_to_cpu(i_data[i]),
> +						&lb, inc);
> 			if (retval)
> 				goto err_out;
> -		} else
> -			lb.curr_block++;
> -	}
> -	if (i_data[EXT4_IND_BLOCK]) {
> -		retval = update_ind_extent_range(handle, tmp_inode,
> -				le32_to_cpu(i_data[EXT4_IND_BLOCK]), &lb);
> -		if (retval)
> -			goto err_out;
> -	} else
> -		lb.curr_block += max_entries;
> -	if (i_data[EXT4_DIND_BLOCK]) {
> -		retval = update_dind_extent_range(handle, tmp_inode,
> -				le32_to_cpu(i_data[EXT4_DIND_BLOCK]), &lb);
> -		if (retval)
> -			goto err_out;
> -	} else
> -		lb.curr_block += max_entries * max_entries;
> -	if (i_data[EXT4_TIND_BLOCK]) {
> -		retval = update_tind_extent_range(handle, tmp_inode,
> -				le32_to_cpu(i_data[EXT4_TIND_BLOCK]), &lb);
> -		if (retval)
> -			goto err_out;
> +
> +		} else {
> +			if (i < EXT4_TIND_BLOCK)
> +				lb.curr_block += inc * mult;
> +		}
> 	}
> 	/*
> 	 * Build the last extent
> --
> 2.17.1
> 


Cheers, Andreas





Attachment: signature.asc
Description: Message signed with OpenPGP


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux