diff mbox series

Fix calculation of chunk size for RAID1/DUP profiles

Message ID 20211116140206.291252-1-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series Fix calculation of chunk size for RAID1/DUP profiles | expand

Commit Message

Nikolay Borisov Nov. 16, 2021, 2:02 p.m. UTC
Current formula calculates the stripe size, however that's not what we want
in the case of RAID1/DUP profiles. In those cases since chunkc are mirrored
across devices we want the full size of the chunk. Without this patch the
'btrfs fi usage' output from an fs which is using RAID1 is:

	<snip>

	Data,RAID1: Size:2.00GiB, Used:1.00GiB (50.03%)
	   /dev/vdc	   1.00GiB
	   /dev/vdf	   1.00GiB

	Metadata,RAID1: Size:256.00MiB, Used:1.34MiB (0.52%)
	   /dev/vdc	 128.00MiB
	   /dev/vdf	 128.00MiB

	System,RAID1: Size:8.00MiB, Used:16.00KiB (0.20%)
	   /dev/vdc	   4.00MiB
	   /dev/vdf	   4.00MiB

	Unallocated:
	   /dev/vdc	   8.87GiB
	   /dev/vdf	   8.87GiB


So a 2 gigabyte RAID1 chunk actually will take up 4 gigabytes on the actual disks
2 each. In this case this is being miscalculated as taking up 1gb on each device.

This also leads to erroneously calculated unallocated space. The correct output
in this case is:

	<snip>

	Data,RAID1: Size:2.00GiB, Used:1.00GiB (50.03%)
	   /dev/vdc	   2.00GiB
	   /dev/vdf	   2.00GiB

	Metadata,RAID1: Size:256.00MiB, Used:1.34MiB (0.52%)
	   /dev/vdc	 256.00MiB
	   /dev/vdf	 256.00MiB

	System,RAID1: Size:8.00MiB, Used:16.00KiB (0.20%)
	   /dev/vdc	   8.00MiB
	   /dev/vdf	   8.00MiB

	Unallocated:
	   /dev/vdc	   7.74GiB
	   /dev/vdf	   7.74GiB


Fix it by only utilising the chunk formula for profiles which are not RAID1/DUP.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 cmds/filesystem-usage.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

--
2.17.1

Comments

David Sterba Nov. 18, 2021, 9:12 a.m. UTC | #1
On Tue, Nov 16, 2021 at 04:02:06PM +0200, Nikolay Borisov wrote:
> Current formula calculates the stripe size, however that's not what we want
> in the case of RAID1/DUP profiles. In those cases since chunkc are mirrored
> across devices we want the full size of the chunk. Without this patch the
> 'btrfs fi usage' output from an fs which is using RAID1 is:
> 
> 	<snip>
> 
> 	Data,RAID1: Size:2.00GiB, Used:1.00GiB (50.03%)
> 	   /dev/vdc	   1.00GiB
> 	   /dev/vdf	   1.00GiB
> 
> 	Metadata,RAID1: Size:256.00MiB, Used:1.34MiB (0.52%)
> 	   /dev/vdc	 128.00MiB
> 	   /dev/vdf	 128.00MiB
> 
> 	System,RAID1: Size:8.00MiB, Used:16.00KiB (0.20%)
> 	   /dev/vdc	   4.00MiB
> 	   /dev/vdf	   4.00MiB
> 
> 	Unallocated:
> 	   /dev/vdc	   8.87GiB
> 	   /dev/vdf	   8.87GiB
> 
> 
> So a 2 gigabyte RAID1 chunk actually will take up 4 gigabytes on the actual disks
> 2 each. In this case this is being miscalculated as taking up 1gb on each device.
> 
> This also leads to erroneously calculated unallocated space. The correct output
> in this case is:
> 
> 	<snip>
> 
> 	Data,RAID1: Size:2.00GiB, Used:1.00GiB (50.03%)
> 	   /dev/vdc	   2.00GiB
> 	   /dev/vdf	   2.00GiB
> 
> 	Metadata,RAID1: Size:256.00MiB, Used:1.34MiB (0.52%)
> 	   /dev/vdc	 256.00MiB
> 	   /dev/vdf	 256.00MiB
> 
> 	System,RAID1: Size:8.00MiB, Used:16.00KiB (0.20%)
> 	   /dev/vdc	   8.00MiB
> 	   /dev/vdf	   8.00MiB
> 
> 	Unallocated:
> 	   /dev/vdc	   7.74GiB
> 	   /dev/vdf	   7.74GiB
> 
> 
> Fix it by only utilising the chunk formula for profiles which are not RAID1/DUP.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Added to devel, thanks.
diff mbox series

Patch

diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
index 6195f633da44..5f2289a9b40d 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -805,11 +805,17 @@  int load_chunk_and_device_info(int fd, struct chunk_info **chunkinfo,
  */
 static u64 calc_chunk_size(struct chunk_info *ci)
 {
-	u32 div;
+	u32 div = 1;

-	/* No parity + sub_stripes, so order of "-" and "/" does not matter */
-	div = (ci->num_stripes - btrfs_bg_type_to_nparity(ci->type)) /
-	      btrfs_bg_type_to_sub_stripes(ci->type);
+	/*
+	 * The formula doesn't work for RAID1/DUP types, we should just return the
+	 * chunk size
+	 */
+	if (!(ci->type & (BTRFS_BLOCK_GROUP_RAID1_MASK|BTRFS_BLOCK_GROUP_DUP))) {
+		/* No parity + sub_stripes, so order of "-" and "/" does not matter */
+		div = (ci->num_stripes - btrfs_bg_type_to_nparity(ci->type)) /
+			btrfs_bg_type_to_sub_stripes(ci->type);
+	}

 	return ci->size / div;
 }