diff mbox series

[08/13] btrfs-progs: Revert "btrfs-progs: Add repair and report function for orphan file extent."

Message ID 20181023094147.7906-9-suy.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: fixes of file extent in original and lowmem check | expand

Commit Message

Su Yue Oct. 23, 2018, 9:41 a.m. UTC 
From: Su Yanjun <suyj.fnst@cn.fujitsu.com>

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 <suyj.fnst@cn.fujitsu.com>
---
 check/main.c | 103 +--------------------------------------------------
 1 file changed, 1 insertion(+), 102 deletions(-)

Comments

Qu Wenruo Oct. 24, 2018, 12:28 a.m. UTC | #1 
On 2018/10/23 下午5:41, Su Yue wrote:
> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> 
> 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.

In fact we should find a place to address the priority when we
check/repair fs trees.

My initial priorities are:

1) Vital trees:
   Chunk, root

2) Data trees
   Fs, file trees

3) Supporting trees
   Extent, csum,

4) Ready to die trees.
   UUID, reloc, log

In that priority, if we found something mismatch in fs and extent tree,
we should trust fs instead of extent tree.


Any good idea on where to place such principle, David?



> So this feature should be removed.
> 
> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>

Looks good to me, although the commit message needs extra explanation.

An example is needed to validate the revert.

Thanks,
Qu

