[PATCH 0/3] btrfs-progs: Check and repair invalid root item generation

[PATCH 0/3] btrfs-progs: Check and repair invalid root item generation

[Qu Wenruo]

Kernel is going to reject invalid root generation.

Consider the existing checks are causing some error reports, we should
handle such problem in advance, so that's the patchset is going to do,
check and repair such invalid root generation.

Qu Wenruo (3):
  btrfs-progs: check/lowmem: Check and repair root generation
  btrfs-progs: check/original: Check and repair root item geneartion
  btrfs-progs: fsck-tests: Add test case for invalid root generation

 check/main.c                                  |  20 ++++++++++++
 check/mode-common.c                           |  29 ++++++++++++++++++
 check/mode-common.h                           |   1 +
 check/mode-lowmem.c                           |  16 ++++++++++
 check/mode-lowmem.h                           |   1 +
 .../default_case.img                          | Bin 0 -> 3072 bytes
 6 files changed, 67 insertions(+)
 create mode 100644 tests/fsck-tests/041-invalid-root-generation/default_case.img

-- 
2.22.0

[PATCH 1/3] btrfs-progs: check/lowmem: Check and repair root generation

[Qu Wenruo]

Since kernel is going to reject any root item which is newer than super
block generation, we need to provide a way to fix such problem in
btrfs-check.

This patch addes the ability to report and repair root generation in
lowmem mode.

This is done by cowing the root node, so we will update the root
generation along with the root node generation at commit transaction
time.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-common.c | 29 +++++++++++++++++++++++++++++
 check/mode-common.h |  1 +
 check/mode-lowmem.c | 16 ++++++++++++++++
 check/mode-lowmem.h |  1 +
 4 files changed, 47 insertions(+)

diff --git a/check/mode-common.c b/check/mode-common.c
index d5f6c8ef97b1..a5b86a0ac32a 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -924,3 +924,32 @@ int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
 	}
 	return ret;
 }
+
+int repair_root_generation(struct btrfs_root *root)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	int ret;
+
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans))
+		return PTR_ERR(trans);
+
+	key.objectid = 0;
+	key.type = 0;
+	key.offset = 0;
+	btrfs_init_path(&path);
+
+	/* Here we only CoW the tree root to update the geneartion */
+	path.lowest_level = btrfs_header_level(root->node);
+	root->node->flags |= EXTENT_BAD_TRANSID;
+
+	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
+	if (ret < 0)
+		return ret;
+
+	btrfs_release_path(&path);
+	ret = btrfs_commit_transaction(trans, root);
+	return ret;
+}
diff --git a/check/mode-common.h b/check/mode-common.h
index 4c169c6e3b29..4a3abeb02c81 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -155,4 +155,5 @@ static inline bool is_valid_imode(u32 imode)
 	return true;
 }
 
+int repair_root_generation(struct btrfs_root *root);
 #endif
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index a2be0e6d7034..bf3b57f5ad2d 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -4957,6 +4957,7 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
 	struct btrfs_path path;
 	struct node_refs nrefs;
 	struct btrfs_root_item *root_item = &root->root_item;
+	u64 super_generation = btrfs_super_generation(root->fs_info->super_copy);
 	int ret;
 	int level;
 	int err = 0;
@@ -4978,6 +4979,21 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
 	level = btrfs_header_level(root->node);
 	btrfs_init_path(&path);
 
+	if (btrfs_root_generation(root_item) > super_generation + 1) {
+		error(
+	"invalid root generation for root %llu, have %llu expect (0, %llu)",
+		      root->root_key.objectid, btrfs_root_generation(root_item),
+		      super_generation + 1);
+		err |= INVALID_GENERATION;
+		if (repair) {
+			ret = repair_root_generation(root);
+			if (!ret) {
+				err &= ~INVALID_GENERATION;
+				printf("Reset generation for root %llu\n",
+					root->root_key.objectid);
+			}
+		}
+	}
 	if (btrfs_root_refs(root_item) > 0 ||
 	    btrfs_disk_key_objectid(&root_item->drop_progress) == 0) {
 		path.nodes[level] = root->node;
diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
index d2983fd12eb4..0361fb3382b1 100644
--- a/check/mode-lowmem.h
+++ b/check/mode-lowmem.h
@@ -47,6 +47,7 @@
 #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
 #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
 #define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
+#define INVALID_GENERATION	(1<<26)	/* Generation is too new */
 
 /*
  * Error bit for low memory mode check.
-- 
2.22.0

[PATCH 2/3] btrfs-progs: check/original: Check and repair root item geneartion

[Qu Wenruo]

Add such ability to original mode to fix root generation mismatch, which
can be rejected by kernel.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/check/main.c b/check/main.c
index 0cc6fdba0289..1b519a67f746 100644
--- a/check/main.c
+++ b/check/main.c
@@ -3437,8 +3437,10 @@ static int check_fs_root(struct btrfs_root *root,
 {
 	int ret = 0;
 	int err = 0;
+	bool generation_err = false;
 	int wret;
 	int level;
+	u64 super_generation;
 	struct btrfs_path path;
 	struct shared_node root_node;
 	struct root_record *rec;
@@ -3449,6 +3451,22 @@ static int check_fs_root(struct btrfs_root *root,
 	struct unaligned_extent_rec_t *urec;
 	struct unaligned_extent_rec_t *tmp;
 
+	super_generation = btrfs_super_generation(root->fs_info->super_copy);
+	if (btrfs_root_generation(root_item) > super_generation + 1) {
+		error(
+	"invalid generation for root %llu, have %llu expect (0, %llu]",
+		      root->root_key.objectid, btrfs_root_generation(root_item),
+		      super_generation + 1);
+		generation_err = true;
+		if (repair) {
+			ret = repair_root_generation(root);
+			if (!ret) {
+				printf("Reset generation for root %llu\n",
+					root->root_key.objectid);
+				generation_err = false;
+			}
+		}
+	}
 	/*
 	 * Reuse the corrupt_block cache tree to record corrupted tree block
 	 *
@@ -3597,6 +3615,8 @@ skip_walking:
 
 	free_corrupt_blocks_tree(&corrupt_blocks);
 	root->fs_info->corrupt_blocks = NULL;
+	if (!ret && generation_err)
+		ret = -1;
 	return ret;
 }
 
-- 
2.22.0

[PATCH 3/3] btrfs-progs: fsck-tests: Add test case for invalid root generation

[Qu Wenruo]

The image contains a fs tree whose generation is over 100 larger than
super block generation.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../041-invalid-root-generation/default_case.img | Bin 0 -> 3072 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/fsck-tests/041-invalid-root-generation/default_case.img

diff --git a/tests/fsck-tests/041-invalid-root-generation/default_case.img b/tests/fsck-tests/041-invalid-root-generation/default_case.img
new file mode 100644
index 0000000000000000000000000000000000000000..4a2c8d88a1a885fb9bcae56adc98dcbe75681441
GIT binary patch
literal 3072
zcmeB9n_BcpHD)gZ6tDnkMlb?0gn%pthX4@Czy>BSfXQ_r0fviUb|5pDGfIqxz-R~z
zy%4ClwfpC>Btd4mfEQECdaGAmZV8>XXi|>uFI66%o`gcT6*YS%y9Y!S?9tcuX+Esl
zF~yldnNQhLK}2{)N;B6XQSOE>PKFciax94`lz7sS!>ruK%Dbk3IZ;A?N9EVRNcQIV
zGhgNx#`NdqtUUDP*9o@&3lom8F_?7PCMGm2eap6FN>9MTmP6?*>b7cARJbno&wC*7
zNqobp%ZHl+4Y=G6YDlmyP7qnZlyO<?;uT(pr}8^GPPJd#q*GuwtFEm(<I@V?WVcPS
zQVrL11f8Cmb1XW^z35gsgYP{@foksEjT4Q^<r;QOp2@iLZU2#chw1z-Ppuc6ynJdd
z-;0kD4X1!2?vLdirY{fO)87>6pv1r+b1Y$D#jUru?XwR#NF4w8{7?HWRd4q2!fQ8O
zUAOb8K8VsPd7-vqiqlocgN_;=(=RQpUNHNC^ox53mnMZ57M7F*7i)QXD8IYUAM>v0
z{N2BC2QRPvdqvgZ^RvRTuW@z1PsJ}!YQ8ghiqF>j;k(wX34CsPb=8-(AIraO-nGu1
z|NEhK%SRSEpC2p5i&PebOj8Q3-l)|v<6YCa>YJic^Q6BS73$zfV0uDUwK==yT0j1E
zpVxVQ;PTqb`#yc&T_Jbs?cL+g>~_xZztvZDWWC-c;r)BQ{In6BV;(yFiROHpr?;-Z
zegFTtXN<q?-GqD#H--1z_v-%X&Yc(S1Pq0<>${vBMc5zg?3!kp^?{Y;^j)=xBlfw8
zlW%4jzu9zFB}3>)lJO0_8#OHV4{XU-?7peA=#c4}2b(_rKK19EWpP^A)+JJMKPq0=
z@7KBYxpZ&Hx&HY5PB&bi*Of@RpUkn)tuJ#snI^5jyZzvkUDJ2Zn>k@$ZS~h19Enq3
ziR3+8UZ{uW*9Y>cPBQ8B(q`{ZYfSyVcK`oK&3)hJom4*%A?XWr?H&INnNS(_58G$^
zUYjAqW0K&&)wQf+$5(+VztVSKj9waWA-lCnS0ubE$Un|3g5Ocg+ti$Yuc)Yt|D%(k
z0{mKPf-Dp7?73z?UF4-=UfHwUvh>aMCdbTv{FE`$|6E(2_Kf$<OLP0$+Ar%24rr^N
z`)jfFQ_0`v<juvx)BECh5_PxN+D$*QGdq9(ziW3^pVx`~{iOR+_4&Uaixg$UdC<Ia
zU`5@JXD2=_*t7iKexs<r|9jWE*p+<i+k8)_y<qF(GbK5H!tVWJN7MAr^=E*VsN0*_
zuQTG~|2)q=CvR0*6|wvBnLFiA9Y24)^8dQq>i_fK>{<W$>fzI~H+M^$hnq=pf7SlF
zw7jpqpj{kPvm=3q|9d}vsd#eTes%LNpk{Nn>;I2eetub~Ww~bCKQpHlTcz@4_#eIt
zdmXY)yF!y~s!D}@-0{w|f_$UU@(&4W8Apz$-^yN7+W%wkoW2z)P4ANfPP51#T`-YT
z@3_L6a`vm=q-H+2`Hcg~Mj*4HS$C~9|4pmU{l$l^MA}MEq+5w>l2vOj-LY}%i`xq&
z@)u8hJ?quKMB{$`-9I*36ffQVytY;@Va|(%Q)_ZBe15m`zrXg~_N)KJt@KUwzUBTA
zI8Zk8zw%wjp!fe>@+VCB>i^bgQR!#<>DA5G9?35Qw@;89z#h1}&+gjf<zJsGuHAb6
z+V1yz|F7H<%KiIty(!PD{eKS4JnMLVF(gXrWB#4{HaTOLNc!KWD+TP%zO7MRAZzm{
zMd(Q8GWAQxY|kdBy<vGPm@e<{=9BXN_v&QD7+!0~>Q(1w9V~JG@aNa``SJJi_T1)?
z%aH57X=*0teQm}Ww$J@)j(<KlTkqH~0VOgHFbc1pcWbNH)UEG=7M^<dxX5|g<NY_c
ztw_6FwsY@ByZ*(S?|(fSy(9PFw$?tUQ(H<3v(>fQ{&c6GpIf=M%5dJv_Mpr)_kDhE
zIsducZ{9!ax3~Y=)@**x6!!g-+_9JE!uj>{&*|l@$uSXo75>V*dheOK{__2z&cDvx
nefQhqUU6Ej{%?zS=?|{uL?@r!*7`x*L{ifYDbz-p!#M;1<_P#~

literal 0
HcmV?d00001

-- 
2.22.0

Re: [PATCH 1/3] btrfs-progs: check/lowmem: Check and repair root generation

[Nikolay Borisov]

On 9.08.19 г. 9:53 ч., Qu Wenruo wrote:
> Since kernel is going to reject any root item which is newer than super
> block generation, we need to provide a way to fix such problem in
> btrfs-check.
> 
> This patch addes the ability to report and repair root generation in
> lowmem mode.
> 
> This is done by cowing the root node, so we will update the root
> generation along with the root node generation at commit transaction
> time.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/mode-common.c | 29 +++++++++++++++++++++++++++++
>  check/mode-common.h |  1 +
>  check/mode-lowmem.c | 16 ++++++++++++++++
>  check/mode-lowmem.h |  1 +
>  4 files changed, 47 insertions(+)
> 
> diff --git a/check/mode-common.c b/check/mode-common.c
> index d5f6c8ef97b1..a5b86a0ac32a 100644
> --- a/check/mode-common.c
> +++ b/check/mode-common.c
> @@ -924,3 +924,32 @@ int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>  	}
>  	return ret;
>  }
> +
> +int repair_root_generation(struct btrfs_root *root)
> +{
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_path path;
> +	struct btrfs_key key;
> +	int ret;
> +
> +	trans = btrfs_start_transaction(root, 1);
> +	if (IS_ERR(trans))
> +		return PTR_ERR(trans);
> +
> +	key.objectid = 0;
> +	key.type = 0;
> +	key.offset = 0;
> +	btrfs_init_path(&path);
> +
> +	/* Here we only CoW the tree root to update the geneartion */
> +	path.lowest_level = btrfs_header_level(root->node);
> +	root->node->flags |= EXTENT_BAD_TRANSID;

Why do you set EXTENT_BAD_TRANSID? This flag is currently used only in
read_tree_block to link blocks which have failed ordinary validation to
the recow_ebs list, and this only happens in case we have a single copy
for this eb. This list is then processed in the main check logic.

So repair_root_generation could possibly be as simple as just linking
root->node to the fs_info->recow_ebs and leave the rest to the existing
logic?

> +
> +	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	btrfs_release_path(&path);
> +	ret = btrfs_commit_transaction(trans, root);
> +	return ret;
> +}
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 4c169c6e3b29..4a3abeb02c81 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -155,4 +155,5 @@ static inline bool is_valid_imode(u32 imode)
>  	return true;
>  }
>  
> +int repair_root_generation(struct btrfs_root *root);
>  #endif
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index a2be0e6d7034..bf3b57f5ad2d 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -4957,6 +4957,7 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>  	struct btrfs_path path;
>  	struct node_refs nrefs;
>  	struct btrfs_root_item *root_item = &root->root_item;
> +	u64 super_generation = btrfs_super_generation(root->fs_info->super_copy);
>  	int ret;
>  	int level;
>  	int err = 0;
> @@ -4978,6 +4979,21 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>  	level = btrfs_header_level(root->node);
>  	btrfs_init_path(&path);
>  
> +	if (btrfs_root_generation(root_item) > super_generation + 1) {
> +		error(
> +	"invalid root generation for root %llu, have %llu expect (0, %llu)",
> +		      root->root_key.objectid, btrfs_root_generation(root_item),
> +		      super_generation + 1);
> +		err |= INVALID_GENERATION;
> +		if (repair) {
> +			ret = repair_root_generation(root);
> +			if (!ret) {
> +				err &= ~INVALID_GENERATION;
> +				printf("Reset generation for root %llu\n",
> +					root->root_key.objectid);
> +			}
> +		}
> +	}
>  	if (btrfs_root_refs(root_item) > 0 ||
>  	    btrfs_disk_key_objectid(&root_item->drop_progress) == 0) {
>  		path.nodes[level] = root->node;
> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
> index d2983fd12eb4..0361fb3382b1 100644
> --- a/check/mode-lowmem.h
> +++ b/check/mode-lowmem.h
> @@ -47,6 +47,7 @@
>  #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
>  #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
>  #define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
> +#define INVALID_GENERATION	(1<<26)	/* Generation is too new */
>  
>  /*
>   * Error bit for low memory mode check.
> 

Re: [PATCH 1/3] btrfs-progs: check/lowmem: Check and repair root generation

[Qu Wenruo]

On 2019/8/10 上午12:10, Nikolay Borisov wrote:
>
>
> On 9.08.19 г. 9:53 ч., Qu Wenruo wrote:
>> Since kernel is going to reject any root item which is newer than super
>> block generation, we need to provide a way to fix such problem in
>> btrfs-check.
>>
>> This patch addes the ability to report and repair root generation in
>> lowmem mode.
>>
>> This is done by cowing the root node, so we will update the root
>> generation along with the root node generation at commit transaction
>> time.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  check/mode-common.c | 29 +++++++++++++++++++++++++++++
>>  check/mode-common.h |  1 +
>>  check/mode-lowmem.c | 16 ++++++++++++++++
>>  check/mode-lowmem.h |  1 +
>>  4 files changed, 47 insertions(+)
>>
>> diff --git a/check/mode-common.c b/check/mode-common.c
>> index d5f6c8ef97b1..a5b86a0ac32a 100644
>> --- a/check/mode-common.c
>> +++ b/check/mode-common.c
>> @@ -924,3 +924,32 @@ int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>>  	}
>>  	return ret;
>>  }
>> +
>> +int repair_root_generation(struct btrfs_root *root)
>> +{
>> +	struct btrfs_trans_handle *trans;
>> +	struct btrfs_path path;
>> +	struct btrfs_key key;
>> +	int ret;
>> +
>> +	trans = btrfs_start_transaction(root, 1);
>> +	if (IS_ERR(trans))
>> +		return PTR_ERR(trans);
>> +
>> +	key.objectid = 0;
>> +	key.type = 0;
>> +	key.offset = 0;
>> +	btrfs_init_path(&path);
>> +
>> +	/* Here we only CoW the tree root to update the geneartion */
>> +	path.lowest_level = btrfs_header_level(root->node);
>> +	root->node->flags |= EXTENT_BAD_TRANSID;
>
> Why do you set EXTENT_BAD_TRANSID? This flag is currently used only in
> read_tree_block to link blocks which have failed ordinary validation to
> the recow_ebs list, and this only happens in case we have a single copy
> for this eb. This list is then processed in the main check logic.

Because we have extra transid check in
btrfs_search_slot()/btrfs_cow_block().

EXTENT_BAD_TRANSID is to suppress such warning.

>
> So repair_root_generation could possibly be as simple as just linking
> root->node to the fs_info->recow_ebs and leave the rest to the existing
> logic?

It doesn't change the root generation.

>
>> +
>> +	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	btrfs_release_path(&path);
>> +	ret = btrfs_commit_transaction(trans, root);
>> +	return ret;
>> +}
>> diff --git a/check/mode-common.h b/check/mode-common.h
>> index 4c169c6e3b29..4a3abeb02c81 100644
>> --- a/check/mode-common.h
>> +++ b/check/mode-common.h
>> @@ -155,4 +155,5 @@ static inline bool is_valid_imode(u32 imode)
>>  	return true;
>>  }
>>
>> +int repair_root_generation(struct btrfs_root *root);
>>  #endif
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index a2be0e6d7034..bf3b57f5ad2d 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -4957,6 +4957,7 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>  	struct btrfs_path path;
>>  	struct node_refs nrefs;
>>  	struct btrfs_root_item *root_item = &root->root_item;
>> +	u64 super_generation = btrfs_super_generation(root->fs_info->super_copy);
>>  	int ret;
>>  	int level;
>>  	int err = 0;
>> @@ -4978,6 +4979,21 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>  	level = btrfs_header_level(root->node);
>>  	btrfs_init_path(&path);
>>
>> +	if (btrfs_root_generation(root_item) > super_generation + 1) {
>> +		error(
>> +	"invalid root generation for root %llu, have %llu expect (0, %llu)",
>> +		      root->root_key.objectid, btrfs_root_generation(root_item),
>> +		      super_generation + 1);
>> +		err |= INVALID_GENERATION;
>> +		if (repair) {
>> +			ret = repair_root_generation(root);
>> +			if (!ret) {
>> +				err &= ~INVALID_GENERATION;
>> +				printf("Reset generation for root %llu\n",
>> +					root->root_key.objectid);
>> +			}
>> +		}
>> +	}
>>  	if (btrfs_root_refs(root_item) > 0 ||
>>  	    btrfs_disk_key_objectid(&root_item->drop_progress) == 0) {
>>  		path.nodes[level] = root->node;
>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>> index d2983fd12eb4..0361fb3382b1 100644
>> --- a/check/mode-lowmem.h
>> +++ b/check/mode-lowmem.h
>> @@ -47,6 +47,7 @@
>>  #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
>>  #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
>>  #define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
>> +#define INVALID_GENERATION	(1<<26)	/* Generation is too new */
>>
>>  /*
>>   * Error bit for low memory mode check.
>>

Re: [PATCH 1/3] btrfs-progs: check/lowmem: Check and repair root generation

[Nikolay Borisov]

On 10.08.19 г. 3:30 ч., Qu Wenruo wrote:
> 
> 
> On 2019/8/10 上午12:10, Nikolay Borisov wrote:
>>
>>
>> On 9.08.19 г. 9:53 ч., Qu Wenruo wrote:
>>> Since kernel is going to reject any root item which is newer than super
>>> block generation, we need to provide a way to fix such problem in
>>> btrfs-check.
>>>
>>> This patch addes the ability to report and repair root generation in
>>> lowmem mode.
>>>
>>> This is done by cowing the root node, so we will update the root
>>> generation along with the root node generation at commit transaction
>>> time.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> ---
>>>  check/mode-common.c | 29 +++++++++++++++++++++++++++++
>>>  check/mode-common.h |  1 +
>>>  check/mode-lowmem.c | 16 ++++++++++++++++
>>>  check/mode-lowmem.h |  1 +
>>>  4 files changed, 47 insertions(+)
>>>
>>> diff --git a/check/mode-common.c b/check/mode-common.c
>>> index d5f6c8ef97b1..a5b86a0ac32a 100644
>>> --- a/check/mode-common.c
>>> +++ b/check/mode-common.c
>>> @@ -924,3 +924,32 @@ int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>>>  	}
>>>  	return ret;
>>>  }
>>> +
>>> +int repair_root_generation(struct btrfs_root *root)
>>> +{
>>> +	struct btrfs_trans_handle *trans;
>>> +	struct btrfs_path path;
>>> +	struct btrfs_key key;
>>> +	int ret;
>>> +
>>> +	trans = btrfs_start_transaction(root, 1);
>>> +	if (IS_ERR(trans))
>>> +		return PTR_ERR(trans);
>>> +
>>> +	key.objectid = 0;
>>> +	key.type = 0;
>>> +	key.offset = 0;
>>> +	btrfs_init_path(&path);
>>> +
>>> +	/* Here we only CoW the tree root to update the geneartion */
>>> +	path.lowest_level = btrfs_header_level(root->node);
>>> +	root->node->flags |= EXTENT_BAD_TRANSID;
>>
>> Why do you set EXTENT_BAD_TRANSID? This flag is currently used only in
>> read_tree_block to link blocks which have failed ordinary validation to
>> the recow_ebs list, and this only happens in case we have a single copy
>> for this eb. This list is then processed in the main check logic.
> 
> Because we have extra transid check in
> btrfs_search_slot()/btrfs_cow_block().
> 
> EXTENT_BAD_TRANSID is to suppress such warning.

nod

> 
>>
>> So repair_root_generation could possibly be as simple as just linking
>> root->node to the fs_info->recow_ebs and leave the rest to the existing
>> logic?
> 
> It doesn't change the root generation.

recow_extent_buffer seems to be doing exactly the same thing
repair_root_generation does - findw the root item and commits the
transaction. What am I missing?

> 
>>
>>> +
>>> +	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	btrfs_release_path(&path);
>>> +	ret = btrfs_commit_transaction(trans, root);
>>> +	return ret;
>>> +}
>>> diff --git a/check/mode-common.h b/check/mode-common.h
>>> index 4c169c6e3b29..4a3abeb02c81 100644
>>> --- a/check/mode-common.h
>>> +++ b/check/mode-common.h
>>> @@ -155,4 +155,5 @@ static inline bool is_valid_imode(u32 imode)
>>>  	return true;
>>>  }
>>>
>>> +int repair_root_generation(struct btrfs_root *root);
>>>  #endif
>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>> index a2be0e6d7034..bf3b57f5ad2d 100644
>>> --- a/check/mode-lowmem.c
>>> +++ b/check/mode-lowmem.c
>>> @@ -4957,6 +4957,7 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>>  	struct btrfs_path path;
>>>  	struct node_refs nrefs;
>>>  	struct btrfs_root_item *root_item = &root->root_item;
>>> +	u64 super_generation = btrfs_super_generation(root->fs_info->super_copy);
>>>  	int ret;
>>>  	int level;
>>>  	int err = 0;
>>> @@ -4978,6 +4979,21 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>>  	level = btrfs_header_level(root->node);
>>>  	btrfs_init_path(&path);
>>>
>>> +	if (btrfs_root_generation(root_item) > super_generation + 1) {
>>> +		error(
>>> +	"invalid root generation for root %llu, have %llu expect (0, %llu)",
>>> +		      root->root_key.objectid, btrfs_root_generation(root_item),
>>> +		      super_generation + 1);
>>> +		err |= INVALID_GENERATION;
>>> +		if (repair) {
>>> +			ret = repair_root_generation(root);
>>> +			if (!ret) {
>>> +				err &= ~INVALID_GENERATION;
>>> +				printf("Reset generation for root %llu\n",
>>> +					root->root_key.objectid);
>>> +			}
>>> +		}
>>> +	}
>>>  	if (btrfs_root_refs(root_item) > 0 ||
>>>  	    btrfs_disk_key_objectid(&root_item->drop_progress) == 0) {
>>>  		path.nodes[level] = root->node;
>>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>>> index d2983fd12eb4..0361fb3382b1 100644
>>> --- a/check/mode-lowmem.h
>>> +++ b/check/mode-lowmem.h
>>> @@ -47,6 +47,7 @@
>>>  #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
>>>  #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
>>>  #define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
>>> +#define INVALID_GENERATION	(1<<26)	/* Generation is too new */
>>>
>>>  /*
>>>   * Error bit for low memory mode check.
>>>
> 

Re: [PATCH 1/3] btrfs-progs: check/lowmem: Check and repair root generation

[Qu Wenruo]

[...]
>>
>> Because we have extra transid check in
>> btrfs_search_slot()/btrfs_cow_block().
>>
>> EXTENT_BAD_TRANSID is to suppress such warning.
>
> nod
>
>>
>>>
>>> So repair_root_generation could possibly be as simple as just linking
>>> root->node to the fs_info->recow_ebs and leave the rest to the existing
>>> logic?
>>
>> It doesn't change the root generation.
>
> recow_extent_buffer seems to be doing exactly the same thing
> repair_root_generation does - findw the root item and commits the
> transaction. What am I missing?

The way it gets the root is not correct.

Using header owner is not good enough for locating the tree owning the
tree blocks.

Cases like log trees and shared tree blocks (especially for cases like
original tree got deleted) are the main corner cases where the old
behavior can't handle.

Thanks,
Qu

>
>>
>>>
>>>> +
>>>> +	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
>>>> +	if (ret < 0)
>>>> +		return ret;
>>>> +
>>>> +	btrfs_release_path(&path);
>>>> +	ret = btrfs_commit_transaction(trans, root);
>>>> +	return ret;
>>>> +}
>>>> diff --git a/check/mode-common.h b/check/mode-common.h
>>>> index 4c169c6e3b29..4a3abeb02c81 100644
>>>> --- a/check/mode-common.h
>>>> +++ b/check/mode-common.h
>>>> @@ -155,4 +155,5 @@ static inline bool is_valid_imode(u32 imode)
>>>>  	return true;
>>>>  }
>>>>
>>>> +int repair_root_generation(struct btrfs_root *root);
>>>>  #endif
>>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>>> index a2be0e6d7034..bf3b57f5ad2d 100644
>>>> --- a/check/mode-lowmem.c
>>>> +++ b/check/mode-lowmem.c
>>>> @@ -4957,6 +4957,7 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>>>  	struct btrfs_path path;
>>>>  	struct node_refs nrefs;
>>>>  	struct btrfs_root_item *root_item = &root->root_item;
>>>> +	u64 super_generation = btrfs_super_generation(root->fs_info->super_copy);
>>>>  	int ret;
>>>>  	int level;
>>>>  	int err = 0;
>>>> @@ -4978,6 +4979,21 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>>>  	level = btrfs_header_level(root->node);
>>>>  	btrfs_init_path(&path);
>>>>
>>>> +	if (btrfs_root_generation(root_item) > super_generation + 1) {
>>>> +		error(
>>>> +	"invalid root generation for root %llu, have %llu expect (0, %llu)",
>>>> +		      root->root_key.objectid, btrfs_root_generation(root_item),
>>>> +		      super_generation + 1);
>>>> +		err |= INVALID_GENERATION;
>>>> +		if (repair) {
>>>> +			ret = repair_root_generation(root);
>>>> +			if (!ret) {
>>>> +				err &= ~INVALID_GENERATION;
>>>> +				printf("Reset generation for root %llu\n",
>>>> +					root->root_key.objectid);
>>>> +			}
>>>> +		}
>>>> +	}
>>>>  	if (btrfs_root_refs(root_item) > 0 ||
>>>>  	    btrfs_disk_key_objectid(&root_item->drop_progress) == 0) {
>>>>  		path.nodes[level] = root->node;
>>>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>>>> index d2983fd12eb4..0361fb3382b1 100644
>>>> --- a/check/mode-lowmem.h
>>>> +++ b/check/mode-lowmem.h
>>>> @@ -47,6 +47,7 @@
>>>>  #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
>>>>  #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
>>>>  #define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
>>>> +#define INVALID_GENERATION	(1<<26)	/* Generation is too new */
>>>>
>>>>  /*
>>>>   * Error bit for low memory mode check.
>>>>
>>

Re: [PATCH 1/3] btrfs-progs: check/lowmem: Check and repair root generation

[Qu Wenruo]

On 2019/8/10 下午5:24, Qu Wenruo wrote:
>
> [...]
>>>
>>> Because we have extra transid check in
>>> btrfs_search_slot()/btrfs_cow_block().
>>>
>>> EXTENT_BAD_TRANSID is to suppress such warning.
>>
>> nod
>>
>>>
>>>>
>>>> So repair_root_generation could possibly be as simple as just linking
>>>> root->node to the fs_info->recow_ebs and leave the rest to the existing
>>>> logic?
>>>
>>> It doesn't change the root generation.
>>
>> recow_extent_buffer seems to be doing exactly the same thing
>> repair_root_generation does - findw the root item and commits the
>> transaction. What am I missing?
>
> The way it gets the root is not correct.
>
> Using header owner is not good enough for locating the tree owning the
> tree blocks.
>
> Cases like log trees and shared tree blocks (especially for cases like
> original tree got deleted) are the main corner cases where the old
> behavior can't handle.

My bad, in fact log tree won't exist in repair mode, and since it's the
tree root node, it won't be shared with other trees, so the existing
facility is completely fine with the use case.

Thanks for pointing this out!
Qu

>
> Thanks,
> Qu
>
>>
>>>
>>>>
>>>>> +
>>>>> +	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
>>>>> +	if (ret < 0)
>>>>> +		return ret;
>>>>> +
>>>>> +	btrfs_release_path(&path);
>>>>> +	ret = btrfs_commit_transaction(trans, root);
>>>>> +	return ret;
>>>>> +}
>>>>> diff --git a/check/mode-common.h b/check/mode-common.h
>>>>> index 4c169c6e3b29..4a3abeb02c81 100644
>>>>> --- a/check/mode-common.h
>>>>> +++ b/check/mode-common.h
>>>>> @@ -155,4 +155,5 @@ static inline bool is_valid_imode(u32 imode)
>>>>>  	return true;
>>>>>  }
>>>>>
>>>>> +int repair_root_generation(struct btrfs_root *root);
>>>>>  #endif
>>>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>>>> index a2be0e6d7034..bf3b57f5ad2d 100644
>>>>> --- a/check/mode-lowmem.c
>>>>> +++ b/check/mode-lowmem.c
>>>>> @@ -4957,6 +4957,7 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>>>>  	struct btrfs_path path;
>>>>>  	struct node_refs nrefs;
>>>>>  	struct btrfs_root_item *root_item = &root->root_item;
>>>>> +	u64 super_generation = btrfs_super_generation(root->fs_info->super_copy);
>>>>>  	int ret;
>>>>>  	int level;
>>>>>  	int err = 0;
>>>>> @@ -4978,6 +4979,21 @@ static int check_btrfs_root(struct btrfs_root *root, int check_all)
>>>>>  	level = btrfs_header_level(root->node);
>>>>>  	btrfs_init_path(&path);
>>>>>
>>>>> +	if (btrfs_root_generation(root_item) > super_generation + 1) {
>>>>> +		error(
>>>>> +	"invalid root generation for root %llu, have %llu expect (0, %llu)",
>>>>> +		      root->root_key.objectid, btrfs_root_generation(root_item),
>>>>> +		      super_generation + 1);
>>>>> +		err |= INVALID_GENERATION;
>>>>> +		if (repair) {
>>>>> +			ret = repair_root_generation(root);
>>>>> +			if (!ret) {
>>>>> +				err &= ~INVALID_GENERATION;
>>>>> +				printf("Reset generation for root %llu\n",
>>>>> +					root->root_key.objectid);
>>>>> +			}
>>>>> +		}
>>>>> +	}
>>>>>  	if (btrfs_root_refs(root_item) > 0 ||
>>>>>  	    btrfs_disk_key_objectid(&root_item->drop_progress) == 0) {
>>>>>  		path.nodes[level] = root->node;
>>>>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>>>>> index d2983fd12eb4..0361fb3382b1 100644
>>>>> --- a/check/mode-lowmem.h
>>>>> +++ b/check/mode-lowmem.h
>>>>> @@ -47,6 +47,7 @@
>>>>>  #define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
>>>>>  #define DIR_ITEM_HASH_MISMATCH	(1<<24) /* Dir item hash mismatch */
>>>>>  #define INODE_MODE_ERROR	(1<<25) /* Bad inode mode */
>>>>> +#define INVALID_GENERATION	(1<<26)	/* Generation is too new */
>>>>>
>>>>>  /*
>>>>>   * Error bit for low memory mode check.
>>>>>
>>>