btrfs: Add chunk allocation ENOSPC debug message for enospc_debug mount option

PATCH Superseded Qu Wenruo 4 participants 7 messages Jan 15, 2018 – Jan 30, 2018 Btrfs file system

Share & tools

Qu Wenruo

author

2018-01-15 06:14 UTC #0 lore raw

PATCH 1/1 btrfs: Add chunk allocation ENOSPC debug message for enospc_debug mount option

Enospc_debug makes extent allocator to print more debug messages, however for chunk allocation, there is no debug message for enospc_debug at all.

This patch will add message for the following parts of chunk allocator:

1) No rw device at all
Quite rare, but at least output one message for this case.

2) No enough space for some device
This debug message is quite handy for unbalanced disks with stripe based profiles (RAID0/10/5/6).

3) Not enough free devices
This debug message should tell us if current chunk allocator is working correctly on minimal device requirement.

Although under most case, we will hit other ENOSPC before we even hit a chunk allocator ENOSPC, but such debug info won't help.

Signed-off-by Qu Wenruo <[email protected]>

---
 fs/btrfs/volumes.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

fs/btrfs/volumes.c +17 -2

index a25684287501..664d8a1b90b3 100644
@@ -4622,8 +4622,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	BUG_ON(!alloc_profile_is_valid(type, 0));
 
-	if (list_empty(&fs_devices->alloc_list))
+	if (list_empty(&fs_devices->alloc_list)) {
+		if (btrfs_test_opt(info, ENOSPC_DEBUG))
+			btrfs_warn(info, "%s: No writable device", __func__);
 		return -ENOSPC;
+	}
 
 	index = __get_raid_index(type);

@@ -4705,8 +4708,14 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		if (ret == 0)
 			max_avail = max_stripe_size * dev_stripes;
 
-		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes)
+		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes) {
+			if (btrfs_test_opt(info, ENOSPC_DEBUG))
+				btrfs_debug(info,
+			"%s: devid %llu has no free space, have=%llu want=%u",
+					    __func__, device->devid, max_avail,
+					    BTRFS_STRIPE_LEN * dev_stripes);
 			continue;
+		}
 
 		if (ndevs == fs_devices->rw_devices) {
 			WARN(1, "%s: found more than %llu devices\n",

@@ -4731,6 +4740,12 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	if (ndevs < devs_increment * sub_stripes || ndevs < devs_min) {
 		ret = -ENOSPC;
+		if (btrfs_test_opt(info, ENOSPC_DEBUG)) {
+			btrfs_debug(info,
+		"%s: not enough devices with free space: have=%d minimal=%d increment=%d",
+				    __func__, ndevs, devs_min,
+				    devs_increment * sub_stripes);
+		}
 		goto error;
 	}

Signature · 1 line

2.15.1

Nikolay Borisov

2018-01-16 13:47 UTC #1 lore raw

Quoted 30 lines · Qu Wenruo

Enospc_debug makes extent allocator to print more debug messages, however for chunk allocation, there is no debug message for enospc_debug at all.
This patch will add message for the following parts of chunk allocator:
1) No rw device at all
Quite rare, but at least output one message for this case.
2) No enough space for some device
This debug message is quite handy for unbalanced disks with stripe based profiles (RAID0/10/5/6).
3) Not enough free devices
This debug message should tell us if current chunk allocator is working correctly on minimal device requirement.
Although under most case, we will hit other ENOSPC before we even hit a chunk allocator ENOSPC, but such debug info won't help.
Signed-off-by Qu Wenruo <[email protected]>
---
 fs/btrfs/volumes.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)
fs/btrfs/volumes.c +0 -0
index a25684287501..664d8a1b90b3 100644

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4627

Quoted 7 lines

fs/btrfs/volumes.c +3 -1

@@ -4622,8 +4622,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	BUG_ON(!alloc_profile_is_valid(type, 0));
 
