[PATCH 00/13] btrfs-progs: fixes of file extent in original and lowmem check

[PATCH 00/13] btrfs-progs: fixes of file extent in original and lowmem check

[Su Yue]

This patchset can be fetched from my repo:
https://github.com/Damenly/btrfs-progs/tree/file_extent_fixes
which is based on kdave/devel whose HEAD is:
commit 4f20c27ab33aab3efffe13cdae1b8837c821d0d7 (kdave/devel)
Author: Nikolay Borisov <nborisov@suse.com>
Date:   Fri Jun 15 07:13:50 2018 +0000

    btrfs-progs: tests: test for FST corruption detection/repair

This set fixes bugs of checking unaligned disk_bytenr extent_data and
orphan extent. Lowmem part patches can be used in common, so I put
this two fixes together.

For unaligned disk_bytenr extent_data, now original and lowmem check
both delete the corrupted part and punch a hole.

For orphan extent, lowmem mode has no change here.
Original mode discards function of detect and repair orphan extents
according extent items. Because as corruptions the community reported,
extent tree is more problematic than fs trees.

patch[1-2] fix minor bugs in lowmem repair.
patch[3] fixes false alert about repaired extent item in lowmem mode.
patch[4] fixes annoying alerts about gap in file extent in lowmem mode.
patch[5,6,7] enable check and repair unaligned disk_bytenr file extent
	     in lowmem mode.
patch[8,9] revert support for orphan extent in original mode.
patch[10] fixes bug about finding right backrefs in original mode.
patch[11] adds support to detect and delete unaligned file extents
	       in original.
patch[12] adds a test image which lacks of a file extent.
patch[13] enables lowmem repair of test case fsck-tests/001 which has
	  	  unaligned disk_bytenr file extent.


Lu Fengqi (2):
  btrfs-progs: lowmem: fix false alert about the existence of gaps in
    the check_file_extent
  btrfs-progs: tests: add case for inode lose one file extent

Su Yanjun (4):
  btrfs-progs: Revert "btrfs-progs: Add repair and report function for
    orphan file extent."
  btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to
    corresponding root."
  btrfs-progs: check: fix bug in find_possible_backrefs
  btrfs-progs: check: Delete file extent item with unaligned extent
    backref

Su Yue (7):
  btrfs-progs: lowmem: add argument path to punch_extent_hole()
  btrfs-progs: lowmem: move nbytes check before isize check
  btrfs-progs: lowmem: fix false alert if extent item has been repaired
  btrfs-progs: lowmem: check unaligned disk_bytenr for extent_data
  btrfs-progs: lowmem: rename delete_extent_tree_item() to delete_item()
  btrfs-progs: lowmem: delete unaligned bytes extent data under repair
  btrfs-progs: fsck-test: enable lowmem repair for case 001

 check/main.c                                  | 581 +++++++++++-------
 check/mode-lowmem.c                           | 272 ++++----
 check/mode-original.h                         |  30 +-
 ctree.h                                       |  10 +-
 disk-io.c                                     |   2 +-
 .../.lowmem_repairable                        |   0
 .../.lowmem_repairable                        |   0
 .../default_case.img                          | Bin 0 -> 3072 bytes
 8 files changed, 538 insertions(+), 357 deletions(-)
 create mode 100644 tests/fsck-tests/001-bad-file-extent-bytenr/.lowmem_repairable
 create mode 100644 tests/fsck-tests/038-missing-one-file-extent/.lowmem_repairable
 create mode 100644 tests/fsck-tests/038-missing-one-file-extent/default_case.img

-- 
2.19.1

[PATCH 01/13] btrfs-progs: lowmem: add argument path to punch_extent_hole()

[Su Yue]

Since repair will do CoW, the outer path may be invalid,
add an argument path to punch_extent_hole().
When punch_extent_hole() returns, path will still point to the item
before calling punch_extent_hole();

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 1bce44f5658a..c8e4f13d816f 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1710,15 +1710,20 @@ out:
 /*
  * Wrapper function of btrfs_punch_hole.
  *
+ * @path:	will point to the item while calling the function.
+ *
  * Returns 0 means success.
  * Returns not 0 means error.
  */
-static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
-			     u64 len)
+static int punch_extent_hole(struct btrfs_root *root, struct btrfs_path *path,
+			     u64 ino, u64 start, u64 len)
 {
 	struct btrfs_trans_handle *trans;
-	int ret = 0;
+	struct btrfs_key key;
+	int ret;
+	int ret2;
 
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
 	trans = btrfs_start_transaction(root, 1);
 	if (IS_ERR(trans))
 		return PTR_ERR(trans);
@@ -1732,6 +1737,12 @@ static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
 		       ino);
 
 	btrfs_commit_transaction(trans, root);
+
+	btrfs_release_path(path);
+	ret2 = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret2 > 0)
+		ret2 = -ENOENT;
+	ret |= ret2;
 	return ret;
 }
 