> ---
>  check/main.c | 103 +--------------------------------------------------
>  1 file changed, 1 insertion(+), 102 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index 9c0e75c19ebc..268de5dd5f26 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -460,8 +460,6 @@ static struct inode_record *clone_inode_rec(struct inode_record *orig_rec)
>  	struct inode_backref *backref;
>  	struct inode_backref *orig;
>  	struct inode_backref *tmp;
> -	struct orphan_data_extent *src_orphan;
> -	struct orphan_data_extent *dst_orphan;
>  	struct rb_node *rb;
>  	size_t size;
>  	int ret;
> @@ -472,7 +470,6 @@ static struct inode_record *clone_inode_rec(struct inode_record *orig_rec)
>  	memcpy(rec, orig_rec, sizeof(*rec));
>  	rec->refs = 1;
>  	INIT_LIST_HEAD(&rec->backrefs);
> -	INIT_LIST_HEAD(&rec->orphan_extents);
>  	rec->holes = RB_ROOT;
>  
>  	list_for_each_entry(orig, &orig_rec->backrefs, list) {
> @@ -485,15 +482,7 @@ static struct inode_record *clone_inode_rec(struct inode_record *orig_rec)
>  		memcpy(backref, orig, size);
>  		list_add_tail(&backref->list, &rec->backrefs);
>  	}
> -	list_for_each_entry(src_orphan, &orig_rec->orphan_extents, list) {
> -		dst_orphan = malloc(sizeof(*dst_orphan));
> -		if (!dst_orphan) {
> -			ret = -ENOMEM;
> -			goto cleanup;
> -		}
> -		memcpy(dst_orphan, src_orphan, sizeof(*src_orphan));
> -		list_add_tail(&dst_orphan->list, &rec->orphan_extents);
> -	}
> +
>  	ret = copy_file_extent_holes(&rec->holes, &orig_rec->holes);
>  	if (ret < 0)
>  		goto cleanup_rb;
> @@ -517,12 +506,6 @@ cleanup:
>  			free(orig);
>  		}
>  
> -	if (!list_empty(&rec->orphan_extents))
> -		list_for_each_entry_safe(orig, tmp, &rec->orphan_extents, list) {
> -			list_del(&orig->list);
> -			free(orig);
> -		}
> -
>  	free(rec);
>  
>  	return ERR_PTR(ret);
> @@ -590,8 +573,6 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
>  		fprintf(stderr, ", some csum missing");
>  	if (errors & I_ERR_LINK_COUNT_WRONG)
>  		fprintf(stderr, ", link count wrong");
> -	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
> -		fprintf(stderr, ", orphan file extent");
>  	if (errors & I_ERR_ODD_INODE_FLAGS)
>  		fprintf(stderr, ", odd inode flags");
>  	if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
> @@ -681,7 +662,6 @@ static struct inode_record *get_inode_rec(struct cache_tree *inode_cache,
>  		rec->extent_start = (u64)-1;
>  		rec->refs = 1;
>  		INIT_LIST_HEAD(&rec->backrefs);
> -		INIT_LIST_HEAD(&rec->orphan_extents);
>  		rec->holes = RB_ROOT;
>  
>  		node = malloc(sizeof(*node));
> @@ -2420,9 +2400,6 @@ static int repair_inode_no_item(struct btrfs_trans_handle *trans,
>  		} else if (rec->found_dir_item) {
>  			type_recovered = 1;
>  			filetype = BTRFS_FT_DIR;
> -		} else if (!list_empty(&rec->orphan_extents)) {
> -			type_recovered = 1;
> -			filetype = BTRFS_FT_REG_FILE;
>  		} else{
>  			printf("Can't determine the filetype for inode %llu, assume it is a normal file\n",
>  			       rec->ino);
> @@ -2453,67 +2430,6 @@ out:
>  	return ret;
>  }
>  
> -static int repair_inode_orphan_extent(struct btrfs_trans_handle *trans,
> -				      struct btrfs_root *root,
> -				      struct btrfs_path *path,
> -				      struct inode_record *rec)
> -{
> -	struct orphan_data_extent *orphan;
> -	struct orphan_data_extent *tmp;
> -	int ret = 0;
> -
> -	list_for_each_entry_safe(orphan, tmp, &rec->orphan_extents, list) {
> -		/*
> -		 * Check for conflicting file extents
> -		 *
> -		 * Here we don't know whether the extents is compressed or not,
> -		 * so we can only assume it not compressed nor data offset,
> -		 * and use its disk_len as extent length.
> -		 */
> -		ret = btrfs_get_extent(NULL, root, path, orphan->objectid,
> -				       orphan->offset, orphan->disk_len, 0);
> -		btrfs_release_path(path);
> -		if (ret < 0)
> -			goto out;
> -		if (!ret) {
> -			fprintf(stderr,
> -				"orphan extent (%llu, %llu) conflicts, delete the orphan\n",
> -				orphan->disk_bytenr, orphan->disk_len);
> -			ret = btrfs_free_extent(trans,
> -					root->fs_info->extent_root,
> -					orphan->disk_bytenr, orphan->disk_len,
> -					0, root->objectid, orphan->objectid,
> -					orphan->offset);
> -			if (ret < 0)
> -				goto out;
> -		}
> -		ret = btrfs_insert_file_extent(trans, root, orphan->objectid,
> -				orphan->offset, orphan->disk_bytenr,
> -				orphan->disk_len, orphan->disk_len);
> -		if (ret < 0)
> -			goto out;
> -
> -		/* Update file size info */
> -		rec->found_size += orphan->disk_len;
> -		if (rec->found_size == rec->nbytes)
> -			rec->errors &= ~I_ERR_FILE_NBYTES_WRONG;
> -
> -		/* Update the file extent hole info too */
> -		ret = del_file_extent_hole(&rec->holes, orphan->offset,
> -					   orphan->disk_len);
> -		if (ret < 0)
> -			goto out;
> -		if (RB_EMPTY_ROOT(&rec->holes))
> -			rec->errors &= ~I_ERR_FILE_EXTENT_DISCOUNT;
> -
> -		list_del(&orphan->list);
> -		free(orphan);
> -	}
> -	rec->errors &= ~I_ERR_FILE_EXTENT_ORPHAN;
> -out:
> -	return ret;
> -}
> -
>  static int repair_inode_discount_extent(struct btrfs_trans_handle *trans,
>  					struct btrfs_root *root,
>  					struct btrfs_path *path,
> @@ -2600,7 +2516,6 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  			     I_ERR_NO_ORPHAN_ITEM |
>  			     I_ERR_LINK_COUNT_WRONG |
>  			     I_ERR_NO_INODE_ITEM |
> -			     I_ERR_FILE_EXTENT_ORPHAN |
>  			     I_ERR_FILE_EXTENT_DISCOUNT |
>  			     I_ERR_FILE_NBYTES_WRONG |
>  			     I_ERR_INLINE_RAM_BYTES_WRONG)))
> @@ -2620,8 +2535,6 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  	btrfs_init_path(&path);
>  	if (rec->errors & I_ERR_NO_INODE_ITEM)
>  		ret = repair_inode_no_item(trans, root, &path, rec);
> -	if (!ret && rec->errors & I_ERR_FILE_EXTENT_ORPHAN)
> -		ret = repair_inode_orphan_extent(trans, root, &path, rec);
>  	if (!ret && rec->errors & I_ERR_FILE_EXTENT_DISCOUNT)
>  		ret = repair_inode_discount_extent(trans, root, &path, rec);
>  	if (!ret && rec->errors & I_ERR_DIR_ISIZE_WRONG)
> @@ -3245,8 +3158,6 @@ static int check_fs_root(struct btrfs_root *root,
>  	struct root_record *rec;
>  	struct btrfs_root_item *root_item = &root->root_item;
>  	struct cache_tree corrupt_blocks;
> -	struct orphan_data_extent *orphan;
> -	struct orphan_data_extent *tmp;
>  	enum btrfs_tree_block_status status;
>  	struct node_refs nrefs;
>  
> @@ -3272,18 +3183,6 @@ static int check_fs_root(struct btrfs_root *root,
>  	cache_tree_init(&root_node.inode_cache);
>  	memset(&nrefs, 0, sizeof(nrefs));
>  
> -	/* Move the orphan extent record to corresponding inode_record */
> -	list_for_each_entry_safe(orphan, tmp,
> -				 &root->orphan_data_extents, list) {
> -		struct inode_record *inode;
> -
> -		inode = get_inode_rec(&root_node.inode_cache, orphan->objectid,
> -				      1);
> -		BUG_ON(IS_ERR(inode));
> -		inode->errors |= I_ERR_FILE_EXTENT_ORPHAN;
> -		list_move(&orphan->list, &inode->orphan_extents);
> -	}
> -
>  	level = btrfs_header_level(root->node);
>  	memset(wc->nodes, 0, sizeof(wc->nodes));
>  	wc->nodes[level] = &root_node;
>
diff mbox series

Patch

diff --git a/check/main.c b/check/main.c
index 9c0e75c19ebc..268de5dd5f26 100644
--- a/check/main.c
+++ b/check/main.c
@@ -460,8 +460,6 @@  static struct inode_record *clone_inode_rec(struct inode_record *orig_rec)
 	struct inode_backref *backref;
 	struct inode_backref *orig;
 	struct inode_backref *tmp;
-	struct orphan_data_extent *src_orphan;
-	struct orphan_data_extent *dst_orphan;
 	struct rb_node *rb;
 	size_t size;
 	int ret;
@@ -472,7 +470,6 @@  static struct inode_record *clone_inode_rec(struct inode_record *orig_rec)
 	memcpy(rec, orig_rec, sizeof(*rec));
 	rec->refs = 1;
 	INIT_LIST_HEAD(&rec->backrefs);
-	INIT_LIST_HEAD(&rec->orphan_extents);
 	rec->holes = RB_ROOT;
 
 	list_for_each_entry(orig, &orig_rec->backrefs, list) {
@@ -485,15 +482,7 @@  static struct inode_record *clone_inode_rec(struct inode_record *orig_rec)
 		memcpy(backref, orig, size);
 		list_add_tail(&backref->list, &rec->backrefs);
 	}
-	list_for_each_entry(src_orphan, &orig_rec->orphan_extents, list) {
-		dst_orphan = malloc(sizeof(*dst_orphan));
-		if (!dst_orphan) {
-			ret = -ENOMEM;
-			goto cleanup;
-		}
-		memcpy(dst_orphan, src_orphan, sizeof(*src_orphan));
-		list_add_tail(&dst_orphan->list, &rec->orphan_extents);
-	}
+
 	ret = copy_file_extent_holes(&rec->holes, &orig_rec->holes);
 	if (ret < 0)
 		goto cleanup_rb;
@@ -517,12 +506,6 @@  cleanup:
 			free(orig);
 		}
 
-	if (!list_empty(&rec->orphan_extents))
-		list_for_each_entry_safe(orig, tmp, &rec->orphan_extents, list) {
-			list_del(&orig->list);
-			free(orig);
-		}
-
 	free(rec);
 
 	return ERR_PTR(ret);
@@ -590,8 +573,6 @@  static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
 		fprintf(stderr, ", some csum missing");
 	if (errors & I_ERR_LINK_COUNT_WRONG)
 		fprintf(stderr, ", link count wrong");
-	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
-		fprintf(stderr, ", orphan file extent");
 	if (errors & I_ERR_ODD_INODE_FLAGS)
 		fprintf(stderr, ", odd inode flags");
 	if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
@@ -681,7 +662,6 @@  static struct inode_record *get_inode_rec(struct cache_tree *inode_cache,
 		rec->extent_start = (u64)-1;
 		rec->refs = 1;
 		INIT_LIST_HEAD(&rec->backrefs);
-		INIT_LIST_HEAD(&rec->orphan_extents);
 		rec->holes = RB_ROOT;
 
 		node = malloc(sizeof(*node));
@@ -2420,9 +2400,6 @@  static int repair_inode_no_item(struct btrfs_trans_handle *trans,
 		} else if (rec->found_dir_item) {
 			type_recovered = 1;
 			filetype = BTRFS_FT_DIR;
-		} else if (!list_empty(&rec->orphan_extents)) {
-			type_recovered = 1;
-			filetype = BTRFS_FT_REG_FILE;
 		} else{
 			printf("Can't determine the filetype for inode %llu, assume it is a normal file\n",
 			       rec->ino);
@@ -2453,67 +2430,6 @@  out:
 	return ret;
 }
 
-static int repair_inode_orphan_extent(struct btrfs_trans_handle *trans,
-				      struct btrfs_root *root,
-				      struct btrfs_path *path,
-				      struct inode_record *rec)
-{
-	struct orphan_data_extent *orphan;
-	struct orphan_data_extent *tmp;
-	int ret = 0;
-
-	list_for_each_entry_safe(orphan, tmp, &rec->orphan_extents, list) {
-		/*
-		 * Check for conflicting file extents
-		 *
-		 * Here we don't know whether the extents is compressed or not,
-		 * so we can only assume it not compressed nor data offset,
-		 * and use its disk_len as extent length.
-		 */
-		ret = btrfs_get_extent(NULL, root, path, orphan->objectid,
-				       orphan->offset, orphan->disk_len, 0);
-		btrfs_release_path(path);
-		if (ret < 0)
-			goto out;
-		if (!ret) {
-			fprintf(stderr,
-				"orphan extent (%llu, %llu) conflicts, delete the orphan\n",
-				orphan->disk_bytenr, orphan->disk_len);
-			ret = btrfs_free_extent(trans,
-					root->fs_info->extent_root,
-					orphan->disk_bytenr, orphan->disk_len,
-					0, root->objectid, orphan->objectid,
-					orphan->offset);
-			if (ret < 0)
-				goto out;
-		}
-		ret = btrfs_insert_file_extent(trans, root, orphan->objectid,
-				orphan->offset, orphan->disk_bytenr,
-				orphan->disk_len, orphan->disk_len);
-		if (ret < 0)
-			goto out;
-
-		/* Update file size info */
-		rec->found_size += orphan->disk_len;
-		if (rec->found_size == rec->nbytes)
-			rec->errors &= ~I_ERR_FILE_NBYTES_WRONG;
-
-		/* Update the file extent hole info too */
-		ret = del_file_extent_hole(&rec->holes, orphan->offset,
-					   orphan->disk_len);
-		if (ret < 0)
-			goto out;
-		if (RB_EMPTY_ROOT(&rec->holes))
-			rec->errors &= ~I_ERR_FILE_EXTENT_DISCOUNT;
-
-		list_del(&orphan->list);
-		free(orphan);
-	}
-	rec->errors &= ~I_ERR_FILE_EXTENT_ORPHAN;
-out:
-	return ret;
-}
-
 static int repair_inode_discount_extent(struct btrfs_trans_handle *trans,
 					struct btrfs_root *root,
 					struct btrfs_path *path,
@@ -2600,7 +2516,6 @@  static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 			     I_ERR_NO_ORPHAN_ITEM |
 			     I_ERR_LINK_COUNT_WRONG |
 			     I_ERR_NO_INODE_ITEM |
-			     I_ERR_FILE_EXTENT_ORPHAN |
 			     I_ERR_FILE_EXTENT_DISCOUNT |
 			     I_ERR_FILE_NBYTES_WRONG |
 			     I_ERR_INLINE_RAM_BYTES_WRONG)))