-	if (list_empty(&fs_devices->alloc_list))
+	if (list_empty(&fs_devices->alloc_list)) {
+		if (btrfs_test_opt(info, ENOSPC_DEBUG))
+			btrfs_warn(info, "%s: No writable device", __func__);

perhaps this shouldn't be gated on ENOSPC_DEBUG if it's a warning, or if it's to be gated then make it a DEBUG.

Quoted 6 lines

fs/btrfs/volumes.c +1 -0

 		return -ENOSPC;
+	}
 
 	index = __get_raid_index(type);

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4716

Quoted 10 lines

fs/btrfs/volumes.c +6 -1

@@ -4705,8 +4708,14 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		if (ret == 0)
 			max_avail = max_stripe_size * dev_stripes;
 
-		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes)
+		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes) {
+			if (btrfs_test_opt(info, ENOSPC_DEBUG))
+				btrfs_debug(info,
+			"%s: devid %llu has no free space, have=%llu want=%u",
+					    __func__, device->devid, max_avail,
+					    BTRFS_STRIPE_LEN * dev_stripes);

Here we have a debug output gated on ENOSCP_DEBUG so let's be consistent (hence my previous comment)

Quoted 6 lines

fs/btrfs/volumes.c +1 -0

 			continue;
+		}
 
 		if (ndevs == fs_devices->rw_devices) {
 			WARN(1, "%s: found more than %llu devices\n",

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4747

Quoted 8 lines

fs/btrfs/volumes.c +5 -0

@@ -4731,6 +4740,12 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	if (ndevs < devs_increment * sub_stripes || ndevs < devs_min) {
 		ret = -ENOSPC;
+		if (btrfs_test_opt(info, ENOSPC_DEBUG)) {
+			btrfs_debug(info,
+		"%s: not enough devices with free space: have=%d minimal=%d increment=%d",
+				    __func__, ndevs, devs_min,
+				    devs_increment * sub_stripes);

Without looking at the code it's not really obvious what increment is. Perhaps you can use a more descriptive word?

Quoted 5 lines

fs/btrfs/volumes.c +1 -0
+		}
 		goto error;
 	}
 

Qu Wenruo

2018-01-17 00:53 UTC #2 lore raw

Quoted 36 lines · Nikolay Borisov

Quoted 33 lines · Qu Wenruo
Enospc_debug makes extent allocator to print more debug messages, however for chunk allocation, there is no debug message for enospc_debug at all.
This patch will add message for the following parts of chunk allocator:
1) No rw device at all
Quite rare, but at least output one message for this case.
2) No enough space for some device
This debug message is quite handy for unbalanced disks with stripe based profiles (RAID0/10/5/6).
3) Not enough free devices
This debug message should tell us if current chunk allocator is working correctly on minimal device requirement.
Although under most case, we will hit other ENOSPC before we even hit a chunk allocator ENOSPC, but such debug info won't help.
Signed-off-by Qu Wenruo <[email protected]>
---
 fs/btrfs/volumes.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)
fs/btrfs/volumes.c +0 -0
index a25684287501..664d8a1b90b3 100644
@@ -4622,8 +4622,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	BUG_ON(!alloc_profile_is_valid(type, 0));
 

Quoted 7 lines

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4627
Quoted 4 lines
fs/btrfs/volumes.c +3 -1
-	if (list_empty(&fs_devices->alloc_list))
+	if (list_empty(&fs_devices->alloc_list)) {
+		if (btrfs_test_opt(info, ENOSPC_DEBUG))
+			btrfs_warn(info, "%s: No writable device", __func__);
perhaps this shouldn't be gated on ENOSPC_DEBUG if it's a warning, or if it's to be gated then make it a DEBUG.

Because the case of no writeable device is rare.

But change it to debug seems good.

Quoted 27 lines

Quoted 7 lines

fs/btrfs/volumes.c +1 -0

 		return -ENOSPC;
+	}
 
 	index = __get_raid_index(type);

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4716

Quoted 10 lines

fs/btrfs/volumes.c +6 -1