@@ -1963,7 +1974,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 	/* Check EXTENT_DATA hole */
 	if (!no_holes && *end != fkey.offset) {
 		if (repair)
-			ret = punch_extent_hole(root, fkey.objectid,
+			ret = punch_extent_hole(root, path, fkey.objectid,
 						*end, fkey.offset - *end);
 		if (!repair || ret) {
 			err |= FILE_EXTENT_ERROR;
@@ -2534,7 +2545,7 @@ out:
 
 		if (!nbytes && !no_holes && extent_end < isize) {
 			if (repair)
-				ret = punch_extent_hole(root, inode_id,
+				ret = punch_extent_hole(root, path, inode_id,
 						extent_end, isize - extent_end);
 			if (!repair || ret) {
 				err |= NBYTES_ERROR;
-- 
2.19.1

Re: [PATCH 01/13] btrfs-progs: lowmem: add argument path to punch_extent_hole()

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> Since repair will do CoW, the outer path may be invalid,
> add an argument path to punch_extent_hole().
> When punch_extent_hole() returns, path will still point to the item
> before calling punch_extent_hole();
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Overall it looks OK, however still some nitpicks with a new bug exposed
inlined below.

> ---
>  check/mode-lowmem.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..c8e4f13d816f 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1710,15 +1710,20 @@ out:
>  /*
>   * Wrapper function of btrfs_punch_hole.
>   *
> + * @path:	will point to the item while calling the function.
> + *
>   * Returns 0 means success.
>   * Returns not 0 means error.
>   */
> -static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
> -			     u64 len)
> +static int punch_extent_hole(struct btrfs_root *root, struct btrfs_path *path,
> +			     u64 ino, u64 start, u64 len)
>  {
>  	struct btrfs_trans_handle *trans;
> -	int ret = 0;
> +	struct btrfs_key key;
> +	int ret;
> +	int ret2;

I didn't see the need to introduce another ret.

@ret is used to record the error from btrfs_punch_hole(), which should
only return <0 or 0.

We should only continue for ret == 0, so we can overwrite @ret.

>  
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>  	trans = btrfs_start_transaction(root, 1);
>  	if (IS_ERR(trans))
>  		return PTR_ERR(trans);
> @@ -1732,6 +1737,12 @@ static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
>  		       ino);
>  
>  	btrfs_commit_transaction(trans, root);

Here since the wrapper is a little old and at that time we don't have
btrfs_abort_transaction() (well, not really have one even now).

The correct behavior is to abort trans and return error if we get error
from btrfs_punch_hole().

And if we get no error from btrfs_punch_hole(), we can reuse @ret for
btrfs_search_slot(), no need to introduce @ret2.

> +
> +	btrfs_release_path(path);
> +	ret2 = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +	if (ret2 > 0)
> +		ret2 = -ENOENT;
> +	ret |= ret2;
>  	return ret;
>  }
>  
> @@ -1963,7 +1974,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  	/* Check EXTENT_DATA hole */
>  	if (!no_holes && *end != fkey.offset) {
>  		if (repair)
> -			ret = punch_extent_hole(root, fkey.objectid,
> +			ret = punch_extent_hole(root, path, fkey.objectid,
>  						*end, fkey.offset - *end);
>  		if (!repair || ret) {
>  			err |= FILE_EXTENT_ERROR;
> @@ -2534,7 +2545,7 @@ out:
>  
>  		if (!nbytes && !no_holes && extent_end < isize) {
>  			if (repair)
> -				ret = punch_extent_hole(root, inode_id,
> +				ret = punch_extent_hole(root, path, inode_id,
>  						extent_end, isize - extent_end);

In this case, it's check_inode_item() and it's repair case for missing
tailing hole.

However isize can be unaligned, but hole extent should always be aligned.
So there is another bug that repair could create invalid hole extents.

Thanks,
Qu

>  			if (!repair || ret) {
>  				err |= NBYTES_ERROR;
> 

Re: [PATCH 01/13] btrfs-progs: lowmem: add argument path to punch_extent_hole()

[Su Yue]

On 10/23/18 6:04 PM, Qu Wenruo wrote:
> 
> 
> On 2018/10/23 下午5:41, Su Yue wrote:
>> Since repair will do CoW, the outer path may be invalid,
>> add an argument path to punch_extent_hole().
>> When punch_extent_hole() returns, path will still point to the item
>> before calling punch_extent_hole();
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> 
> Overall it looks OK, however still some nitpicks with a new bug exposed
> inlined below.
> 
>> ---
>>   check/mode-lowmem.c | 21 ++++++++++++++++-----
>>   1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 1bce44f5658a..c8e4f13d816f 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -1710,15 +1710,20 @@ out:
>>   /*
>>    * Wrapper function of btrfs_punch_hole.
>>    *
>> + * @path:	will point to the item while calling the function.
>> + *
>>    * Returns 0 means success.
>>    * Returns not 0 means error.
>>    */
>> -static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
>> -			     u64 len)
>> +static int punch_extent_hole(struct btrfs_root *root, struct btrfs_path *path,
>> +			     u64 ino, u64 start, u64 len)
>>   {
>>   	struct btrfs_trans_handle *trans;
>> -	int ret = 0;
>> +	struct btrfs_key key;
>> +	int ret;
>> +	int ret2;
> 
> I didn't see the need to introduce another ret.
> 
> @ret is used to record the error from btrfs_punch_hole(), which should
> only return <0 or 0.
> 
> We should only continue for ret == 0, so we can overwrite @ret.
> 
>>   
>> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>   	trans = btrfs_start_transaction(root, 1);
>>   	if (IS_ERR(trans))
>>   		return PTR_ERR(trans);
>> @@ -1732,6 +1737,12 @@ static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
>>   		       ino);
>>   
>>   	btrfs_commit_transaction(trans, root);
> 
> Here since the wrapper is a little old and at that time we don't have
> btrfs_abort_transaction() (well, not really have one even now).
> 
> The correct behavior is to abort trans and return error if we get error
> from btrfs_punch_hole().
> 
> And if we get no error from btrfs_punch_hole(), we can reuse @ret for
> btrfs_search_slot(), no need to introduce @ret2.
> 

Thanks to your clean explanation.

>> +
>> +	btrfs_release_path(path);
>> +	ret2 = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>> +	if (ret2 > 0)
>> +		ret2 = -ENOENT;
>> +	ret |= ret2;
>>   	return ret;
>>   }
>>   
>> @@ -1963,7 +1974,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>>   	/* Check EXTENT_DATA hole */
>>   	if (!no_holes && *end != fkey.offset) {
>>   		if (repair)
>> -			ret = punch_extent_hole(root, fkey.objectid,
>> +			ret = punch_extent_hole(root, path, fkey.objectid,
>>   						*end, fkey.offset - *end);
>>   		if (!repair || ret) {
>>   			err |= FILE_EXTENT_ERROR;
>> @@ -2534,7 +2545,7 @@ out:
>>   
>>   		if (!nbytes && !no_holes && extent_end < isize) {
>>   			if (repair)
>> -				ret = punch_extent_hole(root, inode_id,
>> +				ret = punch_extent_hole(root, path, inode_id,
>>   						extent_end, isize - extent_end);
> 
> In this case, it's check_inode_item() and it's repair case for missing
> tailing hole.
> 
> However isize can be unaligned, but hole extent should always be aligned.
> So there is another bug that repair could create invalid hole extents.
> 

Oh.. Will fix those two.

Thanks,
Su

> Thanks,
> Qu
> 
>>   			if (!repair || ret) {
>>   				err |= NBYTES_ERROR;
>>
> 
> 

[PATCH v2 01/13] btrfs-progs: lowmem: add argument path to punch_extent_hole()

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

Since repair will do CoW, the outer path may be invalid,
add an argument path to punch_extent_hole().
When punch_extent_hole() returns, path will still point to the item
before calling punch_extent_hole();

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
Changelog:
v2:
  Remove varaiable @ret2.
  Call btrfs_abort_transaction() if failed.
  
---
 check/mode-lowmem.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 1bce44f5658a..3dce4d3b5fc1 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1710,28 +1710,37 @@ out:
 /*
  * Wrapper function of btrfs_punch_hole.
  *
+ * @path:	will point to the item while calling the function.
+ *
  * Returns 0 means success.
  * Returns not 0 means error.
  */
-static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
-			     u64 len)
+static int punch_extent_hole(struct btrfs_root *root, struct btrfs_path *path,
+			     u64 ino, u64 start, u64 len)
 {
 	struct btrfs_trans_handle *trans;
-	int ret = 0;
+	struct btrfs_key key;
+	int ret;
 
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
 	trans = btrfs_start_transaction(root, 1);
 	if (IS_ERR(trans))
 		return PTR_ERR(trans);
 
 	ret = btrfs_punch_hole(trans, root, ino, start, len);
-	if (ret)
+	if (ret) {
 		error("failed to add hole [%llu, %llu] in inode [%llu]",
 		      start, len, ino);
-	else
-		printf("Add a hole [%llu, %llu] in inode [%llu]\n", start, len,
-		       ino);
-
+		btrfs_abort_transaction(trans, ret);
+		return ret;
+	}
+	printf("Add a hole [%llu, %llu] in inode [%llu]\n", start, len, ino);
 	btrfs_commit_transaction(trans, root);
+
+	btrfs_release_path(path);
+	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
 	return ret;
 }
 
@@ -1963,7 +1972,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 	/* Check EXTENT_DATA hole */
 	if (!no_holes && *end != fkey.offset) {
 		if (repair)
-			ret = punch_extent_hole(root, fkey.objectid,
+			ret = punch_extent_hole(root, path, fkey.objectid,
 						*end, fkey.offset - *end);
 		if (!repair || ret) {
 			err |= FILE_EXTENT_ERROR;
@@ -2534,7 +2543,7 @@ out:
 
 		if (!nbytes && !no_holes && extent_end < isize) {
 			if (repair)
-				ret = punch_extent_hole(root, inode_id,
+				ret = punch_extent_hole(root, path, inode_id,
 						extent_end, isize - extent_end);
 			if (!repair || ret) {
 				err |= NBYTES_ERROR;
-- 
2.19.1

[PATCH 02/13] btrfs-progs: lowmem: move nbytes check before isize check

[Su Yue]

For files, lowmem repair will try to check nbytes and isize,
but isize check depends nbytes.

Once bytes has been repaired, then isize should be checked and
repaired.
So move nbytes check before isize check. Also set nbytes to
extent_size once repaired successfully.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index c8e4f13d816f..76e7be81ceb1 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2125,7 +2125,7 @@ out:
 		error("failed to set nbytes in inode %llu root %llu",
 		      ino, root->root_key.objectid);
 	else
-		printf("Set nbytes in inode item %llu root %llu\n to %llu", ino,
+		printf("Set nbytes in inode item %llu root %llu to %llu\n", ino,
 		       root->root_key.objectid, nbytes);
 
 	/* research path */
@@ -2543,28 +2543,31 @@ out:
 			}
 		}
 
-		if (!nbytes && !no_holes && extent_end < isize) {
-			if (repair)
-				ret = punch_extent_hole(root, path, inode_id,
-						extent_end, isize - extent_end);
+		if (nbytes != extent_size) {
+			if (repair) {
+				ret = repair_inode_nbytes_lowmem(root, path,
+							 inode_id, extent_size);
+				if (!ret)
+					nbytes = extent_size;
+			}
 			if (!repair || ret) {
 				err |= NBYTES_ERROR;
 				error(
-	"root %llu INODE[%llu] size %llu should have a file extent hole",
-				      root->objectid, inode_id, isize);
+	"root %llu INODE[%llu] nbytes %llu not equal to extent_size %llu",
+				      root->objectid, inode_id, nbytes,
+				      extent_size);
 			}
 		}
 
-		if (nbytes != extent_size) {
+		if (!nbytes && !no_holes && extent_end < isize) {
 			if (repair)
-				ret = repair_inode_nbytes_lowmem(root, path,
-							 inode_id, extent_size);
+				ret = punch_extent_hole(root, path, inode_id,
+						extent_end, isize - extent_end);
 			if (!repair || ret) {
 				err |= NBYTES_ERROR;
 				error(
-	"root %llu INODE[%llu] nbytes %llu not equal to extent_size %llu",
-				      root->objectid, inode_id, nbytes,
-				      extent_size);
+	"root %llu INODE[%llu] size %llu should have a file extent hole",
+				      root->objectid, inode_id, isize);
 			}
 		}
 	}
-- 
2.19.1

Re: [PATCH 02/13] btrfs-progs: lowmem: move nbytes check before isize check

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> For files, lowmem repair will try to check nbytes and isize,
> but isize check depends nbytes.
> 
> Once bytes has been repaired, then isize should be checked and
> repaired.
> So move nbytes check before isize check. Also set nbytes to
> extent_size once repaired successfully.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Overall looks good.

Just a small nitpick.

> ---
>  check/mode-lowmem.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index c8e4f13d816f..76e7be81ceb1 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -2125,7 +2125,7 @@ out:
>  		error("failed to set nbytes in inode %llu root %llu",
>  		      ino, root->root_key.objectid);
>  	else
> -		printf("Set nbytes in inode item %llu root %llu\n to %llu", ino,
> +		printf("Set nbytes in inode item %llu root %llu to %llu\n", ino,

It's better to fix it in another patch.

Thanks,
Qu

>  		       root->root_key.objectid, nbytes);
>  
>  	/* research path */
> @@ -2543,28 +2543,31 @@ out:
>  			}
>  		}
>  
> -		if (!nbytes && !no_holes && extent_end < isize) {
> -			if (repair)
> -				ret = punch_extent_hole(root, path, inode_id,
> -						extent_end, isize - extent_end);
> +		if (nbytes != extent_size) {
> +			if (repair) {
> +				ret = repair_inode_nbytes_lowmem(root, path,
> +							 inode_id, extent_size);
> +				if (!ret)
> +					nbytes = extent_size;
> +			}
>  			if (!repair || ret) {
>  				err |= NBYTES_ERROR;
>  				error(
> -	"root %llu INODE[%llu] size %llu should have a file extent hole",
> -				      root->objectid, inode_id, isize);
> +	"root %llu INODE[%llu] nbytes %llu not equal to extent_size %llu",
> +				      root->objectid, inode_id, nbytes,
> +				      extent_size);
>  			}
>  		}
>  
> -		if (nbytes != extent_size) {
> +		if (!nbytes && !no_holes && extent_end < isize) {
>  			if (repair)
> -				ret = repair_inode_nbytes_lowmem(root, path,
> -							 inode_id, extent_size);
> +				ret = punch_extent_hole(root, path, inode_id,
> +						extent_end, isize - extent_end);
>  			if (!repair || ret) {
>  				err |= NBYTES_ERROR;
>  				error(
> -	"root %llu INODE[%llu] nbytes %llu not equal to extent_size %llu",
> -				      root->objectid, inode_id, nbytes,
> -				      extent_size);
> +	"root %llu INODE[%llu] size %llu should have a file extent hole",
> +				      root->objectid, inode_id, isize);
>  			}
>  		}
>  	}
> 

[PATCH v2 02/13] btrfs-progs: lowmem: move nbytes check before isize check

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

For files, lowmem repair will try to check nbytes and isize,
but isize check depends nbytes.

Once bytes has been repaired, then isize should be checked and
repaired.
So move nbytes check before isize check. Also set nbytes to
extent_size once repaired successfully.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
Changelog:
v2
  Remove one unrelated change.
  
---
 check/mode-lowmem.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 3dce4d3b5fc1..58f8f6fabed7 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2541,28 +2541,31 @@ out:
 			}
 		}
 
-		if (!nbytes && !no_holes && extent_end < isize) {
-			if (repair)
-				ret = punch_extent_hole(root, path, inode_id,
-						extent_end, isize - extent_end);
+		if (nbytes != extent_size) {
+			if (repair) {
+				ret = repair_inode_nbytes_lowmem(root, path,
+							 inode_id, extent_size);
+				if (!ret)
+					nbytes = extent_size;
+			}
 			if (!repair || ret) {
 				err |= NBYTES_ERROR;
 				error(
-	"root %llu INODE[%llu] size %llu should have a file extent hole",
-				      root->objectid, inode_id, isize);
+	"root %llu INODE[%llu] nbytes %llu not equal to extent_size %llu",
+				      root->objectid, inode_id, nbytes,
+				      extent_size);
 			}
 		}
 
-		if (nbytes != extent_size) {
+		if (!nbytes && !no_holes && extent_end < isize) {
 			if (repair)
-				ret = repair_inode_nbytes_lowmem(root, path,
-							 inode_id, extent_size);
+				ret = punch_extent_hole(root, path, inode_id,
+						extent_end, isize - extent_end);
 			if (!repair || ret) {
 				err |= NBYTES_ERROR;
 				error(
-	"root %llu INODE[%llu] nbytes %llu not equal to extent_size %llu",
-				      root->objectid, inode_id, nbytes,
-				      extent_size);
+	"root %llu INODE[%llu] size %llu should have a file extent hole",
+				      root->objectid, inode_id, isize);
 			}
 		}
 	}
-- 
2.19.1

[PATCH 03/13] btrfs-progs: lowmem: fix false alert if extent item has been repaired

[Su Yue]

Previously, @err are assigned immediately after check but before
repair.
repair_extent_item()'s return value also confuses the caller. If
error has been repaired and returns 0, check_extent_item() will try
to continue check the nonexistent and cause flase alerts.

Here make repair_extent_item()'s return codes only represents status
of the extent item, error bits are passed by pointer.
Move the change of @err after repair.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 106 ++++++++++++++++++++++++++++----------------
 1 file changed, 68 insertions(+), 38 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 76e7be81ceb1..769b3141de8b 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -3788,35 +3788,35 @@ out:
 /*
  * Only delete backref if REFERENCER_MISSING now
  *
- * Returns <0   the extent was deleted
- * Returns >0   the backref was deleted but extent still exists, returned value
- *               means error after repair
+ * Returns <0   the whole extent item was deleted
+ * Returns >0   the backref was deleted but extent still exists, @err_ret
+ *		will be set to be error bits after repair.
  * Returns  0   nothing happened
  */
 static int repair_extent_item(struct btrfs_root *root, struct btrfs_path *path,
 		      u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
-		      u64 owner, u64 offset, int err)
+		      u64 owner, u64 offset, int *err_ret)
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_root *extent_root = root->fs_info->extent_root;
 	struct btrfs_key old_key;
-	int freed = 0;
 	int ret;
+	int err = *err_ret;
 
 	btrfs_item_key_to_cpu(path->nodes[0], &old_key, path->slots[0]);
 
 	if ((err & (REFERENCER_MISSING | REFERENCER_MISMATCH)) == 0)
-		return err;
+		return 0;
 
 	ret = avoid_extents_overwrite(root->fs_info);
 	if (ret)
-		return err;
+		return 0;
 
 	trans = btrfs_start_transaction(extent_root, 1);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
 		error("fail to start transaction %s", strerror(-ret));
-		/* nothing happened */
+		err |= FATAL_ERROR;
 		ret = 0;
 		goto out;
 	}
@@ -3824,24 +3824,44 @@ static int repair_extent_item(struct btrfs_root *root, struct btrfs_path *path,
 	ret = btrfs_free_extent(trans, root->fs_info->fs_root, bytenr,
 			num_bytes, parent, root_objectid, owner, offset);
 	if (!ret) {
-		freed = 1;
-		err &= ~REFERENCER_MISSING;
+		err &= ~(REFERENCER_MISSING | REFERENCER_MISMATCH);
 		printf("Delete backref in extent [%llu %llu]\n",
 		       bytenr, num_bytes);
 	} else {
 		error("fail to delete backref in extent [%llu %llu]",
 		      bytenr, num_bytes);
+		if (ret < 0)
+			err |= FATAL_ERROR;
 	}
 	btrfs_commit_transaction(trans, extent_root);
 
-	/* btrfs_free_extent may delete the extent */
 	btrfs_release_path(path);
 	ret = btrfs_search_slot(NULL, root, &old_key, path, 0, 0);
-	if (ret)
+	if (ret > 0) {
+		/* odd, there must be one block group before at least*/
+		if (path->slots[0] == 0) {
+			ret = -EUCLEAN;
+			err |= FATAL_ERROR;
+			goto out;
+		}
+		/*
+		 * btrfs_free_extent() has deleted the extent item,
+		 * let path point to last checked item.
+		 */
+		if (path->slots[0] >= btrfs_header_nritems(path->nodes[0]))
+			path->slots[0] = btrfs_header_nritems(path->nodes[0]) - 1;
+		else
+			path->slots[0]--;
+
 		ret = -ENOENT;
-	else if (freed)
-		ret = err;
+		err = 0;
+	} else if (ret < 0) {
+		err |= FATAL_ERROR;
+	} else {
+		ret = 1;
+	}
 out:
+	*err_ret = err;
 	return ret;
 }
 
@@ -3858,7 +3878,6 @@ static int check_extent_item(struct btrfs_fs_info *fs_info,
 	struct btrfs_extent_inline_ref *iref;
 	struct btrfs_extent_data_ref *dref;
 	struct extent_buffer *eb = path->nodes[0];
-	unsigned long end;
 	unsigned long ptr;
 	int slot = path->slots[0];
 	int type;
@@ -3876,6 +3895,8 @@ static int check_extent_item(struct btrfs_fs_info *fs_info,
 	struct btrfs_key key;
 	int ret;
 	int err = 0;
+	int tmp_err;
+	u32 ptr_offset;
 
 	btrfs_item_key_to_cpu(eb, &key, slot);
 	if (key.type == BTRFS_EXTENT_ITEM_KEY) {
@@ -3921,21 +3942,22 @@ static int check_extent_item(struct btrfs_fs_info *fs_info,
 		/* New METADATA_ITEM */
 		level = key.offset;
 	}
-	end = (unsigned long)ei + item_size;
+	ptr_offset = ptr - (unsigned long)ei;
 
 next:
 	/* Reached extent item end normally */
-	if (ptr == end)
+	if (ptr_offset == item_size)
 		goto out;
 
 	/* Beyond extent item end, wrong item size */
-	if (ptr > end) {
+	if (ptr_offset > item_size) {
 		err |= ITEM_SIZE_MISMATCH;
 		error("extent item at bytenr %llu slot %d has wrong size",
 			eb->start, slot);
 		goto out;
 	}
 
+	ptr = (unsigned long)ei + ptr_offset;
 	parent = 0;
 	root_objectid = 0;
 	owner = 0;
@@ -3948,52 +3970,60 @@ next:
 	case BTRFS_TREE_BLOCK_REF_KEY:
 		root_objectid = offset;
 		owner = level;
-		ret = check_tree_block_backref(fs_info, offset, key.objectid,
-					       level);
-		err |= ret;
+		tmp_err = check_tree_block_backref(fs_info, offset,
+						   key.objectid, level);
 		break;
 	case BTRFS_SHARED_BLOCK_REF_KEY:
 		parent = offset;
-		ret = check_shared_block_backref(fs_info, offset, key.objectid,
-						 level);
-		err |= ret;
+		tmp_err = check_shared_block_backref(fs_info, offset,
+						     key.objectid, level);
 		break;
 	case BTRFS_EXTENT_DATA_REF_KEY:
 		dref = (struct btrfs_extent_data_ref *)(&iref->offset);
 		root_objectid = btrfs_extent_data_ref_root(eb, dref);
 		owner = btrfs_extent_data_ref_objectid(eb, dref);
 		owner_offset = btrfs_extent_data_ref_offset(eb, dref);
-		ret = check_extent_data_backref(fs_info, root_objectid, owner,
-					owner_offset, key.objectid, key.offset,
-					btrfs_extent_data_ref_count(eb, dref));
-		err |= ret;
+		tmp_err = check_extent_data_backref(fs_info, root_objectid,
+			    owner, owner_offset, key.objectid, key.offset,
+			    btrfs_extent_data_ref_count(eb, dref));
 		break;
 	case BTRFS_SHARED_DATA_REF_KEY:
 		parent = offset;
-		ret = check_shared_data_backref(fs_info, offset, key.objectid);
-		err |= ret;
+		tmp_err = check_shared_data_backref(fs_info, offset,
+						    key.objectid);
 		break;
 	default:
 		error("extent[%llu %d %llu] has unknown ref type: %d",
 			key.objectid, key.type, key.offset, type);
-		ret = UNKNOWN_TYPE;
-		err |= ret;
+		err |= UNKNOWN_TYPE;
+
 		goto out;
 	}
 
-	if (err && repair) {
+	if (tmp_err && repair) {
 		ret = repair_extent_item(fs_info->extent_root, path,
 			 key.objectid, num_bytes, parent, root_objectid,
-			 owner, owner_offset, ret);
-		if (ret < 0)
+			 owner, owner_offset, &tmp_err);
+		err |= tmp_err;
+
+		if (tmp_err & FATAL_ERROR || ret < 0)
 			goto out;
-		if (ret) {
+		/*
+		 * the error has been repaired which means the extent item
+		 * is still existed with other backrefs, go to check next.
+		 */
+		if (ret > 0) {
+			eb = path->nodes[0];
+			slot = path->slots[0];
+			ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
+			item_size = btrfs_item_size_nr(eb, slot);
 			goto next;
-			err = ret;
 		}
+	} else {
+		err |= tmp_err;
 	}
 
-	ptr += btrfs_extent_inline_ref_size(type);
+	ptr_offset += btrfs_extent_inline_ref_size(type);
 	goto next;
 
 out:
-- 
2.19.1

Re: [PATCH 03/13] btrfs-progs: lowmem: fix false alert if extent item has been repaired

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> Previously, @err are assigned immediately after check but before
> repair.
> repair_extent_item()'s return value also confuses the caller. If
> error has been repaired and returns 0, check_extent_item() will try
> to continue check the nonexistent and cause flase alerts.
> 
> Here make repair_extent_item()'s return codes only represents status
> of the extent item, error bits are passed by pointer.
> Move the change of @err after repair.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  check/mode-lowmem.c | 106 ++++++++++++++++++++++++++++----------------
>  1 file changed, 68 insertions(+), 38 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 76e7be81ceb1..769b3141de8b 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -3788,35 +3788,35 @@ out:
>  /*
>   * Only delete backref if REFERENCER_MISSING now
>   *
> - * Returns <0   the extent was deleted
> - * Returns >0   the backref was deleted but extent still exists, returned value
> - *               means error after repair
> + * Returns <0   the whole extent item was deleted
> + * Returns >0   the backref was deleted but extent still exists, @err_ret
> + *		will be set to be error bits after repair.

Well, I have to admit the parameter list is really human friendly from
the initial design.

In fact, the only real user of @err is just that "if ((err &
(REFERENCER_MISSING | REFERENCER_MISMATCH)) == 0)" line.

We can in fact get rid of that @err parameter and do that check at the
only caller of repair_extent_item().


And then redefine the return value.

Like "Return < 0 if we failed to repair backref for the extent.
Return 0 if we repaired the backref for the extent".

[snip]
>  
> -	if (err && repair) {
> +	if (tmp_err && repair) {
>  		ret = repair_extent_item(fs_info->extent_root, path,
>  			 key.objectid, num_bytes, parent, root_objectid,
> -			 owner, owner_offset, ret);
> -		if (ret < 0)

If follow my above method, here the code can be a little easier:
	if (tmp_err && repair) {
		if (!(tmp_err & (REFERENCER_MISSING |
		    REFENCER_MISMATCH)) {
			err |= tmp_err;
			goto next;
		}
		ret = repair_extent_item(...);
		if (ret < 0)
			err |= tmp_err;
	} else if (tmp_err) {
		err |= tmp_err;
	}

Without the need to passing @err into repair_extent_item() nor modifying
@err in that function.

Thanks,
Qu

> +			 owner, owner_offset, &tmp_err);
> +		err |= tmp_err;
> +
> +		if (tmp_err & FATAL_ERROR || ret < 0)
>  			goto out;
> -		if (ret) {
> +		/*
> +		 * the error has been repaired which means the extent item
> +		 * is still existed with other backrefs, go to check next.
> +		 */> +		if (ret > 0) {
> +			eb = path->nodes[0];
> +			slot = path->slots[0];
> +			ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
> +			item_size = btrfs_item_size_nr(eb, slot);
>  			goto next;
> -			err = ret;
>  		}
> +	} else {
> +		err |= tmp_err;
>  	}
>  
> -	ptr += btrfs_extent_inline_ref_size(type);
> +	ptr_offset += btrfs_extent_inline_ref_size(type);
>  	goto next;
>  
>  out:
> 

Re: [PATCH 03/13] btrfs-progs: lowmem: fix false alert if extent item has been repaired

[Su Yue]

On 10/23/18 6:30 PM, Qu Wenruo wrote:
> 
> 
> On 2018/10/23 下午5:41, Su Yue wrote:
>> Previously, @err are assigned immediately after check but before
>> repair.
>> repair_extent_item()'s return value also confuses the caller. If
>> error has been repaired and returns 0, check_extent_item() will try
>> to continue check the nonexistent and cause flase alerts.
>>
>> Here make repair_extent_item()'s return codes only represents status
>> of the extent item, error bits are passed by pointer.
>> Move the change of @err after repair.
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>>   check/mode-lowmem.c | 106 ++++++++++++++++++++++++++++----------------
>>   1 file changed, 68 insertions(+), 38 deletions(-)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 76e7be81ceb1..769b3141de8b 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -3788,35 +3788,35 @@ out:
>>   /*
>>    * Only delete backref if REFERENCER_MISSING now
>>    *
>> - * Returns <0   the extent was deleted
>> - * Returns >0   the backref was deleted but extent still exists, returned value
>> - *               means error after repair
>> + * Returns <0   the whole extent item was deleted
>> + * Returns >0   the backref was deleted but extent still exists, @err_ret
>> + *		will be set to be error bits after repair.
> 
> Well, I have to admit the parameter list is really human friendly from
> the initial design.
> 

As my previous design, callers of repair_xxx functions just take
care of successes, failures and pathes. If exposing the logic
to the callers is acceptable, will do it as your suggestions.

Thanks,
Su

> In fact, the only real user of @err is just that "if ((err &
> (REFERENCER_MISSING | REFERENCER_MISMATCH)) == 0)" line.
> 
> We can in fact get rid of that @err parameter and do that check at the
> only caller of repair_extent_item().
> 
> 
> And then redefine the return value.
> 
> Like "Return < 0 if we failed to repair backref for the extent.
> Return 0 if we repaired the backref for the extent".
> 
> [snip]
>>   
>> -	if (err && repair) {
>> +	if (tmp_err && repair) {
>>   		ret = repair_extent_item(fs_info->extent_root, path,
>>   			 key.objectid, num_bytes, parent, root_objectid,
>> -			 owner, owner_offset, ret);
>> -		if (ret < 0)
> 
> If follow my above method, here the code can be a little easier:
> 	if (tmp_err && repair) {
> 		if (!(tmp_err & (REFERENCER_MISSING |
> 		    REFENCER_MISMATCH)) {
> 			err |= tmp_err;
> 			goto next;
> 		}
> 		ret = repair_extent_item(...);
> 		if (ret < 0)
> 			err |= tmp_err;
> 	} else if (tmp_err) {
> 		err |= tmp_err;
> 	}
> 

> Without the need to passing @err into repair_extent_item() nor modifying
> @err in that function.
> 
> Thanks,
> Qu
> 
>> +			 owner, owner_offset, &tmp_err);
>> +		err |= tmp_err;
>> +
>> +		if (tmp_err & FATAL_ERROR || ret < 0)
>>   			goto out;
>> -		if (ret) {
>> +		/*
>> +		 * the error has been repaired which means the extent item
>> +		 * is still existed with other backrefs, go to check next.
>> +		 */> +		if (ret > 0) {
>> +			eb = path->nodes[0];
>> +			slot = path->slots[0];
>> +			ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
>> +			item_size = btrfs_item_size_nr(eb, slot);
>>   			goto next;
>> -			err = ret;
>>   		}
>> +	} else {
>> +		err |= tmp_err;
>>   	}
>>   
>> -	ptr += btrfs_extent_inline_ref_size(type);
>> +	ptr_offset += btrfs_extent_inline_ref_size(type);
>>   	goto next;
>>   
>>   out:
>>
> 
> 

Re: [PATCH 03/13] btrfs-progs: lowmem: fix false alert if extent item has been repaired

[Qu Wenruo]

On 2018/10/24 上午9:27, Su Yue wrote:
> 
> 
> On 10/23/18 6:30 PM, Qu Wenruo wrote:
>>
>>
>> On 2018/10/23 下午5:41, Su Yue wrote:
>>> Previously, @err are assigned immediately after check but before
>>> repair.
>>> repair_extent_item()'s return value also confuses the caller. If
>>> error has been repaired and returns 0, check_extent_item() will try
>>> to continue check the nonexistent and cause flase alerts.
>>>
>>> Here make repair_extent_item()'s return codes only represents status
>>> of the extent item, error bits are passed by pointer.
>>> Move the change of @err after repair.
>>>
>>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>>> ---
>>>   check/mode-lowmem.c | 106 ++++++++++++++++++++++++++++----------------
>>>   1 file changed, 68 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>> index 76e7be81ceb1..769b3141de8b 100644
>>> --- a/check/mode-lowmem.c
>>> +++ b/check/mode-lowmem.c
>>> @@ -3788,35 +3788,35 @@ out:
>>>   /*
>>>    * Only delete backref if REFERENCER_MISSING now
>>>    *
>>> - * Returns <0   the extent was deleted
>>> - * Returns >0   the backref was deleted but extent still exists,
>>> returned value
>>> - *               means error after repair
>>> + * Returns <0   the whole extent item was deleted
>>> + * Returns >0   the backref was deleted but extent still exists,
>>> @err_ret
>>> + *        will be set to be error bits after repair.
>>
>> Well, I have to admit the parameter list is really human friendly from
>> the initial design.
>>
> 
> As my previous design, callers of repair_xxx functions just take
> care of successes, failures and pathes. If exposing the logic
> to the callers is acceptable, will do it as your suggestions.

Sorry, I miss the "not" word, so the correct line should be:

> Well, I have to admit the parameter list is really mot human friendly
> from the initial design.

Thanks,
Qu



> 
> Thanks,
> Su
> 
>> In fact, the only real user of @err is just that "if ((err &
>> (REFERENCER_MISSING | REFERENCER_MISMATCH)) == 0)" line.
>>
>> We can in fact get rid of that @err parameter and do that check at the
>> only caller of repair_extent_item().
>>
>>
>> And then redefine the return value.
>>
>> Like "Return < 0 if we failed to repair backref for the extent.
>> Return 0 if we repaired the backref for the extent".
>>
>> [snip]
>>>   -    if (err && repair) {
>>> +    if (tmp_err && repair) {
>>>           ret = repair_extent_item(fs_info->extent_root, path,
>>>                key.objectid, num_bytes, parent, root_objectid,
>>> -             owner, owner_offset, ret);
>>> -        if (ret < 0)
>>
>> If follow my above method, here the code can be a little easier:
>>     if (tmp_err && repair) {
>>         if (!(tmp_err & (REFERENCER_MISSING |
>>             REFENCER_MISMATCH)) {
>>             err |= tmp_err;
>>             goto next;
>>         }
>>         ret = repair_extent_item(...);
>>         if (ret < 0)
>>             err |= tmp_err;
>>     } else if (tmp_err) {
>>         err |= tmp_err;
>>     }
>>
> 
>> Without the need to passing @err into repair_extent_item() nor modifying
>> @err in that function.
>>
>> Thanks,
>> Qu
>>
>>> +             owner, owner_offset, &tmp_err);
>>> +        err |= tmp_err;
>>> +
>>> +        if (tmp_err & FATAL_ERROR || ret < 0)
>>>               goto out;
>>> -        if (ret) {
>>> +        /*
>>> +         * the error has been repaired which means the extent item
>>> +         * is still existed with other backrefs, go to check next.
>>> +         */> +        if (ret > 0) {
>>> +            eb = path->nodes[0];
>>> +            slot = path->slots[0];
>>> +            ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
>>> +            item_size = btrfs_item_size_nr(eb, slot);
>>>               goto next;
>>> -            err = ret;
>>>           }
>>> +    } else {
>>> +        err |= tmp_err;
>>>       }
>>>   -    ptr += btrfs_extent_inline_ref_size(type);
>>> +    ptr_offset += btrfs_extent_inline_ref_size(type);
>>>       goto next;
>>>     out:
>>>
>>
>>
> 
> 

[PATCH v2 03/13] btrfs-progs: lowmem: fix false alert if extent item has been repaired

[damenly.su]

From: Su Yue <suy.fnst@cn.fujitsu.com>

Previously, @err are assigned immediately after check but before
repair.
repair_extent_item()'s return value also confuses the caller. If
error has been repaired and returns 0, check_extent_item() will try
to continue check the nonexistent and cause false alerts.

Here make repair_extent_item()'s return codes only represents status
of the extent item, error bits are handled in caller of the repair
function.
Change of @err after repair.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
Changelog:
v2
  Error bits are handled in check_extent_item().
  Simplify repair_extent_item() according to Qu Wenruo's suggestion.
  
---
 check/mode-lowmem.c | 118 +++++++++++++++++++++++++++-----------------
 1 file changed, 72 insertions(+), 46 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 58f8f6fabed7..a5b7d9d35772 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -3784,61 +3784,69 @@ out:
 }
 
 /*
- * Only delete backref if REFERENCER_MISSING now
+ * Only delete backref if REFERENCER_MISSING or REFERENCER_MISMATCH.
  *
- * Returns <0   the extent was deleted
- * Returns >0   the backref was deleted but extent still exists, returned value
- *               means error after repair
- * Returns  0   nothing happened
+ * Returns <0   error
+ * Returns >0   the backref was deleted but extent still exists
+ * Returns =0   the whole extent item was deleted
  */
 static int repair_extent_item(struct btrfs_root *root, struct btrfs_path *path,
 		      u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
-		      u64 owner, u64 offset, int err)
+		      u64 owner, u64 offset)
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_root *extent_root = root->fs_info->extent_root;
 	struct btrfs_key old_key;
-	int freed = 0;
 	int ret;
 
 	btrfs_item_key_to_cpu(path->nodes[0], &old_key, path->slots[0]);
 
-	if ((err & (REFERENCER_MISSING | REFERENCER_MISMATCH)) == 0)
-		return err;
-
 	ret = avoid_extents_overwrite(root->fs_info);
 	if (ret)
-		return err;
+		return ret;
 
 	trans = btrfs_start_transaction(extent_root, 1);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
 		error("fail to start transaction %s", strerror(-ret));
-		/* nothing happened */
-		ret = 0;
 		goto out;
 	}
 	/* delete the backref */
 	ret = btrfs_free_extent(trans, root->fs_info->fs_root, bytenr,
 			num_bytes, parent, root_objectid, owner, offset);
-	if (!ret) {
-		freed = 1;
-		err &= ~REFERENCER_MISSING;
+	if (!ret)
 		printf("Delete backref in extent [%llu %llu]\n",
 		       bytenr, num_bytes);
-	} else {
+	else {
 		error("fail to delete backref in extent [%llu %llu]",
 		      bytenr, num_bytes);
+		btrfs_abort_transaction(trans, ret);
+		goto out;
 	}
 	btrfs_commit_transaction(trans, extent_root);
 
-	/* btrfs_free_extent may delete the extent */
 	btrfs_release_path(path);
 	ret = btrfs_search_slot(NULL, root, &old_key, path, 0, 0);
-	if (ret)
-		ret = -ENOENT;
-	else if (freed)
-		ret = err;
+	if (ret > 0) {
+		/* odd, there must be one block group before at least */
+		if (path->slots[0] == 0) {
+			ret = -EUCLEAN;
+			goto out;
+		}
+		/*
+		 * btrfs_free_extent() has deleted the extent item,
+		 * let path point to last checked item.
+		 */
+		if (path->slots[0] >= btrfs_header_nritems(path->nodes[0]))
+			path->slots[0] = btrfs_header_nritems(path->nodes[0]) - 1;
+		else
+			path->slots[0]--;
+
+		ret = 0;
+	} else if (ret == 0) {
+		ret = 1;
+	}
+
 out:
 	return ret;
 }
@@ -3856,7 +3864,6 @@ static int check_extent_item(struct btrfs_fs_info *fs_info,
 	struct btrfs_extent_inline_ref *iref;
 	struct btrfs_extent_data_ref *dref;
 	struct extent_buffer *eb = path->nodes[0];
-	unsigned long end;
 	unsigned long ptr;
 	int slot = path->slots[0];
 	int type;
@@ -3874,6 +3881,8 @@ static int check_extent_item(struct btrfs_fs_info *fs_info,
 	struct btrfs_key key;
 	int ret;
 	int err = 0;
+	int tmp_err;
+	u32 ptr_offset;
 
 	btrfs_item_key_to_cpu(eb, &key, slot);
 	if (key.type == BTRFS_EXTENT_ITEM_KEY) {
@@ -3919,21 +3928,22 @@ static int check_extent_item(struct btrfs_fs_info *fs_info,
 		/* New METADATA_ITEM */
 		level = key.offset;
 	}
-	end = (unsigned long)ei + item_size;
+	ptr_offset = ptr - (unsigned long)ei;
 
 next:
 	/* Reached extent item end normally */
-	if (ptr == end)
+	if (ptr_offset == item_size)
 		goto out;
 
 	/* Beyond extent item end, wrong item size */
-	if (ptr > end) {
+	if (ptr_offset > item_size) {
 		err |= ITEM_SIZE_MISMATCH;
 		error("extent item at bytenr %llu slot %d has wrong size",
 			eb->start, slot);
 		goto out;
 	}
 
+	ptr = (unsigned long)ei + ptr_offset;
 	parent = 0;
 	root_objectid = 0;
 	owner = 0;
@@ -3946,52 +3956,68 @@ next:
 	case BTRFS_TREE_BLOCK_REF_KEY:
 		root_objectid = offset;
 		owner = level;
-		ret = check_tree_block_backref(fs_info, offset, key.objectid,
-					       level);
-		err |= ret;
+		tmp_err = check_tree_block_backref(fs_info, offset,
+						   key.objectid, level);
 		break;
 	case BTRFS_SHARED_BLOCK_REF_KEY:
 		parent = offset;
-		ret = check_shared_block_backref(fs_info, offset, key.objectid,
-						 level);
-		err |= ret;
+		tmp_err = check_shared_block_backref(fs_info, offset,
+						     key.objectid, level);
 		break;
 	case BTRFS_EXTENT_DATA_REF_KEY:
 		dref = (struct btrfs_extent_data_ref *)(&iref->offset);
 		root_objectid = btrfs_extent_data_ref_root(eb, dref);
 		owner = btrfs_extent_data_ref_objectid(eb, dref);
 		owner_offset = btrfs_extent_data_ref_offset(eb, dref);
-		ret = check_extent_data_backref(fs_info, root_objectid, owner,
-					owner_offset, key.objectid, key.offset,
-					btrfs_extent_data_ref_count(eb, dref));
-		err |= ret;
+		tmp_err = check_extent_data_backref(fs_info, root_objectid,
+			    owner, owner_offset, key.objectid, key.offset,
+			    btrfs_extent_data_ref_count(eb, dref));
 		break;
 	case BTRFS_SHARED_DATA_REF_KEY:
 		parent = offset;
-		ret = check_shared_data_backref(fs_info, offset, key.objectid);
-		err |= ret;
+		tmp_err = check_shared_data_backref(fs_info, offset,
+						    key.objectid);
 		break;
 	default:
 		error("extent[%llu %d %llu] has unknown ref type: %d",
 			key.objectid, key.type, key.offset, type);
-		ret = UNKNOWN_TYPE;
-		err |= ret;
+		err |= UNKNOWN_TYPE;
+
 		goto out;
 	}
 
-	if (err && repair) {
+	if ((tmp_err & (REFERENCER_MISSING | REFERENCER_MISMATCH))
+	    && repair) {
 		ret = repair_extent_item(fs_info->extent_root, path,
 			 key.objectid, num_bytes, parent, root_objectid,
-			 owner, owner_offset, ret);
-		if (ret < 0)
+			 owner, owner_offset);
+		if (ret < 0) {
+			err |= tmp_err;
+			err |= FATAL_ERROR;
 			goto out;
-		if (ret) {
+		} else if (ret == 0) {
+			err = 0;
+			goto out;
+		} else if (ret > 0) {
+			/*
+			 * The error has been repaired which means the
+			 * extent item is still existed with other backrefs,
+			 * go to check next.
+			 */
+			tmp_err &= ~REFERENCER_MISSING;
+			tmp_err &= ~REFERENCER_MISMATCH;
+			err |= tmp_err;
+			eb = path->nodes[0];
+			slot = path->slots[0];
+			ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
+			item_size = btrfs_item_size_nr(eb, slot);
 			goto next;
-			err = ret;
 		}
+	} else {
+		err |= tmp_err;
 	}
 
-	ptr += btrfs_extent_inline_ref_size(type);
+	ptr_offset += btrfs_extent_inline_ref_size(type);
 	goto next;
 
 out:
-- 
2.19.1

[PATCH 04/13] btrfs-progs: lowmem: fix false alert about the existence of gaps in the check_file_extent

[Su Yue]

From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

The 'end' parameter of check_file_extent tracks the ending offset of the
last checked extent. This is used to detect gaps between adjacent extents.

Currently such gaps are wrongly detected since for regular extents only
the size of the extent is added to the 'end' parameter. This results in
wrongly considering all extents of a file as having gaps between them
when only 2 of them really have a gap as seen in the example below.

Solution:
The extent_end variable should set to the sum of the offset and the
extent_num_bytes of the file extent.

Example:
Suppose that lowmem check the following file extent of inode 257.

        item 6 key (257 EXTENT_DATA 0) itemoff 15813 itemsize 53
                generation 6 type 1 (regular)
                extent data disk byte 13631488 nr 4096
                extent data offset 0 nr 4096 ram 4096
                extent compression 0 (none)
        item 7 key (257 EXTENT_DATA 8192) itemoff 15760 itemsize 53
                generation 6 type 1 (regular)
                extent data disk byte 13631488 nr 4096
                extent data offset 0 nr 4096 ram 4096
                extent compression 0 (none)
        item 8 key (257 EXTENT_DATA 12288) itemoff 15707 itemsize 53
                generation 6 type 1 (regular)
                extent data disk byte 13631488 nr 4096
                extent data offset 0 nr 4096 ram 4096
                extent compression 0 (none)

For inode 257, check_inode_item set extent_end to 0, then call
check_file_extent to check item {6,7,8}.
item 6)
	offset(0) == extent_end(0)
	extent_end = extent_end(0) + extent_num_bytes(4096)
item 7)
	offset(8192) != extent_end(4096)
	extent_end = extent_end(4096) + extent_num_bytes(4096)
			^^^
	The old extent_end should replace by offset(8192).
item 8)
	offset(12288) != extent_end(8192)
		^^^
	But there is no gap between item {7,8}.

Fixes: d88da10ddd42 ("btrfs-progs: check: introduce function to check file extent")
Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 769b3141de8b..35fe1adf58e6 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1985,7 +1985,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 		}
 	}
 
-	*end += extent_num_bytes;
+	*end = fkey.offset + extent_num_bytes;
 	if (!is_hole)
 		*size += extent_num_bytes;
 
-- 
2.19.1

Re: [PATCH 04/13] btrfs-progs: lowmem: fix false alert about the existence of gaps in the check_file_extent

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> 
> The 'end' parameter of check_file_extent tracks the ending offset of the
> last checked extent. This is used to detect gaps between adjacent extents.
> 
> Currently such gaps are wrongly detected since for regular extents only
> the size of the extent is added to the 'end' parameter. This results in
> wrongly considering all extents of a file as having gaps between them
> when only 2 of them really have a gap as seen in the example below.
> 
> Solution:
> The extent_end variable should set to the sum of the offset and the
> extent_num_bytes of the file extent.
> 
> Example:
> Suppose that lowmem check the following file extent of inode 257.
> 
>         item 6 key (257 EXTENT_DATA 0) itemoff 15813 itemsize 53
>                 generation 6 type 1 (regular)
>                 extent data disk byte 13631488 nr 4096
>                 extent data offset 0 nr 4096 ram 4096
>                 extent compression 0 (none)
>         item 7 key (257 EXTENT_DATA 8192) itemoff 15760 itemsize 53
>                 generation 6 type 1 (regular)
>                 extent data disk byte 13631488 nr 4096
>                 extent data offset 0 nr 4096 ram 4096
>                 extent compression 0 (none)
>         item 8 key (257 EXTENT_DATA 12288) itemoff 15707 itemsize 53
>                 generation 6 type 1 (regular)
>                 extent data disk byte 13631488 nr 4096
>                 extent data offset 0 nr 4096 ram 4096
>                 extent compression 0 (none)
> 
> For inode 257, check_inode_item set extent_end to 0, then call
> check_file_extent to check item {6,7,8}.
> item 6)
> 	offset(0) == extent_end(0)
> 	extent_end = extent_end(0) + extent_num_bytes(4096)
> item 7)
> 	offset(8192) != extent_end(4096)
> 	extent_end = extent_end(4096) + extent_num_bytes(4096)
> 			^^^

The problem is all about a gap screwing up all later accounting.

So it's indeed a problem.

> 	The old extent_end should replace by offset(8192).
> item 8)
> 	offset(12288) != extent_end(8192)
> 		^^^
> 	But there is no gap between item {7,8}.
> 
> Fixes: d88da10ddd42 ("btrfs-progs: check: introduce function to check file extent")
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  check/mode-lowmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 769b3141de8b..35fe1adf58e6 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1985,7 +1985,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  		}
>  	}
>  
> -	*end += extent_num_bytes;
> +	*end = fkey.offset + extent_num_bytes;
>  	if (!is_hole)
>  		*size += extent_num_bytes;
>  
> 

[PATCH 05/13] btrfs-progs: lowmem: check unaligned disk_bytenr for extent_data

[Su Yue]

Add support to check unaligned disk_bytenr for extent_data.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 35fe1adf58e6..e8a2e825c0f3 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -3103,7 +3103,14 @@ static int check_extent_data_item(struct btrfs_root *root,
 	extent_num_bytes = btrfs_file_extent_num_bytes(eb, fi);
 	offset = btrfs_file_extent_offset(eb, fi);
 
-	/* Check unaligned disk_num_bytes and num_bytes */
+	/* Check unaligned disk_bytenr, disk_num_bytes and num_bytes */
+	if (!IS_ALIGNED(disk_bytenr, root->fs_info->sectorsize)) {
+		error(
+"file extent [%llu, %llu] has unaligned disk bytenr: %llu, should be aligned to %u",
+			fi_key.objectid, fi_key.offset, disk_bytenr,
+			root->fs_info->sectorsize);
+		err |= BYTES_UNALIGNED;
+	}
 	if (!IS_ALIGNED(disk_num_bytes, root->fs_info->sectorsize)) {
 		error(
 "file extent [%llu, %llu] has unaligned disk num bytes: %llu, should be aligned to %u",
-- 
2.19.1

Re: [PATCH 05/13] btrfs-progs: lowmem: check unaligned disk_bytenr for extent_data

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> Add support to check unaligned disk_bytenr for extent_data.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  check/mode-lowmem.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 35fe1adf58e6..e8a2e825c0f3 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -3103,7 +3103,14 @@ static int check_extent_data_item(struct btrfs_root *root,
>  	extent_num_bytes = btrfs_file_extent_num_bytes(eb, fi);
>  	offset = btrfs_file_extent_offset(eb, fi);
>  
> -	/* Check unaligned disk_num_bytes and num_bytes */
> +	/* Check unaligned disk_bytenr, disk_num_bytes and num_bytes */
> +	if (!IS_ALIGNED(disk_bytenr, root->fs_info->sectorsize)) {
> +		error(
> +"file extent [%llu, %llu] has unaligned disk bytenr: %llu, should be aligned to %u",
> +			fi_key.objectid, fi_key.offset, disk_bytenr,
> +			root->fs_info->sectorsize);
> +		err |= BYTES_UNALIGNED;
> +	}
>  	if (!IS_ALIGNED(disk_num_bytes, root->fs_info->sectorsize)) {
>  		error(
>  "file extent [%llu, %llu] has unaligned disk num bytes: %llu, should be aligned to %u",
> 

[PATCH 06/13] btrfs-progs: lowmem: rename delete_extent_tree_item() to delete_item()

[Su Yue]

The function can delete items in trees besides extent tree.
Rename and move it for further use.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 99 +++++++++++++++++++++++----------------------
 1 file changed, 51 insertions(+), 48 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index e8a2e825c0f3..3c9ecff7e498 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -537,6 +537,52 @@ static int end_avoid_extents_overwrite(struct btrfs_fs_info *fs_info)
 	return ret;
 }
 
+/*
+ * Delete the item @path point to.
+ * If deleted, path will point to the previous item to the deleted item.
+ */
+static int delete_item(struct btrfs_root *root, struct btrfs_path *path)
+{
+	struct btrfs_key key;
+	struct btrfs_trans_handle *trans;
+	int ret = 0;
+
+	ret = avoid_extents_overwrite(root->fs_info);
+	if (ret)
+		return ret;
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		error("fail to start transaction %s", strerror(-ret));
+		goto out;
+	}
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+	btrfs_release_path(path);
+	ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
+	if (ret) {
+		ret = -ENOENT;
+		goto out;
+	}
+
+	ret = btrfs_del_item(trans, root, path);
+	if (ret)
+		goto out;
+
+	if (path->slots[0] == 0)
+		btrfs_prev_leaf(root, path);
+	else
+		path->slots[0]--;
+out:
+	btrfs_commit_transaction(trans, root);
+	if (ret)
+		error("failed to delete root %llu item[%llu, %u, %llu]",
+		      root->objectid, key.objectid, key.type, key.offset);
+	else
+		printf("Deleted root %llu item[%llu, %u, %llu]\n",
+		       root->objectid, key.objectid, key.type, key.offset);
+	return ret;
+}
+
 /*
  * Wrapper function for btrfs_fix_block_accounting().
  *
@@ -4335,49 +4381,6 @@ static int repair_chunk_item(struct btrfs_root *chunk_root,
 	return err;
 }
 
-static int delete_extent_tree_item(struct btrfs_root *root,
-				   struct btrfs_path *path)
-{
-	struct btrfs_key key;
-	struct btrfs_trans_handle *trans;
-	int ret = 0;
-
-	ret = avoid_extents_overwrite(root->fs_info);
-	if (ret)
-		return ret;
-	trans = btrfs_start_transaction(root, 1);
-	if (IS_ERR(trans)) {
-		ret = PTR_ERR(trans);
-		error("fail to start transaction %s", strerror(-ret));
-		goto out;
-	}
-	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
-	btrfs_release_path(path);
-	ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
-	if (ret) {
-		ret = -ENOENT;
-		goto out;
-	}
-
-	ret = btrfs_del_item(trans, root, path);
-	if (ret)
-		goto out;
-
-	if (path->slots[0] == 0)
-		btrfs_prev_leaf(root, path);
-	else
-		path->slots[0]--;
-out:
-	btrfs_commit_transaction(trans, root);
-	if (ret)
-		error("failed to delete root %llu item[%llu, %u, %llu]",
-		      root->objectid, key.objectid, key.type, key.offset);
-	else
-		printf("Deleted root %llu item[%llu, %u, %llu]\n",
-		       root->objectid, key.objectid, key.type, key.offset);
-	return ret;
-}
-
 /*
  * Main entry function to check known items and update related accounting info
  */
@@ -4419,7 +4422,7 @@ again:
 		ret = check_block_group_item(fs_info, eb, slot);
 		if (repair &&
 		    ret & REFERENCER_MISSING)
-			ret = delete_extent_tree_item(root, path);
+			ret = delete_item(root, path);
 		err |= ret;
 		break;
 	case BTRFS_DEV_ITEM_KEY:
@@ -4450,7 +4453,7 @@ again:
 					       key.objectid, -1);
 		if (repair &&
 		    ret & (REFERENCER_MISMATCH | REFERENCER_MISSING))
-			ret = delete_extent_tree_item(root, path);
+			ret = delete_item(root, path);
 		err |= ret;
 		break;
 	case BTRFS_EXTENT_DATA_REF_KEY:
@@ -4463,7 +4466,7 @@ again:
 				btrfs_extent_data_ref_count(eb, dref));
 		if (repair &&
 		    ret & (REFERENCER_MISMATCH | REFERENCER_MISSING))
-			ret = delete_extent_tree_item(root, path);
+			ret = delete_item(root, path);
 		err |= ret;
 		break;
 	case BTRFS_SHARED_BLOCK_REF_KEY:
@@ -4471,7 +4474,7 @@ again:
 						 key.objectid, -1);
 		if (repair &&
 		    ret & (REFERENCER_MISMATCH | REFERENCER_MISSING))
-			ret = delete_extent_tree_item(root, path);
+			ret = delete_item(root, path);
 		err |= ret;
 		break;
 	case BTRFS_SHARED_DATA_REF_KEY:
@@ -4479,7 +4482,7 @@ again:
 						key.objectid);
 		if (repair &&
 		    ret & (REFERENCER_MISMATCH | REFERENCER_MISSING))
-			ret = delete_extent_tree_item(root, path);
+			ret = delete_item(root, path);
 		err |= ret;
 		break;
 	default:
-- 
2.19.1

Re: [PATCH 06/13] btrfs-progs: lowmem: rename delete_extent_tree_item() to delete_item()

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> The function can delete items in trees besides extent tree.
> Rename and move it for further use.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

> ---
>  check/mode-lowmem.c | 99 +++++++++++++++++++++++----------------------
>  1 file changed, 51 insertions(+), 48 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index e8a2e825c0f3..3c9ecff7e498 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -537,6 +537,52 @@ static int end_avoid_extents_overwrite(struct btrfs_fs_info *fs_info)
>  	return ret;
>  }
>  
> +/*
> + * Delete the item @path point to.

Although I'd prefer to add some more words about the fact it's a wrapper
for btrfs_del_item().

Thanks,
Qu

> + * If deleted, path will point to the previous item to the deleted item.
> + */
> +static int delete_item(struct btrfs_root *root, struct btrfs_path *path)
> +{
> +	struct btrfs_key key;
> +	struct btrfs_trans_handle *trans;
> +	int ret = 0;
> +
> +	ret = avoid_extents_overwrite(root->fs_info);
> +	if (ret)
> +		return ret;
> +	trans = btrfs_start_transaction(root, 1);
> +	if (IS_ERR(trans)) {
> +		ret = PTR_ERR(trans);
> +		error("fail to start transaction %s", strerror(-ret));
> +		goto out;
> +	}
> +	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +	btrfs_release_path(path);
> +	ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
> +	if (ret) {
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	ret = btrfs_del_item(trans, root, path);
> +	if (ret)
> +		goto out;
> +
> +	if (path->slots[0] == 0)
> +		btrfs_prev_leaf(root, path);
> +	else
> +		path->slots[0]--;
> +out:
> +	btrfs_commit_transaction(trans, root);
> +	if (ret)
> +		error("failed to delete root %llu item[%llu, %u, %llu]",
> +		      root->objectid, key.objectid, key.type, key.offset);
> +	else
> +		printf("Deleted root %llu item[%llu, %u, %llu]\n",
> +		       root->objectid, key.objectid, key.type, key.offset);
> +	return ret;
> +}
> +
>  /*
>   * Wrapper function for btrfs_fix_block_accounting().
>   *
> @@ -4335,49 +4381,6 @@ static int repair_chunk_item(struct btrfs_root *chunk_root,
>  	return err;
>  }
>  
> -static int delete_extent_tree_item(struct btrfs_root *root,
> -				   struct btrfs_path *path)
> -{
> -	struct btrfs_key key;
> -	struct btrfs_trans_handle *trans;
> -	int ret = 0;
> -
> -	ret = avoid_extents_overwrite(root->fs_info);
> -	if (ret)
> -		return ret;
> -	trans = btrfs_start_transaction(root, 1);
> -	if (IS_ERR(trans)) {
> -		ret = PTR_ERR(trans);
> -		error("fail to start transaction %s", strerror(-ret));
> -		goto out;
> -	}
> -	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> -	btrfs_release_path(path);
> -	ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
> -	if (ret) {
> -		ret = -ENOENT;
> -		goto out;
> -	}
> -
> -	ret = btrfs_del_item(trans, root, path);
> -	if (ret)
> -		goto out;
> -
> -	if (path->slots[0] == 0)
> -		btrfs_prev_leaf(root, path);
> -	else
> -		path->slots[0]--;
> -out:
> -	btrfs_commit_transaction(trans, root);
> -	if (ret)
> -		error("failed to delete root %llu item[%llu, %u, %llu]",
> -		      root->objectid, key.objectid, key.type, key.offset);
> -	else
> -		printf("Deleted root %llu item[%llu, %u, %llu]\n",
> -		       root->objectid, key.objectid, key.type, key.offset);
> -	return ret;
> -}
> -
>  /*
>   * Main entry function to check known items and update related accounting info
>   */
> @@ -4419,7 +4422,7 @@ again:
>  		ret = check_block_group_item(fs_info, eb, slot);
>  		if (repair &&
>  		    ret & REFERENCER_MISSING)
> -			ret = delete_extent_tree_item(root, path);
> +			ret = delete_item(root, path);
>  		err |= ret;
>  		break;
>  	case BTRFS_DEV_ITEM_KEY:
> @@ -4450,7 +4453,7 @@ again:
>  					       key.objectid, -1);
>  		if (repair &&
>  		    ret & (REFERENCER_MISMATCH | REFERENCER_MISSING))
> -			ret = delete_extent_tree_item(root, path);
> +			ret = delete_item(root, path);
>  		err |= ret;
>  		break;
>  	case BTRFS_EXTENT_DATA_REF_KEY:
> @@ -4463,7 +4466,7 @@ again:
>  				btrfs_extent_data_ref_count(eb, dref));
>  		if (repair &&
>  		    ret & (REFERENCER_MISMATCH | REFERENCER_MISSING))
> -			ret = delete_extent_tree_item(root, path);
> +			ret = delete_item(root, path);
>  		err |= ret;
>  		break;
>  	case BTRFS_SHARED_BLOCK_REF_KEY:
> @@ -4471,7 +4474,7 @@ again:
>  						 key.objectid, -1);
>  		if (repair &&
>  		    ret & (REFERENCER_MISMATCH | REFERENCER_MISSING))
> -			ret = delete_extent_tree_item(root, path);
> +			ret = delete_item(root, path);
>  		err |= ret;
>  		break;
>  	case BTRFS_SHARED_DATA_REF_KEY:
> @@ -4479,7 +4482,7 @@ again:
>  						key.objectid);
>  		if (repair &&
>  		    ret & (REFERENCER_MISMATCH | REFERENCER_MISSING))
> -			ret = delete_extent_tree_item(root, path);
> +			ret = delete_item(root, path);
>  		err |= ret;
>  		break;
>  	default:
> 

