On 2017/12/14 11:54, Yunlei He wrote:
> 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 <[email protected]>
> ---
> fs/f2fs/data.c | 5 +++++
> fs/f2fs/recovery.c | 23 +++++++++++++++--------
> 2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 7aca6cc..2ac992f 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2146,6 +2146,9 @@ void f2fs_invalidate_page(struct page *page, unsigned
> int offset,
> }
> }
>
> + if (PageChecked(page))
> + ClearPageChecked(page);
IMO, we don't need to care about this flag like most other PG_* flags
during truncation.
> +
> /* This is atomic written page, keep Private */
> if (IS_ATOMIC_WRITTEN_PAGE(page))
> return drop_inmem_page(inode, page);
> @@ -2292,6 +2295,8 @@ int f2fs_migrate_page(struct address_space *mapping,
> SetPagePrivate(newpage);
> set_page_private(newpage, page_private(page));
>
> + if (PageChecked(page))
> + SetPageChecked(page);
Actually, migrate_page_copy will do this job.
> if (mode != MIGRATE_SYNC_NO_COPY)
> migrate_page_copy(newpage, page);
> else
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index 7d63faf..905be02 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -220,6 +220,14 @@ static void recover_inode(struct inode *inode, struct
> page *page)
> ino_of_node(page), name);
> }
>
> +static void destroy_fsync_dnodes(struct list_head *head)
> +{
> + struct fsync_inode_entry *entry, *tmp;
> +
> + list_for_each_entry_safe(entry, tmp, head, list)
> + del_fsync_inode(entry);
> +}
How about moving this function behind del_fsync_inode?
> +
> static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head
> *head,
> bool check_only)
> {
> @@ -239,7 +247,14 @@ 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 dead loop node
> block list");
"Abandon looped node block list"
> + destroy_fsync_dnodes(head);
> + break;
> + }
>
> + /* No need ClearPageChecked for page cache truncated */
/*
* 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);
Will looks more neat to leave a blank line here.
Thanks,
> if (!is_recoverable_dnode(page))
> break;
>
> @@ -288,14 +303,6 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi,
> struct list_head *head,
> return err;
> }
>
> -static void destroy_fsync_dnodes(struct list_head *head)
> -{
> - struct fsync_inode_entry *entry, *tmp;
> -
> - list_for_each_entry_safe(entry, tmp, head, list)
> - del_fsync_inode(entry);
> -}
> -
> static int check_index_in_prev_nodes(struct f2fs_sb_info *sbi,
> block_t blkaddr, struct dnode_of_data *dn)
> {
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