@@ -4705,8 +4708,14 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		if (ret == 0)
 			max_avail = max_stripe_size * dev_stripes;
 
-		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes)
+		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes) {
+			if (btrfs_test_opt(info, ENOSPC_DEBUG))
+				btrfs_debug(info,
+			"%s: devid %llu has no free space, have=%llu want=%u",
+					    __func__, device->devid, max_avail,
+					    BTRFS_STRIPE_LEN * dev_stripes);

Here we have a debug output gated on ENOSCP_DEBUG so let's be consistent (hence my previous comment)

Quoted 7 lines

fs/btrfs/volumes.c +1 -0

 			continue;
+		}
 
 		if (ndevs == fs_devices->rw_devices) {
 			WARN(1, "%s: found more than %llu devices\n",

@@ -4731,6 +4740,12 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,

Quoted 10 lines

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4747
Quoted 7 lines
fs/btrfs/volumes.c +5 -0
 	if (ndevs < devs_increment * sub_stripes || ndevs < devs_min) {
 		ret = -ENOSPC;
+		if (btrfs_test_opt(info, ENOSPC_DEBUG)) {
+			btrfs_debug(info,
+		"%s: not enough devices with free space: have=%d minimal=%d increment=%d",
+				    __func__, ndevs, devs_min,
+				    devs_increment * sub_stripes);
Without looking at the code it's not really obvious what increment is. Perhaps you can use a more descriptive word?

"increment" is indeed less meaningful.

I'll change it to only output "minimal" just min(minimal, devs_min).

Thanks,
Qu

Quoted 11 lines

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4751
Quoted 6 lines
fs/btrfs/volumes.c +1 -0
+		}
 		goto error;
 	}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Nikolay Borisov

2018-01-17 16:14 UTC #3 lore raw

Quoted 89 lines · Qu Wenruo

Quoted 36 lines · Nikolay Borisov
Quoted 33 lines · Qu Wenruo
Enospc_debug makes extent allocator to print more debug messages, however for chunk allocation, there is no debug message for enospc_debug at all.
This patch will add message for the following parts of chunk allocator:
1) No rw device at all
Quite rare, but at least output one message for this case.
2) No enough space for some device
This debug message is quite handy for unbalanced disks with stripe based profiles (RAID0/10/5/6).
3) Not enough free devices
This debug message should tell us if current chunk allocator is working correctly on minimal device requirement.
Although under most case, we will hit other ENOSPC before we even hit a chunk allocator ENOSPC, but such debug info won't help.
Signed-off-by Qu Wenruo <[email protected]>
---
 fs/btrfs/volumes.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)
fs/btrfs/volumes.c +0 -0
index a25684287501..664d8a1b90b3 100644
@@ -4622,8 +4622,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	BUG_ON(!alloc_profile_is_valid(type, 0));
 
Quoted 7 lines
fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4627
Quoted 4 lines
fs/btrfs/volumes.c +3 -1
-	if (list_empty(&fs_devices->alloc_list))
+	if (list_empty(&fs_devices->alloc_list)) {
+		if (btrfs_test_opt(info, ENOSPC_DEBUG))
+			btrfs_warn(info, "%s: No writable device", __func__);
perhaps this shouldn't be gated on ENOSPC_DEBUG if it's a warning, or if it's to be gated then make it a DEBUG.
Because the case of no writeable device is rare.
But change it to debug seems good.
Quoted 27 lines
Quoted 7 lines
fs/btrfs/volumes.c +1 -0
 		return -ENOSPC;
+	}
 
 	index = __get_raid_index(type);
 
fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4716
Quoted 10 lines
fs/btrfs/volumes.c +6 -1
@@ -4705,8 +4708,14 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		if (ret == 0)
 			max_avail = max_stripe_size * dev_stripes;
 
