btrfs-progs: check: Skip data csum verification for metadata dump
PATCH 1/2 btrfs-progs: check: Skip data csum verification for metadata dump
For metadata dump (fs restored by btrfs-image), since no data is restored check sum verification will definitely report error.
Add such check in check_csums() and prompt for user.
Issue: #103
--- check/main.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
check/main.c +12 -1
@@ -5601,6 +5601,7 @@ static int check_csums(struct btrfs_root *root)int ret;u64 data_len;unsigned long leaf_offset; +bool verify_csum = !!check_data_csum; root = root->fs_info->csum_root;if (!extent_buffer_uptodate (root->node)) {
@@ -5623,6 +5624,16 @@ static int check_csums(struct btrfs_root *root) path.slots[0 ]--; ret =0 ; +/* +* For metadata dump (btrfs-image) all data is wiped so verifying data +* csum is meaningless and will always report csum error. +*/ +if (check_data_csum && (btrfs_super_flags (root->fs_info->super_copy) & + (BTRFS_SUPER_FLAG_METADUMP |BTRFS_SUPER_FLAG_METADUMP_V2 ))) { +printf ("skip data csum verification for metadata dump\n" ); + verify_csum = false; + } +while (1 ) {if (path.slots[0 ] >=btrfs_header_nritems (path.nodes[0 ])) { ret =btrfs_next_leaf (root, &path);
@@ -5644,7 +5655,7 @@ static int check_csums(struct btrfs_root *root) data_len = (btrfs_item_size_nr (leaf, path.slots[0 ]) / csum_size) * root->fs_info->sectorsize; -if (!check_data_csum) +if (!verify_csum) goto skip_csum_check; leaf_offset =btrfs_item_ptr_offset (leaf, path.slots[0 ]); ret = check_extent_csums(root, key.offset, data_len,
Signature · 1 line
2.16.3
PATCH 2/2 btrfs-progs: tests/fsck: Add test case to check if btrfs check can skip data csum verfication for metadata dump
--- .../031-metadatadump-check-data-csum/test.sh | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100755 tests/fsck-tests/031-metadatadump-check-data-csum/test.sh
tests/fsck-tests/031-metadatadump-check-data-csum/test.sh new file +32 -0
@@ -0,0 +1,32 @@ +#!/bin/bash +# To check if "btrfs check" can detect metadata dump (restored by btrfs-iamge) +# and ignore --check-data-csum option + +source "$TEST_TOP/common" + +check_prereq btrfs +check_prereq mkfs.btrfs +check_prereq btrfs-image +setup_root_helper +prepare_test_dev + +tmp=$(mktemp --tmpdir btrfs-progs-fsck.XXXXXXX) + +run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV" +run_check_mount_test_dev + +run_check $SUDO_HELPER dd if=/dev/urandom of="$TEST_MNT/file" bs=4k count=16 +run_check_umount_test_dev + +run_check $SUDO_HELPER "$TOP/btrfs-image" "$TEST_DEV" "$tmp.image" + +# use prepare_test_dev() to wipe all existing data on $TEST_DEV +# so there is no way that restored image could have mathcing data csum +prepare_test_dev + +run_check $SUDO_HELPER "$TOP/btrfs-image" -r "$tmp.image" "$TEST_DEV" + +# Should not report any error +run_check "$TOP/btrfs" check --check-data-csum "$TEST_DEV" + +rm -rf -- "$tmp*"
Signature · 1 line
2.16.3
Re: [PATCH 2/2] btrfs-progs: tests/fsck: Add test case to check if btrfs check can skip data csum verfication for metadata dump
Quoted 15 lines · Qu Wenruo
Signed-off-by Qu Wenruo <[email protected]>--- .../031-metadatadump-check-data-csum/test.sh | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100755 tests/fsck-tests/031-metadatadump-check-data-csum/test.shtests/fsck-tests/031-metadatadump-check-data-csum/test.sh new file +3
-0@@ -0,0 +1,32 @@ +#!/bin/bash +# To check if "btrfs check" can detect metadata dump (restored by btrfs-iamge) +# and ignore --check-data-csum option
tests/fsck-tests/031-metadatadump-check-data-csum/test.sh line 13
Quoted 10 lines
tests/fsck-tests/031-metadatadump-check-data-csum/test.sh new file +10
-0+ +source "$TEST_TOP/common" + +check_prereq btrfs +check_prereq mkfs.btrfs +check_prereq btrfs-image +setup_root_helper +prepare_test_dev + +tmp=$(mktemp --tmpdir btrfs-progs-fsck.XXXXXXX)
For a single temporary file I'd suggest not to use mktemp, so I'll remove the line.
Quoted 25 lines
tests/fsck-tests/031-metadatadump-check-data-csum/test.sh new file +18
-0+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV" +run_check_mount_test_dev + +run_check $SUDO_HELPER dd if=/dev/urandom of="$TEST_MNT/file" bs=4k count=16 +run_check_umount_test_dev + +run_check $SUDO_HELPER "$TOP/btrfs-image" "$TEST_DEV" "$tmp.image" + +# use prepare_test_dev() to wipe all existing data on $TEST_DEV +# so there is no way that restored image could have mathcing data csum +prepare_test_dev + +run_check $SUDO_HELPER "$TOP/btrfs-image" -r "$tmp.image" "$TEST_DEV" + +# Should not report any error +run_check "$TOP/btrfs" check --check-data-csum "$TEST_DEV" + +rm -rf -- "$tmp*"Signature · 6 lines
2.16.3 -- 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
Quoted 7 lines · Qu Wenruo
For metadata dump (fs restored by btrfs-image), since no data is restored check sum verification will definitely report error.
Add such check in
check_csums()and prompt for user.Issue: #103
Signed-off-by Qu Wenruo <[email protected]>
check/main.c static int check_csums(struct btrfs_root *root) · line 5604
Quoted 13 lines
--- check/main.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)check/main.c +1
-0@@ -5601,6 +5601,7 @@ static int check_csums(struct btrfs_root *root)int ret;u64 data_len;unsigned long leaf_offset; +bool verify_csum = !!check_data_csum;
nit: Why don't you convert check_data_csum to bool in this patch. My recent testing in !! usage showed that it's generating quite a number of additional instructions. Not that it will have big impact in this case though.
Quoted 30 lines
check/main.c +11
-1root = root->fs_info->csum_root;if (!extent_buffer_uptodate (root->node)) {@@ -5623,6 +5624,16 @@ static int check_csums(struct btrfs_root *root) path.slots[0 ]--; ret =0 ; +/* +* For metadata dump (btrfs-image) all data is wiped so verifying data +* csum is meaningless and will always report csum error. +*/ +if (check_data_csum && (btrfs_super_flags (root->fs_info->super_copy) & + (BTRFS_SUPER_FLAG_METADUMP |BTRFS_SUPER_FLAG_METADUMP_V2 ))) { +printf ("skip data csum verification for metadata dump\n" ); + verify_csum = false; + } +while (1 ) {if (path.slots[0 ] >=btrfs_header_nritems (path.nodes[0 ])) { ret =btrfs_next_leaf (root, &path);@@ -5644,7 +5655,7 @@ static int check_csums(struct btrfs_root *root) data_len = (btrfs_item_size_nr (leaf, path.slots[0 ]) / csum_size) * root->fs_info->sectorsize; -if (!check_data_csum) +if (!verify_csum) goto skip_csum_check; leaf_offset =btrfs_item_ptr_offset (leaf, path.slots[0 ]); ret = check_extent_csums(root, key.offset, data_len,
Quoted 17 lines · Nikolay Borisov
Quoted 7 lines · Qu Wenruo
For metadata dump (fs restored by btrfs-image), since no data is restored check sum verification will definitely report error.
Add such check in
check_csums()and prompt for user.Issue: #103
Signed-off-by Qu Wenruo <[email protected]>Reviewed-by Nikolay Borisov <[email protected]>Quoted 4 lines
--- check/main.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
Quoted 11 lines
check/main.c static int check_csums(struct btrfs_root *root) · line 5604
Quoted 9 lines
check/main.c +1
-0@@ -5601,6 +5601,7 @@ static int check_csums(struct btrfs_root *root)int ret;u64 data_len;unsigned long leaf_offset; +bool verify_csum = !!check_data_csum;nit: Why don't you convert
check_data_csumto bool in this patch.
Next thing to do.
Quoted 3 lines
My
recent testing in !! usage showed that it's generating quite a number of additional instructions.
That's pretty interesting.
Would you please shared some more info about this?
Thanks,
Qu
Quoted 38 lines
Not that it will have big impact in this case
though.Quoted 22 lines
check/main.c +10
-0root = root->fs_info->csum_root;if (!extent_buffer_uptodate (root->node)) {@@ -5623,6 +5624,16 @@ static int check_csums(struct btrfs_root *root) path.slots[0 ]--; ret =0 ; +/* +* For metadata dump (btrfs-image) all data is wiped so verifying data +* csum is meaningless and will always report csum error. +*/ +if (check_data_csum && (btrfs_super_flags (root->fs_info->super_copy) & + (BTRFS_SUPER_FLAG_METADUMP |BTRFS_SUPER_FLAG_METADUMP_V2 ))) { +printf ("skip data csum verification for metadata dump\n" ); + verify_csum = false; + } +while (1 ) {if (path.slots[0 ] >=btrfs_header_nritems (path.nodes[0 ])) { ret =btrfs_next_leaf (root, &path);check/main.c static int check_csums(struct btrfs_root *root) · line 5661
Quoted 8 lines
check/main.c +1
-1@@ -5644,7 +5655,7 @@ static int check_csums(struct btrfs_root *root) data_len = (btrfs_item_size_nr (leaf, path.slots[0 ]) / csum_size) * root->fs_info->sectorsize; -if (!check_data_csum) +if (!verify_csum) goto skip_csum_check; leaf_offset =btrfs_item_ptr_offset (leaf, path.slots[0 ]); ret = check_extent_csums(root, key.offset, data_len,--
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
Quoted 34 lines · Qu Wenruo
Quoted 17 lines · Nikolay Borisov
Quoted 7 lines · Qu Wenruo
For metadata dump (fs restored by btrfs-image), since no data is restored check sum verification will definitely report error.
Add such check in
check_csums()and prompt for user.Issue: #103
Signed-off-by Qu Wenruo <[email protected]>Reviewed-by Nikolay Borisov <[email protected]>Quoted 4 lines
--- check/main.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)Quoted 11 lines
check/main.c static int check_csums(struct btrfs_root *root) · line 5604
Quoted 9 lines
check/main.c +1
-0@@ -5601,6 +5601,7 @@ static int check_csums(struct btrfs_root *root)int ret;u64 data_len;unsigned long leaf_offset; +bool verify_csum = !!check_data_csum;nit: Why don't you convert
check_data_csumto bool in this patch.Next thing to do.
Quoted 7 lines
Quoted 3 lines
My
recent testing in !! usage showed that it's generating quite a number of additional instructions.That's pretty interesting.
Would you please shared some more info about this?
So it seems I might have been mistaken. The latest experiment I did was with the attached diff. So I did a bloat-o-meter comparison between stock random.c against the diff applied and then modified the diff to use uuid rather than !!uuid. And in my initial experiment I did see a reduction in the size of the function. However, now that I tried reproducing I couldn't. So just ignore my comment.
Bloat-o-meter shows in both cases:
nborisov@fisk:~/projects/kernel/source$ ./scripts/bloat-o-meter random.original drivers/char/random.o add/remove: 0/0 grow/shrink: 1/0 up/down: 49/0 (49) Function old new delta proc_do_uuid 227 276 +49 Total: Before=30542, After=30591, chg +0.16%
diff --git a/drivers/char/random.c b/drivers/char/random.c
index ec42c8bb9b0d..e05daf7f38f4 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1960,6 +1960,10 @@ static int proc_do_uuid(struct ctl_table *table, int write,
unsigned char buf[64], tmp_uuid[16], *uuid;
uuid = table->data;+#ifdef CONFIG_UTS_NS
+ if (!!uuid && (uuid == (unsigned char *)sysctl_bootid))
+ uuid = current->nsproxy->uts_ns->sysctl_bootid;
+#endif
if (!uuid) {
uuid = tmp_uuid;
generate_random_uuid(uuid);include/linux/utsname.h +0 -0
index c8060c2ecd04..f704aca3e95a 100644
--- a/include/linux/utsname.h
+++ b/include/linux/utsname.h
@@ -27,6 +27,7 @@ struct uts_namespace {
struct user_namespace *user_ns;
struct ucounts *ucounts;
struct ns_common ns;+ char sysctl_bootid[16];
} __randomize_layout;
extern struct uts_namespace init_uts_ns;
kernel/utsname.c +0 -0
index 913fe4336d2b..f1749cdcd341 100644
--- a/kernel/utsname.c
+++ b/kernel/utsname.c
@@ -34,8 +34,10 @@ static struct uts_namespace *create_uts_ns(void)
struct uts_namespace *uts_ns;
uts_ns = kmalloc(sizeof(struct uts_namespace), GFP_KERNEL);- if (uts_ns)
+ if (uts_ns) {
kref_init(&uts_ns->kref);
+ memset(uts_ns->sysctl_bootid, 0, 16);
+ }
return uts_ns;
}
David Sterba
AppliedQuoted 7 lines · Qu Wenruo
For metadata dump (fs restored by btrfs-image), since no data is restored check sum verification will definitely report error.
Add such check in
check_csums()and prompt for user.Issue: #103
Signed-off-by Qu Wenruo <[email protected]>
Applied, thanks.