| Message ID | 20231225144335.2548-1-chao@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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: 000000c0, but has 191 blocks > [FIX] (fsck_chk_inode_blk:1260) --> [0x1c100] i_blocks=0x000000c0 -> 0xbf > [FIX] (fsck_chk_inode_blk:1269) --> [0x1c100] i_compr_blocks=0x00000026 -> 0x27 > [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1cadb has i_blocks: 0000002f, but has 46 blocks > [FIX] (fsck_chk_inode_blk:1260) --> [0x1cadb] i_blocks=0x0000002f -> 0x2e > [FIX] (fsck_chk_inode_blk:1269) --> [0x1cadb] i_compr_blocks=0x00000011 -> 0x12 > [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c62c has i_blocks: 00000002, but has 1 blocks > [FIX] (fsck_chk_inode_blk:1260) --> [0x1c62c] i_blocks=0x00000002 -> 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 <chao@kernel.org> > --- > 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 0x000010000 > FAULT_LOCK_OP 0x000020000 > FAULT_BLKADDR 0x000040000 > + FAULT_INCONSISTENCE 0x000080000 > =================== =========== > > What: /sys/fs/f2fs/<disk>/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 0x000010000 > FAULT_LOCK_OP 0x000020000 > FAULT_BLKADDR 0x000040000 > + FAULT_INCONSISTENCE 0x000080000 > =================== =========== > 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, > + FAULT_INCONSISTENCE, > FAULT_MAX, > }; > > @@ -3767,6 +3768,8 @@ struct page *f2fs_get_meta_page_retry(struct f2fs_sb_info *sbi, pgoff_t index); > struct page *f2fs_get_tmp_page(struct f2fs_sb_info *sbi, pgoff_t index); > bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, > block_t blkaddr, int type); > +bool f2fs_is_valid_blkaddr_raw(struct f2fs_sb_info *sbi, > + block_t blkaddr, int type); > int f2fs_ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages, > int type, bool sync); > void f2fs_ra_meta_pages_cond(struct f2fs_sb_info *sbi, pgoff_t index, > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 9f4e21b5916c..b5149f1f2a20 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -590,9 +590,13 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count) > f2fs_set_data_blkaddr(dn, NULL_ADDR); > > if (__is_valid_data_blkaddr(blkaddr)) { > - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, > - DATA_GENERIC_ENHANCE)) > + if (time_to_inject(sbi, FAULT_INCONSISTENCE)) > + continue; > + if (!f2fs_is_valid_blkaddr_raw(sbi, blkaddr, > + DATA_GENERIC_ENHANCE)) { > + f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR); > continue; > + } > if (compressed_cluster) > valid_blocks++; > } > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 206d03c82d96..9a5c5e06f766 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -62,6 +62,7 @@ const char *f2fs_fault_name[FAULT_MAX] = { > [FAULT_DQUOT_INIT] = "dquot initialize", > [FAULT_LOCK_OP] = "lock_op", > [FAULT_BLKADDR] = "invalid blkaddr", > + [FAULT_INCONSISTENCE] = "inconsistence", > }; > > void f2fs_build_fault_attr(struct f2fs_sb_info *sbi, unsigned int rate, > -- > 2.40.1
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: 000000c0, but has 191 blocks >> [FIX] (fsck_chk_inode_blk:1260) --> [0x1c100] i_blocks=0x000000c0 -> 0xbf >> [FIX] (fsck_chk_inode_blk:1269) --> [0x1c100] i_compr_blocks=0x00000026 -> 0x27 >> [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1cadb has i_blocks: 0000002f, but has 46 blocks >> [FIX] (fsck_chk_inode_blk:1260) --> [0x1cadb] i_blocks=0x0000002f -> 0x2e >> [FIX] (fsck_chk_inode_blk:1269) --> [0x1cadb] i_compr_blocks=0x00000011 -> 0x12 >> [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c62c has i_blocks: 00000002, but has 1 blocks >> [FIX] (fsck_chk_inode_blk:1260) --> [0x1c62c] i_blocks=0x00000002 -> 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 <chao@kernel.org> >> --- >> 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 0x000010000 >> FAULT_LOCK_OP 0x000020000 >> FAULT_BLKADDR 0x000040000 >> + FAULT_INCONSISTENCE 0x000080000 >> =================== =========== >> >> What: /sys/fs/f2fs/<disk>/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 0x000010000 >> FAULT_LOCK_OP 0x000020000 >> FAULT_BLKADDR 0x000040000 >> + FAULT_INCONSISTENCE 0x000080000 >> =================== =========== >> 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, >> + FAULT_INCONSISTENCE, >> FAULT_MAX, >> }; >> >> @@ -3767,6 +3768,8 @@ struct page *f2fs_get_meta_page_retry(struct f2fs_sb_info *sbi, pgoff_t index); >> struct page *f2fs_get_tmp_page(struct f2fs_sb_info *sbi, pgoff_t index); >> bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, >> block_t blkaddr, int type); >> +bool f2fs_is_valid_blkaddr_raw(struct f2fs_sb_info *sbi, >> + block_t blkaddr, int type); >> int f2fs_ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages, >> int type, bool sync); >> void f2fs_ra_meta_pages_cond(struct f2fs_sb_info *sbi, pgoff_t index, >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index 9f4e21b5916c..b5149f1f2a20 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -590,9 +590,13 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count) >> f2fs_set_data_blkaddr(dn, NULL_ADDR); >> >> if (__is_valid_data_blkaddr(blkaddr)) { >> - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, >> - DATA_GENERIC_ENHANCE)) >> + if (time_to_inject(sbi, FAULT_INCONSISTENCE)) >> + continue; >> + if (!f2fs_is_valid_blkaddr_raw(sbi, blkaddr, >> + DATA_GENERIC_ENHANCE)) { >> + f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR); >> continue; >> + } >> if (compressed_cluster) >> valid_blocks++; >> } >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index 206d03c82d96..9a5c5e06f766 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -62,6 +62,7 @@ const char *f2fs_fault_name[FAULT_MAX] = { >> [FAULT_DQUOT_INIT] = "dquot initialize", >> [FAULT_LOCK_OP] = "lock_op", >> [FAULT_BLKADDR] = "invalid blkaddr", >> + [FAULT_INCONSISTENCE] = "inconsistence", >> }; >> >> void f2fs_build_fault_attr(struct f2fs_sb_info *sbi, unsigned int rate, >> -- >> 2.40.1
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 0x000010000 FAULT_LOCK_OP 0x000020000 FAULT_BLKADDR 0x000040000 + FAULT_INCONSISTENCE 0x000080000 =================== =========== What: /sys/fs/f2fs/<disk>/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 0x000010000 FAULT_LOCK_OP 0x000020000 FAULT_BLKADDR 0x000040000 + FAULT_INCONSISTENCE 0x000080000 =================== =========== 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, + FAULT_INCONSISTENCE, FAULT_MAX, }; @@ -3767,6 +3768,8 @@ struct page *f2fs_get_meta_page_retry(struct f2fs_sb_info *sbi, pgoff_t index); struct page *f2fs_get_tmp_page(struct f2fs_sb_info *sbi, pgoff_t index); bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, block_t blkaddr, int type); +bool f2fs_is_valid_blkaddr_raw(struct f2fs_sb_info *sbi, + block_t blkaddr, int type); int f2fs_ra_meta_pages(struct f2fs_sb_info *sbi, block_t start, int nrpages, int type, bool sync); void f2fs_ra_meta_pages_cond(struct f2fs_sb_info *sbi, pgoff_t index, diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 9f4e21b5916c..b5149f1f2a20 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -590,9 +590,13 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count) f2fs_set_data_blkaddr(dn, NULL_ADDR); if (__is_valid_data_blkaddr(blkaddr)) { - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, - DATA_GENERIC_ENHANCE)) + if (time_to_inject(sbi, FAULT_INCONSISTENCE)) + continue; + if (!f2fs_is_valid_blkaddr_raw(sbi, blkaddr, + DATA_GENERIC_ENHANCE)) { + f2fs_handle_error(sbi, ERROR_INVALID_BLKADDR); continue; + } if (compressed_cluster) valid_blocks++; } diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 206d03c82d96..9a5c5e06f766 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -62,6 +62,7 @@ const char *f2fs_fault_name[FAULT_MAX] = { [FAULT_DQUOT_INIT] = "dquot initialize", [FAULT_LOCK_OP] = "lock_op", [FAULT_BLKADDR] = "invalid blkaddr", + [FAULT_INCONSISTENCE] = "inconsistence", }; void f2fs_build_fault_attr(struct f2fs_sb_info *sbi, unsigned int rate,
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: 000000c0, but has 191 blocks [FIX] (fsck_chk_inode_blk:1260) --> [0x1c100] i_blocks=0x000000c0 -> 0xbf [FIX] (fsck_chk_inode_blk:1269) --> [0x1c100] i_compr_blocks=0x00000026 -> 0x27 [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1cadb has i_blocks: 0000002f, but has 46 blocks [FIX] (fsck_chk_inode_blk:1260) --> [0x1cadb] i_blocks=0x0000002f -> 0x2e [FIX] (fsck_chk_inode_blk:1269) --> [0x1cadb] i_compr_blocks=0x00000011 -> 0x12 [ASSERT] (fsck_chk_inode_blk:1254) --> ino: 0x1c62c has i_blocks: 00000002, but has 1 blocks [FIX] (fsck_chk_inode_blk:1260) --> [0x1c62c] i_blocks=0x00000002 -> 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 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 <chao@kernel.org> --- 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(-)