-		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes)
+		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes) {
+			if (btrfs_test_opt(info, ENOSPC_DEBUG))
+				btrfs_debug(info,
+			"%s: devid %llu has no free space, have=%llu want=%u",
+					    __func__, device->devid, max_avail,
+					    BTRFS_STRIPE_LEN * dev_stripes);
Here we have a debug output gated on ENOSCP_DEBUG so let's be consistent (hence my previous comment)
Quoted 7 lines
fs/btrfs/volumes.c +1 -0
 			continue;
+		}
 
 		if (ndevs == fs_devices->rw_devices) {
 			WARN(1, "%s: found more than %llu devices\n",
@@ -4731,6 +4740,12 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
Quoted 10 lines
fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4747
Quoted 7 lines
fs/btrfs/volumes.c +5 -0
 	if (ndevs < devs_increment * sub_stripes || ndevs < devs_min) {
 		ret = -ENOSPC;
+		if (btrfs_test_opt(info, ENOSPC_DEBUG)) {
+			btrfs_debug(info,
+		"%s: not enough devices with free space: have=%d minimal=%d increment=%d",
+				    __func__, ndevs, devs_min,
+				    devs_increment * sub_stripes);
Without looking at the code it's not really obvious what increment is. Perhaps you can use a more descriptive word?

Quoted 3 lines

"increment" is indeed less meaningful.
I'll change it to only output "minimal" just min(minimal, devs_min).

I think that 'minimal' should be 'minimal required'

Quoted 16 lines

Thanks,
Qu
Quoted 11 lines
fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4751
Quoted 6 lines
fs/btrfs/volumes.c +1 -0
+		}
 		goto error;
 	}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Qu Wenruo

author

2018-01-22 05:51 UTC #4 lore raw

PATCH v2 1/1 btrfs: Add chunk allocation ENOSPC debug message for enospc_debug mount option

In reply to Qu Wenruo

Enospc_debug makes extent allocator to print more debug messages, however for chunk allocation, there is no debug message for enospc_debug at all.

This patch will add message for the following parts of chunk allocator:

1) No rw device at all
Quite rare, but at least output one message for this case.

2) No enough space for some device
This debug message is quite handy for unbalanced disks with stripe based profiles (RAID0/10/5/6).

3) Not enough free devices
This debug message should tell us if current chunk allocator is working correctly on minimal device requirement.

Although under most case, we will hit other ENOSPC before we even hit a chunk allocator ENOSPC, but such debug info won't help.

Signed-off-by Qu Wenruo <[email protected]>

---
v2:
Unify all message level to btrfs_debug().
More meaningful message if we don't have enough device.

---
 fs/btrfs/volumes.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

fs/btrfs/volumes.c +17 -2

index a25684287501..86cae6a15b1e 100644
@@ -4622,8 +4622,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	BUG_ON(!alloc_profile_is_valid(type, 0));
 
-	if (list_empty(&fs_devices->alloc_list))
+	if (list_empty(&fs_devices->alloc_list)) {
+		if (btrfs_test_opt(info, ENOSPC_DEBUG))
+			btrfs_debug(info, "%s: No writable device", __func__);
 		return -ENOSPC;
+	}
 
 	index = __get_raid_index(type);

@@ -4705,8 +4708,14 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		if (ret == 0)
 			max_avail = max_stripe_size * dev_stripes;
 
-		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes)
+		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes) {
+			if (btrfs_test_opt(info, ENOSPC_DEBUG))
+				btrfs_debug(info,
+			"%s: devid %llu has no free space, have=%llu want=%u",
+					    __func__, device->devid, max_avail,
+					    BTRFS_STRIPE_LEN * dev_stripes);
 			continue;
+		}
 
 		if (ndevs == fs_devices->rw_devices) {
 			WARN(1, "%s: found more than %llu devices\n",

@@ -4731,6 +4740,12 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	if (ndevs < devs_increment * sub_stripes || ndevs < devs_min) {
 		ret = -ENOSPC;
+		if (btrfs_test_opt(info, ENOSPC_DEBUG)) {
+			btrfs_debug(info,
+		"%s: not enough devices with free space: have=%d minimal required=%d",
+				    __func__, ndevs, min(devs_min,
+				    devs_increment * sub_stripes));
+		}
 		goto error;
 	}

Signature · 1 line

2.15.1

Nikolay Borisov

Reviewed-by on v2 1/1

2018-01-22 08:11 UTC #5 lore raw

Quoted 13 lines · Qu Wenruo

Enospc_debug makes extent allocator to print more debug messages, however for chunk allocation, there is no debug message for enospc_debug at all.
This patch will add message for the following parts of chunk allocator:
1) No rw device at all
Quite rare, but at least output one message for this case.
2) No enough space for some device
This debug message is quite handy for unbalanced disks with stripe based profiles (RAID0/10/5/6).

