Re: [f2fs-dev] [PATCH v2 6/6] f2fs: introduce FAULT_INCONSISTENCE
On 2023/12/28 7:06, Jaegeuk Kim wrote: On 12/25, Chao Yu wrote: We will encounter below inconsistent status when FAULT_BLKADDR type fault injection is on. Info: checkpoint state = d6 : nat_bits crc fsck compacted_summary orphan_inodes sudden-power-off [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c100 has i_blocks: 00c0, but has 191 blocks [FIX] (fsck_chk_inode_blk:1260) --> [0x1c100] i_blocks=0x00c0 -> 0xbf [FIX] (fsck_chk_inode_blk:1269) --> [0x1c100] i_compr_blocks=0x0026 -> 0x27 [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1cadb has i_blocks: 002f, but has 46 blocks [FIX] (fsck_chk_inode_blk:1260) --> [0x1cadb] i_blocks=0x002f -> 0x2e [FIX] (fsck_chk_inode_blk:1269) --> [0x1cadb] i_compr_blocks=0x0011 -> 0x12 [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c62c has i_blocks: 0002, but has 1 blocks [FIX] (fsck_chk_inode_blk:1260) --> [0x1c62c] i_blocks=0x0002 -> 0x1 After we inject fault into f2fs_is_valid_blkaddr() during truncation, a) it missed to increase @nr_free or @valid_blocks b) it can cause in blkaddr leak in truncated dnode Which may cause inconsistent status. This patch separates FAULT_INCONSISTENCE from FAULT_BLKADDR, so that Could you please rename FAULT_INCONSISTENCE to give exactly what it tries to break? Sure, maybe FAULT_BLKADDR_INCONSISTENCE... let me know if you want/have a better one. :) Thanks, we can: a) use FAULT_INCONSISTENCE in f2fs_truncate_data_blocks_range() to simulate inconsistent issue independently, b) FAULT_BLKADDR fault will not cause any inconsistent status, we can just use it to check error path handling in kernel side. Signed-off-by: Chao Yu --- v2: - make __f2fs_is_valid_blkaddr() void. Documentation/ABI/testing/sysfs-fs-f2fs | 1 + Documentation/filesystems/f2fs.rst | 1 + fs/f2fs/checkpoint.c| 19 +++ fs/f2fs/f2fs.h | 3 +++ fs/f2fs/file.c | 8 ++-- fs/f2fs/super.c | 1 + 6 files changed, 27 insertions(+), 6 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs index 4f1d4e636d67..649aabac16c2 100644 --- a/Documentation/ABI/testing/sysfs-fs-f2fs +++ b/Documentation/ABI/testing/sysfs-fs-f2fs @@ -708,6 +708,7 @@ Description:Support configuring fault injection type, should be FAULT_DQUOT_INIT 0x1 FAULT_LOCK_OP0x2 FAULT_BLKADDR0x4 + FAULT_INCONSISTENCE 0x8 === === What: /sys/fs/f2fs//discard_io_aware_gran diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst index d32c6209685d..5616fb8ae207 100644 --- a/Documentation/filesystems/f2fs.rst +++ b/Documentation/filesystems/f2fs.rst @@ -206,6 +206,7 @@ fault_type=%dSupport configuring fault injection type, should be FAULT_DQUOT_INIT 0x1 FAULT_LOCK_OP0x2 FAULT_BLKADDR0x4 +FAULT_INCONSISTENCE 0x8 === === mode=%sControl block allocation mode which supports "adaptive" and "lfs". In "lfs" mode, there should be no random diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index b0597a539fc5..84546f529cf0 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -170,12 +170,9 @@ static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, block_t blkaddr, return exist; } -bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, +static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, block_t blkaddr, int type) { - if (time_to_inject(sbi, FAULT_BLKADDR)) - return false; - switch (type) { case META_NAT: break; @@ -230,6 +227,20 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, return true; } +bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, + block_t blkaddr, int type) +{ + if (time_to_inject(sbi, FAULT_BLKADDR)) + return false; + return __f2fs_is_valid_blkaddr(sbi, blkaddr, type); +} + +bool f2fs_is_valid_blkaddr_raw(struct f2fs_sb_info *sbi, + block_t blkaddr, int type) +{ + return __f2fs_is_valid_blkaddr(sbi, blkaddr, type); +} + /* * Readahead CP/NAT/SIT/SSA/POR pages */ diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 34b20700b5ec..3985296e64cb 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -61,6 +61,7 @@ enum { FAULT_DQUOT_INIT, FAULT_LOCK_OP, FAULT_BLKADDR, + FAULT_INCONSISTENCE,
Re: [f2fs-dev] [PATCH v2 6/6] f2fs: introduce FAULT_INCONSISTENCE
On 12/25, Chao Yu wrote: > We will encounter below inconsistent status when FAULT_BLKADDR type > fault injection is on. > > Info: checkpoint state = d6 : nat_bits crc fsck compacted_summary > orphan_inodes sudden-power-off > [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c100 has i_blocks: 00c0, > but has 191 blocks > [FIX] (fsck_chk_inode_blk:1260) --> [0x1c100] i_blocks=0x00c0 -> 0xbf > [FIX] (fsck_chk_inode_blk:1269) --> [0x1c100] i_compr_blocks=0x0026 -> > 0x27 > [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1cadb has i_blocks: 002f, > but has 46 blocks > [FIX] (fsck_chk_inode_blk:1260) --> [0x1cadb] i_blocks=0x002f -> 0x2e > [FIX] (fsck_chk_inode_blk:1269) --> [0x1cadb] i_compr_blocks=0x0011 -> > 0x12 > [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c62c has i_blocks: 0002, > but has 1 blocks > [FIX] (fsck_chk_inode_blk:1260) --> [0x1c62c] i_blocks=0x0002 -> 0x1 > > After we inject fault into f2fs_is_valid_blkaddr() during truncation, > a) it missed to increase @nr_free or @valid_blocks > b) it can cause in blkaddr leak in truncated dnode > Which may cause inconsistent status. > > This patch separates FAULT_INCONSISTENCE from FAULT_BLKADDR, so that Could you please rename FAULT_INCONSISTENCE to give exactly what it tries to break? > we can: > a) use FAULT_INCONSISTENCE in f2fs_truncate_data_blocks_range() to > simulate inconsistent issue independently, > b) FAULT_BLKADDR fault will not cause any inconsistent status, we can > just use it to check error path handling in kernel side. > > Signed-off-by: Chao Yu > --- > v2: > - make __f2fs_is_valid_blkaddr() void. > Documentation/ABI/testing/sysfs-fs-f2fs | 1 + > Documentation/filesystems/f2fs.rst | 1 + > fs/f2fs/checkpoint.c| 19 +++ > fs/f2fs/f2fs.h | 3 +++ > fs/f2fs/file.c | 8 ++-- > fs/f2fs/super.c | 1 + > 6 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs > b/Documentation/ABI/testing/sysfs-fs-f2fs > index 4f1d4e636d67..649aabac16c2 100644 > --- a/Documentation/ABI/testing/sysfs-fs-f2fs > +++ b/Documentation/ABI/testing/sysfs-fs-f2fs > @@ -708,6 +708,7 @@ Description: Support configuring fault injection > type, should be > FAULT_DQUOT_INIT 0x1 > FAULT_LOCK_OP0x2 > FAULT_BLKADDR0x4 > + FAULT_INCONSISTENCE 0x8 > === === > > What:/sys/fs/f2fs//discard_io_aware_gran > diff --git a/Documentation/filesystems/f2fs.rst > b/Documentation/filesystems/f2fs.rst > index d32c6209685d..5616fb8ae207 100644 > --- a/Documentation/filesystems/f2fs.rst > +++ b/Documentation/filesystems/f2fs.rst > @@ -206,6 +206,7 @@ fault_type=%d Support configuring fault > injection type, should be >FAULT_DQUOT_INIT 0x1 >FAULT_LOCK_OP0x2 >FAULT_BLKADDR0x4 > + FAULT_INCONSISTENCE 0x8 >=== === > mode=%s Control block allocation mode which supports > "adaptive" >and "lfs". In "lfs" mode, there should be no random > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index b0597a539fc5..84546f529cf0 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -170,12 +170,9 @@ static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, > block_t blkaddr, > return exist; > } > > -bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, > +static bool __f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, > block_t blkaddr, int type) > { > - if (time_to_inject(sbi, FAULT_BLKADDR)) > - return false; > - > switch (type) { > case META_NAT: > break; > @@ -230,6 +227,20 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, > return true; > } > > +bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, > + block_t blkaddr, int type) > +{ > + if (time_to_inject(sbi, FAULT_BLKADDR)) > + return false; > + return __f2fs_is_valid_blkaddr(sbi, blkaddr, type); > +} > + > +bool f2fs_is_valid_blkaddr_raw(struct f2fs_sb_info *sbi, > + block_t blkaddr, int type) > +{ > + return __f2fs_is_valid_blkaddr(sbi, blkaddr, type); > +} > + > /* > * Readahead CP/NAT/SIT/SSA/POR pages > */ > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 34b20700b5ec..3985296e64cb 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -61,6 +61,7 @@ enum { > FAULT_DQUOT_INIT, > FAULT_LOCK_OP, > FAULT_BLKADDR, > +