Skip to contentSkip to conversation

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

PATCH Applied [email protected] 3 participants 3 messages Jul 25, 2019 Btrfs file system

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Btrfs-progs: mkfs, fix metadata corruption when using mixed mode

From: Filipe Manana <[email protected]>

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")
---
 mkfs/main.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
mkfs/main.c +12 -6
index 8dbec071..971cb395 100644
@@ -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);
Signature · 1 line
2.11.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 19 lines · On 2019/7/25 下午6:27, [email protected] wrote:

From: Filipe Manana <[email protected]>

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.

Quoted 5 lines

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

Thanks,
Qu

Quoted 47 lines
---
 mkfs/main.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
mkfs/main.c +12 -6
index 8dbec071..971cb395 100644
@@ -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);
Applied
#2 lore raw
In reply to [email protected]
Quoted 19 lines · On Thu, Jul 25, 2019 at 11:27:17AM +0100, [email protected] wrote:

From: Filipe Manana <[email protected]>

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.

Quoted 5 lines

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

Applied, thanks.