diff mbox series

[2/3] ext4: WANR_ON when detect abnormal 'i_reserved_data_blocks'

Message ID 20221117014246.610202-3-yebin@huaweicloud.com
State New
Headers
Series Fix two issues about bigalloc feature |

Commit Message

Ye Bin Nov. 17, 2022, 1:42 a.m. UTC 
  From: Ye Bin <yebin10@huawei.com>

If 'i_reserved_data_blocks' is not cleared which mean something wrong
with code, so emit WARN_ON to capture this abnormal closer to the first
scene.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 fs/ext4/super.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Jan Kara Nov. 21, 2022, 9:47 a.m. UTC | #1 
On Thu 17-11-22 09:42:45, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> If 'i_reserved_data_blocks' is not cleared which mean something wrong
> with code, so emit WARN_ON to capture this abnormal closer to the first
> scene.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/super.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 63ef74eb8091..30885a6fe18b 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1385,11 +1385,14 @@ static void ext4_destroy_inode(struct inode *inode)
>  		dump_stack();
>  	}
>  
> -	if (EXT4_I(inode)->i_reserved_data_blocks)
> -		ext4_msg(inode->i_sb, KERN_ERR,
> -			 "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!",
> -			 inode->i_ino, EXT4_I(inode),
> -			 EXT4_I(inode)->i_reserved_data_blocks);
> +	if (EXT4_I(inode)->i_reserved_data_blocks) {
> +		ext4_warning(inode->i_sb, "Inode %lu (%p): "
> +			    "i_reserved_data_blocks (%u) not cleared!",
> +			     inode->i_ino, EXT4_I(inode),
> +			     EXT4_I(inode)->i_reserved_data_blocks);
> +
> +		WARN_ON(1);
> +	}

Hum, so I'd think that if this happens, the free space accounting is likely
wrong so we might as well just force the filesystem to error mode with
ext4_error() to force fsck?  I also gives a good chance to various test
systems to detect there is some problem so we don't need the WARN_ON then?
What do others think?

								Honza
Ye Bin Nov. 21, 2022, 11:14 a.m. UTC | #2 
On 2022/11/21 17:47, Jan Kara wrote:
> On Thu 17-11-22 09:42:45, Ye Bin wrote:
>> From: Ye Bin <yebin10@huawei.com>
>>
>> If 'i_reserved_data_blocks' is not cleared which mean something wrong
>> with code, so emit WARN_ON to capture this abnormal closer to the first
>> scene.
>>
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>   fs/ext4/super.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index 63ef74eb8091..30885a6fe18b 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -1385,11 +1385,14 @@ static void ext4_destroy_inode(struct inode *inode)
>>   		dump_stack();
>>   	}
>>   
>> -	if (EXT4_I(inode)->i_reserved_data_blocks)
>> -		ext4_msg(inode->i_sb, KERN_ERR,
>> -			 "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!",
>> -			 inode->i_ino, EXT4_I(inode),
>> -			 EXT4_I(inode)->i_reserved_data_blocks);
>> +	if (EXT4_I(inode)->i_reserved_data_blocks) {
>> +		ext4_warning(inode->i_sb, "Inode %lu (%p): "
>> +			    "i_reserved_data_blocks (%u) not cleared!",
>> +			     inode->i_ino, EXT4_I(inode),
>> +			     EXT4_I(inode)->i_reserved_data_blocks);
>> +
>> +		WARN_ON(1);
>> +	}
> Hum, so I'd think that if this happens, the free space accounting is likely
> wrong so we might as well just force the filesystem to error mode with
> ext4_error() to force fsck?  I also gives a good chance to various test
> systems to detect there is some problem so we don't need the WARN_ON then?
> What do others think?
>
> 								Honza
Thanks for your advice, use ext4_error() maybe is suitable and also 
testable.
diff mbox series

Patch

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 63ef74eb8091..30885a6fe18b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1385,11 +1385,14 @@  static void ext4_destroy_inode(struct inode *inode)
 		dump_stack();
 	}
 
-	if (EXT4_I(inode)->i_reserved_data_blocks)
-		ext4_msg(inode->i_sb, KERN_ERR,
-			 "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!",
-			 inode->i_ino, EXT4_I(inode),
-			 EXT4_I(inode)->i_reserved_data_blocks);
+	if (EXT4_I(inode)->i_reserved_data_blocks) {
+		ext4_warning(inode->i_sb, "Inode %lu (%p): "
+			    "i_reserved_data_blocks (%u) not cleared!",
+			     inode->i_ino, EXT4_I(inode),
+			     EXT4_I(inode)->i_reserved_data_blocks);
+
+		WARN_ON(1);
+	}
 }
 
 static void init_once(void *foo)