Quoted 8 lines

3) Not enough free devices
This debug message should tell us if current chunk allocator is working correctly on minimal device requirement.
Although under most case, we will hit other ENOSPC before we even hit a chunk allocator ENOSPC, but such debug info won't help.
Signed-off-by Qu Wenruo <[email protected]>

Reviewed-by Nikolay Borisov <[email protected]>

Quoted 42 lines

---
v2:
Unify all message level to btrfs_debug().
More meaningful message if we don't have enough device.

---
 fs/btrfs/volumes.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

fs/btrfs/volumes.c +11 -2

index a25684287501..86cae6a15b1e 100644
@@ -4622,8 +4622,11 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	BUG_ON(!alloc_profile_is_valid(type, 0));
 
-	if (list_empty(&fs_devices->alloc_list))
+	if (list_empty(&fs_devices->alloc_list)) {
+		if (btrfs_test_opt(info, ENOSPC_DEBUG))
+			btrfs_debug(info, "%s: No writable device", __func__);
 		return -ENOSPC;
+	}
 
 	index = __get_raid_index(type);

@@ -4705,8 +4708,14 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 		if (ret == 0)
 			max_avail = max_stripe_size * dev_stripes;
 
-		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes)
+		if (max_avail < BTRFS_STRIPE_LEN * dev_stripes) {
+			if (btrfs_test_opt(info, ENOSPC_DEBUG))
+				btrfs_debug(info,
+			"%s: devid %llu has no free space, have=%llu want=%u",
+					    __func__, device->devid, max_avail,
+					    BTRFS_STRIPE_LEN * dev_stripes);
 			continue;
+		}
 
 		if (ndevs == fs_devices->rw_devices) {
 			WARN(1, "%s: found more than %llu devices\n",

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4745

Quoted 6 lines

fs/btrfs/volumes.c +3 -0

@@ -4731,6 +4740,12 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	if (ndevs < devs_increment * sub_stripes || ndevs < devs_min) {
 		ret = -ENOSPC;
+		if (btrfs_test_opt(info, ENOSPC_DEBUG)) {
+			btrfs_debug(info,
+		"%s: not enough devices with free space: have=%d minimal required=%d",

nit: s/minimal/minimum
But there is no point in resending just for that, I guess David could fix it while merging.

Quoted 7 lines

fs/btrfs/volumes.c +3 -0

+				    __func__, ndevs, min(devs_min,
+				    devs_increment * sub_stripes));
+		}
 		goto error;
 	}

David Sterba

2018-01-30 13:50 UTC #6 lore raw

Quoted 11 lines · Nikolay Borisov

fs/btrfs/volumes.c static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, · line 4745
Quoted 7 lines
fs/btrfs/volumes.c +3 -0
@@ -4731,6 +4740,12 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
 
 	if (ndevs < devs_increment * sub_stripes || ndevs < devs_min) {
 		ret = -ENOSPC;
+		if (btrfs_test_opt(info, ENOSPC_DEBUG)) {
+			btrfs_debug(info,
+		"%s: not enough devices with free space: have=%d minimal required=%d",
nit: s/minimal/minimum
But there is no point in resending just for that, I guess David could fix it while merging.

Yes, string updated and added to next, thanks.