[PATCH 07/13] btrfs-progs: lowmem: delete unaligned bytes extent data under repair

[Su Yue]

If found a extent data item has unaligned part, lowmem repair
just deletes it.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 check/mode-lowmem.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 3c9ecff7e498..5381096fa8b2 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2972,6 +2972,7 @@ out:
 }
 
 /*
+ * If @err contains BYTES_UNALIGNED then delete the extent data item.
  * If @err contains BACKREF_MISSING then add extent of the
  * file_extent_data_item.
  *
@@ -3023,6 +3024,13 @@ static int repair_extent_data_item(struct btrfs_root *root,
 	else
 		parent = 0;
 
+	if (err & BYTES_UNALIGNED) {
+		ret = delete_item(root, pathp);
+		if (!ret)
+			err = 0;
+		goto out;
+	}
+
 	/* now repair only adds backref */
 	if ((err & BACKREF_MISSING) == 0)
 		return err;
-- 
2.19.1

Re: [PATCH 07/13] btrfs-progs: lowmem: delete unaligned bytes extent data under repair

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> If found a extent data item has unaligned part, lowmem repair
> just deletes it.

Fair enough solution.

Much better than the unpredictable original mode solution.

> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  check/mode-lowmem.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 3c9ecff7e498..5381096fa8b2 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -2972,6 +2972,7 @@ out:
>  }
>  
>  /*
> + * If @err contains BYTES_UNALIGNED then delete the extent data item.
>   * If @err contains BACKREF_MISSING then add extent of the
>   * file_extent_data_item.
>   *
> @@ -3023,6 +3024,13 @@ static int repair_extent_data_item(struct btrfs_root *root,
>  	else
>  		parent = 0;
>  
> +	if (err & BYTES_UNALIGNED) {
> +		ret = delete_item(root, pathp);
> +		if (!ret)
> +			err = 0;
> +		goto out;
> +	}
> +
>  	/* now repair only adds backref */
>  	if ((err & BACKREF_MISSING) == 0)
>  		return err;
> 

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

