Btrfs-progs: mkfs, fix metadata corruption when using mixed mode
diff mbox series

Message ID 20190725102717.11688-1-fdmanana@kernel.org
State New
Headers show
Series
  • Btrfs-progs: mkfs, fix metadata corruption when using mixed mode
Related show

Commit Message

Filipe Manana July 25, 2019, 10:27 a.m. UTC
From: Filipe Manana <fdmanana@suse.com>

When creating a filesystem with mixed block groups, we are creating two
space info objects to track used/reserved/pinned space, one only for data
and another one only for metadata.

This is making fstests test case generic/416 fail, with btrfs' check
reporting over an hundred errors about bad extents:

  (...)
  bad extent [17186816, 17190912), type mismatch with chunk
  bad extent [17195008, 17199104), type mismatch with chunk
  bad extent [17203200, 17207296), type mismatch with chunk
  (...)

Because, surprisingly, this results in block groups that do not have the
BTRFS_BLOCK_GROUP_DATA flag set but have data extents allocated in them.
This is a regression introduced in btrfs-progs v5.2.

So fix this by making sure we only create one space info object, for both
metadata and data, when mixed block groups are enabled.

Fixes: c31edf610cbe1e ("btrfs-progs: Fix false ENOSPC alert by tracking used space correctly")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 mkfs/main.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Qu Wenruo July 25, 2019, 12:06 p.m. UTC | #1
On 2019/7/25 下午6:27, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When creating a filesystem with mixed block groups, we are creating two
> space info objects to track used/reserved/pinned space, one only for data
> and another one only for metadata.
> 
> This is making fstests test case generic/416 fail, with btrfs' check
> reporting over an hundred errors about bad extents:
> 
>   (...)
>   bad extent [17186816, 17190912), type mismatch with chunk
>   bad extent [17195008, 17199104), type mismatch with chunk
>   bad extent [17203200, 17207296), type mismatch with chunk
>   (...)
> 
> Because, surprisingly, this results in block groups that do not have the
> BTRFS_BLOCK_GROUP_DATA flag set but have data extents allocated in them.
> This is a regression introduced in btrfs-progs v5.2.
> 
> So fix this by making sure we only create one space info object, for both
> metadata and data, when mixed block groups are enabled.
> 
> Fixes: c31edf610cbe1e ("btrfs-progs: Fix false ENOSPC alert by tracking used space correctly")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu
> ---
>  mkfs/main.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 8dbec071..971cb395 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -64,18 +64,17 @@ static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
>  	struct btrfs_fs_info *fs_info = root->fs_info;
>  	struct btrfs_trans_handle *trans;
>  	struct btrfs_space_info *sinfo;
> +	u64 flags = BTRFS_BLOCK_GROUP_METADATA;
>  	u64 bytes_used;
>  	u64 chunk_start = 0;
>  	u64 chunk_size = 0;
>  	int ret;
>  
> +	if (mixed)
> +		flags |= BTRFS_BLOCK_GROUP_DATA;
> +
>  	/* Create needed space info to trace extents reservation */
> -	ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_METADATA,
> -				0, 0, &sinfo);
> -	if (ret < 0)
> -		return ret;
> -	ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_DATA,
> -				0, 0, &sinfo);
> +	ret = update_space_info(fs_info, flags, 0, 0, &sinfo);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -149,6 +148,13 @@ static int create_data_block_groups(struct btrfs_trans_handle *trans,
>  	int ret = 0;
>  
>  	if (!mixed) {
> +		struct btrfs_space_info *sinfo;
> +
> +		ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_DATA,
> +					0, 0, &sinfo);
> +		if (ret < 0)
> +			return ret;
> +
>  		ret = btrfs_alloc_chunk(trans, fs_info,
>  					&chunk_start, &chunk_size,
>  					BTRFS_BLOCK_GROUP_DATA);
>
David Sterba July 25, 2019, 6:03 p.m. UTC | #2
On Thu, Jul 25, 2019 at 11:27:17AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When creating a filesystem with mixed block groups, we are creating two
> space info objects to track used/reserved/pinned space, one only for data
> and another one only for metadata.
> 
> This is making fstests test case generic/416 fail, with btrfs' check
> reporting over an hundred errors about bad extents:
> 
>   (...)
>   bad extent [17186816, 17190912), type mismatch with chunk
>   bad extent [17195008, 17199104), type mismatch with chunk
>   bad extent [17203200, 17207296), type mismatch with chunk
>   (...)
> 
> Because, surprisingly, this results in block groups that do not have the
> BTRFS_BLOCK_GROUP_DATA flag set but have data extents allocated in them.
> This is a regression introduced in btrfs-progs v5.2.
> 
> So fix this by making sure we only create one space info object, for both
> metadata and data, when mixed block groups are enabled.
> 
> Fixes: c31edf610cbe1e ("btrfs-progs: Fix false ENOSPC alert by tracking used space correctly")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Applied, thanks.

Patch
diff mbox series

diff --git a/mkfs/main.c b/mkfs/main.c
index 8dbec071..971cb395 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -64,18 +64,17 @@  static int create_metadata_block_groups(struct btrfs_root *root, int mixed,
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_trans_handle *trans;
 	struct btrfs_space_info *sinfo;
+	u64 flags = BTRFS_BLOCK_GROUP_METADATA;
 	u64 bytes_used;
 	u64 chunk_start = 0;
 	u64 chunk_size = 0;
 	int ret;
 
+	if (mixed)
+		flags |= BTRFS_BLOCK_GROUP_DATA;
+
 	/* Create needed space info to trace extents reservation */
-	ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_METADATA,
-				0, 0, &sinfo);
-	if (ret < 0)
-		return ret;
-	ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_DATA,
-				0, 0, &sinfo);
+	ret = update_space_info(fs_info, flags, 0, 0, &sinfo);
 	if (ret < 0)
 		return ret;
 
@@ -149,6 +148,13 @@  static int create_data_block_groups(struct btrfs_trans_handle *trans,
 	int ret = 0;
 
 	if (!mixed) {
+		struct btrfs_space_info *sinfo;
+
+		ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_DATA,
+					0, 0, &sinfo);
+		if (ret < 0)
+			return ret;
+
 		ret = btrfs_alloc_chunk(trans, fs_info,
 					&chunk_start, &chunk_size,
 					BTRFS_BLOCK_GROUP_DATA);