diff mbox series

[v2,2/3] ext4: record error when detect abnormal 'i_reserved_data_blocks'

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

Commit Message

Ye Bin Nov. 21, 2022, 12:14 p.m. UTC 
  From: Ye Bin <yebin10@huawei.com>

If 'i_reserved_data_blocks' is not cleared which mean something wrong with
code, free space accounting is likely wrong, according to Jan Kara's advice
use ext4_error() to record this abnormal let fsck to repair and also we can
capture this issue.

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

Comments

Eric Whitney Nov. 22, 2022, 3:34 a.m. UTC | #1 
* Ye Bin <yebin@huaweicloud.com>:
> From: Ye Bin <yebin10@huawei.com>
> 
> If 'i_reserved_data_blocks' is not cleared which mean something wrong with
> code, free space accounting is likely wrong, according to Jan Kara's advice
> use ext4_error() to record this abnormal let fsck to repair and also we can
> capture this issue.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  fs/ext4/super.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 0690e2e0b74d..3d30007502a4 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1387,10 +1387,9 @@ static void ext4_destroy_inode(struct inode *inode)
>  	}
>  
>  	if (EXT4_I(inode)->i_reserved_data_blocks)
> -		ext4_msg(inode->i_sb, KERN_ERR,

Per the coding standard, IIRC, the string should not be split across lines
for "greppability", so it should remain as is.  It's always good to run
checkpatch to catch stuff like this.

> -			 "Inode %lu (%p): i_reserved_data_blocks (%u) not cleared!",
> -			 inode->i_ino, EXT4_I(inode),
> -			 EXT4_I(inode)->i_reserved_data_blocks);
> +		ext4_error(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);
>  }
>  
>  static void init_once(void *foo)

This is an improvement over the first version.  If i_reserved_data_blocks is
non-zero, something is definitely broken, but it's perhaps less likely to
indicate file system damage than if it hits zero while there are still
outstanding delayed blocks (handled well elsewhere).  So, it's not clear
we need to escalate handling this case but it doesn't hurt, either.

Eric

> -- 
> 2.31.1
>
diff mbox series

Patch

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 0690e2e0b74d..3d30007502a4 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1387,10 +1387,9 @@  static void ext4_destroy_inode(struct inode *inode)
 	}
 
 	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);
+		ext4_error(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);
 }
 
 static void init_once(void *foo)