[Su Yue]

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(-)

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;
-- 
2.19.1

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

[Qu Wenruo]

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;
> 

[PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."

[Su Yue]

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          | 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;
-- 
2.19.1

Re: [PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."

[Qu Wenruo]

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.
> 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 <suyj.fnst@cn.fujitsu.com>
> ---
>  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;
> 

Re: [PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."

[Su Yanjun <suyj.fnst@cn.fujitsu.com>]

On 10/24/2018 8:29 AM, Qu Wenruo wrote:
>
> 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.
>> 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.

Thanks,
Su

>> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>> ---
>>   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;
>>
>

Re: [PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."

[Qu Wenruo]

On 2018/11/7 下午5:09, Su Yanjun <suyj.fnst@cn.fujitsu.com> 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@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.
>>>
>> 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@cn.fujitsu.com>
>>> ---
>>>   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;
>>>
>>
> 
> 
> 

[PATCH 10/13] btrfs-progs: check: fix bug in find_possible_backrefs

[Su Yue]

From: Su Yanjun <suyj.fnst@cn.fujitsu.com>

It may cost more time to search all extent data of correspond files but
should not influence total speed too much cause that only corrupted
extent items are participated in.

Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
---
 check/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 92 insertions(+), 18 deletions(-)

diff --git a/check/main.c b/check/main.c
index bd1f322e0f12..90d9fd570287 100644
--- a/check/main.c
+++ b/check/main.c
@@ -7015,6 +7015,89 @@ out:
 	return ret ? ret : nr_del;
 }
 
+/*
+ * Based extent backref item, we find all file extent items in the fs tree. By
+ * the info we can rebuild the extent backref item
+ */
+static int __find_possible_backrefs(struct btrfs_root *root,
+		u64 owner, u64 offset, u64 bytenr, u64 *refs_ret,
+		u64 *bytes_ret)
+{
+	int ret = 0;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct btrfs_key found_key;
+	struct btrfs_file_extent_item *fi;
+	struct extent_buffer *leaf;
+	u64 backref_offset, disk_bytenr;
+	int slot;
+
+	btrfs_init_path(&path);
+
+	key.objectid = owner;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret) {
+		btrfs_release_path(&path);
+		return ret;
+	}
+
+	btrfs_release_path(&path);
+
+	key.objectid = owner;
+	key.type = BTRFS_EXTENT_DATA_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret < 0) {
+		btrfs_release_path(&path);
+		return ret;
+	}
+
+	while (1) {
+		leaf = path.nodes[0];
+		slot = path.slots[0];
+
+		if (slot >= btrfs_header_nritems(leaf)) {
+			ret = btrfs_next_leaf(root, &path);
+			if (ret) {
+				if (ret > 0)
+					ret = 0;
+				break;
+			}
+
+			leaf = path.nodes[0];
+			slot = path.slots[0];
+		}
+
+		btrfs_item_key_to_cpu(leaf, &found_key, slot);
+		if ((found_key.objectid != owner) ||
+			(found_key.type != BTRFS_EXTENT_DATA_KEY))
+			break;
+
+		fi = btrfs_item_ptr(leaf, slot,
+				struct btrfs_file_extent_item);
+
+		backref_offset = found_key.offset -
+			btrfs_file_extent_offset(leaf, fi);
+		disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
+		*bytes_ret = btrfs_file_extent_disk_num_bytes(leaf,
+								fi);
+		if ((disk_bytenr == bytenr) &&
+			(backref_offset == offset)) {
+			(*refs_ret)++;
+		}
+		path.slots[0]++;
+	}
+
+	btrfs_release_path(&path);
+	return ret;
+}
+
 static int find_possible_backrefs(struct btrfs_fs_info *info,
 				  struct btrfs_path *path,
 				  struct cache_tree *extent_cache,
@@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
 	struct extent_backref *back, *tmp;
 	struct data_backref *dback;
 	struct cache_extent *cache;
-	struct btrfs_file_extent_item *fi;
 	struct btrfs_key key;
 	u64 bytenr, bytes;
+	u64 refs;
 	int ret;
 
 	rbtree_postorder_for_each_entry_safe(back, tmp,
@@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
 		if (IS_ERR(root))
 			return PTR_ERR(root);
 
-		key.objectid = dback->owner;
-		key.type = BTRFS_EXTENT_DATA_KEY;
-		key.offset = dback->offset;
-		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-		if (ret) {
-			btrfs_release_path(path);
-			if (ret < 0)
-				return ret;
-			/* Didn't find it, we can carry on */
-			ret = 0;
+		refs = 0;
+		bytes = 0;
+		ret = __find_possible_backrefs(root, dback->owner,
+				dback->offset, rec->start, &refs, &bytes);
+		if (ret)
 			continue;
-		}
 
-		fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
-				    struct btrfs_file_extent_item);
-		bytenr = btrfs_file_extent_disk_bytenr(path->nodes[0], fi);
-		bytes = btrfs_file_extent_disk_num_bytes(path->nodes[0], fi);
-		btrfs_release_path(path);
+		bytenr = rec->start;
+
 		cache = lookup_cache_extent(extent_cache, bytenr, 1);
 		if (cache) {
 			struct extent_record *tmp;
@@ -7090,7 +7164,7 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
 				continue;
 		}
 
-		dback->found_ref += 1;
+		dback->found_ref += refs;
 		dback->disk_bytenr = bytenr;
 		dback->bytes = bytes;
 
-- 
2.19.1

Re: [PATCH 10/13] btrfs-progs: check: fix bug in find_possible_backrefs

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> 
> It may cost more time to search all extent data of correspond files but
> should not influence total speed too much cause that only corrupted
> extent items are participated in.

Sorry, I didn't really get the point of the modification from the commit
message.

Would you please explain the purpose of this patch first?

Thanks,
Qu

> 
> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> ---
>  check/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 92 insertions(+), 18 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index bd1f322e0f12..90d9fd570287 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -7015,6 +7015,89 @@ out:
>  	return ret ? ret : nr_del;
>  }
>  
> +/*
> + * Based extent backref item, we find all file extent items in the fs tree. By
> + * the info we can rebuild the extent backref item
> + */
> +static int __find_possible_backrefs(struct btrfs_root *root,
> +		u64 owner, u64 offset, u64 bytenr, u64 *refs_ret,
> +		u64 *bytes_ret)
> +{
> +	int ret = 0;
> +	struct btrfs_path path;
> +	struct btrfs_key key;
> +	struct btrfs_key found_key;
> +	struct btrfs_file_extent_item *fi;
> +	struct extent_buffer *leaf;
> +	u64 backref_offset, disk_bytenr;
> +	int slot;
> +
> +	btrfs_init_path(&path);
> +
> +	key.objectid = owner;
> +	key.type = BTRFS_INODE_ITEM_KEY;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret > 0)
> +		ret = -ENOENT;
> +	if (ret) {
> +		btrfs_release_path(&path);
> +		return ret;
> +	}
> +
> +	btrfs_release_path(&path);
> +
> +	key.objectid = owner;
> +	key.type = BTRFS_EXTENT_DATA_KEY;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret < 0) {
> +		btrfs_release_path(&path);
> +		return ret;
> +	}
> +
> +	while (1) {
> +		leaf = path.nodes[0];
> +		slot = path.slots[0];
> +
> +		if (slot >= btrfs_header_nritems(leaf)) {
> +			ret = btrfs_next_leaf(root, &path);
> +			if (ret) {
> +				if (ret > 0)
> +					ret = 0;
> +				break;
> +			}
> +
> +			leaf = path.nodes[0];
> +			slot = path.slots[0];
> +		}
> +
> +		btrfs_item_key_to_cpu(leaf, &found_key, slot);
> +		if ((found_key.objectid != owner) ||
> +			(found_key.type != BTRFS_EXTENT_DATA_KEY))
> +			break;
> +
> +		fi = btrfs_item_ptr(leaf, slot,
> +				struct btrfs_file_extent_item);
> +
> +		backref_offset = found_key.offset -
> +			btrfs_file_extent_offset(leaf, fi);
> +		disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
> +		*bytes_ret = btrfs_file_extent_disk_num_bytes(leaf,
> +								fi);
> +		if ((disk_bytenr == bytenr) &&
> +			(backref_offset == offset)) {
> +			(*refs_ret)++;
> +		}
> +		path.slots[0]++;
> +	}
> +
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> +
>  static int find_possible_backrefs(struct btrfs_fs_info *info,
>  				  struct btrfs_path *path,
>  				  struct cache_tree *extent_cache,
> @@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
>  	struct extent_backref *back, *tmp;
>  	struct data_backref *dback;
>  	struct cache_extent *cache;
> -	struct btrfs_file_extent_item *fi;
>  	struct btrfs_key key;
>  	u64 bytenr, bytes;
> +	u64 refs;
>  	int ret;
>  
>  	rbtree_postorder_for_each_entry_safe(back, tmp,
> @@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
>  		if (IS_ERR(root))
>  			return PTR_ERR(root);
>  
> -		key.objectid = dback->owner;
> -		key.type = BTRFS_EXTENT_DATA_KEY;
> -		key.offset = dback->offset;
> -		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> -		if (ret) {
> -			btrfs_release_path(path);
> -			if (ret < 0)
> -				return ret;
> -			/* Didn't find it, we can carry on */
> -			ret = 0;
> +		refs = 0;
> +		bytes = 0;
> +		ret = __find_possible_backrefs(root, dback->owner,
> +				dback->offset, rec->start, &refs, &bytes);
> +		if (ret)
>  			continue;
> -		}
>  
> -		fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
> -				    struct btrfs_file_extent_item);
> -		bytenr = btrfs_file_extent_disk_bytenr(path->nodes[0], fi);
> -		bytes = btrfs_file_extent_disk_num_bytes(path->nodes[0], fi);
> -		btrfs_release_path(path);
> +		bytenr = rec->start;
> +
>  		cache = lookup_cache_extent(extent_cache, bytenr, 1);
>  		if (cache) {
>  			struct extent_record *tmp;
> @@ -7090,7 +7164,7 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
>  				continue;
>  		}
>  
> -		dback->found_ref += 1;
> +		dback->found_ref += refs;
>  		dback->disk_bytenr = bytenr;
>  		dback->bytes = bytes;
>  
> 

Re: [PATCH 10/13] btrfs-progs: check: fix bug in find_possible_backrefs

[Su Yanjun <suyj.fnst@cn.fujitsu.com>]

On 10/24/2018 8:34 AM, Qu Wenruo wrote:
>
> On 2018/10/23 下午5:41, Su Yue wrote:
>> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>>
>> It may cost more time to search all extent data of correspond files but
>> should not influence total speed too much cause that only corrupted
>> extent items are participated in.
> Sorry, I didn't really get the point of the modification from the commit
> message.
>
> Would you please explain the purpose of this patch first?
>
> Thanks,
> Qu

In find_possible_backrefs() function:

	key.objectid = dback->owner;
	key.type = BTRFS_EXTENT_DATA_KEY;
	key.offset = dback->offset;//
	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);

