diff mbox series

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

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

Commit Message

Ye Bin Dec. 8, 2022, 3:34 a.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>
Reviewed-by: Eric Whitney <enwlinux@gmail.com>
---
 fs/ext4/super.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Theodore Ts'o Dec. 9, 2022, 5:50 a.m. UTC | #1 
On Thu, Dec 08, 2022 at 11:34:25AM +0800, Ye Bin wrote:
> 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.

If i_reserved_data_block, it means that there is something wrong with
our delayed allocation accounting.  However, this accounting is
usually only an in-memory error.  The one exception is if quota is
enabled, in which case the space is decremented from the
user/group/project quota.

However, if quota is not in use, which is the overwhelmingly common
case, there will be nothing for fsck to repair.  (It does mean that df
will show a slightly smaller free space value due to the messed up
delayed allocation accounting, but that disappears after a reboot.)

Marking the file system as in need of repair when nothing is actually
wrong with the on-disk file system can backfire, in that it confuses
users when they see the ext4 error but then when they run fsck, fsck
reports nothing wrong --- at which point they file a bug.

We could use ext4_error if quotas are enabled, and ext4_msg if not,
but it might not worth the extra complexity.

Cheers,

					- Ted
diff mbox series

Patch

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 840e0a614959..4b2d257d3845 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1387,10 +1387,10 @@  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)