All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ext4: remove code duplication in free_ind_block()
@ 2019-03-12 13:09 Vasily Averin
  2019-05-29 15:01 ` Jan Kara
  0 siblings, 1 reply; 4+ messages in thread
From: Vasily Averin @ 2019-03-12 13:09 UTC (permalink / raw)
  To: Theodore Ts'o, Andreas Dilger, linux-ext4

free_ind_block(), free_dind_blocks() and free_tind_blocks() are replaced
by a single recursive function.
v2: rebase to v5.0

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/ext4/migrate.c | 115 +++++++++++++---------------------------------
 1 file changed, 32 insertions(+), 83 deletions(-)

diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index fde2f1bc96b0..6b811b7d110c 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -157,100 +157,43 @@ static int extend_credit_for_blkdel(handle_t *handle, struct inode *inode)
 	return retval;
 }
 
-static int free_dind_blocks(handle_t *handle,
-				struct inode *inode, __le32 i_data)
+static int free_ind_blocks(handle_t *handle,
+				struct inode *inode, __le32 i_data, int ind)
 {
-	int i;
-	__le32 *tmp_idata;
-	struct buffer_head *bh;
-	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
-
-	bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
-	if (IS_ERR(bh))
-		return PTR_ERR(bh);
-
-	tmp_idata = (__le32 *)bh->b_data;
-	for (i = 0; i < max_entries; i++) {
-		if (tmp_idata[i]) {
-			extend_credit_for_blkdel(handle, inode);
-			ext4_free_blocks(handle, inode, NULL,
-					 le32_to_cpu(tmp_idata[i]), 1,
-					 EXT4_FREE_BLOCKS_METADATA |
-					 EXT4_FREE_BLOCKS_FORGET);
-		}
-	}
-	put_bh(bh);
-	extend_credit_for_blkdel(handle, inode);
-	ext4_free_blocks(handle, inode, NULL, le32_to_cpu(i_data), 1,
-			 EXT4_FREE_BLOCKS_METADATA |
-			 EXT4_FREE_BLOCKS_FORGET);
-	return 0;
-}
-
-static int free_tind_blocks(handle_t *handle,
-				struct inode *inode, __le32 i_data)
-{
-	int i, retval = 0;
-	__le32 *tmp_idata;
-	struct buffer_head *bh;
-	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
-
-	bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
-	if (IS_ERR(bh))
-		return PTR_ERR(bh);
-
-	tmp_idata = (__le32 *)bh->b_data;
-	for (i = 0; i < max_entries; i++) {
-		if (tmp_idata[i]) {
-			retval = free_dind_blocks(handle,
-					inode, tmp_idata[i]);
-			if (retval) {
-				put_bh(bh);
-				return retval;
+	if (ind > 0) {
+		int retval = 0;
+		__le32 *tmp_idata;
+		ext4_lblk_t i, max_entries;
+		struct buffer_head *bh;
+
+		bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
+		if (IS_ERR(bh))
+			return PTR_ERR(bh);
+
+		tmp_idata = (__le32 *)bh->b_data;
+		max_entries = inode->i_sb->s_blocksize >> 2;
+		for (i = 0; i < max_entries; i++) {
+			if (tmp_idata[i]) {
+				retval = free_ind_blocks(handle,
+						inode, tmp_idata[i], ind - 1);
+				if (retval) {
+					put_bh(bh);
+					return retval;
+				}
 			}
 		}
+		put_bh(bh);
 	}
-	put_bh(bh);
 	extend_credit_for_blkdel(handle, inode);
 	ext4_free_blocks(handle, inode, NULL, le32_to_cpu(i_data), 1,
-			 EXT4_FREE_BLOCKS_METADATA |
-			 EXT4_FREE_BLOCKS_FORGET);
-	return 0;
-}
-
-static int free_ind_block(handle_t *handle, struct inode *inode, __le32 *i_data)
-{
-	int retval;
-
-	/* ei->i_data[EXT4_IND_BLOCK] */
-	if (i_data[0]) {
-		extend_credit_for_blkdel(handle, inode);
-		ext4_free_blocks(handle, inode, NULL,
-				le32_to_cpu(i_data[0]), 1,
-				 EXT4_FREE_BLOCKS_METADATA |
-				 EXT4_FREE_BLOCKS_FORGET);
-	}
-
-	/* ei->i_data[EXT4_DIND_BLOCK] */
-	if (i_data[1]) {
-		retval = free_dind_blocks(handle, inode, i_data[1]);
-		if (retval)
-			return retval;
-	}
-
-	/* ei->i_data[EXT4_TIND_BLOCK] */
-	if (i_data[2]) {
-		retval = free_tind_blocks(handle, inode, i_data[2]);
-		if (retval)
-			return retval;
-	}
+			 EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
 	return 0;
 }
 
 static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
 						struct inode *tmp_inode)
 {
-	int retval;
+	int i, retval;
 	__le32	i_data[3];
 	struct ext4_inode_info *ei = EXT4_I(inode);
 	struct ext4_inode_info *tmp_ei = EXT4_I(tmp_inode);
@@ -307,7 +250,13 @@ static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
 	 * We mark the inode dirty after, because we decrement the
 	 * i_blocks when freeing the indirect meta-data blocks
 	 */
-	retval = free_ind_block(handle, inode, i_data);
+	for (i = 0; i < ARRAY_SIZE(i_data); i++) {
+		if (i_data[i]) {
+			retval = free_ind_blocks(handle, inode, i_data[i], i);
+			if (retval)
+				break;
+		}
+	}
 	ext4_mark_inode_dirty(handle, inode);
 
 err_out:
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ext4: remove code duplication in free_ind_block()
  2019-03-12 13:09 [PATCH v2] ext4: remove code duplication in free_ind_block() Vasily Averin
@ 2019-05-29 15:01 ` Jan Kara
  2019-05-30  7:13   ` Vasily Averin
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Kara @ 2019-05-29 15:01 UTC (permalink / raw)
  To: Vasily Averin; +Cc: Theodore Ts'o, Andreas Dilger, linux-ext4

On Tue 12-03-19 16:09:12, Vasily Averin wrote:
> free_ind_block(), free_dind_blocks() and free_tind_blocks() are replaced
> by a single recursive function.
> v2: rebase to v5.0
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Thanks for the patch! Nice cleanup. The patch looks good to me. Feel free
to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Just one question. How did you test this? And if you have a testcase for
this code, can you please add it to fstests so that it gets excercised?
Thanks!

								Honza

> ---
>  fs/ext4/migrate.c | 115 +++++++++++++---------------------------------
>  1 file changed, 32 insertions(+), 83 deletions(-)
> 
> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
> index fde2f1bc96b0..6b811b7d110c 100644
> --- a/fs/ext4/migrate.c
> +++ b/fs/ext4/migrate.c
> @@ -157,100 +157,43 @@ static int extend_credit_for_blkdel(handle_t *handle, struct inode *inode)
>  	return retval;
>  }
>  
> -static int free_dind_blocks(handle_t *handle,
> -				struct inode *inode, __le32 i_data)
> +static int free_ind_blocks(handle_t *handle,
> +				struct inode *inode, __le32 i_data, int ind)
>  {
> -	int i;
> -	__le32 *tmp_idata;
> -	struct buffer_head *bh;
> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
> -
> -	bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
> -	if (IS_ERR(bh))
> -		return PTR_ERR(bh);
> -
> -	tmp_idata = (__le32 *)bh->b_data;
> -	for (i = 0; i < max_entries; i++) {
> -		if (tmp_idata[i]) {
> -			extend_credit_for_blkdel(handle, inode);
> -			ext4_free_blocks(handle, inode, NULL,
> -					 le32_to_cpu(tmp_idata[i]), 1,
> -					 EXT4_FREE_BLOCKS_METADATA |
> -					 EXT4_FREE_BLOCKS_FORGET);
> -		}
> -	}
> -	put_bh(bh);
> -	extend_credit_for_blkdel(handle, inode);
> -	ext4_free_blocks(handle, inode, NULL, le32_to_cpu(i_data), 1,
> -			 EXT4_FREE_BLOCKS_METADATA |
> -			 EXT4_FREE_BLOCKS_FORGET);
> -	return 0;
> -}
> -
> -static int free_tind_blocks(handle_t *handle,
> -				struct inode *inode, __le32 i_data)
> -{
> -	int i, retval = 0;
> -	__le32 *tmp_idata;
> -	struct buffer_head *bh;
> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
> -
> -	bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
> -	if (IS_ERR(bh))
> -		return PTR_ERR(bh);
> -
> -	tmp_idata = (__le32 *)bh->b_data;
> -	for (i = 0; i < max_entries; i++) {
> -		if (tmp_idata[i]) {
> -			retval = free_dind_blocks(handle,
> -					inode, tmp_idata[i]);
> -			if (retval) {
> -				put_bh(bh);
> -				return retval;
> +	if (ind > 0) {
> +		int retval = 0;
> +		__le32 *tmp_idata;
> +		ext4_lblk_t i, max_entries;
> +		struct buffer_head *bh;
> +
> +		bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
> +		if (IS_ERR(bh))
> +			return PTR_ERR(bh);
> +
> +		tmp_idata = (__le32 *)bh->b_data;
> +		max_entries = inode->i_sb->s_blocksize >> 2;
> +		for (i = 0; i < max_entries; i++) {
> +			if (tmp_idata[i]) {
> +				retval = free_ind_blocks(handle,
> +						inode, tmp_idata[i], ind - 1);
> +				if (retval) {
> +					put_bh(bh);
> +					return retval;
> +				}
>  			}
>  		}
> +		put_bh(bh);
>  	}
> -	put_bh(bh);
>  	extend_credit_for_blkdel(handle, inode);
>  	ext4_free_blocks(handle, inode, NULL, le32_to_cpu(i_data), 1,
> -			 EXT4_FREE_BLOCKS_METADATA |
> -			 EXT4_FREE_BLOCKS_FORGET);
> -	return 0;
> -}
> -
> -static int free_ind_block(handle_t *handle, struct inode *inode, __le32 *i_data)
> -{
> -	int retval;
> -
> -	/* ei->i_data[EXT4_IND_BLOCK] */
> -	if (i_data[0]) {
> -		extend_credit_for_blkdel(handle, inode);
> -		ext4_free_blocks(handle, inode, NULL,
> -				le32_to_cpu(i_data[0]), 1,
> -				 EXT4_FREE_BLOCKS_METADATA |
> -				 EXT4_FREE_BLOCKS_FORGET);
> -	}
> -
> -	/* ei->i_data[EXT4_DIND_BLOCK] */
> -	if (i_data[1]) {
> -		retval = free_dind_blocks(handle, inode, i_data[1]);
> -		if (retval)
> -			return retval;
> -	}
> -
> -	/* ei->i_data[EXT4_TIND_BLOCK] */
> -	if (i_data[2]) {
> -		retval = free_tind_blocks(handle, inode, i_data[2]);
> -		if (retval)
> -			return retval;
> -	}
> +			 EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
>  	return 0;
>  }
>  
>  static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
>  						struct inode *tmp_inode)
>  {
> -	int retval;
> +	int i, retval;
>  	__le32	i_data[3];
>  	struct ext4_inode_info *ei = EXT4_I(inode);
>  	struct ext4_inode_info *tmp_ei = EXT4_I(tmp_inode);
> @@ -307,7 +250,13 @@ static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
>  	 * We mark the inode dirty after, because we decrement the
>  	 * i_blocks when freeing the indirect meta-data blocks
>  	 */
> -	retval = free_ind_block(handle, inode, i_data);
> +	for (i = 0; i < ARRAY_SIZE(i_data); i++) {
> +		if (i_data[i]) {
> +			retval = free_ind_blocks(handle, inode, i_data[i], i);
> +			if (retval)
> +				break;
> +		}
> +	}
>  	ext4_mark_inode_dirty(handle, inode);
>  
>  err_out:
> -- 
> 2.17.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ext4: remove code duplication in free_ind_block()
  2019-05-29 15:01 ` Jan Kara
