* [PATCH v2] f2fs: avoid dead loop in function find_fsync_dnodes
@ 2017-12-11 4:23 Yunlei He
2017-12-11 13:46 ` Chao Yu
0 siblings, 1 reply; 2+ messages in thread
From: Yunlei He @ 2017-12-11 4:23 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/f2fs.h | 6 +++++
fs/f2fs/recovery.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 67 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 82f1dc3..4649663 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -289,6 +289,12 @@ struct fsync_inode_entry {
block_t last_dentry; /* block address locating the last dentry */
};
+/* for the block of node chain, used only during recovery */
+struct fsync_node_entry {
+ struct list_head list; /* list head */
+ block_t blkaddr; /* locating the node block in recovery chain */
+};
+
#define nats_in_cursum(jnl) (le16_to_cpu((jnl)->n_nats))
#define sits_in_cursum(jnl) (le16_to_cpu((jnl)->n_sits))
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 7d63faf..9958abb 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -46,6 +46,7 @@
*/
static struct kmem_cache *fsync_entry_slab;
+static struct kmem_cache *fsync_node_chain_slab;
bool space_for_roll_forward(struct f2fs_sb_info *sbi)
{
@@ -220,14 +221,55 @@ static void recover_inode(struct inode *inode, struct page *page)
ino_of_node(page), name);
}
+static void destroy_node_blocks(struct list_head *head)
+{
+ struct fsync_node_entry *entry, *tmp;
+
+ list_for_each_entry_safe(entry, tmp, head, list) {
+ list_del(&entry->list);
+ kmem_cache_free(fsync_node_chain_slab, entry);
+ }
+}
+
+static bool blkaddr_already_in_list(struct list_head *head, block_t blkaddr)
+{
+ struct fsync_node_entry *entry;
+
+ list_for_each_entry(entry, head, list)
+ if (entry->blkaddr == blkaddr)
+ return true;
+
+ return false;
+}
+
+static void add_fsync_node(struct list_head *head, block_t blkaddr)
+{
+ struct fsync_node_entry *entry;
+
+ entry = f2fs_kmem_cache_alloc(fsync_node_chain_slab, GFP_F2FS_ZERO);
+ entry->blkaddr = blkaddr;
+ list_add_tail(&entry->list, head);
+}
+
+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)
{
struct curseg_info *curseg;
struct page *page = NULL;
+ struct list_head node_block_list;
block_t blkaddr;
int err = 0;
+ INIT_LIST_HEAD(&node_block_list);
+
/* get node pages in the current segment */
curseg = CURSEG_I(sbi, CURSEG_WARM_NODE);
blkaddr = NEXT_FREE_BLKADDR(sbi, curseg);
@@ -239,7 +281,13 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head,
return 0;
page = get_tmp_page(sbi, blkaddr);
+ if (unlikely(blkaddr_already_in_list(&node_block_list, blkaddr))) {
+ f2fs_msg(sbi->sb, KERN_ERR, "Abandon dead loop node block list");
+ destroy_fsync_dnodes(head);
+ break;
+ }
+ add_fsync_node(&node_block_list, blkaddr);
if (!is_recoverable_dnode(page))
break;
@@ -284,18 +332,12 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head,
ra_meta_pages_cond(sbi, blkaddr);
}
+
+ destroy_node_blocks(&node_block_list);
f2fs_put_page(page, 1);
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)
{
@@ -614,7 +656,14 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
sizeof(struct fsync_inode_entry));
if (!fsync_entry_slab) {
err = -ENOMEM;
- goto out;
+ goto free_inode_entry;
+ }
+
+ fsync_node_chain_slab = f2fs_kmem_cache_create("f2fs_fsync_node_entry",
+ sizeof(struct fsync_node_entry));
+ if (!fsync_node_chain_slab) {
+ err = -ENOMEM;
+ goto free_node_entry;
}
INIT_LIST_HEAD(&inode_list);
@@ -664,8 +713,10 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
err = write_checkpoint(sbi, &cpc);
}
+ kmem_cache_destroy(fsync_node_chain_slab);
+free_node_entry:
kmem_cache_destroy(fsync_entry_slab);
-out:
+free_inode_entry:
#ifdef CONFIG_QUOTA
/* Turn quotas off */
if (quota_enabled)
--
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 v2] f2fs: avoid dead loop in function find_fsync_dnodes
2017-12-11 4:23 [PATCH v2] f2fs: avoid dead loop in function find_fsync_dnodes Yunlei He
@ 2017-12-11 13:46 ` Chao Yu
0 siblings, 0 replies; 2+ messages in thread
From: Chao Yu @ 2017-12-11 13:46 UTC (permalink / raw)
To: Yunlei He, jaegeuk, yuchao0, linux-f2fs-devel; +Cc: ning.jia
On 2017/12/11 12:23, 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.
What about tagging page with PG_checked flag to indicate current meta page
has been traversed?
Thanks,
>
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
> fs/f2fs/f2fs.h | 6 +++++
> fs/f2fs/recovery.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 67 insertions(+), 10 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 82f1dc3..4649663 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -289,6 +289,12 @@ struct fsync_inode_entry {
> block_t last_dentry; /* block address locating the last dentry */
> };
>
> +/* for the block of node chain, used only during recovery */
> +struct fsync_node_entry {
> + struct list_head list; /* list head */
> + block_t blkaddr; /* locating the node block in recovery chain */
> +};
> +
> #define nats_in_cursum(jnl) (le16_to_cpu((jnl)->n_nats))
> #define sits_in_cursum(jnl) (le16_to_cpu((jnl)->n_sits))
>
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index 7d63faf..9958abb 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -46,6 +46,7 @@
> */
>
> static struct kmem_cache *fsync_entry_slab;
> +static struct kmem_cache *fsync_node_chain_slab;
>
> bool space_for_roll_forward(struct f2fs_sb_info *sbi)
> {
> @@ -220,14 +221,55 @@ static void recover_inode(struct inode *inode, struct page *page)
> ino_of_node(page), name);
> }
>
> +static void destroy_node_blocks(struct list_head *head)
> +{
> + struct fsync_node_entry *entry, *tmp;
> +
> + list_for_each_entry_safe(entry, tmp, head, list) {
> + list_del(&entry->list);
> + kmem_cache_free(fsync_node_chain_slab, entry);
> + }
> +}
> +
> +static bool blkaddr_already_in_list(struct list_head *head, block_t blkaddr)
> +{
> + struct fsync_node_entry *entry;
> +
> + list_for_each_entry(entry, head, list)
> + if (entry->blkaddr == blkaddr)
> + return true;
> +
> + return false;
> +}
> +
> +static void add_fsync_node(struct list_head *head, block_t blkaddr)
> +{
> + struct fsync_node_entry *entry;
> +
> + entry = f2fs_kmem_cache_alloc(fsync_node_chain_slab, GFP_F2FS_ZERO);
> + entry->blkaddr = blkaddr;
> + list_add_tail(&entry->list, head);
> +}
> +
> +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)
> {
> struct curseg_info *curseg;
> struct page *page = NULL;
> + struct list_head node_block_list;
> block_t blkaddr;
> int err = 0;
>
> + INIT_LIST_HEAD(&node_block_list);
> +
> /* get node pages in the current segment */
> curseg = CURSEG_I(sbi, CURSEG_WARM_NODE);
> blkaddr = NEXT_FREE_BLKADDR(sbi, curseg);
> @@ -239,7 +281,13 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head,
> return 0;
>
> page = get_tmp_page(sbi, blkaddr);
> + if (unlikely(blkaddr_already_in_list(&node_block_list, blkaddr))) {
> + f2fs_msg(sbi->sb, KERN_ERR, "Abandon dead loop node block list");
> + destroy_fsync_dnodes(head);
> + break;
> + }
>
> + add_fsync_node(&node_block_list, blkaddr);
> if (!is_recoverable_dnode(page))
> break;
>
> @@ -284,18 +332,12 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head,
>
> ra_meta_pages_cond(sbi, blkaddr);
> }
> +
> + destroy_node_blocks(&node_block_list);
> f2fs_put_page(page, 1);
> 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)
> {
> @@ -614,7 +656,14 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
> sizeof(struct fsync_inode_entry));
> if (!fsync_entry_slab) {
> err = -ENOMEM;
> - goto out;
> + goto free_inode_entry;
> + }
> +
> + fsync_node_chain_slab = f2fs_kmem_cache_create("f2fs_fsync_node_entry",
> + sizeof(struct fsync_node_entry));
> + if (!fsync_node_chain_slab) {
> + err = -ENOMEM;
> + goto free_node_entry;
> }
>
> INIT_LIST_HEAD(&inode_list);
> @@ -664,8 +713,10 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool check_only)
> err = write_checkpoint(sbi, &cpc);
> }
>
> + kmem_cache_destroy(fsync_node_chain_slab);
> +free_node_entry:
> kmem_cache_destroy(fsync_entry_slab);
> -out:
> +free_inode_entry:
> #ifdef CONFIG_QUOTA
> /* Turn quotas off */
> if (quota_enabled)
>
------------------------------------------------------------------------------
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-11 13:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-11 4:23 [PATCH v2] f2fs: avoid dead loop in function find_fsync_dnodes Yunlei He
2017-12-11 13:46 ` Chao Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).