On 2018/11/7 下午5:09, Su Yanjun <suyj.fnst@xxxxxxxxxxxxxx> wrote:
>
>
> On 10/24/2018 8:29 AM, Qu Wenruo wrote:
>>
>> On 2018/10/23 下午5:41, Su Yue wrote:
>>> From: Su Yanjun <suyj.fnst@xxxxxxxxxxxxxx>
>>>
>>> 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, &key, &path, 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:
(XXXX EXTENT_DATA XXXX)
#And some description about missing INODE_ITEM
After repair:
(XXXX EXTENT_DATA XXXX)
# 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 <suyj.fnst@xxxxxxxxxxxxxx>
>>> ---
>>> 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(&rec->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(&orphan->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(&backref->list);
>>> free(backref);
>>> }
>>> - free_orphan_data_extents(&rec->orphan_extents);
>>> free_file_extent_holes(&rec->holes);
>>> free(rec);
>>> }
>>> @@ -3286,7 +3254,6 @@ skip_walking:
>>> free_corrupt_blocks_tree(&corrupt_blocks);
>>> root->fs_info->corrupt_blocks = NULL;
>>> - free_orphan_data_extents(&root->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(&path);
>>> - rbtree_postorder_for_each_entry_safe(back, tmp,
>>> - &rec->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, &key);
>>> -
>>> - /* 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, &key, &path, 0, 0);
>>> - btrfs_release_path(&path);
>>> - /*
>>> - * For ret < 0, it's OK since the fs-tree may be corrupted,
>>> - * we need to record it for inode/file extent rebuild.
>>> - * For ret > 0, we record it only for file extent rebuild.
>>> - * For ret == 0, the file extent exists but only bytenr
>>> - * mismatch, let the original bytenr fix routine to handle,
>>> - * don't record it.
>>> - */
>>> - if (ret == 0)
>>> - continue;
>>> - ret = 0;
>>> - orphan = malloc(sizeof(*orphan));
>>> - if (!orphan) {
>>> - ret = -ENOMEM;
>>> - goto out;
>>> - }
>>> - INIT_LIST_HEAD(&orphan->list);
>>> - orphan->root = dback->root;
>>> - orphan->objectid = dback->owner;
>>> - orphan->offset = dback->offset;
>>> - orphan->disk_bytenr = rec->cache.start;
>>> - orphan->disk_len = rec->cache.size;
>>> - list_add(&dest_root->orphan_data_extents, &orphan->list);
>>> - recorded_data_ref = 1;
>>> - }
>>> -out:
>>> - btrfs_release_path(&path);
>>> - if (!ret)
>>> - return !recorded_data_ref;
>>> - else
>>> - return ret;
>>> -}
>>> -
>>> /*
>>> * when an incorrect extent item is found, this will delete
>>> * all of the existing entries for it and recreate them
>>> @@ -7560,10 +7445,7 @@ static int check_extent_refs(struct btrfs_root
>>> *root,
>>> fprintf(stderr, "extent item %llu, found %llu\n",
>>> (unsigned long long)rec->extent_item_refs,
>>> (unsigned long long)rec->refs);
>>> - ret = record_orphan_data_extents(root->fs_info, rec);
>>> - if (ret < 0)
>>> - goto repair_abort;
>>> - fix = ret;
>>> + fix = 1;
>>> cur_err = 1;
>>> }
>>> if (all_backpointers_checked(rec, 1)) {
>>> diff --git a/check/mode-original.h b/check/mode-original.h
>>> index ec2842e0b3be..ed995931fcd5 100644
>>> --- a/check/mode-original.h
>>> +++ b/check/mode-original.h
>>> @@ -57,21 +57,6 @@ static inline struct data_backref*
>>> to_data_backref(struct extent_backref *back)
>>> return container_of(back, struct data_backref, node);
>>> }
>>> -/*
>>> - * Much like data_backref, just removed the undetermined members
>>> - * and change it to use list_head.
>>> - * During extent scan, it is stored in root->orphan_data_extent.
>>> - * During fs tree scan, it is then moved to
>>> inode_rec->orphan_data_extents.
>>> - */
>>> -struct orphan_data_extent {
>>> - struct list_head list;
>>> - u64 root;
>>> - u64 objectid;
>>> - u64 offset;
>>> - u64 disk_bytenr;
>>> - u64 disk_len;
>>> -};
>>> -
>>> struct tree_backref {
>>> struct extent_backref node;
>>> union {
>>> @@ -184,7 +169,6 @@ struct file_extent_hole {
>>> #define I_ERR_ODD_CSUM_ITEM (1 << 11)
>>> #define I_ERR_SOME_CSUM_MISSING (1 << 12)
>>> #define I_ERR_LINK_COUNT_WRONG (1 << 13)
>>> -#define I_ERR_FILE_EXTENT_ORPHAN (1 << 14)
>>> #define I_ERR_FILE_EXTENT_TOO_LARGE (1 << 15)
>>> #define I_ERR_ODD_INODE_FLAGS (1 << 16)
>>> #define I_ERR_INLINE_RAM_BYTES_WRONG (1 << 17)
>>> @@ -212,7 +196,6 @@ struct inode_record {
>>> u64 extent_start;
>>> u64 extent_end;
>>> struct rb_root holes;
>>> - struct list_head orphan_extents;
>>> u32 refs;
>>> };
>>> diff --git a/ctree.h b/ctree.h
>>> index 2a2437070ef9..2e0896390434 100644
>>> --- a/ctree.h
>>> +++ b/ctree.h
>>> @@ -1177,16 +1177,6 @@ struct btrfs_root {
>>> u32 type;
>>> u64 last_inode_alloc;
>>> - /*
>>> - * Record orphan data extent ref
>>> - *
>>> - * TODO: Don't restore things in btrfs_root.
>>> - * Directly record it into inode_record, which needs a lot of
>>> - * infrastructure change to allow cooperation between extent
>>> - * and fs tree scan.
>>> - */
>>> - struct list_head orphan_data_extents;
>>> -
>>> /* the dirty list is only used by non-reference counted roots */
>>> struct list_head dirty_list;
>>> struct rb_node rb_node;
>>> diff --git a/disk-io.c b/disk-io.c
>>> index 4bc2e77d438a..992f4b870e9f 100644
>>> --- a/disk-io.c
>>> +++ b/disk-io.c
>>> @@ -480,7 +480,6 @@ void btrfs_setup_root(struct btrfs_root *root,
>>> struct btrfs_fs_info *fs_info,
>>> root->last_inode_alloc = 0;
>>> INIT_LIST_HEAD(&root->dirty_list);
>>> - INIT_LIST_HEAD(&root->orphan_data_extents);
>>> memset(&root->root_key, 0, sizeof(root->root_key));
>>> memset(&root->root_item, 0, sizeof(root->root_item));
>>> root->root_key.objectid = objectid;
>>>
>>
>
>
>