@@ -2620,8 +2535,6 @@  static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 	btrfs_init_path(&path);
 	if (rec->errors & I_ERR_NO_INODE_ITEM)
 		ret = repair_inode_no_item(trans, root, &path, rec);
-	if (!ret && rec->errors & I_ERR_FILE_EXTENT_ORPHAN)
-		ret = repair_inode_orphan_extent(trans, root, &path, rec);
 	if (!ret && rec->errors & I_ERR_FILE_EXTENT_DISCOUNT)
 		ret = repair_inode_discount_extent(trans, root, &path, rec);
 	if (!ret && rec->errors & I_ERR_DIR_ISIZE_WRONG)
@@ -3245,8 +3158,6 @@  static int check_fs_root(struct btrfs_root *root,
 	struct root_record *rec;
 	struct btrfs_root_item *root_item = &root->root_item;
 	struct cache_tree corrupt_blocks;
-	struct orphan_data_extent *orphan;
-	struct orphan_data_extent *tmp;
 	enum btrfs_tree_block_status status;
 	struct node_refs nrefs;
 
@@ -3272,18 +3183,6 @@  static int check_fs_root(struct btrfs_root *root,
 	cache_tree_init(&root_node.inode_cache);
 	memset(&nrefs, 0, sizeof(nrefs));
 
-	/* Move the orphan extent record to corresponding inode_record */
-	list_for_each_entry_safe(orphan, tmp,
-				 &root->orphan_data_extents, list) {
-		struct inode_record *inode;
-
-		inode = get_inode_rec(&root_node.inode_cache, orphan->objectid,
-				      1);
-		BUG_ON(IS_ERR(inode));
-		inode->errors |= I_ERR_FILE_EXTENT_ORPHAN;
-		list_move(&orphan->list, &inode->orphan_extents);
-	}
-
 	level = btrfs_header_level(root->node);
 	memset(wc->nodes, 0, sizeof(wc->nodes));
 	wc->nodes[level] = &root_node;