Re: [PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."
On 2018/11/7 下午5:09, Su Yanjun wrote:
>
>
> On 10/24/2018 8:29 AM, Qu Wenruo wrote:
>>
>> On 2018/10/23 下午5:41, Su Yue wrote:
>>> From: Su Yanjun
>>>
>>> The reason for revert is that according to the existing situation, the
>>> probability of problem in the extent tree is higher than in the fs Tree.
>>> So this feature should be removed.
>>>
>> The same problem as previous patch.
>>
>> We need an example on how such repair could lead to further corruption.
>>
>> Thanks,
>> Qu
>
> Firstly In record_orphan_data_extents() function:
>
> key.objectid = dback->owner;
> key.type = BTRFS_EXTENT_DATA_KEY;
> key.offset = dback->offset;//
> ret = btrfs_search_slot(NULL, dest_root, , , 0, 0);
>
> 'dback->offset' is wrong use here.
>
> Secondly when disk bytenr in file extent item is wrong, the file extent
> data is always inserted to fs tree which will lead to fs check failed.
What about an example like:
Before:
( EXTENT_DATA )
#And some description about missing INODE_ITEM
After repair:
( EXTENT_DATA )
# Then describe what's wrong.
IMHO, with a format similar to inspect tree, along with some
description, it will be much easier for us to understand why the old
repair is causing more problem.
Thanks,
Qu
>
> Thanks,
> Su
>
>>> Signed-off-by: Su Yanjun
>>> ---
>>> check/main.c | 120 +-
>>> check/mode-original.h | 17 --
>>> ctree.h | 10
>>> disk-io.c | 1 -
>>> 4 files changed, 1 insertion(+), 147 deletions(-)
>>>
>>> diff --git a/check/main.c b/check/main.c
>>> index 268de5dd5f26..bd1f322e0f12 100644
>>> --- a/check/main.c
>>> +++ b/check/main.c
>>> @@ -511,22 +511,6 @@ cleanup:
>>> return ERR_PTR(ret);
>>> }
>>> -static void print_orphan_data_extents(struct list_head
>>> *orphan_extents,
>>> - u64 objectid)
>>> -{
>>> - struct orphan_data_extent *orphan;
>>> -
>>> - if (list_empty(orphan_extents))
>>> - return;
>>> - printf("The following data extent is lost in tree %llu:\n",
>>> - objectid);
>>> - list_for_each_entry(orphan, orphan_extents, list) {
>>> - printf("\tinode: %llu, offset:%llu, disk_bytenr: %llu,
>>> disk_len: %llu\n",
>>> - orphan->objectid, orphan->offset, orphan->disk_bytenr,
>>> - orphan->disk_len);
>>> - }
>>> -}
>>> -
>>> static void print_inode_error(struct btrfs_root *root, struct
>>> inode_record *rec)
>>> {
>>> u64 root_objectid = root->root_key.objectid;
>>> @@ -578,9 +562,6 @@ static void print_inode_error(struct btrfs_root
>>> *root, struct inode_record *rec)
>>> if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
>>> fprintf(stderr, ", invalid inline ram bytes");
>>> fprintf(stderr, "\n");
>>> - /* Print the orphan extents if needed */
>>> - if (errors & I_ERR_FILE_EXTENT_ORPHAN)
>>> - print_orphan_data_extents(>orphan_extents,
>>> root->objectid);
>>> /* Print the holes if needed */
>>> if (errors & I_ERR_FILE_EXTENT_DISCOUNT) {
>>> @@ -683,18 +664,6 @@ static struct inode_record *get_inode_rec(struct
>>> cache_tree *inode_cache,
>>> return rec;
>>> }
>>> -static void free_orphan_data_extents(struct list_head
>>> *orphan_extents)
>>> -{
>>> - struct orphan_data_extent *orphan;
>>> -
>>> - while (!list_empty(orphan_extents)) {
>>> - orphan = list_entry(orphan_extents->next,
>>> - struct orphan_data_extent, list);
>>> - list_del(>list);
>>> - free(orphan);
>>> - }
>>> -}
>>> -
>>> static void free_inode_rec(struct inode_record *rec)
>>> {
>>> struct inode_backref *backref;
>>> @@ -707,7 +676,6 @@ static void free_inode_rec(struct inode_record *rec)
>>> list_del(>list);
>>> free(backref);
>>> }
>>> - free_orphan_data_extents(>orphan_extents);
>>> free_file_extent_holes(>holes);
>>> free(rec);
>>> }
>>> @@ -3286,7 +3254,6 @@ skip_walking:
>>> free_corrupt_blocks_tree(_blocks);
>>> root->fs_info->corrupt_blocks = NULL;
>>> - free_orphan_data_extents(>orphan_data_extents);
>>> return ret;
>>> }
>>> @@ -7137,88 +7104,6 @@ static int find_possible_backrefs(struct
>>> btrfs_fs_info *info,
>>> return 0;
>>> }
>>> -/*
>>> - * Record orphan data ref into corresponding root.
>>> - *
>>> - * Return 0 if the extent item contains data ref and recorded.
>>> - * Return 1 if the extent item contains no useful data ref
>>> - * On that case, it may contains only shared_dataref or metadata
>>> backref
>>> - * or the file extent exists(this should be handled by the extent
>>> bytenr
>>> - * recovery routine)
>>> - * Return <0 if something goes wrong.
>>> - */
>>> -static int record_orphan_data_extents(struct btrfs_fs_info *fs_info,
>>> - struct extent_record *rec)
>>> -{
>>> - struct btrfs_key key;
>>> - struct btrfs_root *dest_root;
Re: [PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."
On 10/24/2018 8:29 AM, Qu Wenruo wrote:
On 2018/10/23 下午5:41, Su Yue wrote:
From: Su Yanjun
The reason for revert is that according to the existing situation, the
probability of problem in the extent tree is higher than in the fs Tree.
So this feature should be removed.
The same problem as previous patch.
We need an example on how such repair could lead to further corruption.
Thanks,
Qu
Firstly In record_orphan_data_extents() function:
key.objectid = dback->owner;
key.type = BTRFS_EXTENT_DATA_KEY;
key.offset = dback->offset;//
ret = btrfs_search_slot(NULL, dest_root, , , 0, 0);
'dback->offset' is wrong use here.
Secondly when disk bytenr in file extent item is wrong, the file extent data is
always inserted to fs tree which will lead to fs check failed.
Thanks,
Su
Signed-off-by: Su Yanjun
---
check/main.c | 120 +-
check/mode-original.h | 17 --
ctree.h | 10
disk-io.c | 1 -
4 files changed, 1 insertion(+), 147 deletions(-)
diff --git a/check/main.c b/check/main.c
index 268de5dd5f26..bd1f322e0f12 100644
--- a/check/main.c
+++ b/check/main.c
@@ -511,22 +511,6 @@ cleanup:
return ERR_PTR(ret);
}
-static void print_orphan_data_extents(struct list_head *orphan_extents,
- u64 objectid)
-{
- struct orphan_data_extent *orphan;
-
- if (list_empty(orphan_extents))
- return;
- printf("The following data extent is lost in tree %llu:\n",
- objectid);
- list_for_each_entry(orphan, orphan_extents, list) {
- printf("\tinode: %llu, offset:%llu, disk_bytenr: %llu, disk_len:
%llu\n",
- orphan->objectid, orphan->offset, orphan->disk_bytenr,
- orphan->disk_len);
- }
-}
-
static void print_inode_error(struct btrfs_root *root, struct inode_record
*rec)
{
u64 root_objectid = root->root_key.objectid;
@@ -578,9 +562,6 @@ static void print_inode_error(struct btrfs_root *root,
struct inode_record *rec)
if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
fprintf(stderr, ", invalid inline ram bytes");
fprintf(stderr, "\n");
- /* Print the orphan extents if needed */
- if (errors & I_ERR_FILE_EXTENT_ORPHAN)
- print_orphan_data_extents(>orphan_extents, root->objectid);
/* Print the holes if needed */
if (errors & I_ERR_FILE_EXTENT_DISCOUNT) {
@@ -683,18 +664,6 @@ static struct inode_record *get_inode_rec(struct
cache_tree *inode_cache,
return rec;
}
-static void free_orphan_data_extents(struct list_head *orphan_extents)
-{
- struct orphan_data_extent *orphan;
-
- while (!list_empty(orphan_extents)) {
- orphan = list_entry(orphan_extents->next,
- struct orphan_data_extent, list);
- list_del(>list);
- free(orphan);
- }
-}
-
static void free_inode_rec(struct inode_record *rec)
{
struct inode_backref *backref;
@@ -707,7 +676,6 @@ static void free_inode_rec(struct inode_record *rec)
list_del(>list);
free(backref);
}
- free_orphan_data_extents(>orphan_extents);
free_file_extent_holes(>holes);
free(rec);
}
@@ -3286,7 +3254,6 @@ skip_walking:
free_corrupt_blocks_tree(_blocks);
root->fs_info->corrupt_blocks = NULL;
- free_orphan_data_extents(>orphan_data_extents);
return ret;
}
@@ -7137,88 +7104,6 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
return 0;
}
-/*
- * Record orphan data ref into corresponding root.
- *
- * Return 0 if the extent item contains data ref and recorded.
- * Return 1 if the extent item contains no useful data ref
- * On that case, it may contains only shared_dataref or metadata backref
- * or the file extent exists(this should be handled by the extent bytenr
- * recovery routine)
- * Return <0 if something goes wrong.
- */
-static int record_orphan_data_extents(struct btrfs_fs_info *fs_info,
- struct extent_record *rec)
-{
- struct btrfs_key key;
- struct btrfs_root *dest_root;
- struct extent_backref *back, *tmp;
- struct data_backref *dback;
- struct orphan_data_extent *orphan;
- struct btrfs_path path;
- int recorded_data_ref = 0;
- int ret = 0;
-
- if (rec->metadata)
- return 1;
- btrfs_init_path();
- rbtree_postorder_for_each_entry_safe(back, tmp,
->backref_tree, node) {
- if (back->full_backref || !back->is_data ||
- !back->found_extent_tree)
- continue;
- dback = to_data_backref(back);
- if (dback->found_ref)
-
Re: [PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."
On 2018/10/23 下午5:41, Su Yue wrote:
> From: Su Yanjun
>
> The reason for revert is that according to the existing situation, the
> probability of problem in the extent tree is higher than in the fs Tree.
> So this feature should be removed.
>
The same problem as previous patch.
We need an example on how such repair could lead to further corruption.
Thanks,
Qu
> Signed-off-by: Su Yanjun
> ---
> check/main.c | 120 +-
> check/mode-original.h | 17 --
> ctree.h | 10
> disk-io.c | 1 -
> 4 files changed, 1 insertion(+), 147 deletions(-)
>
> diff --git a/check/main.c b/check/main.c
> index 268de5dd5f26..bd1f322e0f12 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -511,22 +511,6 @@ cleanup:
> return ERR_PTR(ret);
> }
>
> -static void print_orphan_data_extents(struct list_head *orphan_extents,
> - u64 objectid)
> -{
> - struct orphan_data_extent *orphan;
> -
> - if (list_empty(orphan_extents))
> - return;
> - printf("The following data extent is lost in tree %llu:\n",
> -objectid);
> - list_for_each_entry(orphan, orphan_extents, list) {
> - printf("\tinode: %llu, offset:%llu, disk_bytenr: %llu,
> disk_len: %llu\n",
> -orphan->objectid, orphan->offset, orphan->disk_bytenr,
> -orphan->disk_len);
> - }
> -}
> -
> static void print_inode_error(struct btrfs_root *root, struct inode_record
> *rec)
> {
> u64 root_objectid = root->root_key.objectid;
> @@ -578,9 +562,6 @@ static void print_inode_error(struct btrfs_root *root,
> struct inode_record *rec)
> if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
> fprintf(stderr, ", invalid inline ram bytes");
> fprintf(stderr, "\n");
> - /* Print the orphan extents if needed */
> - if (errors & I_ERR_FILE_EXTENT_ORPHAN)
> - print_orphan_data_extents(>orphan_extents, root->objectid);
>
> /* Print the holes if needed */
> if (errors & I_ERR_FILE_EXTENT_DISCOUNT) {
> @@ -683,18 +664,6 @@ static struct inode_record *get_inode_rec(struct
> cache_tree *inode_cache,
> return rec;
> }
>
> -static void free_orphan_data_extents(struct list_head *orphan_extents)
> -{
> - struct orphan_data_extent *orphan;
> -
> - while (!list_empty(orphan_extents)) {
> - orphan = list_entry(orphan_extents->next,
> - struct orphan_data_extent, list);
> - list_del(>list);
> - free(orphan);
> - }
> -}
> -
> static void free_inode_rec(struct inode_record *rec)
> {
> struct inode_backref *backref;
> @@ -707,7 +676,6 @@ static void free_inode_rec(struct inode_record *rec)
> list_del(>list);
> free(backref);
> }
> - free_orphan_data_extents(>orphan_extents);
> free_file_extent_holes(>holes);
> free(rec);
> }
> @@ -3286,7 +3254,6 @@ skip_walking:
>
> free_corrupt_blocks_tree(_blocks);
> root->fs_info->corrupt_blocks = NULL;
> - free_orphan_data_extents(>orphan_data_extents);
> return ret;
> }
>
> @@ -7137,88 +7104,6 @@ static int find_possible_backrefs(struct btrfs_fs_info
> *info,
> return 0;
> }
>
> -/*
> - * Record orphan data ref into corresponding root.
> - *
> - * Return 0 if the extent item contains data ref and recorded.
> - * Return 1 if the extent item contains no useful data ref
> - * On that case, it may contains only shared_dataref or metadata backref
> - * or the file extent exists(this should be handled by the extent bytenr
> - * recovery routine)
> - * Return <0 if something goes wrong.
> - */
> -static int record_orphan_data_extents(struct btrfs_fs_info *fs_info,
> - struct extent_record *rec)
> -{
> - struct btrfs_key key;
> - struct btrfs_root *dest_root;
> - struct extent_backref *back, *tmp;
> - struct data_backref *dback;
> - struct orphan_data_extent *orphan;
> - struct btrfs_path path;
> - int recorded_data_ref = 0;
> - int ret = 0;
> -
> - if (rec->metadata)
> - return 1;
> - btrfs_init_path();
> - rbtree_postorder_for_each_entry_safe(back, tmp,
> - >backref_tree, node) {
> - if (back->full_backref || !back->is_data ||
> - !back->found_extent_tree)
> - continue;
> - dback = to_data_backref(back);
> - if (dback->found_ref)
> - continue;
> - key.objectid = dback->root;
> - key.type = BTRFS_ROOT_ITEM_KEY;
> - key.offset = (u64)-1;
> -
> - dest_root = btrfs_read_fs_root(fs_info, );
> -
> - /* For non-exist root we just skip it */
> - if (IS_ERR(dest_root) || !dest_root)
> - continue;
> -
>
[PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."
From: Su Yanjun
The reason for revert is that according to the existing situation, the
probability of problem in the extent tree is higher than in the fs Tree.
So this feature should be removed.
Signed-off-by: Su Yanjun
---
check/main.c | 120 +-
check/mode-original.h | 17 --
ctree.h | 10
disk-io.c | 1 -
4 files changed, 1 insertion(+), 147 deletions(-)
diff --git a/check/main.c b/check/main.c
index 268de5dd5f26..bd1f322e0f12 100644
--- a/check/main.c
+++ b/check/main.c
@@ -511,22 +511,6 @@ cleanup:
return ERR_PTR(ret);
}
-static void print_orphan_data_extents(struct list_head *orphan_extents,
- u64 objectid)
-{
- struct orphan_data_extent *orphan;
-
- if (list_empty(orphan_extents))
- return;
- printf("The following data extent is lost in tree %llu:\n",
- objectid);
- list_for_each_entry(orphan, orphan_extents, list) {
- printf("\tinode: %llu, offset:%llu, disk_bytenr: %llu,
disk_len: %llu\n",
- orphan->objectid, orphan->offset, orphan->disk_bytenr,
- orphan->disk_len);
- }
-}
-
static void print_inode_error(struct btrfs_root *root, struct inode_record
*rec)
{
u64 root_objectid = root->root_key.objectid;
@@ -578,9 +562,6 @@ static void print_inode_error(struct btrfs_root *root,
struct inode_record *rec)
if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
fprintf(stderr, ", invalid inline ram bytes");
fprintf(stderr, "\n");
- /* Print the orphan extents if needed */
- if (errors & I_ERR_FILE_EXTENT_ORPHAN)
- print_orphan_data_extents(>orphan_extents, root->objectid);
/* Print the holes if needed */
if (errors & I_ERR_FILE_EXTENT_DISCOUNT) {
@@ -683,18 +664,6 @@ static struct inode_record *get_inode_rec(struct
cache_tree *inode_cache,
return rec;
}
-static void free_orphan_data_extents(struct list_head *orphan_extents)
-{
- struct orphan_data_extent *orphan;
-
- while (!list_empty(orphan_extents)) {
- orphan = list_entry(orphan_extents->next,
- struct orphan_data_extent, list);
- list_del(>list);
- free(orphan);
- }
-}
-
static void free_inode_rec(struct inode_record *rec)
{
struct inode_backref *backref;
@@ -707,7 +676,6 @@ static void free_inode_rec(struct inode_record *rec)
list_del(>list);
free(backref);
}
- free_orphan_data_extents(>orphan_extents);
free_file_extent_holes(>holes);
free(rec);
}
@@ -3286,7 +3254,6 @@ skip_walking:
free_corrupt_blocks_tree(_blocks);
root->fs_info->corrupt_blocks = NULL;
- free_orphan_data_extents(>orphan_data_extents);
return ret;
}
@@ -7137,88 +7104,6 @@ static int find_possible_backrefs(struct btrfs_fs_info
*info,
return 0;
}
-/*
- * Record orphan data ref into corresponding root.
- *
- * Return 0 if the extent item contains data ref and recorded.
- * Return 1 if the extent item contains no useful data ref
- * On that case, it may contains only shared_dataref or metadata backref
- * or the file extent exists(this should be handled by the extent bytenr
- * recovery routine)
- * Return <0 if something goes wrong.
- */
-static int record_orphan_data_extents(struct btrfs_fs_info *fs_info,
- struct extent_record *rec)
-{
- struct btrfs_key key;
- struct btrfs_root *dest_root;
- struct extent_backref *back, *tmp;
- struct data_backref *dback;
- struct orphan_data_extent *orphan;
- struct btrfs_path path;
- int recorded_data_ref = 0;
- int ret = 0;
-
- if (rec->metadata)
- return 1;
- btrfs_init_path();
- rbtree_postorder_for_each_entry_safe(back, tmp,
->backref_tree, node) {
- if (back->full_backref || !back->is_data ||
- !back->found_extent_tree)
- continue;
- dback = to_data_backref(back);
- if (dback->found_ref)
- continue;
- key.objectid = dback->root;
- key.type = BTRFS_ROOT_ITEM_KEY;
- key.offset = (u64)-1;
-
- dest_root = btrfs_read_fs_root(fs_info, );
-
- /* For non-exist root we just skip it */
- if (IS_ERR(dest_root) || !dest_root)
- continue;
-
- key.objectid = dback->owner;
- key.type = BTRFS_EXTENT_DATA_KEY;
- key.offset = dback->offset;
-
- ret = btrfs_search_slot(NULL, dest_root, , , 0, 0);
- btrfs_release_path();
- /*
-* For ret < 0, it's
