btrfs-progs: fi-usage: Fix wrong RAID10 used and unallocated space
PATCH 1/2 btrfs-progs: fi-usage: Fix wrong RAID10 used and unallocated space
[BUG]
For a very basic RAID10 with 4 disks, "fi usage" and "fi show" are outputting conflicting result:
------
# btrfs fi show /mnt/btrfs/ Label: none uuid: 6d0229db-28d1-4696-ac20-e828cc45dc40 Total devices 4 FS bytes used 1.12MiB devid 1 size 5.00GiB used 2.01GiB path /dev/mapper/data-disk1 devid 2 size 5.00GiB used 2.01GiB path /dev/mapper/data-disk2 devid 3 size 5.00GiB used 2.01GiB path /dev/mapper/data-disk3 devid 4 size 5.00GiB used 2.01GiB path /dev/mapper/data-disk4
Here the unallocated space for disk4 should be a little less than 3G. (5.00 Gib - 2.01GiB)
# btrfs fi usage /mnt/btrfs/
Overall:
Device size: 20.00GiB
Device allocated: 8.03GiB
Device unallocated: 11.97GiB
Device missing: 0.00B
Used: 2.25MiB
Free (estimated): 7.98GiB (min: 7.98GiB)
Data ratio: 2.00
Metadata ratio: 2.00
Global reserve: 16.00MiB (used: 0.00B)Data,RAID10: Size:2.00GiB, Used:1.00MiB
...
/dev/mapper/data-disk4 512.00MiB
Metadata,RAID10: Size:2.00GiB, Used:112.00KiB
...
/dev/mapper/data-disk4 512.00MiB
System,RAID10: Size:16.00MiB, Used:16.00KiB
...
/dev/mapper/data-disk4 4.00MiB
Unallocated:
...
/dev/mapper/data-disk4 4.00GiB
While fi usage shows we still have 4.00GiB unallocated space.
------
[CAUSE]calc_chunk_size() is used to convert chunk size to device extent size, which is used to get the per-device data/meta/sys used space.
However, for RAID10 we just divide the chunk size with num_stripes, without taking sub stripes into account.
Resulting data/meta/sys usage in RAID10 halved.
[FIX]
Take the missing sub stripes into consideration.
--- cmds-fi-usage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
cmds-fi-usage.c +1 -1
@@ -664,7 +664,7 @@ static u64 calc_chunk_size(struct chunk_info *ci)else if (ci->type &BTRFS_BLOCK_GROUP_RAID6 )return ci->size / (ci->num_stripes -2 );else if (ci->type &BTRFS_BLOCK_GROUP_RAID10 ) -return ci->size / ci->num_stripes; +return ci->size / (ci->num_stripes /2 );return ci->size; }
Signature · 1 line
2.15.1
PATCH 2/2 btrfs-progs: misc-test: Add test case to check if fi usage and show report consistent unallocated space
--- tests/misc-tests/028-fi-usage-cross-check/test.sh | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100755 tests/misc-tests/028-fi-usage-cross-check/test.sh
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +46 -0
@@ -0,0 +1,46 @@ +#!/bin/bash +# check fi-usage is output correct unallocated space for RAID10 + +source "$TOP/tests/common" + +check_prereq mkfs.btrfs +check_prereq btrfs + +setup_root_helper + +setup_loopdevs 4 +prepare_loopdevs +dev1=${loopdevs[1]} + +run_check "$TOP/mkfs.btrfs" -d raid10 -m raid10 -f ${loopdevs[@]} +run_check $SUDO_HELPER mount $dev1 "$TEST_MNT" + +output_dir=$(mktemp -d --tmpdir btrfs-progs-misc.XXXXXXXX) + +run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem show --raw "$TEST_MNT" |\ + grep "$dev1" > "$output_dir/fi_show_output" +if [ ! -s "$output_dir/fi_show_output" ]; then + _fail "failed to get correct fi show output" +fi + +dev1_total=$(cat "$output_dir/fi_show_output" | awk '{print $4}') +dev1_used=$(cat "$output_dir/fi_show_output" | awk '{print $6}') +dev1_fi_show_unallocated=$(( $dev1_total - $dev1_used)) + +run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem usage --raw "$TEST_MNT" |\ + grep "$dev1" | tail -n1 > "$output_dir/fi_usage_output" +if [ ! -s "$output_dir/fi_usage_output" ]; then + _fail "failed to get correct fi usage output" +fi + +dev1_fi_usage_unallocated=$(cat "$output_dir/fi_usage_output" | awk '{print $2}') +echo "fi usage unallocated for devid1 is $dev1_fi_usage_unallocated" >> "$RESULTS" +echo "fi show unallocated for devid1 is $dev1_fi_show_unallocated" >> "$RESULTS" + +if [ $dev1_fi_show_unallocated -ne $dev1_fi_usage_unallocated ]; then + _fail "fi usage unallocated mismatch with fi show" +fi + +run_check $SUDO_HELPER umount "$TEST_MNT" +cleanup_loopdevs +rm $output_dir -rf
Signature · 1 line
2.15.1
Re: [PATCH 2/2] btrfs-progs: misc-test: Add test case to check if fi usage and show report consistent unallocated space
Quoted 18 lines · Qu Wenruo
Signed-off-by Qu Wenruo <[email protected]>--- tests/misc-tests/028-fi-usage-cross-check/test.sh | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100755 tests/misc-tests/028-fi-usage-cross-check/test.shtests/misc-tests/028-fi-usage-cross-check/test.sh new file +6
-0@@ -0,0 +1,46 @@ +#!/bin/bash +# check fi-usage is output correct unallocated space for RAID10 + +source "$TOP/tests/common" + +check_prereq mkfs.btrfs
tests/misc-tests/028-fi-usage-cross-check/test.sh line 16
Quoted 10 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +10
-0+check_prereq btrfs + +setup_root_helper + +setup_loopdevs 4 +prepare_loopdevs +dev1=${loopdevs[1]} + +run_check "$TOP/mkfs.btrfs" -d raid10 -m raid10 -f ${loopdevs[@]} +run_check $SUDO_HELPER mount $dev1 "$TEST_MNT"
There's a helper macro for that, run_check_mount_test_dev, that expects the TEST_DEV to be set. I suggest to use this variable instead of dev1, and then the umount command can be replaced by run_check_umount_test_dev.
tests/misc-tests/028-fi-usage-cross-check/test.sh line 18
Quoted 2 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +2
-0+ +output_dir=$(mktemp -d --tmpdir btrfs-progs-misc.XXXXXXXX)
This is not IMO necessary, you get only one line from the fi show output and check if it's not empty. A simple 'if grep ...' . The output is reused so you can store it to a variable.
Quoted 18 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +18
-0+ +run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem show --raw "$TEST_MNT" |\ + grep "$dev1" > "$output_dir/fi_show_output" +if [ ! -s "$output_dir/fi_show_output" ]; then + _fail "failed to get correct fi show output" +fi + +dev1_total=$(cat "$output_dir/fi_show_output" | awk '{print $4}') +dev1_used=$(cat "$output_dir/fi_show_output" | awk '{print $6}') +dev1_fi_show_unallocated=$(( $dev1_total - $dev1_used)) + +run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem usage --raw "$TEST_MNT" |\ + grep "$dev1" | tail -n1 > "$output_dir/fi_usage_output" +if [ ! -s "$output_dir/fi_usage_output" ]; then + _fail "failed to get correct fi usage output" +fi + +dev1_fi_usage_unallocated=$(cat "$output_dir/fi_usage_output" | awk '{print $2}')
tests/misc-tests/028-fi-usage-cross-check/test.sh line 46
Quoted 10 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +10
-0+echo "fi usage unallocated for devid1 is $dev1_fi_usage_unallocated" >> "$RESULTS" +echo "fi show unallocated for devid1 is $dev1_fi_show_unallocated" >> "$RESULTS" + +if [ $dev1_fi_show_unallocated -ne $dev1_fi_usage_unallocated ]; then + _fail "fi usage unallocated mismatch with fi show" +fi + +run_check $SUDO_HELPER umount "$TEST_MNT" +cleanup_loopdevs +rm $output_dir -rf
Please quote all arguments, namely if it's passed to 'rm', and options should go before arugments.
Re: [PATCH 2/2] btrfs-progs: misc-test: Add test case to check if fi usage and show report consistent unallocated space
Quoted 64 lines · David Sterba
Quoted 18 lines · Qu Wenruo
Signed-off-by Qu Wenruo <[email protected]>--- tests/misc-tests/028-fi-usage-cross-check/test.sh | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100755 tests/misc-tests/028-fi-usage-cross-check/test.shtests/misc-tests/028-fi-usage-cross-check/test.sh new file +6
-0@@ -0,0 +1,46 @@ +#!/bin/bash +# check fi-usage is output correct unallocated space for RAID10 + +source "$TOP/tests/common" + +check_prereq mkfs.btrfstests/misc-tests/028-fi-usage-cross-check/test.sh line 16
Quoted 10 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +10
-0+check_prereq btrfs + +setup_root_helper + +setup_loopdevs 4 +prepare_loopdevs +dev1=${loopdevs[1]} + +run_check "$TOP/mkfs.btrfs" -d raid10 -m raid10 -f ${loopdevs[@]} +run_check $SUDO_HELPER mount $dev1 "$TEST_MNT"There's a helper macro for that,
run_check_mount_test_dev, that expects theTEST_DEVto be set. I suggest to use this variable instead of dev1, and then the umount command can be replaced byrun_check_umount_test_dev.tests/misc-tests/028-fi-usage-cross-check/test.sh line 18
Quoted 2 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +2
-0+ +output_dir=$(mktemp -d --tmpdir btrfs-progs-misc.XXXXXXXX)This is not IMO necessary, you get only one line from the fi show output and check if it's not empty. A simple 'if grep ...' . The output is reused so you can store it to a variable.
Quoted 23 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +23
-0+ +run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem show --raw "$TEST_MNT" |\ + grep "$dev1" > "$output_dir/fi_show_output" +if [ ! -s "$output_dir/fi_show_output" ]; then + _fail "failed to get correct fi show output" +fi + +dev1_total=$(cat "$output_dir/fi_show_output" | awk '{print $4}') +dev1_used=$(cat "$output_dir/fi_show_output" | awk '{print $6}') +dev1_fi_show_unallocated=$(( $dev1_total - $dev1_used)) + +run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem usage --raw "$TEST_MNT" |\ + grep "$dev1" | tail -n1 > "$output_dir/fi_usage_output" +if [ ! -s "$output_dir/fi_usage_output" ]; then + _fail "failed to get correct fi usage output" +fi + +dev1_fi_usage_unallocated=$(cat "$output_dir/fi_usage_output" | awk '{print $2}') +echo "fi usage unallocated for devid1 is $dev1_fi_usage_unallocated" >> "$RESULTS" +echo "fi show unallocated for devid1 is $dev1_fi_show_unallocated" >> "$RESULTS" + +if [ $dev1_fi_show_unallocated -ne $dev1_fi_usage_unallocated ]; then + _fail "fi usage unallocated mismatch with fi show"
Quoted 8 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh line 46
Quoted 5 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +5
-0+fi + +run_check $SUDO_HELPER umount "$TEST_MNT" +cleanup_loopdevs +rm $output_dir -rfPlease quote all arguments, namely if it's passed to 'rm', and options should go before arugments.
However the $output_dir won't contain any space or any escaping.
It will always be something like "/tmp/btrfs-progs-misc.1234567", do we still need to quota something we're sure it's safe?
Thanks,
Qu
Quoted 5 lines
--
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
Re: [PATCH 2/2] btrfs-progs: misc-test: Add test case to check if fi usage and show report consistent unallocated space
Quoted 74 lines · Qu Wenruo
Quoted 64 lines · David Sterba
Quoted 18 lines · Qu Wenruo
Signed-off-by Qu Wenruo <[email protected]>--- tests/misc-tests/028-fi-usage-cross-check/test.sh | 46 +++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100755 tests/misc-tests/028-fi-usage-cross-check/test.shtests/misc-tests/028-fi-usage-cross-check/test.sh new file +6
-0@@ -0,0 +1,46 @@ +#!/bin/bash +# check fi-usage is output correct unallocated space for RAID10 + +source "$TOP/tests/common" + +check_prereq mkfs.btrfstests/misc-tests/028-fi-usage-cross-check/test.sh line 16
Quoted 10 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +10
-0+check_prereq btrfs + +setup_root_helper + +setup_loopdevs 4 +prepare_loopdevs +dev1=${loopdevs[1]} + +run_check "$TOP/mkfs.btrfs" -d raid10 -m raid10 -f ${loopdevs[@]} +run_check $SUDO_HELPER mount $dev1 "$TEST_MNT"There's a helper macro for that,
run_check_mount_test_dev, that expects theTEST_DEVto be set. I suggest to use this variable instead of dev1, and then the umount command can be replaced byrun_check_umount_test_dev.tests/misc-tests/028-fi-usage-cross-check/test.sh line 18
Quoted 2 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +2
-0+ +output_dir=$(mktemp -d --tmpdir btrfs-progs-misc.XXXXXXXX)This is not IMO necessary, you get only one line from the fi show output and check if it's not empty. A simple 'if grep ...' . The output is reused so you can store it to a variable.
Quoted 23 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +23
-0+ +run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem show --raw "$TEST_MNT" |\ + grep "$dev1" > "$output_dir/fi_show_output" +if [ ! -s "$output_dir/fi_show_output" ]; then + _fail "failed to get correct fi show output" +fi + +dev1_total=$(cat "$output_dir/fi_show_output" | awk '{print $4}') +dev1_used=$(cat "$output_dir/fi_show_output" | awk '{print $6}') +dev1_fi_show_unallocated=$(( $dev1_total - $dev1_used)) + +run_check_stdout $SUDO_HELPER "$TOP/btrfs" filesystem usage --raw "$TEST_MNT" |\ + grep "$dev1" | tail -n1 > "$output_dir/fi_usage_output" +if [ ! -s "$output_dir/fi_usage_output" ]; then + _fail "failed to get correct fi usage output" +fi + +dev1_fi_usage_unallocated=$(cat "$output_dir/fi_usage_output" | awk '{print $2}') +echo "fi usage unallocated for devid1 is $dev1_fi_usage_unallocated" >> "$RESULTS" +echo "fi show unallocated for devid1 is $dev1_fi_show_unallocated" >> "$RESULTS" + +if [ $dev1_fi_show_unallocated -ne $dev1_fi_usage_unallocated ]; then + _fail "fi usage unallocated mismatch with fi show"Quoted 8 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh line 46
Quoted 5 lines
tests/misc-tests/028-fi-usage-cross-check/test.sh new file +5
-0+fi + +run_check $SUDO_HELPER umount "$TEST_MNT" +cleanup_loopdevs +rm $output_dir -rfPlease quote all arguments, namely if it's passed to 'rm', and options should go before arugments.
Quoted 4 lines
However the $output_dir won't contain any space or any escaping.
It will always be something like "
/tmp/btrfs-progs-misc.1234567", do we still need to quota something we're sure it's safe?
For sake of consistency, yes. There are only a few exceptions where we don't want quotes around variables.