'dback->offset' is not the offset in file range, we'll find wrong file extent.

Thanks,

Su

>> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>> ---
>>   check/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++---------
>>   1 file changed, 92 insertions(+), 18 deletions(-)
>>
>> diff --git a/check/main.c b/check/main.c
>> index bd1f322e0f12..90d9fd570287 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -7015,6 +7015,89 @@ out:
>>   	return ret ? ret : nr_del;
>>   }
>>   
>> +/*
>> + * Based extent backref item, we find all file extent items in the fs tree. By
>> + * the info we can rebuild the extent backref item
>> + */
>> +static int __find_possible_backrefs(struct btrfs_root *root,
>> +		u64 owner, u64 offset, u64 bytenr, u64 *refs_ret,
>> +		u64 *bytes_ret)
>> +{
>> +	int ret = 0;
>> +	struct btrfs_path path;
>> +	struct btrfs_key key;
>> +	struct btrfs_key found_key;
>> +	struct btrfs_file_extent_item *fi;
>> +	struct extent_buffer *leaf;
>> +	u64 backref_offset, disk_bytenr;
>> +	int slot;
>> +
>> +	btrfs_init_path(&path);
>> +
>> +	key.objectid = owner;
>> +	key.type = BTRFS_INODE_ITEM_KEY;
>> +	key.offset = 0;
>> +
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +	if (ret > 0)
>> +		ret = -ENOENT;
>> +	if (ret) {
>> +		btrfs_release_path(&path);
>> +		return ret;
>> +	}
>> +
>> +	btrfs_release_path(&path);
>> +
>> +	key.objectid = owner;
>> +	key.type = BTRFS_EXTENT_DATA_KEY;
>> +	key.offset = 0;
>> +
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +	if (ret < 0) {
>> +		btrfs_release_path(&path);
>> +		return ret;
>> +	}
>> +
>> +	while (1) {
>> +		leaf = path.nodes[0];
>> +		slot = path.slots[0];
>> +
>> +		if (slot >= btrfs_header_nritems(leaf)) {
>> +			ret = btrfs_next_leaf(root, &path);
>> +			if (ret) {
>> +				if (ret > 0)
>> +					ret = 0;
>> +				break;
>> +			}
>> +
>> +			leaf = path.nodes[0];
>> +			slot = path.slots[0];
>> +		}
>> +
>> +		btrfs_item_key_to_cpu(leaf, &found_key, slot);
>> +		if ((found_key.objectid != owner) ||
>> +			(found_key.type != BTRFS_EXTENT_DATA_KEY))
>> +			break;
>> +
>> +		fi = btrfs_item_ptr(leaf, slot,
>> +				struct btrfs_file_extent_item);
>> +
>> +		backref_offset = found_key.offset -
>> +			btrfs_file_extent_offset(leaf, fi);
>> +		disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
>> +		*bytes_ret = btrfs_file_extent_disk_num_bytes(leaf,
>> +								fi);
>> +		if ((disk_bytenr == bytenr) &&
>> +			(backref_offset == offset)) {
>> +			(*refs_ret)++;
>> +		}
>> +		path.slots[0]++;
>> +	}
>> +
>> +	btrfs_release_path(&path);
>> +	return ret;
>> +}
>> +
>>   static int find_possible_backrefs(struct btrfs_fs_info *info,
>>   				  struct btrfs_path *path,
>>   				  struct cache_tree *extent_cache,
>> @@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
>>   	struct extent_backref *back, *tmp;
>>   	struct data_backref *dback;
>>   	struct cache_extent *cache;
>> -	struct btrfs_file_extent_item *fi;
>>   	struct btrfs_key key;
>>   	u64 bytenr, bytes;
>> +	u64 refs;
>>   	int ret;
>>   
>>   	rbtree_postorder_for_each_entry_safe(back, tmp,
>> @@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
>>   		if (IS_ERR(root))
>>   			return PTR_ERR(root);
>>   
>> -		key.objectid = dback->owner;
>> -		key.type = BTRFS_EXTENT_DATA_KEY;
>> -		key.offset = dback->offset;
>> -		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>> -		if (ret) {
>> -			btrfs_release_path(path);
>> -			if (ret < 0)
>> -				return ret;
>> -			/* Didn't find it, we can carry on */
>> -			ret = 0;
>> +		refs = 0;
>> +		bytes = 0;
>> +		ret = __find_possible_backrefs(root, dback->owner,
>> +				dback->offset, rec->start, &refs, &bytes);
>> +		if (ret)
>>   			continue;
>> -		}
>>   
>> -		fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> -				    struct btrfs_file_extent_item);
>> -		bytenr = btrfs_file_extent_disk_bytenr(path->nodes[0], fi);
>> -		bytes = btrfs_file_extent_disk_num_bytes(path->nodes[0], fi);
>> -		btrfs_release_path(path);
>> +		bytenr = rec->start;
>> +
>>   		cache = lookup_cache_extent(extent_cache, bytenr, 1);
>>   		if (cache) {
>>   			struct extent_record *tmp;
>> @@ -7090,7 +7164,7 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,
>>   				continue;
>>   		}
>>   
>> -		dback->found_ref += 1;
>> +		dback->found_ref += refs;
>>   		dback->disk_bytenr = bytenr;
>>   		dback->bytes = bytes;
>>   
>>
>

Re: [PATCH 10/13] btrfs-progs: check: fix bug in find_possible_backrefs

[Qu Wenruo]

On 2018/11/7 下午2:28, Su Yanjun <suyj.fnst@cn.fujitsu.com> wrote:
> 
> 
> On 10/24/2018 8:34 AM, Qu Wenruo wrote:
>>
>> On 2018/10/23 下午5:41, Su Yue wrote:
>>> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>>>
>>> It may cost more time to search all extent data of correspond files but
>>> should not influence total speed too much cause that only corrupted
>>> extent items are participated in.
>> Sorry, I didn't really get the point of the modification from the commit
>> message.
>>
>> Would you please explain the purpose of this patch first?
>>
>> Thanks,
>> Qu
> 
> In find_possible_backrefs() function:
> 
>     key.objectid = dback->owner;
>     key.type = BTRFS_EXTENT_DATA_KEY;
>     key.offset = dback->offset;//
>     ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> 
> 'dback->offset' is not the offset in file range, we'll find wrong file
> extent.

Now this makes sense.

The original commit message doesn't touch this part, and focused on the
side effect, not the problem itself.

Please update the commit message.

Thanks,
Qu

> 
> Thanks,
> 
> Su
> 
>>> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>>> ---
>>>   check/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++---------
>>>   1 file changed, 92 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/check/main.c b/check/main.c
>>> index bd1f322e0f12..90d9fd570287 100644
>>> --- a/check/main.c
>>> +++ b/check/main.c
>>> @@ -7015,6 +7015,89 @@ out:
>>>       return ret ? ret : nr_del;
>>>   }
>>>   +/*
>>> + * Based extent backref item, we find all file extent items in the
>>> fs tree. By
>>> + * the info we can rebuild the extent backref item
>>> + */
>>> +static int __find_possible_backrefs(struct btrfs_root *root,
>>> +        u64 owner, u64 offset, u64 bytenr, u64 *refs_ret,
>>> +        u64 *bytes_ret)
>>> +{
>>> +    int ret = 0;
>>> +    struct btrfs_path path;
>>> +    struct btrfs_key key;
>>> +    struct btrfs_key found_key;
>>> +    struct btrfs_file_extent_item *fi;
>>> +    struct extent_buffer *leaf;
>>> +    u64 backref_offset, disk_bytenr;
>>> +    int slot;
>>> +
>>> +    btrfs_init_path(&path);
>>> +
>>> +    key.objectid = owner;
>>> +    key.type = BTRFS_INODE_ITEM_KEY;
>>> +    key.offset = 0;
>>> +
>>> +    ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>>> +    if (ret > 0)
>>> +        ret = -ENOENT;
>>> +    if (ret) {
>>> +        btrfs_release_path(&path);
>>> +        return ret;
>>> +    }
>>> +
>>> +    btrfs_release_path(&path);
>>> +
>>> +    key.objectid = owner;
>>> +    key.type = BTRFS_EXTENT_DATA_KEY;
>>> +    key.offset = 0;
>>> +
>>> +    ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>>> +    if (ret < 0) {
>>> +        btrfs_release_path(&path);
>>> +        return ret;
>>> +    }
>>> +
>>> +    while (1) {
>>> +        leaf = path.nodes[0];
>>> +        slot = path.slots[0];
>>> +
>>> +        if (slot >= btrfs_header_nritems(leaf)) {
>>> +            ret = btrfs_next_leaf(root, &path);
>>> +            if (ret) {
>>> +                if (ret > 0)
>>> +                    ret = 0;
>>> +                break;
>>> +            }
>>> +
>>> +            leaf = path.nodes[0];
>>> +            slot = path.slots[0];
>>> +        }
>>> +
>>> +        btrfs_item_key_to_cpu(leaf, &found_key, slot);
>>> +        if ((found_key.objectid != owner) ||
>>> +            (found_key.type != BTRFS_EXTENT_DATA_KEY))
>>> +            break;
>>> +
>>> +        fi = btrfs_item_ptr(leaf, slot,
>>> +                struct btrfs_file_extent_item);
>>> +
>>> +        backref_offset = found_key.offset -
>>> +            btrfs_file_extent_offset(leaf, fi);
>>> +        disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
>>> +        *bytes_ret = btrfs_file_extent_disk_num_bytes(leaf,
>>> +                                fi);
>>> +        if ((disk_bytenr == bytenr) &&
>>> +            (backref_offset == offset)) {
>>> +            (*refs_ret)++;
>>> +        }
>>> +        path.slots[0]++;
>>> +    }
>>> +
>>> +    btrfs_release_path(&path);
>>> +    return ret;
>>> +}
>>> +
>>>   static int find_possible_backrefs(struct btrfs_fs_info *info,
>>>                     struct btrfs_path *path,
>>>                     struct cache_tree *extent_cache,
>>> @@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct
>>> btrfs_fs_info *info,
>>>       struct extent_backref *back, *tmp;
>>>       struct data_backref *dback;
>>>       struct cache_extent *cache;
>>> -    struct btrfs_file_extent_item *fi;
>>>       struct btrfs_key key;
>>>       u64 bytenr, bytes;
>>> +    u64 refs;
>>>       int ret;
>>>         rbtree_postorder_for_each_entry_safe(back, tmp,
>>> @@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct
>>> btrfs_fs_info *info,
>>>           if (IS_ERR(root))
>>>               return PTR_ERR(root);
>>>   -        key.objectid = dback->owner;
>>> -        key.type = BTRFS_EXTENT_DATA_KEY;
>>> -        key.offset = dback->offset;
>>> -        ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
>>> -        if (ret) {
>>> -            btrfs_release_path(path);
>>> -            if (ret < 0)
>>> -                return ret;
>>> -            /* Didn't find it, we can carry on */
>>> -            ret = 0;
>>> +        refs = 0;
>>> +        bytes = 0;
>>> +        ret = __find_possible_backrefs(root, dback->owner,
>>> +                dback->offset, rec->start, &refs, &bytes);
>>> +        if (ret)
>>>               continue;
>>> -        }
>>>   -        fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
>>> -                    struct btrfs_file_extent_item);
>>> -        bytenr = btrfs_file_extent_disk_bytenr(path->nodes[0], fi);
>>> -        bytes = btrfs_file_extent_disk_num_bytes(path->nodes[0], fi);
>>> -        btrfs_release_path(path);
>>> +        bytenr = rec->start;
>>> +
>>>           cache = lookup_cache_extent(extent_cache, bytenr, 1);
>>>           if (cache) {
>>>               struct extent_record *tmp;
>>> @@ -7090,7 +7164,7 @@ static int find_possible_backrefs(struct
>>> btrfs_fs_info *info,
>>>                   continue;
>>>           }
>>>   -        dback->found_ref += 1;
>>> +        dback->found_ref += refs;
>>>           dback->disk_bytenr = bytenr;
>>>           dback->bytes = bytes;
>>>  
>>
> 
> 
> 

[PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref

[Su Yue]

From: Su Yanjun <suyj.fnst@cn.fujitsu.com>

In original mode, if some file extent item has unaligned extent backref,
fixup_extent_refs can't repair it. This patch will check extent alignment
then delete file extent with unaligned extent backref.

Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
---
 check/main.c          | 278 +++++++++++++++++++++++++++++++++++++++++-
 check/mode-original.h |  13 ++
 ctree.h               |   2 +
 disk-io.c             |   1 +
 4 files changed, 293 insertions(+), 1 deletion(-)

diff --git a/check/main.c b/check/main.c
index 90d9fd570287..b5e68b3241e5 100644
--- a/check/main.c
+++ b/check/main.c
@@ -460,6 +460,8 @@ 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 unaligned_extent_rec_t *src;
+	struct unaligned_extent_rec_t *dst;
 	struct rb_node *rb;
 	size_t size;
 	int ret;
@@ -470,6 +472,7 @@ 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->unaligned_extent_recs);
 	rec->holes = RB_ROOT;
 
 	list_for_each_entry(orig, &orig_rec->backrefs, list) {
@@ -483,6 +486,17 @@ static struct inode_record *clone_inode_rec(struct inode_record *orig_rec)
 		list_add_tail(&backref->list, &rec->backrefs);
 	}
 
+	list_for_each_entry(src, &orig_rec->unaligned_extent_recs, list) {
+		size = sizeof(*src);
+		dst = malloc(size);
+		if (!dst) {
+			ret = -ENOMEM;
+			goto cleanup;
+		}
+		memcpy(dst, src, size);
+		list_add_tail(&dst->list, &rec->unaligned_extent_recs);
+	}
+
 	ret = copy_file_extent_holes(&rec->holes, &orig_rec->holes);
 	if (ret < 0)
 		goto cleanup_rb;
@@ -506,6 +520,13 @@ cleanup:
 			free(orig);
 		}
 
+	if (!list_empty(&rec->unaligned_extent_recs))
+		list_for_each_entry_safe(src, dst, &rec->unaligned_extent_recs,
+				list) {
+			list_del(&src->list);
+			free(src);
+		}
+
 	free(rec);
 
 	return ERR_PTR(ret);
@@ -643,6 +664,7 @@ 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->unaligned_extent_recs);
 		rec->holes = RB_ROOT;
 
 		node = malloc(sizeof(*node));
@@ -664,6 +686,18 @@ static struct inode_record *get_inode_rec(struct cache_tree *inode_cache,
 	return rec;
 }
 
+static void free_unaligned_extent_recs(struct list_head *unaligned_extent_recs)
+{
+	struct unaligned_extent_rec_t *urec;
+
+	while (!list_empty(unaligned_extent_recs)) {
+		urec = list_entry(unaligned_extent_recs->next,
+				struct unaligned_extent_rec_t, list);
+		list_del(&urec->list);
+		free(urec);
+	}
+}
+
 static void free_inode_rec(struct inode_record *rec)
 {
 	struct inode_backref *backref;
@@ -676,6 +710,7 @@ static void free_inode_rec(struct inode_record *rec)
 		list_del(&backref->list);
 		free(backref);
 	}
+	free_unaligned_extent_recs(&rec->unaligned_extent_recs);
 	free_file_extent_holes(&rec->holes);
 	free(rec);
 }
@@ -2474,18 +2509,154 @@ out:
 	return ret;
 }
 
+static int btrfs_delete_item(struct btrfs_trans_handle *trans,
+		struct btrfs_root *root, struct btrfs_key *key)
+{
+	struct btrfs_path path;
+	int ret = 0;
+
+	btrfs_init_path(&path);
+
+	ret = btrfs_search_slot(trans, root, key, &path, -1, 1);
+	if (ret) {
+		if (ret > 0)
+			ret = -ENOENT;
+
+		btrfs_release_path(&path);
+		return ret;
+	}
+
+	ret = btrfs_del_item(trans, root, &path);
+
+	btrfs_release_path(&path);
+	return ret;
+}
+
+static int find_file_extent_offset_by_bytenr(struct btrfs_root *root,
+		u64 owner, u64 bytenr, u64 *offset_ret)
+{
+	int ret = 0;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct btrfs_key found_key;
+	struct btrfs_file_extent_item *fi;
+	struct extent_buffer *leaf;
+	u64 disk_bytenr;
+	int slot;
+
+	btrfs_init_path(&path);
+
+	key.objectid = owner;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret) {
+		if (ret > 0)
+			ret = -ENOENT;
+		btrfs_release_path(&path);
+		return ret;
+	}
+
+	btrfs_release_path(&path);
+
+	key.objectid = owner;
+	key.type = BTRFS_EXTENT_DATA_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret < 0) {
+		btrfs_release_path(&path);
+		return ret;
+	}
+
+	while (1) {
+		leaf = path.nodes[0];
+		slot = path.slots[0];
+
+		if (slot >= btrfs_header_nritems(leaf)) {
+			ret = btrfs_next_leaf(root, &path);
+			if (ret) {
+				if (ret > 0)
+					ret = 0;
+				break;
+			}
+
+			leaf = path.nodes[0];
+			slot = path.slots[0];
+		}
+
+		btrfs_item_key_to_cpu(leaf, &found_key, slot);
+		if ((found_key.objectid != owner) ||
+			(found_key.type != BTRFS_EXTENT_DATA_KEY))
+			break;
+
+		fi = btrfs_item_ptr(leaf, slot,
+				struct btrfs_file_extent_item);
+
+		disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
+		if (disk_bytenr == bytenr) {
+			*offset_ret = found_key.offset;
+			ret = 0;
+			break;
+		}
+		path.slots[0]++;
+	}
+
+	btrfs_release_path(&path);
+	return ret;
+}
+
+static int repair_unaligned_extent_recs(struct btrfs_trans_handle *trans,
+				struct btrfs_root *root,
+				struct btrfs_path *path,
+				struct inode_record *rec)
+{
+	int ret = 0;
+	struct btrfs_key key;
+	struct unaligned_extent_rec_t *urec;
+	struct unaligned_extent_rec_t *tmp;
+
+	list_for_each_entry_safe(urec, tmp, &rec->unaligned_extent_recs, list) {
+
+		key.objectid = urec->owner;
+		key.type = BTRFS_EXTENT_DATA_KEY;
+		key.offset = urec->offset;
+		fprintf(stderr, "delete file extent item [%llu,%llu]\n",
+					urec->owner, urec->offset);
+		ret = btrfs_delete_item(trans, root, &key);
+		if (ret)
+			return ret;
+
+		list_del(&urec->list);
+		free(urec);
+	}
+	rec->errors &= ~I_ERR_UNALIGNED_EXTENT_REC;
+
+	return ret;
+}
+
 static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_path path;
 	int ret = 0;
 
+	/*
+	 * unaligned extent recs always lead to csum missing error, clean it
+	 */
+	if ((rec->errors & I_ERR_SOME_CSUM_MISSING) &&
+			(rec->errors & I_ERR_UNALIGNED_EXTENT_REC))
+		rec->errors &= ~I_ERR_SOME_CSUM_MISSING;
+
+
 	if (!(rec->errors & (I_ERR_DIR_ISIZE_WRONG |
 			     I_ERR_NO_ORPHAN_ITEM |
 			     I_ERR_LINK_COUNT_WRONG |
 			     I_ERR_NO_INODE_ITEM |
 			     I_ERR_FILE_EXTENT_DISCOUNT |
 			     I_ERR_FILE_NBYTES_WRONG |
+			     I_ERR_UNALIGNED_EXTENT_REC |
 			     I_ERR_INLINE_RAM_BYTES_WRONG)))
 		return rec->errors;
 
@@ -2515,6 +2686,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 		ret = repair_inode_nbytes(trans, root, &path, rec);
 	if (!ret && rec->errors & I_ERR_INLINE_RAM_BYTES_WRONG)
 		ret = repair_inline_ram_bytes(trans, root, &path, rec);
+	if (!ret && rec->errors & I_ERR_UNALIGNED_EXTENT_REC)
+		ret = repair_unaligned_extent_recs(trans, root, &path, rec);
 	btrfs_commit_transaction(trans, root);
 	btrfs_release_path(&path);
 	return ret;
@@ -3128,6 +3301,8 @@ static int check_fs_root(struct btrfs_root *root,
 	struct cache_tree corrupt_blocks;
 	enum btrfs_tree_block_status status;
 	struct node_refs nrefs;
+	struct unaligned_extent_rec_t *urec;
+	struct unaligned_extent_rec_t *tmp;
 
 	/*
 	 * Reuse the corrupt_block cache tree to record corrupted tree block
@@ -3151,6 +3326,30 @@ static int check_fs_root(struct btrfs_root *root,
 	cache_tree_init(&root_node.inode_cache);
 	memset(&nrefs, 0, sizeof(nrefs));
 
+	/*
+	 * Mode unaligned extent recs to corresponding inode record
+	 */
+	list_for_each_entry_safe(urec, tmp,
+			&root->unaligned_extent_recs, list) {
+		struct inode_record *inode;
+
+		inode = get_inode_rec(&root_node.inode_cache, urec->owner, 1);
+
+		if (IS_ERR_OR_NULL(inode)) {
+			fprintf(stderr,
+				"fail to get inode rec on [%llu,%llu]\n",
+				urec->objectid, urec->owner);
+
+			list_del(&urec->list);
+			free(urec);
+
+			continue;
+		}
+
+		inode->errors |= I_ERR_UNALIGNED_EXTENT_REC;
+		list_move(&urec->list, &inode->unaligned_extent_recs);
+	}
+
 	level = btrfs_header_level(root->node);
 	memset(wc->nodes, 0, sizeof(wc->nodes));
 	wc->nodes[level] = &root_node;
@@ -7425,6 +7624,68 @@ static int prune_corrupt_blocks(struct btrfs_fs_info *info)
 	return 0;
 }
 
