* [PATCH v3] f2fs: avoid dead loop in function find_fsync_dnodes
@ 2017-12-14 3:54 Yunlei He
2017-12-14 11:03 ` Chao Yu
0 siblings, 1 reply; 2+ messages in thread
From: Yunlei He @ 2017-12-14 3:54 UTC (permalink / raw)
To: jaegeuk, yuchao0, linux-f2fs-devel; +Cc: ning.jia, heyunlei
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/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);
+
/* 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);
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);
+}
+
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");
+ destroy_fsync_dnodes(head);
+ break;
+ }
+ /* No need ClearPageChecked for page cache truncated */
+ SetPageChecked(page);
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)
{
--
1.9.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] f2fs: avoid dead loop in function find_fsync_dnodes
2017-12-14 3:54 [PATCH v3] f2fs: avoid dead loop in function find_fsync_dnodes Yunlei He
@ 2017-12-14 11:03 ` Chao Yu
0 siblings, 0 replies; 2+ messages in thread
From: Chao Yu @ 2017-12-14 11:03 UTC (permalink / raw)
To: Yunlei He, jaegeuk, linux-f2fs-devel; +Cc: ning.jia
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 <heyunlei@huawei.com>
> ---
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-12-14 11:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-14 3:54 [PATCH v3] f2fs: avoid dead loop in function find_fsync_dnodes Yunlei He
2017-12-14 11:03 ` Chao Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.