Hi Chao, On Wed, Jan 24, 2024 at 10:54 PM Chao Yu <c...@kernel.org> wrote: > > Zhiguo, >m > Can you please check below version? Is it fine to you? > > https://lore.kernel.org/linux-f2fs-devel/20240124144915.19445-1-c...@kernel.org it is ok to me and more reasonable than my version thanks~ > > On 2024/1/22 13:46, Zhiguo Niu wrote: > > Hi Chao > > > > On Mon, Jan 22, 2024 at 11:46 AM Chao Yu <c...@kernel.org> wrote: > >> > >> On 2023/12/25 19:11, Zhiguo Niu wrote: > >>> There is a potentail deadloop issue in the corner case of > >>> CONFIG_F2FS_FAULT_INJECTION is enabled and the return value > >>> of f2fs_reserve_new_block is error but not -ENOSPC, such as > >>> this error case: > >>> if (unlikely(is_inode_flag_set(dn->inode, FI_NO_ALLOC))) > >>> return -EPERM; > >> > >> I don't see any path to trigger this error? am I missing something? > >> > >>> besides, the mainly error -ENOSPC has been handled as bug on, > >>> so other error cases can be proecssed normally without looping. > >> > >> commit 975756c41332bc5e523e9f843271ed5ab6aaaaaa > >> Author: Jaegeuk Kim <jaeg...@kernel.org> > >> Date: Thu May 19 11:57:21 2016 -0700 > >> > >> f2fs: avoid ENOSPC fault in the recovery process > >> > >> This patch avoids impossible error injection, ENOSPC, during > >> recovery process. > >> > >> Please check above patch, I guess intention of adding such loop is > >> to avoid mount failure due to fault injection was triggered in > >> f2fs_reserve_new_block(). > >> > >> What about change as blew? > >> - keep the loop to avoid mount failure. > >> - remove bug_on() to avoid panic due to fault injection error. > >> > >> #define DEFAULT_RETRY_COUNT 8 > >> > >> for (loops = DEFAULT_RETRY_COUNT; loops > 0; loops--) { > >> err = f2fs_reserve_new_block(&dn); > >> if (!err || > >> !IS_ENABLED(CONFIG_F2FS_FAULT_INJECTION)) > >> break; > >> } > > > > Thanks for your detailed explanation and I understand. > > It seems that the original process is also reasonable, > > so it’s okay to keep it as it is. > >> > >> Thanks, > >> > >>> > >>> Fixes: 956fa1ddc132 ("f2fs: fix to check return value of > >>> f2fs_reserve_new_block()") > >>> Signed-off-by: Zhiguo Niu <zhiguo....@unisoc.com> > >>> --- > >>> fs/f2fs/recovery.c | 26 ++++++++------------------ > >>> 1 file changed, 8 insertions(+), 18 deletions(-) > >>> > >>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c > >>> index 21381b7..5d658f6 100644 > >>> --- a/fs/f2fs/recovery.c > >>> +++ b/fs/f2fs/recovery.c > >>> @@ -710,15 +710,10 @@ static int do_recover_data(struct f2fs_sb_info > >>> *sbi, struct inode *inode, > >>> */ > >>> if (dest == NEW_ADDR) { > >>> f2fs_truncate_data_blocks_range(&dn, 1); > >>> - do { > >>> - err = f2fs_reserve_new_block(&dn); > >>> - if (err == -ENOSPC) { > >>> - f2fs_bug_on(sbi, 1); > >>> - break; > >>> - } > >>> - } while (err && > >>> - IS_ENABLED(CONFIG_F2FS_FAULT_INJECTION)); > >>> - if (err) > >>> + err = f2fs_reserve_new_block(&dn); > >>> + if (err == -ENOSPC) > >>> + f2fs_bug_on(sbi, 1); > >>> + else if (err) > >>> goto err; > >>> continue; > >>> } > >>> @@ -727,15 +722,10 @@ static int do_recover_data(struct f2fs_sb_info > >>> *sbi, struct inode *inode, > >>> if (f2fs_is_valid_blkaddr(sbi, dest, META_POR)) { > >>> > >>> if (src == NULL_ADDR) { > >>> - do { > >>> - err = f2fs_reserve_new_block(&dn); > >>> - if (err == -ENOSPC) { > >>> - f2fs_bug_on(sbi, 1); > >>> - break; > >>> - } > >>> - } while (err && > >>> - > >>> IS_ENABLED(CONFIG_F2FS_FAULT_INJECTION)); > >>> - if (err) > >>> + err = f2fs_reserve_new_block(&dn); > >>> + if (err == -ENOSPC) > >>> + f2fs_bug_on(sbi, 1); > >>> + else if (err) > >>> goto err; > >>> } > >>> retry_prev:
_______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
