Re: [PATCH v5] f2fs: avoid dead loop in function find_fsync_dnodes

[Chao Yu <yuchao0@huawei.com>]

On 2017/12/20 13:03, Yunlei He wrote:
> v4 -> v5 return err instead destory inode list for two reasons:
> i.  avoid duplicated destroy inode list in function recover_fsync_data.
> ii. report an error for recovery, and set need_fsck flag

fsck can't fix this issue so far, it doesn't work even we set need_fsck flag,
would it be better to just drop last dnode with corrupted next_blkaddr and try
our best to recover fsynced data?

Thanks,

>         
> Came across a dead loop in recovery like this:
> 
> ......
> [   24.680480s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597696
> [   24.698394s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597697
> [   24.724334s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724334s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724365s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724365s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724365s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724395s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724395s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724395s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724395s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> [   24.724426s][pid:320,cpu0,init]find_fsync_dnodes: blkaddr =13597698
> ......
> 
> Mount process will block in dead loop and fsck can do nothing with this
> error, This patch abandon recovery if node chain is cyclical.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/recovery.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index d025aa8..a535ec2 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -216,6 +216,17 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
>  			return 0;
>  
>  		page = get_tmp_page(sbi, blkaddr);
> +		if (PageChecked(page)) {
> +			f2fs_msg(sbi->sb, KERN_ERR, "Abandon looped node block list");
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		/*
> +		 * it's not needed to clear PG_checked flag in temp page since we
> +		 * will truncate all those pages in the end of recovery.
> +		 */
> +		SetPageChecked(page);
>  
>  		if (!is_recoverable_dnode(page))
>  			break;
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot