[PATCH] btrfs-progs: Update backup roots when writing super blocks

[PATCH] btrfs-progs: Update backup roots when writing super blocks

[Qu Wenruo]

The code is mostly ported from kernel with minimal change.

Since btrfs-progs doesn't support replaying log, there is some code
unnecessary for btrfs-progs, but to keep the code the same, that
unnecessary code is kept as it.

Now "btrfs check --repair" will update backup roots correctly.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 disk-io.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/disk-io.c b/disk-io.c
index 133835a4d063..f28a6b526391 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1621,6 +1621,83 @@ write_err:
 	return ret;
 }
 
+/*
+ * copy all the root pointers into the super backup array.
+ * this will bump the backup pointer by one when it is
+ * done
+ */
+static void backup_super_roots(struct btrfs_fs_info *info)
+{
+	struct btrfs_root_backup *root_backup;
+	int next_backup;
+	int last_backup;
+
+	last_backup = find_best_backup_root(info->super_copy);
+	next_backup = (last_backup + 1) % BTRFS_NUM_BACKUP_ROOTS;
+
+	/* just overwrite the last backup if we're at the same generation */
+	root_backup = info->super_copy->super_roots + last_backup;
+	if (btrfs_backup_tree_root_gen(root_backup) ==
+	    btrfs_header_generation(info->tree_root->node))
+		next_backup = last_backup;
+
+	root_backup = info->super_copy->super_roots + next_backup;
+
+	/*
+	 * make sure all of our padding and empty slots get zero filled
+	 * regardless of which ones we use today
+	 */
+	memset(root_backup, 0, sizeof(*root_backup));
+	btrfs_set_backup_tree_root(root_backup, info->tree_root->node->start);
+	btrfs_set_backup_tree_root_gen(root_backup,
+			       btrfs_header_generation(info->tree_root->node));
+	btrfs_set_backup_tree_root_level(root_backup,
+			       btrfs_header_level(info->tree_root->node));
+
+	btrfs_set_backup_chunk_root(root_backup, info->chunk_root->node->start);
+	btrfs_set_backup_chunk_root_gen(root_backup,
+			       btrfs_header_generation(info->chunk_root->node));
+	btrfs_set_backup_chunk_root_level(root_backup,
+			       btrfs_header_level(info->chunk_root->node));
+
+	btrfs_set_backup_extent_root(root_backup, info->extent_root->node->start);
+	btrfs_set_backup_extent_root_gen(root_backup,
+			       btrfs_header_generation(info->extent_root->node));
+	btrfs_set_backup_extent_root_level(root_backup,
+			       btrfs_header_level(info->extent_root->node));
+	/*
+	 * we might commit during log recovery, which happens before we set
+	 * the fs_root.  Make sure it is valid before we fill it in.
+	 */
+	if (info->fs_root && info->fs_root->node) {
+		btrfs_set_backup_fs_root(root_backup,
+					 info->fs_root->node->start);
+		btrfs_set_backup_fs_root_gen(root_backup,
+			       btrfs_header_generation(info->fs_root->node));
+		btrfs_set_backup_fs_root_level(root_backup,
+			       btrfs_header_level(info->fs_root->node));
+	}
+
+	btrfs_set_backup_dev_root(root_backup, info->dev_root->node->start);
+	btrfs_set_backup_dev_root_gen(root_backup,
+			       btrfs_header_generation(info->dev_root->node));
+	btrfs_set_backup_dev_root_level(root_backup,
+				       btrfs_header_level(info->dev_root->node));
+
+	btrfs_set_backup_csum_root(root_backup, info->csum_root->node->start);
+	btrfs_set_backup_csum_root_gen(root_backup,
+			       btrfs_header_generation(info->csum_root->node));
+	btrfs_set_backup_csum_root_level(root_backup,
+			       btrfs_header_level(info->csum_root->node));
+
+	btrfs_set_backup_total_bytes(root_backup,
+			     btrfs_super_total_bytes(info->super_copy));
+	btrfs_set_backup_bytes_used(root_backup,
+			     btrfs_super_bytes_used(info->super_copy));
+	btrfs_set_backup_num_devices(root_backup,
+			     btrfs_super_num_devices(info->super_copy));
+};
+
 int write_all_supers(struct btrfs_fs_info *fs_info)
 {
 	struct list_head *head = &fs_info->fs_devices->devices;
@@ -1630,6 +1707,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info)
 	int ret;
 	u64 flags;
 
+	backup_super_roots(fs_info);
 	sb = fs_info->super_copy;
 	dev_item = &sb->dev_item;
 	list_for_each_entry(dev, head, dev_list) {
-- 
2.20.1

Re: [PATCH] btrfs-progs: Update backup roots when writing super blocks

[Nikolay Borisov]

On 25.12.18 г. 7:28 ч., Qu Wenruo wrote:
> The code is mostly ported from kernel with minimal change.
> 
> Since btrfs-progs doesn't support replaying log, there is some code
> unnecessary for btrfs-progs, but to keep the code the same, that
> unnecessary code is kept as it.
> 
> Now "btrfs check --repair" will update backup roots correctly.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  disk-io.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/disk-io.c b/disk-io.c
> index 133835a4d063..f28a6b526391 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1621,6 +1621,83 @@ write_err:
>  	return ret;
>  }
>  
> +/*
> + * copy all the root pointers into the super backup array.
> + * this will bump the backup pointer by one when it is
> + * done
> + */
> +static void backup_super_roots(struct btrfs_fs_info *info)
> +{
> +	struct btrfs_root_backup *root_backup;
> +	int next_backup;
> +	int last_backup;
> +
> +	last_backup = find_best_backup_root(info->super_copy);

Slightly offtopic: but find_best_backup_root only partially resembles
it's kernel counterpart - find_newest_super_backup. And it's missing the
code for handling wraparound. So in kernel space if the newest root is
at index 3 and index 0 backup also has the same gen then we switch the
index to 0, why is this needed? In any case I think
'find_best_backup_root' in progs should be renamed to
'find_newest_super_backup' and copied from kernel space.

> +	next_backup = (last_backup + 1) % BTRFS_NUM_BACKUP_ROOTS;

This logic is different than the kernel one which does:
last_backup = (next_backup + BTRFS_NUM_BACKUP_ROOTS - 1) %
                BTRFS_NUM_BACKUP_ROOTS;

So in the kernel when info->backup_root_index == 0 then the newest
backup root is going to be written at position 3, the next one in pos 1,
the next in pos 2 and so on. OTOH in userspace we might potentially
start writing at 1 (which doesn't seem problematic per se). So why the
difference in logic?

> +
> +	/* just overwrite the last backup if we're at the same generation */
> +	root_backup = info->super_copy->super_roots + last_backup;
> +	if (btrfs_backup_tree_root_gen(root_backup) ==
> +	    btrfs_header_generation(info->tree_root->node))
> +		next_backup = last_backup;
> +
> +	root_backup = info->super_copy->super_roots + next_backup;
> +
> +	/*
> +	 * make sure all of our padding and empty slots get zero filled
> +	 * regardless of which ones we use today
> +	 */
> +	memset(root_backup, 0, sizeof(*root_backup));
> +	btrfs_set_backup_tree_root(root_backup, info->tree_root->node->start);
> +	btrfs_set_backup_tree_root_gen(root_backup,
> +			       btrfs_header_generation(info->tree_root->node));
> +	btrfs_set_backup_tree_root_level(root_backup,
> +			       btrfs_header_level(info->tree_root->node));
> +
> +	btrfs_set_backup_chunk_root(root_backup, info->chunk_root->node->start);
> +	btrfs_set_backup_chunk_root_gen(root_backup,
> +			       btrfs_header_generation(info->chunk_root->node));
> +	btrfs_set_backup_chunk_root_level(root_backup,
> +			       btrfs_header_level(info->chunk_root->node));
> +
> +	btrfs_set_backup_extent_root(root_backup, info->extent_root->node->start);
> +	btrfs_set_backup_extent_root_gen(root_backup,
> +			       btrfs_header_generation(info->extent_root->node));
> +	btrfs_set_backup_extent_root_level(root_backup,
> +			       btrfs_header_level(info->extent_root->node));
> +	/*
> +	 * we might commit during log recovery, which happens before we set
> +	 * the fs_root.  Make sure it is valid before we fill it in.
> +	 */
> +	if (info->fs_root && info->fs_root->node) {
> +		btrfs_set_backup_fs_root(root_backup,
> +					 info->fs_root->node->start);
> +		btrfs_set_backup_fs_root_gen(root_backup,
> +			       btrfs_header_generation(info->fs_root->node));
> +		btrfs_set_backup_fs_root_level(root_backup,
> +			       btrfs_header_level(info->fs_root->node));
> +	}
> +
> +	btrfs_set_backup_dev_root(root_backup, info->dev_root->node->start);
> +	btrfs_set_backup_dev_root_gen(root_backup,
> +			       btrfs_header_generation(info->dev_root->node));
> +	btrfs_set_backup_dev_root_level(root_backup,
> +				       btrfs_header_level(info->dev_root->node));
> +
> +	btrfs_set_backup_csum_root(root_backup, info->csum_root->node->start);
> +	btrfs_set_backup_csum_root_gen(root_backup,
> +			       btrfs_header_generation(info->csum_root->node));
> +	btrfs_set_backup_csum_root_level(root_backup,
> +			       btrfs_header_level(info->csum_root->node));
> +
> +	btrfs_set_backup_total_bytes(root_backup,
> +			     btrfs_super_total_bytes(info->super_copy));
> +	btrfs_set_backup_bytes_used(root_backup,
> +			     btrfs_super_bytes_used(info->super_copy));
> +	btrfs_set_backup_num_devices(root_backup,
> +			     btrfs_super_num_devices(info->super_copy));
> +};
> +
>  int write_all_supers(struct btrfs_fs_info *fs_info)
>  {
>  	struct list_head *head = &fs_info->fs_devices->devices;
> @@ -1630,6 +1707,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info)
>  	int ret;
>  	u64 flags;
>  
> +	backup_super_roots(fs_info);
>  	sb = fs_info->super_copy;
>  	dev_item = &sb->dev_item;
>  	list_for_each_entry(dev, head, dev_list) {
> 

Re: [PATCH] btrfs-progs: Update backup roots when writing super blocks

[Qu Wenruo]

On 2019/1/2 下午4:57, Nikolay Borisov wrote:
> 
> 
> On 25.12.18 г. 7:28 ч., Qu Wenruo wrote:
>> The code is mostly ported from kernel with minimal change.
>>
>> Since btrfs-progs doesn't support replaying log, there is some code
>> unnecessary for btrfs-progs, but to keep the code the same, that
>> unnecessary code is kept as it.
>>
>> Now "btrfs check --repair" will update backup roots correctly.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  disk-io.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 78 insertions(+)
>>
>> diff --git a/disk-io.c b/disk-io.c
>> index 133835a4d063..f28a6b526391 100644
>> --- a/disk-io.c
>> +++ b/disk-io.c
>> @@ -1621,6 +1621,83 @@ write_err:
>>  	return ret;
>>  }
>>  
>> +/*
>> + * copy all the root pointers into the super backup array.
>> + * this will bump the backup pointer by one when it is
>> + * done
>> + */
>> +static void backup_super_roots(struct btrfs_fs_info *info)
>> +{
>> +	struct btrfs_root_backup *root_backup;
>> +	int next_backup;
>> +	int last_backup;
>> +
>> +	last_backup = find_best_backup_root(info->super_copy);
> 
> Slightly offtopic: but find_best_backup_root only partially resembles
> it's kernel counterpart - find_newest_super_backup. And it's missing the
> code for handling wraparound. So in kernel space if the newest root is
> at index 3 and index 0 backup also has the same gen then we switch the
> index to 0, why is this needed?

I'm not sure either.

IIRC kernel shouldn't create two backup roots with the same gen.

> In any case I think
> 'find_best_backup_root' in progs should be renamed to
> 'find_newest_super_backup' and copied from kernel space.
> 
>> +	next_backup = (last_backup + 1) % BTRFS_NUM_BACKUP_ROOTS;
> 
> This logic is different than the kernel one which does:
> last_backup = (next_backup + BTRFS_NUM_BACKUP_ROOTS - 1) %
>                 BTRFS_NUM_BACKUP_ROOTS;
> 
> So in the kernel when info->backup_root_index == 0 then the newest
> backup root is going to be written at position 3, the next one in pos 1,
> the next in pos 2 and so on. OTOH in userspace we might potentially
> start writing at 1 (which doesn't seem problematic per se). So why the
> difference in logic?

The logic is the same, for kernel it's 'last_backup' which should, and
is, going backward one slot.

And the next one is still written to next slot.

Just as the dump super shows:
$ btrfs ins dump-super -f /dev/data/btrfs | grep -A1 "backup "
	backup 0:
		backup_tree_root:	30818304	gen: 10	level: 0
--
	backup 1:
		backup_tree_root:	30621696	gen: 7	level: 0
--
	backup 2:
		backup_tree_root:	30703616	gen: 8	level: 0
--
	backup 3:
		backup_tree_root:	30785536	gen: 9	level: 0

So the logic should be the same.

Just simpler implementation in btrfs-progs, without
fs_info->backup_root_index.

Thanks,
Qu

> 
>> +
>> +	/* just overwrite the last backup if we're at the same generation */
>> +	root_backup = info->super_copy->super_roots + last_backup;
>> +	if (btrfs_backup_tree_root_gen(root_backup) ==
>> +	    btrfs_header_generation(info->tree_root->node))
>> +		next_backup = last_backup;
>> +
>> +	root_backup = info->super_copy->super_roots + next_backup;
>> +
>> +	/*
>> +	 * make sure all of our padding and empty slots get zero filled
>> +	 * regardless of which ones we use today
>> +	 */
>> +	memset(root_backup, 0, sizeof(*root_backup));
>> +	btrfs_set_backup_tree_root(root_backup, info->tree_root->node->start);
>> +	btrfs_set_backup_tree_root_gen(root_backup,
>> +			       btrfs_header_generation(info->tree_root->node));
>> +	btrfs_set_backup_tree_root_level(root_backup,
>> +			       btrfs_header_level(info->tree_root->node));
>> +
>> +	btrfs_set_backup_chunk_root(root_backup, info->chunk_root->node->start);
>> +	btrfs_set_backup_chunk_root_gen(root_backup,
>> +			       btrfs_header_generation(info->chunk_root->node));
>> +	btrfs_set_backup_chunk_root_level(root_backup,
>> +			       btrfs_header_level(info->chunk_root->node));
>> +
>> +	btrfs_set_backup_extent_root(root_backup, info->extent_root->node->start);
>> +	btrfs_set_backup_extent_root_gen(root_backup,
>> +			       btrfs_header_generation(info->extent_root->node));
>> +	btrfs_set_backup_extent_root_level(root_backup,
>> +			       btrfs_header_level(info->extent_root->node));
>> +	/*
>> +	 * we might commit during log recovery, which happens before we set
>> +	 * the fs_root.  Make sure it is valid before we fill it in.
>> +	 */
>> +	if (info->fs_root && info->fs_root->node) {
>> +		btrfs_set_backup_fs_root(root_backup,
>> +					 info->fs_root->node->start);
>> +		btrfs_set_backup_fs_root_gen(root_backup,
>> +			       btrfs_header_generation(info->fs_root->node));
>> +		btrfs_set_backup_fs_root_level(root_backup,
>> +			       btrfs_header_level(info->fs_root->node));
>> +	}
>> +
>> +	btrfs_set_backup_dev_root(root_backup, info->dev_root->node->start);
>> +	btrfs_set_backup_dev_root_gen(root_backup,
>> +			       btrfs_header_generation(info->dev_root->node));
>> +	btrfs_set_backup_dev_root_level(root_backup,
>> +				       btrfs_header_level(info->dev_root->node));
>> +
>> +	btrfs_set_backup_csum_root(root_backup, info->csum_root->node->start);
>> +	btrfs_set_backup_csum_root_gen(root_backup,
>> +			       btrfs_header_generation(info->csum_root->node));
>> +	btrfs_set_backup_csum_root_level(root_backup,
>> +			       btrfs_header_level(info->csum_root->node));
>> +
>> +	btrfs_set_backup_total_bytes(root_backup,
>> +			     btrfs_super_total_bytes(info->super_copy));
>> +	btrfs_set_backup_bytes_used(root_backup,
>> +			     btrfs_super_bytes_used(info->super_copy));
>> +	btrfs_set_backup_num_devices(root_backup,
>> +			     btrfs_super_num_devices(info->super_copy));
>> +};
>> +
>>  int write_all_supers(struct btrfs_fs_info *fs_info)
>>  {
>>  	struct list_head *head = &fs_info->fs_devices->devices;
>> @@ -1630,6 +1707,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info)
>>  	int ret;
>>  	u64 flags;
>>  
>> +	backup_super_roots(fs_info);
>>  	sb = fs_info->super_copy;
>>  	dev_item = &sb->dev_item;
>>  	list_for_each_entry(dev, head, dev_list) {
>>

Re: [PATCH] btrfs-progs: Update backup roots when writing super blocks

[Nikolay Borisov]

On 2.01.19 г. 12:08 ч., Qu Wenruo wrote:
> 
> 
> On 2019/1/2 下午4:57, Nikolay Borisov wrote:
>>
>>
>> On 25.12.18 г. 7:28 ч., Qu Wenruo wrote:
>>> The code is mostly ported from kernel with minimal change.
>>>
>>> Since btrfs-progs doesn't support replaying log, there is some code
>>> unnecessary for btrfs-progs, but to keep the code the same, that
>>> unnecessary code is kept as it.
>>>
>>> Now "btrfs check --repair" will update backup roots correctly.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> ---
>>>  disk-io.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 78 insertions(+)
>>>
>>> diff --git a/disk-io.c b/disk-io.c
>>> index 133835a4d063..f28a6b526391 100644
>>> --- a/disk-io.c
>>> +++ b/disk-io.c
>>> @@ -1621,6 +1621,83 @@ write_err:
>>>  	return ret;
>>>  }
>>>  
>>> +/*
>>> + * copy all the root pointers into the super backup array.
>>> + * this will bump the backup pointer by one when it is
>>> + * done
>>> + */
>>> +static void backup_super_roots(struct btrfs_fs_info *info)
>>> +{
>>> +	struct btrfs_root_backup *root_backup;
>>> +	int next_backup;
>>> +	int last_backup;
>>> +
>>> +	last_backup = find_best_backup_root(info->super_copy);
>>
>> Slightly offtopic: but find_best_backup_root only partially resembles
>> it's kernel counterpart - find_newest_super_backup. And it's missing the
>> code for handling wraparound. So in kernel space if the newest root is
>> at index 3 and index 0 backup also has the same gen then we switch the
>> index to 0, why is this needed?
> 
> I'm not sure either.
> 
> IIRC kernel shouldn't create two backup roots with the same gen.
> 
>> In any case I think
>> 'find_best_backup_root' in progs should be renamed to
>> 'find_newest_super_backup' and copied from kernel space.
>>
>>> +	next_backup = (last_backup + 1) % BTRFS_NUM_BACKUP_ROOTS;
>>
>> This logic is different than the kernel one which does:
>> last_backup = (next_backup + BTRFS_NUM_BACKUP_ROOTS - 1) %
>>                 BTRFS_NUM_BACKUP_ROOTS;
>>
>> So in the kernel when info->backup_root_index == 0 then the newest
>> backup root is going to be written at position 3, the next one in pos 1,
>> the next in pos 2 and so on. OTOH in userspace we might potentially
>> start writing at 1 (which doesn't seem problematic per se). So why the
>> difference in logic?
> 
> The logic is the same, for kernel it's 'last_backup' which should, and
> is, going backward one slot.
> 
> And the next one is still written to next slot.

On a second look at the code you are right and I like more progs'
version - it's more explicit. I think the performance cost is going to
be negligible so it might be worth it copying the implementation to
kernel (once  the overflow handling is figured out, I've asked Chris for
clarification).

> 
> Just as the dump super shows:
> $ btrfs ins dump-super -f /dev/data/btrfs | grep -A1 "backup "
> 	backup 0:
> 		backup_tree_root:	30818304	gen: 10	level: 0
> --
> 	backup 1:
> 		backup_tree_root:	30621696	gen: 7	level: 0
> --
> 	backup 2:
> 		backup_tree_root:	30703616	gen: 8	level: 0
> --
> 	backup 3:
> 		backup_tree_root:	30785536	gen: 9	level: 0
> 
> So the logic should be the same.
> 
> Just simpler implementation in btrfs-progs, without
> fs_info->backup_root_index.



<snip>

Re: [PATCH] btrfs-progs: Update backup roots when writing super blocks

[Nikolay Borisov]

On 2.01.19 г. 12:08 ч., Qu Wenruo wrote:
> 
> 
> On 2019/1/2 下午4:57, Nikolay Borisov wrote:
>>
>>
>> On 25.12.18 г. 7:28 ч., Qu Wenruo wrote:
>>> The code is mostly ported from kernel with minimal change.
>>>
>>> Since btrfs-progs doesn't support replaying log, there is some code
>>> unnecessary for btrfs-progs, but to keep the code the same, that
>>> unnecessary code is kept as it.
>>>
>>> Now "btrfs check --repair" will update backup roots correctly.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> ---
>>>  disk-io.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 78 insertions(+)
>>>
>>> diff --git a/disk-io.c b/disk-io.c
>>> index 133835a4d063..f28a6b526391 100644
>>> --- a/disk-io.c
>>> +++ b/disk-io.c
>>> @@ -1621,6 +1621,83 @@ write_err:
>>>  	return ret;
>>>  }
>>>  
>>> +/*
>>> + * copy all the root pointers into the super backup array.
>>> + * this will bump the backup pointer by one when it is
>>> + * done
>>> + */
>>> +static void backup_super_roots(struct btrfs_fs_info *info)
>>> +{
>>> +	struct btrfs_root_backup *root_backup;
>>> +	int next_backup;
>>> +	int last_backup;
>>> +
>>> +	last_backup = find_best_backup_root(info->super_copy);
>>
>> Slightly offtopic: but find_best_backup_root only partially resembles
>> it's kernel counterpart - find_newest_super_backup. And it's missing the
>> code for handling wraparound. So in kernel space if the newest root is
>> at index 3 and index 0 backup also has the same gen then we switch the
>> index to 0, why is this needed?
> 
> I'm not sure either.
> 
> IIRC kernel shouldn't create two backup roots with the same gen.

So I heard back from Chris, his original code could have created
duplicates but since fed3b381145e ("Btrfs: do not backup tree roots when
fsync") this is no longer the case. The overflow handling should have
been removed as part of fed3b381145e but it didn't. So it makes sense to
actually remove it in a future patch, perhaps when the progs and kernel
versions are unified.

> 
>> In any case I think
>> 'find_best_backup_root' in progs should be renamed to
>> 'find_newest_super_backup' and copied from kernel space.
>>
>>> +	next_backup = (last_backup + 1) % BTRFS_NUM_BACKUP_ROOTS;
>>
>> This logic is different than the kernel one which does:
>> last_backup = (next_backup + BTRFS_NUM_BACKUP_ROOTS - 1) %
>>                 BTRFS_NUM_BACKUP_ROOTS;
>>
>> So in the kernel when info->backup_root_index == 0 then the newest
>> backup root is going to be written at position 3, the next one in pos 1,
>> the next in pos 2 and so on. OTOH in userspace we might potentially
>> start writing at 1 (which doesn't seem problematic per se). So why the
>> difference in logic?
> 
> The logic is the same, for kernel it's 'last_backup' which should, and
> is, going backward one slot.
> 
> And the next one is still written to next slot.
> 
> Just as the dump super shows:
> $ btrfs ins dump-super -f /dev/data/btrfs | grep -A1 "backup "
> 	backup 0:
> 		backup_tree_root:	30818304	gen: 10	level: 0
> --
> 	backup 1:
> 		backup_tree_root:	30621696	gen: 7	level: 0
> --
> 	backup 2:
> 		backup_tree_root:	30703616	gen: 8	level: 0
> --
> 	backup 3:
> 		backup_tree_root:	30785536	gen: 9	level: 0
> 
> So the logic should be the same.
> 
> Just simpler implementation in btrfs-progs, without
> fs_info->backup_root_index.
> 
> Thanks,
> Qu
> 
>>
>>> +
>>> +	/* just overwrite the last backup if we're at the same generation */
>>> +	root_backup = info->super_copy->super_roots + last_backup;
>>> +	if (btrfs_backup_tree_root_gen(root_backup) ==
>>> +	    btrfs_header_generation(info->tree_root->node))
>>> +		next_backup = last_backup;
>>> +
>>> +	root_backup = info->super_copy->super_roots + next_backup;
>>> +
>>> +	/*
>>> +	 * make sure all of our padding and empty slots get zero filled
>>> +	 * regardless of which ones we use today
>>> +	 */
>>> +	memset(root_backup, 0, sizeof(*root_backup));
>>> +	btrfs_set_backup_tree_root(root_backup, info->tree_root->node->start);
>>> +	btrfs_set_backup_tree_root_gen(root_backup,
>>> +			       btrfs_header_generation(info->tree_root->node));
>>> +	btrfs_set_backup_tree_root_level(root_backup,
>>> +			       btrfs_header_level(info->tree_root->node));
>>> +
>>> +	btrfs_set_backup_chunk_root(root_backup, info->chunk_root->node->start);
>>> +	btrfs_set_backup_chunk_root_gen(root_backup,
>>> +			       btrfs_header_generation(info->chunk_root->node));
>>> +	btrfs_set_backup_chunk_root_level(root_backup,
>>> +			       btrfs_header_level(info->chunk_root->node));
>>> +
>>> +	btrfs_set_backup_extent_root(root_backup, info->extent_root->node->start);
>>> +	btrfs_set_backup_extent_root_gen(root_backup,
>>> +			       btrfs_header_generation(info->extent_root->node));
>>> +	btrfs_set_backup_extent_root_level(root_backup,
>>> +			       btrfs_header_level(info->extent_root->node));
>>> +	/*
>>> +	 * we might commit during log recovery, which happens before we set
>>> +	 * the fs_root.  Make sure it is valid before we fill it in.
>>> +	 */
>>> +	if (info->fs_root && info->fs_root->node) {
>>> +		btrfs_set_backup_fs_root(root_backup,
>>> +					 info->fs_root->node->start);
>>> +		btrfs_set_backup_fs_root_gen(root_backup,
>>> +			       btrfs_header_generation(info->fs_root->node));
>>> +		btrfs_set_backup_fs_root_level(root_backup,
>>> +			       btrfs_header_level(info->fs_root->node));
>>> +	}
>>> +
>>> +	btrfs_set_backup_dev_root(root_backup, info->dev_root->node->start);
>>> +	btrfs_set_backup_dev_root_gen(root_backup,
>>> +			       btrfs_header_generation(info->dev_root->node));
>>> +	btrfs_set_backup_dev_root_level(root_backup,
>>> +				       btrfs_header_level(info->dev_root->node));
>>> +
>>> +	btrfs_set_backup_csum_root(root_backup, info->csum_root->node->start);
>>> +	btrfs_set_backup_csum_root_gen(root_backup,
>>> +			       btrfs_header_generation(info->csum_root->node));
>>> +	btrfs_set_backup_csum_root_level(root_backup,
>>> +			       btrfs_header_level(info->csum_root->node));
>>> +
>>> +	btrfs_set_backup_total_bytes(root_backup,
>>> +			     btrfs_super_total_bytes(info->super_copy));
>>> +	btrfs_set_backup_bytes_used(root_backup,
>>> +			     btrfs_super_bytes_used(info->super_copy));
>>> +	btrfs_set_backup_num_devices(root_backup,
>>> +			     btrfs_super_num_devices(info->super_copy));
>>> +};
>>> +
>>>  int write_all_supers(struct btrfs_fs_info *fs_info)
>>>  {
>>>  	struct list_head *head = &fs_info->fs_devices->devices;
>>> @@ -1630,6 +1707,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info)
>>>  	int ret;
>>>  	u64 flags;
>>>  
>>> +	backup_super_roots(fs_info);
>>>  	sb = fs_info->super_copy;
>>>  	dev_item = &sb->dev_item;
>>>  	list_for_each_entry(dev, head, dev_list) {
>>>
>