@ 2019-05-30  7:13   ` Vasily Averin
  2019-07-11 14:02     ` Vasily Averin
  0 siblings, 1 reply; 4+ messages in thread
From: Vasily Averin @ 2019-05-30  7:13 UTC (permalink / raw)
  To: Jan Kara; +Cc: Theodore Ts'o, Andreas Dilger, linux-ext4

On 5/29/19 6:01 PM, Jan Kara wrote:
> On Tue 12-03-19 16:09:12, Vasily Averin wrote:
>> free_ind_block(), free_dind_blocks() and free_tind_blocks() are replaced
>> by a single recursive function.
>> v2: rebase to v5.0
>>
>> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> 
> Thanks for the patch! Nice cleanup. The patch looks good to me. Feel free
> to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
> Just one question. How did you test this? And if you have a testcase for
> this code, can you please add it to fstests so that it gets excercised?

Frankly speaking I've very carefully reviewed these changes,
complied and booted, but not tested it under any real load.

Thank you,
	Vasily Averin

>> ---
>>  fs/ext4/migrate.c | 115 +++++++++++++---------------------------------
>>  1 file changed, 32 insertions(+), 83 deletions(-)
>>
>> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
>> index fde2f1bc96b0..6b811b7d110c 100644
>> --- a/fs/ext4/migrate.c
>> +++ b/fs/ext4/migrate.c
>> @@ -157,100 +157,43 @@ static int extend_credit_for_blkdel(handle_t *handle, struct inode *inode)
>>  	return retval;
>>  }
>>  
>> -static int free_dind_blocks(handle_t *handle,
>> -				struct inode *inode, __le32 i_data)
>> +static int free_ind_blocks(handle_t *handle,
>> +				struct inode *inode, __le32 i_data, int ind)
>>  {
>> -	int i;
>> -	__le32 *tmp_idata;
>> -	struct buffer_head *bh;
>> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
>> -
>> -	bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
>> -	if (IS_ERR(bh))
>> -		return PTR_ERR(bh);
>> -
>> -	tmp_idata = (__le32 *)bh->b_data;
>> -	for (i = 0; i < max_entries; i++) {
>> -		if (tmp_idata[i]) {
>> -			extend_credit_for_blkdel(handle, inode);
>> -			ext4_free_blocks(handle, inode, NULL,
>> -					 le32_to_cpu(tmp_idata[i]), 1,
>> -					 EXT4_FREE_BLOCKS_METADATA |
>> -					 EXT4_FREE_BLOCKS_FORGET);
>> -		}
>> -	}
>> -	put_bh(bh);
>> -	extend_credit_for_blkdel(handle, inode);
>> -	ext4_free_blocks(handle, inode, NULL, le32_to_cpu(i_data), 1,
>> -			 EXT4_FREE_BLOCKS_METADATA |
>> -			 EXT4_FREE_BLOCKS_FORGET);
>> -	return 0;
>> -}
>> -
>> -static int free_tind_blocks(handle_t *handle,
>> -				struct inode *inode, __le32 i_data)
>> -{
>> -	int i, retval = 0;
>> -	__le32 *tmp_idata;
>> -	struct buffer_head *bh;
>> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
>> -
>> -	bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
>> -	if (IS_ERR(bh))
>> -		return PTR_ERR(bh);
>> -
>> -	tmp_idata = (__le32 *)bh->b_data;
>> -	for (i = 0; i < max_entries; i++) {
>> -		if (tmp_idata[i]) {
>> -			retval = free_dind_blocks(handle,
>> -					inode, tmp_idata[i]);
>> -			if (retval) {
>> -				put_bh(bh);
>> -				return retval;
>> +	if (ind > 0) {
>> +		int retval = 0;
>> +		__le32 *tmp_idata;
>> +		ext4_lblk_t i, max_entries;
>> +		struct buffer_head *bh;
>> +
>> +		bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
>> +		if (IS_ERR(bh))
>> +			return PTR_ERR(bh);
>> +
>> +		tmp_idata = (__le32 *)bh->b_data;
>> +		max_entries = inode->i_sb->s_blocksize >> 2;
>> +		for (i = 0; i < max_entries; i++) {
>> +			if (tmp_idata[i]) {
>> +				retval = free_ind_blocks(handle,
>> +						inode, tmp_idata[i], ind - 1);
>> +				if (retval) {
>> +					put_bh(bh);
>> +					return retval;
>> +				}
>>  			}
>>  		}
>> +		put_bh(bh);
>>  	}
>> -	put_bh(bh);
>>  	extend_credit_for_blkdel(handle, inode);
>>  	ext4_free_blocks(handle, inode, NULL, le32_to_cpu(i_data), 1,
>> -			 EXT4_FREE_BLOCKS_METADATA |
>> -			 EXT4_FREE_BLOCKS_FORGET);
>> -	return 0;
>> -}
>> -
>> -static int free_ind_block(handle_t *handle, struct inode *inode, __le32 *i_data)
>> -{
>> -	int retval;
>> -
>> -	/* ei->i_data[EXT4_IND_BLOCK] */
>> -	if (i_data[0]) {
>> -		extend_credit_for_blkdel(handle, inode);
>> -		ext4_free_blocks(handle, inode, NULL,
>> -				le32_to_cpu(i_data[0]), 1,
>> -				 EXT4_FREE_BLOCKS_METADATA |
>> -				 EXT4_FREE_BLOCKS_FORGET);
>> -	}
>> -
>> -	/* ei->i_data[EXT4_DIND_BLOCK] */
>> -	if (i_data[1]) {
>> -		retval = free_dind_blocks(handle, inode, i_data[1]);
>> -		if (retval)
>> -			return retval;
>> -	}
>> -
>> -	/* ei->i_data[EXT4_TIND_BLOCK] */
>> -	if (i_data[2]) {
>> -		retval = free_tind_blocks(handle, inode, i_data[2]);
>> -		if (retval)
>> -			return retval;
>> -	}
>> +			 EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
>>  	return 0;
>>  }
>>  
>>  static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
>>  						struct inode *tmp_inode)
>>  {
>> -	int retval;
>> +	int i, retval;
>>  	__le32	i_data[3];
>>  	struct ext4_inode_info *ei = EXT4_I(inode);
>>  	struct ext4_inode_info *tmp_ei = EXT4_I(tmp_inode);
>> @@ -307,7 +250,13 @@ static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
>>  	 * We mark the inode dirty after, because we decrement the
>>  	 * i_blocks when freeing the indirect meta-data blocks
>>  	 */
>> -	retval = free_ind_block(handle, inode, i_data);
>> +	for (i = 0; i < ARRAY_SIZE(i_data); i++) {
>> +		if (i_data[i]) {
>> +			retval = free_ind_blocks(handle, inode, i_data[i], i);
>> +			if (retval)
>> +				break;
>> +		}
>> +	}
>>  	ext4_mark_inode_dirty(handle, inode);
>>  
>>  err_out:
>> -- 
>> 2.17.1
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ext4: remove code duplication in free_ind_block()
  2019-05-30  7:13   ` Vasily Averin
@ 2019-07-11 14:02     ` Vasily Averin
  0 siblings, 0 replies; 4+ messages in thread
From: Vasily Averin @ 2019-07-11 14:02 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Jan Kara, Andreas Dilger, linux-ext4

Dear Ted,
could you please comment the state of this patch?
Should I re-new or resend it?
Do you probably have some objections or may be expect some troubles related to this patch?

On 5/30/19 10:13 AM, Vasily Averin wrote:
> On 5/29/19 6:01 PM, Jan Kara wrote:
>> On Tue 12-03-19 16:09:12, Vasily Averin wrote:
>>> free_ind_block(), free_dind_blocks() and free_tind_blocks() are replaced
>>> by a single recursive function.
>>> v2: rebase to v5.0
>>>
>>> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
>>
>> Thanks for the patch! Nice cleanup. The patch looks good to me. Feel free
>> to add:
>>
>> Reviewed-by: Jan Kara <jack@suse.cz>
>>
>> Just one question. How did you test this? And if you have a testcase for
>> this code, can you please add it to fstests so that it gets excercised?
> 
> Frankly speaking I've very carefully reviewed these changes,
> complied and booted, but not tested it under any real load.
> 
> Thank you,
> 	Vasily Averin
> 
>>> ---
>>>  fs/ext4/migrate.c | 115 +++++++++++++---------------------------------
>>>  1 file changed, 32 insertions(+), 83 deletions(-)
>>>
>>> diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
>>> index fde2f1bc96b0..6b811b7d110c 100644
>>> --- a/fs/ext4/migrate.c
>>> +++ b/fs/ext4/migrate.c
>>> @@ -157,100 +157,43 @@ static int extend_credit_for_blkdel(handle_t *handle, struct inode *inode)
>>>  	return retval;
>>>  }
>>>  
>>> -static int free_dind_blocks(handle_t *handle,
>>> -				struct inode *inode, __le32 i_data)
>>> +static int free_ind_blocks(handle_t *handle,
>>> +				struct inode *inode, __le32 i_data, int ind)
>>>  {
>>> -	int i;
>>> -	__le32 *tmp_idata;
>>> -	struct buffer_head *bh;
>>> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
>>> -
>>> -	bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
>>> -	if (IS_ERR(bh))
>>> -		return PTR_ERR(bh);
>>> -
>>> -	tmp_idata = (__le32 *)bh->b_data;
>>> -	for (i = 0; i < max_entries; i++) {
>>> -		if (tmp_idata[i]) {
>>> -			extend_credit_for_blkdel(handle, inode);
>>> -			ext4_free_blocks(handle, inode, NULL,
>>> -					 le32_to_cpu(tmp_idata[i]), 1,
>>> -					 EXT4_FREE_BLOCKS_METADATA |
>>> -					 EXT4_FREE_BLOCKS_FORGET);
>>> -		}
>>> -	}
>>> -	put_bh(bh);
>>> -	extend_credit_for_blkdel(handle, inode);
>>> -	ext4_free_blocks(handle, inode, NULL, le32_to_cpu(i_data), 1,
>>> -			 EXT4_FREE_BLOCKS_METADATA |
>>> -			 EXT4_FREE_BLOCKS_FORGET);
>>> -	return 0;
>>> -}
>>> -
>>> -static int free_tind_blocks(handle_t *handle,
>>> -				struct inode *inode, __le32 i_data)
>>> -{
>>> -	int i, retval = 0;
>>> -	__le32 *tmp_idata;
>>> -	struct buffer_head *bh;
>>> -	unsigned long max_entries = inode->i_sb->s_blocksize >> 2;
>>> -
>>> -	bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
>>> -	if (IS_ERR(bh))
>>> -		return PTR_ERR(bh);
>>> -
>>> -	tmp_idata = (__le32 *)bh->b_data;
>>> -	for (i = 0; i < max_entries; i++) {
>>> -		if (tmp_idata[i]) {
>>> -			retval = free_dind_blocks(handle,
>>> -					inode, tmp_idata[i]);
>>> -			if (retval) {
>>> -				put_bh(bh);
>>> -				return retval;
>>> +	if (ind > 0) {
>>> +		int retval = 0;
>>> +		__le32 *tmp_idata;
>>> +		ext4_lblk_t i, max_entries;
>>> +		struct buffer_head *bh;
>>> +
>>> +		bh = ext4_sb_bread(inode->i_sb, le32_to_cpu(i_data), 0);
>>> +		if (IS_ERR(bh))
>>> +			return PTR_ERR(bh);
>>> +
>>> +		tmp_idata = (__le32 *)bh->b_data;
>>> +		max_entries = inode->i_sb->s_blocksize >> 2;
>>> +		for (i = 0; i < max_entries; i++) {
>>> +			if (tmp_idata[i]) {
>>> +				retval = free_ind_blocks(handle,
>>> +						inode, tmp_idata[i], ind - 1);
>>> +				if (retval) {
>>> +					put_bh(bh);
>>> +					return retval;
>>> +				}
>>>  			}
>>>  		}
>>> +		put_bh(bh);
>>>  	}
>>> -	put_bh(bh);
>>>  	extend_credit_for_blkdel(handle, inode);
>>>  	ext4_free_blocks(handle, inode, NULL, le32_to_cpu(i_data), 1,
>>> -			 EXT4_FREE_BLOCKS_METADATA |
>>> -			 EXT4_FREE_BLOCKS_FORGET);
>>> -	return 0;
>>> -}
>>> -
>>> -static int free_ind_block(handle_t *handle, struct inode *inode, __le32 *i_data)
>>> -{
>>> -	int retval;
>>> -
>>> -	/* ei->i_data[EXT4_IND_BLOCK] */
>>> -	if (i_data[0]) {
>>> -		extend_credit_for_blkdel(handle, inode);
>>> -		ext4_free_blocks(handle, inode, NULL,
>>> -				le32_to_cpu(i_data[0]), 1,
>>> -				 EXT4_FREE_BLOCKS_METADATA |
>>> -				 EXT4_FREE_BLOCKS_FORGET);
>>> -	}
>>> -
>>> -	/* ei->i_data[EXT4_DIND_BLOCK] */
>>> -	if (i_data[1]) {
>>> -		retval = free_dind_blocks(handle, inode, i_data[1]);
>>> -		if (retval)
>>> -			return retval;
>>> -	}
>>> -
>>> -	/* ei->i_data[EXT4_TIND_BLOCK] */
>>> -	if (i_data[2]) {
>>> -		retval = free_tind_blocks(handle, inode, i_data[2]);
>>> -		if (retval)
>>> -			return retval;
>>> -	}
>>> +			 EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
>>>  	return 0;
>>>  }
>>>  
>>>  static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
>>>  						struct inode *tmp_inode)
>>>  {
>>> -	int retval;
>>> +	int i, retval;
>>>  	__le32	i_data[3];
>>>  	struct ext4_inode_info *ei = EXT4_I(inode);
>>>  	struct ext4_inode_info *tmp_ei = EXT4_I(tmp_inode);
>>> @@ -307,7 +250,13 @@ static int ext4_ext_swap_inode_data(handle_t *handle, struct inode *inode,
>>>  	 * We mark the inode dirty after, because we decrement the
>>>  	 * i_blocks when freeing the indirect meta-data blocks
>>>  	 */
>>> -	retval = free_ind_block(handle, inode, i_data);
>>> +	for (i = 0; i < ARRAY_SIZE(i_data); i++) {
>>> +		if (i_data[i]) {
>>> +			retval = free_ind_blocks(handle, inode, i_data[i], i);
>>> +			if (retval)
>>> +				break;
>>> +		}
>>> +	}
>>>  	ext4_mark_inode_dirty(handle, inode);
>>>  
>>>  err_out:
>>> -- 
>>> 2.17.1
>>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-07-11 14:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-12 13:09 [PATCH v2] ext4: remove code duplication in free_ind_block() Vasily Averin
2019-05-29 15:01 ` Jan Kara
2019-05-30  7:13   ` Vasily Averin
2019-07-11 14:02     ` Vasily Averin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.