+static int record_unaligned_extent_rec(struct btrfs_fs_info *fs_info,
+					struct extent_record *rec)
+{
+
+	struct extent_backref *back, *tmp;
+	struct data_backref *dback;
+	struct btrfs_root *dest_root;
+	struct btrfs_key key;
+	struct unaligned_extent_rec_t *urec;
+	LIST_HEAD(entries);
+	int ret = 0;
+
+	fprintf(stderr, "record unaligned extent record on %llu %llu\n",
+			rec->start, rec->nr);
+
+	/*
+	 * Metadata is easy and the backrefs should always agree on bytenr and
+	 * size, if not we've got bigger issues.
+	 */
+	if (rec->metadata)
+		return 0;
+
+	rbtree_postorder_for_each_entry_safe(back, tmp,
+					     &rec->backref_tree, node) {
+		if (back->full_backref || !back->is_data)
+			continue;
+
+		dback = to_data_backref(back);
+
+		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_OR_NULL(dest_root))
+			continue;
+
+		urec = malloc(sizeof(struct unaligned_extent_rec_t));
+		if (!urec)
+			return -ENOMEM;
+
+		INIT_LIST_HEAD(&urec->list);
+		urec->objectid = dest_root->objectid;
+		urec->owner = dback->owner;
+		urec->offset = 0;
+		urec->bytenr = rec->start;
+		ret = find_file_extent_offset_by_bytenr(dest_root,
+				dback->owner, rec->start, &urec->offset);
+		if (ret) {
+			free(urec);
+			return ret;
+		}
+		list_add(&urec->list, &dest_root->unaligned_extent_recs);
+	}
+
+	return ret;
+}
+
 static int check_extent_refs(struct btrfs_root *root,
 			     struct cache_tree *extent_cache)
 {
@@ -7522,6 +7783,21 @@ static int check_extent_refs(struct btrfs_root *root,
 			fix = 1;
 			cur_err = 1;
 		}
+
+		if (!IS_ALIGNED(rec->start, root->fs_info->sectorsize)) {
+			fprintf(stderr, "unaligned extent rec on [%llu %llu]\n",
+				(unsigned long long)rec->start,
+				(unsigned long long)rec->nr);
+			ret = record_unaligned_extent_rec(root->fs_info, rec);
+			if (ret)
+				goto repair_abort;
+
+			/*
+			 * free extent record
+			 */
+			goto next;
+		}
+
 		if (all_backpointers_checked(rec, 1)) {
 			fprintf(stderr, "backpointer mismatch on [%llu %llu]\n",
 				(unsigned long long)rec->start,
@@ -7574,7 +7850,7 @@ static int check_extent_refs(struct btrfs_root *root,
 				rec->start, rec->start + rec->max_size);
 			cur_err = 1;
 		}
-
+next:
 		err = cur_err;
 		remove_cache_extent(extent_cache, cache);
 		free_all_extent_backrefs(rec);
diff --git a/check/mode-original.h b/check/mode-original.h
index ed995931fcd5..b23594863199 100644
--- a/check/mode-original.h
+++ b/check/mode-original.h
@@ -155,6 +155,16 @@ struct file_extent_hole {
 	u64 len;
 };
 
+struct unaligned_extent_rec_t {
+	struct list_head list;
+
+	u64 objectid;
+	u64 owner;
+	u64 offset;
+
+	u64 bytenr;
+};
+
 #define I_ERR_NO_INODE_ITEM		(1 << 0)
 #define I_ERR_NO_ORPHAN_ITEM		(1 << 1)
 #define I_ERR_DUP_INODE_ITEM		(1 << 2)
@@ -169,6 +179,7 @@ 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_UNALIGNED_EXTENT_REC	(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)
@@ -185,6 +196,8 @@ struct inode_record {
 	unsigned int nodatasum:1;
 	int errors;
 
+	struct list_head unaligned_extent_recs;
+
 	u64 ino;
 	u32 nlink;
 	u32 imode;
diff --git a/ctree.h b/ctree.h
index 2e0896390434..d0f441587f9f 100644
--- a/ctree.h
+++ b/ctree.h
@@ -1177,6 +1177,8 @@ struct btrfs_root {
 	u32 type;
 	u64 last_inode_alloc;
 
+	struct list_head unaligned_extent_recs;
+
 	/* 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 992f4b870e9f..0dfd51ed87bf 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -480,6 +480,7 @@ 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->unaligned_extent_recs);
 	memset(&root->root_key, 0, sizeof(root->root_key));
 	memset(&root->root_item, 0, sizeof(root->root_item));
 	root->root_key.objectid = objectid;
-- 
2.19.1

Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> 
> In original mode, if some file extent item has unaligned extent backref,
> fixup_extent_refs can't repair it. This patch will check extent alignment
> then delete file extent with unaligned extent backref.

This looks a little strange to me.

You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM?

Then why not just delete the EXTENT_ITEM directly? No need to go back
checking if it has a corresponding EXTENT_DATA since unaligned one is
definitely corrupted.

For corrupted EXTENT_DATA, it should get deleted when we check fs tree.

This would save you a lot of codes.

Thanks,
Qu

> 
> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> ---
>  check/main.c          | 278 +++++++++++++++++++++++++++++++++++++++++-
>  check/mode-original.h |  13 ++
>  ctree.h               |   2 +
>  disk-io.c             |   1 +
>  4 files changed, 293 insertions(+), 1 deletion(-)
> 
> diff --git a/check/main.c b/check/main.c
> index 90d9fd570287..b5e68b3241e5 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -460,6 +460,8 @@ 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 unaligned_extent_rec_t *src;
> +	struct unaligned_extent_rec_t *dst;
>  	struct rb_node *rb;
>  	size_t size;
>  	int ret;
> @@ -470,6 +472,7 @@ 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->unaligned_extent_recs);
>  	rec->holes = RB_ROOT;
>  
>  	list_for_each_entry(orig, &orig_rec->backrefs, list) {
> @@ -483,6 +486,17 @@ static struct inode_record *clone_inode_rec(struct inode_record *orig_rec)
>  		list_add_tail(&backref->list, &rec->backrefs);
>  	}
>  
> +	list_for_each_entry(src, &orig_rec->unaligned_extent_recs, list) {
> +		size = sizeof(*src);
> +		dst = malloc(size);
> +		if (!dst) {
> +			ret = -ENOMEM;
> +			goto cleanup;
> +		}
> +		memcpy(dst, src, size);
> +		list_add_tail(&dst->list, &rec->unaligned_extent_recs);
> +	}
> +
>  	ret = copy_file_extent_holes(&rec->holes, &orig_rec->holes);
>  	if (ret < 0)
>  		goto cleanup_rb;
> @@ -506,6 +520,13 @@ cleanup:
>  			free(orig);
>  		}
>  
> +	if (!list_empty(&rec->unaligned_extent_recs))
> +		list_for_each_entry_safe(src, dst, &rec->unaligned_extent_recs,
> +				list) {
> +			list_del(&src->list);
> +			free(src);
> +		}
> +
>  	free(rec);
>  
>  	return ERR_PTR(ret);
> @@ -643,6 +664,7 @@ 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->unaligned_extent_recs);
>  		rec->holes = RB_ROOT;
>  
>  		node = malloc(sizeof(*node));
> @@ -664,6 +686,18 @@ static struct inode_record *get_inode_rec(struct cache_tree *inode_cache,
>  	return rec;
>  }
>  
> +static void free_unaligned_extent_recs(struct list_head *unaligned_extent_recs)
> +{
> +	struct unaligned_extent_rec_t *urec;
> +
> +	while (!list_empty(unaligned_extent_recs)) {
> +		urec = list_entry(unaligned_extent_recs->next,
> +				struct unaligned_extent_rec_t, list);
> +		list_del(&urec->list);
> +		free(urec);
> +	}
> +}
> +
>  static void free_inode_rec(struct inode_record *rec)
>  {
>  	struct inode_backref *backref;
> @@ -676,6 +710,7 @@ static void free_inode_rec(struct inode_record *rec)
>  		list_del(&backref->list);
>  		free(backref);
>  	}
> +	free_unaligned_extent_recs(&rec->unaligned_extent_recs);
>  	free_file_extent_holes(&rec->holes);
>  	free(rec);
>  }
> @@ -2474,18 +2509,154 @@ out:
>  	return ret;
>  }
>  
> +static int btrfs_delete_item(struct btrfs_trans_handle *trans,
> +		struct btrfs_root *root, struct btrfs_key *key)
> +{
> +	struct btrfs_path path;
> +	int ret = 0;
> +
> +	btrfs_init_path(&path);
> +
> +	ret = btrfs_search_slot(trans, root, key, &path, -1, 1);
> +	if (ret) {
> +		if (ret > 0)
> +			ret = -ENOENT;
> +
> +		btrfs_release_path(&path);
> +		return ret;
> +	}
> +
> +	ret = btrfs_del_item(trans, root, &path);
> +
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> +
> +static int find_file_extent_offset_by_bytenr(struct btrfs_root *root,
> +		u64 owner, u64 bytenr, u64 *offset_ret)
> +{
> +	int ret = 0;
> +	struct btrfs_path path;
> +	struct btrfs_key key;
> +	struct btrfs_key found_key;
> +	struct btrfs_file_extent_item *fi;
> +	struct extent_buffer *leaf;
> +	u64 disk_bytenr;
> +	int slot;
> +
> +	btrfs_init_path(&path);
> +
> +	key.objectid = owner;
> +	key.type = BTRFS_INODE_ITEM_KEY;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret) {
> +		if (ret > 0)
> +			ret = -ENOENT;
> +		btrfs_release_path(&path);
> +		return ret;
> +	}
> +
> +	btrfs_release_path(&path);
> +
> +	key.objectid = owner;
> +	key.type = BTRFS_EXTENT_DATA_KEY;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret < 0) {
> +		btrfs_release_path(&path);
> +		return ret;
> +	}
> +
> +	while (1) {
> +		leaf = path.nodes[0];
> +		slot = path.slots[0];
> +
> +		if (slot >= btrfs_header_nritems(leaf)) {
> +			ret = btrfs_next_leaf(root, &path);
> +			if (ret) {
> +				if (ret > 0)
> +					ret = 0;
> +				break;
> +			}
> +
> +			leaf = path.nodes[0];
> +			slot = path.slots[0];
> +		}
> +
> +		btrfs_item_key_to_cpu(leaf, &found_key, slot);
> +		if ((found_key.objectid != owner) ||
> +			(found_key.type != BTRFS_EXTENT_DATA_KEY))
> +			break;
> +
> +		fi = btrfs_item_ptr(leaf, slot,
> +				struct btrfs_file_extent_item);
> +
> +		disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
> +		if (disk_bytenr == bytenr) {
> +			*offset_ret = found_key.offset;
> +			ret = 0;
> +			break;
> +		}
> +		path.slots[0]++;
> +	}
> +
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> +
> +static int repair_unaligned_extent_recs(struct btrfs_trans_handle *trans,
> +				struct btrfs_root *root,
> +				struct btrfs_path *path,
> +				struct inode_record *rec)
> +{
> +	int ret = 0;
> +	struct btrfs_key key;
> +	struct unaligned_extent_rec_t *urec;
> +	struct unaligned_extent_rec_t *tmp;
> +
> +	list_for_each_entry_safe(urec, tmp, &rec->unaligned_extent_recs, list) {
> +
> +		key.objectid = urec->owner;
> +		key.type = BTRFS_EXTENT_DATA_KEY;
> +		key.offset = urec->offset;
> +		fprintf(stderr, "delete file extent item [%llu,%llu]\n",
> +					urec->owner, urec->offset);
> +		ret = btrfs_delete_item(trans, root, &key);
> +		if (ret)
> +			return ret;
> +
> +		list_del(&urec->list);
> +		free(urec);
> +	}
> +	rec->errors &= ~I_ERR_UNALIGNED_EXTENT_REC;
> +
> +	return ret;
> +}
> +
>  static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  {
>  	struct btrfs_trans_handle *trans;
>  	struct btrfs_path path;
>  	int ret = 0;
>  
> +	/*
> +	 * unaligned extent recs always lead to csum missing error, clean it
> +	 */
> +	if ((rec->errors & I_ERR_SOME_CSUM_MISSING) &&
> +			(rec->errors & I_ERR_UNALIGNED_EXTENT_REC))
> +		rec->errors &= ~I_ERR_SOME_CSUM_MISSING;
> +
> +
>  	if (!(rec->errors & (I_ERR_DIR_ISIZE_WRONG |
>  			     I_ERR_NO_ORPHAN_ITEM |
>  			     I_ERR_LINK_COUNT_WRONG |
>  			     I_ERR_NO_INODE_ITEM |
>  			     I_ERR_FILE_EXTENT_DISCOUNT |
>  			     I_ERR_FILE_NBYTES_WRONG |
> +			     I_ERR_UNALIGNED_EXTENT_REC |
>  			     I_ERR_INLINE_RAM_BYTES_WRONG)))
>  		return rec->errors;
>  
> @@ -2515,6 +2686,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  		ret = repair_inode_nbytes(trans, root, &path, rec);
>  	if (!ret && rec->errors & I_ERR_INLINE_RAM_BYTES_WRONG)
>  		ret = repair_inline_ram_bytes(trans, root, &path, rec);
> +	if (!ret && rec->errors & I_ERR_UNALIGNED_EXTENT_REC)
> +		ret = repair_unaligned_extent_recs(trans, root, &path, rec);
>  	btrfs_commit_transaction(trans, root);
>  	btrfs_release_path(&path);
>  	return ret;
> @@ -3128,6 +3301,8 @@ static int check_fs_root(struct btrfs_root *root,
>  	struct cache_tree corrupt_blocks;
>  	enum btrfs_tree_block_status status;
>  	struct node_refs nrefs;
> +	struct unaligned_extent_rec_t *urec;
> +	struct unaligned_extent_rec_t *tmp;
>  
>  	/*
>  	 * Reuse the corrupt_block cache tree to record corrupted tree block
> @@ -3151,6 +3326,30 @@ static int check_fs_root(struct btrfs_root *root,
>  	cache_tree_init(&root_node.inode_cache);
>  	memset(&nrefs, 0, sizeof(nrefs));
>  
> +	/*
> +	 * Mode unaligned extent recs to corresponding inode record
> +	 */
> +	list_for_each_entry_safe(urec, tmp,
> +			&root->unaligned_extent_recs, list) {
> +		struct inode_record *inode;
> +
> +		inode = get_inode_rec(&root_node.inode_cache, urec->owner, 1);
> +
> +		if (IS_ERR_OR_NULL(inode)) {
> +			fprintf(stderr,
> +				"fail to get inode rec on [%llu,%llu]\n",
> +				urec->objectid, urec->owner);
> +
> +			list_del(&urec->list);
> +			free(urec);
> +
> +			continue;
> +		}
> +
> +		inode->errors |= I_ERR_UNALIGNED_EXTENT_REC;
> +		list_move(&urec->list, &inode->unaligned_extent_recs);
> +	}
> +
>  	level = btrfs_header_level(root->node);
>  	memset(wc->nodes, 0, sizeof(wc->nodes));
>  	wc->nodes[level] = &root_node;
> @@ -7425,6 +7624,68 @@ static int prune_corrupt_blocks(struct btrfs_fs_info *info)
>  	return 0;
>  }
>  
> +static int record_unaligned_extent_rec(struct btrfs_fs_info *fs_info,
> +					struct extent_record *rec)
> +{
> +
> +	struct extent_backref *back, *tmp;
> +	struct data_backref *dback;
> +	struct btrfs_root *dest_root;
> +	struct btrfs_key key;
> +	struct unaligned_extent_rec_t *urec;
> +	LIST_HEAD(entries);
> +	int ret = 0;
> +
> +	fprintf(stderr, "record unaligned extent record on %llu %llu\n",
> +			rec->start, rec->nr);
> +
> +	/*
> +	 * Metadata is easy and the backrefs should always agree on bytenr and
> +	 * size, if not we've got bigger issues.
> +	 */
> +	if (rec->metadata)
> +		return 0;
> +
> +	rbtree_postorder_for_each_entry_safe(back, tmp,
> +					     &rec->backref_tree, node) {
> +		if (back->full_backref || !back->is_data)
> +			continue;
> +
> +		dback = to_data_backref(back);
> +
> +		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_OR_NULL(dest_root))
> +			continue;
> +
> +		urec = malloc(sizeof(struct unaligned_extent_rec_t));
> +		if (!urec)
> +			return -ENOMEM;
> +
> +		INIT_LIST_HEAD(&urec->list);
> +		urec->objectid = dest_root->objectid;
> +		urec->owner = dback->owner;
> +		urec->offset = 0;
> +		urec->bytenr = rec->start;
> +		ret = find_file_extent_offset_by_bytenr(dest_root,
> +				dback->owner, rec->start, &urec->offset);
> +		if (ret) {
> +			free(urec);
> +			return ret;
> +		}
> +		list_add(&urec->list, &dest_root->unaligned_extent_recs);
> +	}
> +
> +	return ret;
> +}
> +
>  static int check_extent_refs(struct btrfs_root *root,
>  			     struct cache_tree *extent_cache)
>  {
> @@ -7522,6 +7783,21 @@ static int check_extent_refs(struct btrfs_root *root,
>  			fix = 1;
>  			cur_err = 1;
>  		}
> +
> +		if (!IS_ALIGNED(rec->start, root->fs_info->sectorsize)) {
> +			fprintf(stderr, "unaligned extent rec on [%llu %llu]\n",
> +				(unsigned long long)rec->start,
> +				(unsigned long long)rec->nr);
> +			ret = record_unaligned_extent_rec(root->fs_info, rec);
> +			if (ret)
> +				goto repair_abort;
> +
> +			/*
> +			 * free extent record
> +			 */
> +			goto next;
> +		}
> +
>  		if (all_backpointers_checked(rec, 1)) {
>  			fprintf(stderr, "backpointer mismatch on [%llu %llu]\n",
>  				(unsigned long long)rec->start,
> @@ -7574,7 +7850,7 @@ static int check_extent_refs(struct btrfs_root *root,
>  				rec->start, rec->start + rec->max_size);
>  			cur_err = 1;
>  		}
> -
> +next:
>  		err = cur_err;
>  		remove_cache_extent(extent_cache, cache);
>  		free_all_extent_backrefs(rec);
> diff --git a/check/mode-original.h b/check/mode-original.h
> index ed995931fcd5..b23594863199 100644
> --- a/check/mode-original.h
> +++ b/check/mode-original.h
> @@ -155,6 +155,16 @@ struct file_extent_hole {
>  	u64 len;
>  };
>  
> +struct unaligned_extent_rec_t {
> +	struct list_head list;
> +
> +	u64 objectid;
> +	u64 owner;
> +	u64 offset;
> +
> +	u64 bytenr;
> +};
> +
>  #define I_ERR_NO_INODE_ITEM		(1 << 0)
>  #define I_ERR_NO_ORPHAN_ITEM		(1 << 1)
>  #define I_ERR_DUP_INODE_ITEM		(1 << 2)
> @@ -169,6 +179,7 @@ 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_UNALIGNED_EXTENT_REC	(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)
> @@ -185,6 +196,8 @@ struct inode_record {
>  	unsigned int nodatasum:1;
>  	int errors;
>  
> +	struct list_head unaligned_extent_recs;
> +
>  	u64 ino;
>  	u32 nlink;
>  	u32 imode;
> diff --git a/ctree.h b/ctree.h
> index 2e0896390434..d0f441587f9f 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -1177,6 +1177,8 @@ struct btrfs_root {
>  	u32 type;
>  	u64 last_inode_alloc;
>  
> +	struct list_head unaligned_extent_recs;
> +
>  	/* 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 992f4b870e9f..0dfd51ed87bf 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -480,6 +480,7 @@ 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->unaligned_extent_recs);
>  	memset(&root->root_key, 0, sizeof(root->root_key));
>  	memset(&root->root_item, 0, sizeof(root->root_item));
>  	root->root_key.objectid = objectid;
> 

Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref

[Su Yanjun <suyj.fnst@cn.fujitsu.com>]

On 10/24/2018 8:45 AM, Qu Wenruo wrote:
>
> On 2018/10/23 下午5:41, Su Yue wrote:
>> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>>
>> In original mode, if some file extent item has unaligned extent backref,
>> fixup_extent_refs can't repair it. This patch will check extent alignment
>> then delete file extent with unaligned extent backref.
> This looks a little strange to me.
>
> You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM?
>
> Then why not just delete the EXTENT_ITEM directly? No need to go back
> checking if it has a corresponding EXTENT_DATA since unaligned one is
> definitely corrupted.
>
> For corrupted EXTENT_DATA, it should get deleted when we check fs tree.
>
> This would save you a lot of codes.
>
> Thanks,
> Qu
The situation is that the file extent has wrong extent backref, actually 
it doesn't exist.

Thanks,
Su

Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref

[Qu Wenruo]

On 2018/11/7 下午2:21, Su Yanjun <suyj.fnst@cn.fujitsu.com> wrote:
> 
> 
> On 10/24/2018 8:45 AM, Qu Wenruo wrote:
>>
>> On 2018/10/23 下午5:41, Su Yue wrote:
>>> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>>>
>>> In original mode, if some file extent item has unaligned extent backref,
>>> fixup_extent_refs can't repair it. This patch will check extent
>>> alignment
>>> then delete file extent with unaligned extent backref.
>> This looks a little strange to me.
>>
>> You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM?
>>
>> Then why not just delete the EXTENT_ITEM directly? No need to go back
>> checking if it has a corresponding EXTENT_DATA since unaligned one is
>> definitely corrupted.
>>
>> For corrupted EXTENT_DATA, it should get deleted when we check fs tree.
>>
>> This would save you a lot of codes.
>>
>> Thanks,
>> Qu
> The situation is that the file extent has wrong extent backref, actually
> it doesn't exist.

Did you mean extent EXTENT_ITEM key's objectid is unaligned?

Would you please give an example on this case? Like:
(<ino> EXTENT_DATA <offset>
   disk bytenr <XXXX> disk len <YYYY>

And its backref like:
(<XXXX> EXTENT_ITEM <YYYY>)

And then mark where the number is incorrect.

Thanks,
Qu

> 
> Thanks,
> Su
> 
> 
> 
> 
> 

Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref

[Su Yanjun <suyj.fnst@cn.fujitsu.com>]

On 11/7/2018 2:38 PM, Qu Wenruo wrote:
>
> On 2018/11/7 下午2:21, Su Yanjun <suyj.fnst@cn.fujitsu.com> wrote:
>>
>> On 10/24/2018 8:45 AM, Qu Wenruo wrote:
>>> On 2018/10/23 下午5:41, Su Yue wrote:
>>>> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>>>>
>>>> In original mode, if some file extent item has unaligned extent backref,
>>>> fixup_extent_refs can't repair it. This patch will check extent
>>>> alignment
>>>> then delete file extent with unaligned extent backref.
>>> This looks a little strange to me.
>>>
>>> You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM?
>>>
>>> Then why not just delete the EXTENT_ITEM directly? No need to go back
>>> checking if it has a corresponding EXTENT_DATA since unaligned one is
>>> definitely corrupted.
>>>
>>> For corrupted EXTENT_DATA, it should get deleted when we check fs tree.
>>>
>>> This would save you a lot of codes.
>>>
>>> Thanks,
>>> Qu
>> The situation is that the file extent has wrong extent backref, actually
>> it doesn't exist.
> Did you mean extent EXTENT_ITEM key's objectid is unaligned?
>
> Would you please give an example on this case? Like:
> (<ino> EXTENT_DATA <offset>
>     disk bytenr <XXXX> disk len <YYYY>
>
> And its backref like:
> (<XXXX> EXTENT_ITEM <YYYY>)
>
> And then mark where the number is incorrect.
>
> Thanks,
> Qu

As in /btrfs-progs/tests/fsck-tests/001-bad-file-extent-bytenr case:

item 7 key (257 EXTENT_DATA 0) itemoff 3453 itemsize 53

                 generation 6 type 1 (regular)

                 extent data disk byte 755944791 nr 1048576
                                 ^^^^^^^^^

                 extent data offset 0 nr 1048576 ram 1048576

                 extent compression 0 (none)

Thanks,

Su

>> Thanks,
>> Su
>>
>>
>>
>>
>>
>

Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref

[Qu Wenruo]

On 2018/11/7 下午3:04, Su Yanjun <suyj.fnst@cn.fujitsu.com> wrote:
> 
> 
> On 11/7/2018 2:38 PM, Qu Wenruo wrote:
>>
>> On 2018/11/7 下午2:21, Su Yanjun <suyj.fnst@cn.fujitsu.com> wrote:
>>>
>>> On 10/24/2018 8:45 AM, Qu Wenruo wrote:
>>>> On 2018/10/23 下午5:41, Su Yue wrote:
>>>>> From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
>>>>>
>>>>> In original mode, if some file extent item has unaligned extent
>>>>> backref,
>>>>> fixup_extent_refs can't repair it. This patch will check extent
>>>>> alignment
>>>>> then delete file extent with unaligned extent backref.
>>>> This looks a little strange to me.
>>>>
>>>> You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM?
>>>>
>>>> Then why not just delete the EXTENT_ITEM directly? No need to go back
>>>> checking if it has a corresponding EXTENT_DATA since unaligned one is
>>>> definitely corrupted.
>>>>
>>>> For corrupted EXTENT_DATA, it should get deleted when we check fs tree.
>>>>
>>>> This would save you a lot of codes.
>>>>
>>>> Thanks,
>>>> Qu
>>> The situation is that the file extent has wrong extent backref, actually
>>> it doesn't exist.
>> Did you mean extent EXTENT_ITEM key's objectid is unaligned?
>>
>> Would you please give an example on this case? Like:
>> (<ino> EXTENT_DATA <offset>
>>     disk bytenr <XXXX> disk len <YYYY>
>>
>> And its backref like:
>> (<XXXX> EXTENT_ITEM <YYYY>)
>>
>> And then mark where the number is incorrect.
>>
>> Thanks,
>> Qu
> 
> As in /btrfs-progs/tests/fsck-tests/001-bad-file-extent-bytenr case:
> 
> item 7 key (257 EXTENT_DATA 0) itemoff 3453 itemsize 53
> 
>                 generation 6 type 1 (regular)
> 
>                 extent data disk byte 755944791 nr 1048576
>                                 ^^^^^^^^^
> 
>                 extent data offset 0 nr 1048576 ram 1048576
> 
>                 extent compression 0 (none)

Then there is no "unaligned extent backref".

It's just a unaligned disk bytenr of a file extent.
Nothing to do with backref.

Please update the commit message to avoid such confusing words, and
include above info, which is pretty easy to understand.

Thanks,
Qu

> 
> Thanks,
> 
> Su
> 
>>> Thanks,
>>> Su
>>>
>>>
>>>
>>>
>>>
>>
> 
> 
> 

[PATCH 12/13] btrfs-progs: tests: add case for inode lose one file extent

[Su Yue]

From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

The missing extent will lead to the existence of the gap between adjacent
extents. The fsck should can detect the gap correctly and repair by punch
a hole.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 .../.lowmem_repairable                           |   0
 .../038-missing-one-file-extent/default_case.img | Bin 0 -> 3072 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/fsck-tests/038-missing-one-file-extent/.lowmem_repairable
 create mode 100644 tests/fsck-tests/038-missing-one-file-extent/default_case.img

diff --git a/tests/fsck-tests/038-missing-one-file-extent/.lowmem_repairable b/tests/fsck-tests/038-missing-one-file-extent/.lowmem_repairable
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/fsck-tests/038-missing-one-file-extent/default_case.img b/tests/fsck-tests/038-missing-one-file-extent/default_case.img
new file mode 100644
index 0000000000000000000000000000000000000000..7cc2cd403977a805a58c61f007d517f24412800c
GIT binary patch
literal 3072
zcmeH{i!<Bl8pqKRLc?y;hF+rDQnDI!b>kMH-NSCzC9O-$A};5kw5V%@DiYed7p<*T
zBCb_x>r!!xoRp>As<<aENgC8$98|M}{EnJ4d#0T^f54gPGw(d__j%vvo%z0>dFM^_
zeGodS=|-3OPwd~FrT50J;D21S!0y&kQVM&yZ!a5s?6Mf%(*qwjzr+^;UkLm^5{Mcb
z=ZD!ANGr<=Cz{?bgfa@di095{Dd%#|>FbkzSEH`2+QBXTnXB!`ZXAbcT!BcZ8Kos^
zAM%uOxQ?}K@|M1b*Ose0ngIO<LjF1{(DQp*JT>7^&<Uj}S@uwb@K%dpGho?nsy;{Y
z&pZRVd7ZTB5%i>#VKUmwGrnwDPXAS6`2~d}mb_iGjrNbwVs$iAV_MJiLB$=}5>IFv
z>9Ri9im7d}52u!uU;RuA*+v|@A?qZOI>MK>1rih=-)ty1IMv*HJ)YB%@wx!iHx2ru
zVDDgptW-h_?q1Z`@;K(&3k%im)!FTRCg;f|Inb8yu+XcA9<^L8>Khow-FyDFC;^gG
zOwJ+OT{;7@rp79iArF~CkGX{pp^9c4vsD}>80t+E?+RPRbW+8i_X2*nD`tBEv-J1Q
zE9F)8^}KYb&GT_wOZ6C|e&%(H8I6H4p=5e@i$uj@;~5j(!H$!^z$%*%VOh2HE)yva
zd8FPL-eFz-0lz#m=>K>!e#yr$5N>bm{3aiAk#e~^cCh(ebBn~)b|sg5`9qbD`}gh}
zE?D<cElXk^u<jVeb1BHFkXgI$=RspodUFyB16r888|hdHY-eZtsEUm{e-{MT{_bI)
zW1xTe;kg^`#NTb@zczd5&+&qh|4509)l9T_d_u!p1AGOXh#al#XwAlY4rypB-@fxA
z5%R$r@z_l!*b;HQNQJ0k(s@$Ve}>6p6Dh>mf)GEPo96tw@shabCWYu*m?z?LU#}4D
z>1~rwv4T7r2KdL;+nks)n4dSg!u-tPih`(tQA%k@-O5lNLA1R&G|I)q(EzbuhcH^y
zlptKhz7%DY;Ni*wiy-?`-}R;wzKdoyp0RRvn=`x>01{1;U;$hzB!XNTl|dC6XHObS
z{O!GoNRC>j(Fg>-r086cF1p0L-C)pv;4``xRMXN0y!O+HQ)LC4%dP}$^#EMn6hGR$
zBVkTDp%j0khkX0=)mk-sWR^ha%3aQ%Ym>zC{Rmwj)~nc=ZdKfQ2gFGhvH?w=UVzT@
zYX-4j%*_>F_^r%KsK&aEWSv%EqoziEDZzxB5kaF_PgXB&0B;ty03#MvxLCYB%wkC5
z-nrCC-o77M|0UB<%TEhAY!C1t3+BSloc45Bc$s<t9s^7KM^%!z_8KR^V;F9CYxK%_
ze5Hip<PLbSW(osJ_2v;@QM!CIa)ZHM!N`YTYQ*!95VOSAn#MxAEfU|m&xU+)RVMB#
zh){8H10+^tI<`GQk7AQf;Zv*|z|1Lwhlu^x>cSq)DQBx1Dl+qGRc%Bd>%V{IM!(f1
zAI=y@k3_2D-R7P(aRieO9G9@w5X_^Vh!o8vNZD5bVd*N2WOJL1IK`c@W-GXMxT<#K
z%K&ly4w~J$)3GB?rTIU*LDH-HR_h%I3&G)%I-CTz@lkVvF_aUl-4Vd--bsCSNHl>A
z=gU2HSpk^kJonFh!~-;C0=-@Mdj9Oj6Z5hyK#1b9oB?%E>Q4uBm$&bJqt&KnjUI>3
zE|${dEDooc<;sP;B3ZfFdpemmoqA7Ny1SfNZViPw$aljP1Qv6$qgZ|;*$T;tT<b1b
z2|uzzBS%nak(QAe9;otJRfL?#{JvaBL8@-lMuOodUcf04sInD*x)meu4NbUp7^YW5
zEC#ak*Yj~9Mw!dyM|9DRQ>ZnL9J<j}%qiB%JUb26X)Q~|MfYc8WBkP#Fv9Z9v%<=x
z{{Fw!r-$bhu5;>Q53&dG@glD(z+AUFdR``IE>`u9t8s2M#)!VLoz*qn(^<APzbG;4
z2^|N1<rrl%=&14RzE~DFl41Yr;4R9w2vEBJWWrT(ZMBuV6A~I;iqIPdP*sb-6sDy1
zWP2_nd6eE}RyZ=#jZWtnp?1Z-Pi`gkPg3)7XWLDfQnc6f$Di{l_ytzAYn6a#m9Fad
zY)Do2#4%6c?+h0m?6A;3YN+E|yKr2-MwcTO4bsXVUsb|rT~w*vj`LBD;f1L#1~YqS
zPFoG)&rbLR4pDqs0)z4QQ?Bh_Np}sjf$*ZDYADq^zAb^tJ_}(wJBI3dt#b|oI;qn_
z+Dp66%hUzf=4>%F)Z}L)g!)hY2a``!^Pvk>axI@a{;S%yJf|cuX~L_EDg#&9=npHi
cM@=@SQR^9<Z{oH*YHS^F;J<pmi2oA!H;vM1aR2}S

literal 0
HcmV?d00001

-- 
2.19.1

[PATCH 13/13] btrfs-progs: fsck-test: enable lowmem repair for case 001

[Su Yue]

Lowmem can repair after commit
       'btrfs-progs: lowmem: move nbytes check before isize check',
so add the beacon file.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 tests/fsck-tests/001-bad-file-extent-bytenr/.lowmem_repairable | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/fsck-tests/001-bad-file-extent-bytenr/.lowmem_repairable

diff --git a/tests/fsck-tests/001-bad-file-extent-bytenr/.lowmem_repairable b/tests/fsck-tests/001-bad-file-extent-bytenr/.lowmem_repairable
new file mode 100644
index 000000000000..e69de29bb2d1
-- 
2.19.1

Re: [PATCH 00/13] btrfs-progs: fixes of file extent in original and lowmem check

[Qu Wenruo]

On 2018/10/23 下午5:41, Su Yue wrote:
> This patchset can be fetched from my repo:
> https://github.com/Damenly/btrfs-progs/tree/file_extent_fixes
> which is based on kdave/devel whose HEAD is:
> commit 4f20c27ab33aab3efffe13cdae1b8837c821d0d7 (kdave/devel)
> Author: Nikolay Borisov <nborisov@suse.com>
> Date:   Fri Jun 15 07:13:50 2018 +0000
> 
>     btrfs-progs: tests: test for FST corruption detection/repair
> 
> This set fixes bugs of checking unaligned disk_bytenr extent_data and
> orphan extent. Lowmem part patches can be used in common, so I put
> this two fixes together.
> 
> For unaligned disk_bytenr extent_data, now original and lowmem check
> both delete the corrupted part and punch a hole.

This method looks pretty valid.

> 
> For orphan extent, lowmem mode has no change here.
> Original mode discards function of detect and repair orphan extents
> according extent items. Because as corruptions the community reported,
> extent tree is more problematic than fs trees.

This also looks pretty nice to me.

Extent tree can't really be trusted, not to mention under most case it's
extent tree itself corrupted.

More over, even we lost every tree block of extent tree, it's still
possible for btrfs-progs to recover the whole fs using btrfs-restore.

(In theory, we should provide a mount option to allow RO mount to read
the fs without a valid extent tree).

So this is the correct way to do.

Thanks,
Qu
> 
> patch[1-2] fix minor bugs in lowmem repair.
> patch[3] fixes false alert about repaired extent item in lowmem mode.
> patch[4] fixes annoying alerts about gap in file extent in lowmem mode.
> patch[5,6,7] enable check and repair unaligned disk_bytenr file extent
> 	     in lowmem mode.
> patch[8,9] revert support for orphan extent in original mode.
> patch[10] fixes bug about finding right backrefs in original mode.
> patch[11] adds support to detect and delete unaligned file extents
> 	       in original.
> patch[12] adds a test image which lacks of a file extent.
> patch[13] enables lowmem repair of test case fsck-tests/001 which has
> 	  	  unaligned disk_bytenr file extent.
> 
> 
> Lu Fengqi (2):
>   btrfs-progs: lowmem: fix false alert about the existence of gaps in
>     the check_file_extent
>   btrfs-progs: tests: add case for inode lose one file extent
> 
> Su Yanjun (4):
>   btrfs-progs: Revert "btrfs-progs: Add repair and report function for
>     orphan file extent."
>   btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to
>     corresponding root."
>   btrfs-progs: check: fix bug in find_possible_backrefs
>   btrfs-progs: check: Delete file extent item with unaligned extent
>     backref
> 
> Su Yue (7):
>   btrfs-progs: lowmem: add argument path to punch_extent_hole()
>   btrfs-progs: lowmem: move nbytes check before isize check
>   btrfs-progs: lowmem: fix false alert if extent item has been repaired
>   btrfs-progs: lowmem: check unaligned disk_bytenr for extent_data
>   btrfs-progs: lowmem: rename delete_extent_tree_item() to delete_item()
>   btrfs-progs: lowmem: delete unaligned bytes extent data under repair
>   btrfs-progs: fsck-test: enable lowmem repair for case 001
> 
>  check/main.c                                  | 581 +++++++++++-------
>  check/mode-lowmem.c                           | 272 ++++----
>  check/mode-original.h                         |  30 +-
>  ctree.h                                       |  10 +-
>  disk-io.c                                     |   2 +-
>  .../.lowmem_repairable                        |   0
>  .../.lowmem_repairable                        |   0
>  .../default_case.img                          | Bin 0 -> 3072 bytes
>  8 files changed, 538 insertions(+), 357 deletions(-)
>  create mode 100644 tests/fsck-tests/001-bad-file-extent-bytenr/.lowmem_repairable
>  create mode 100644 tests/fsck-tests/038-missing-one-file-extent/.lowmem_repairable
>  create mode 100644 tests/fsck-tests/038-missing-one-file-extent/default_case.img
> 

[PATCH 00/13] btrfs-progs: check: Fix

[Qu Wenruo]

This patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/file_extents_fixes

The base commit is devel branch:
commit e1e7519c2d225554004b55a5c19eb196db92285a (david/devel)
Author: Qu Wenruo <wqu@suse.com>
Date:   Thu Oct 25 15:35:47 2018 +0800

    btrfs-progs: tests: check: Make 026-bad-dir-item-name verify if btrfs-check can also repair
    
    Just remove the customized 'test.sh', then generic fsck test will do the
    check-repair-check.

Originally for fsck-tests/001-bad-file-extent-bytenr we're going to fix
it by resetting the corrupted file extent, using the disk bytenr from
data backref.

However in real world, extent tree is the most fragile one while fs tree
is more reliable.
This means fixing using extent tree can sometimes be more dangerous, and
causing extra corruption from repair.

Further more, that test case is a big blockage for any later
modification on file extent repair code.

So the goal of this patchset is to change the repair behavior from
rebuilding file extents using data backref, to dropping invalid file
extents and modify extent tree.

Despite above change, we have some small bug fixes for:
- False alert for file extents gap
- Bad data backref -> file extent search in repair mode

The patchset is mostly from Fujitsu guys (Lu, Su Yue, Su Yanjun).
My work here is:
- Solve rebase conflicts
  Mostly with DIR_ITEM hash mismatch patchset

- Commit message/comment update
  Make them a little more reader friendly

- Reorder the patchset
  Make small fixes first.

Lu Fengqi (2):
  btrfs-progs: lowmem: fix false alert about the existence of gaps in
    the check_file_extent
  btrfs-progs: tests: add case for inode lose one file extent

Su Yanjun (4):
  btrfs-progs: Revert "btrfs-progs: Add repair and report function for
    orphan file extent."
  btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to
    corresponding root."
  btrfs-progs: check: fix wrong @offset used in find_possible_backrefs()
  btrfs-progs: check: Delete file extent item with unaligned disk bytenr

Su Yue (7):
  btrfs-progs: lowmem: add argument path to punch_extent_hole()
  btrfs-progs: lowmem: move nbytes check before isize check
  btrfs-progs: lowmem: fix false alert if extent item has been repaired
  btrfs-progs: lowmem: check unaligned disk_bytenr for extent_data
  btrfs-progs: lowmem: rename delete_extent_tree_item() to delete_item()
  btrfs-progs: lowmem: delete unaligned bytes extent data under repair
  btrfs-progs: fsck-test: enable lowmem repair for case 001

 check/main.c                                  | 584 +++++++++++-------
 check/mode-lowmem.c                           | 293 +++++----
 check/mode-original.h                         |  30 +-
 ctree.h                                       |  10 +-
 disk-io.c                                     |   2 +-
 .../.lowmem_repairable                        |   0
 .../.lowmem_repairable                        |   0
 .../default_case.img                          | Bin 0 -> 3072 bytes
 8 files changed, 543 insertions(+), 376 deletions(-)
 create mode 100644 tests/fsck-tests/001-bad-file-extent-bytenr/.lowmem_repairable
 create mode 100644 tests/fsck-tests/038-missing-one-file-extent/.lowmem_repairable
 create mode 100644 tests/fsck-tests/038-missing-one-file-extent/default_case.img

-- 
2.20.0

[PATCH 05/13] btrfs-progs: lowmem: check unaligned disk_bytenr for extent_data

[Qu Wenruo]

From: Su Yue <suy.fnst@cn.fujitsu.com>

Add support to check unaligned disk_bytenr for extent_data.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-lowmem.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index b7690c6dc1f1..20663617d242 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -3166,7 +3166,14 @@ static int check_extent_data_item(struct btrfs_root *root,
 	extent_num_bytes = btrfs_file_extent_num_bytes(eb, fi);
 	offset = btrfs_file_extent_offset(eb, fi);
 
-	/* Check unaligned disk_num_bytes and num_bytes */
+	/* Check unaligned disk_bytenr, disk_num_bytes and num_bytes */
+	if (!IS_ALIGNED(disk_bytenr, root->fs_info->sectorsize)) {
+		error(
+"file extent [%llu, %llu] has unaligned disk bytenr: %llu, should be aligned to %u",
+			fi_key.objectid, fi_key.offset, disk_bytenr,
+			root->fs_info->sectorsize);
+		err |= BYTES_UNALIGNED;
+	}
 	if (!IS_ALIGNED(disk_num_bytes, root->fs_info->sectorsize)) {
 		error(
 "file extent [%llu, %llu] has unaligned disk num bytes: %llu, should be aligned to %u",
-- 
2.20.0