btrfs: qgroup: Search commit root for rescan to avoid missing extent

PATCH Superseded Qu Wenruo 4 participants 4 messages May 3, 2018 – May 12, 2018 Btrfs file system

Share & tools

Qu Wenruo

author

2018-05-03 07:21 UTC #0 lore raw

PATCH 1/1 btrfs: qgroup: Search commit root for rescan to avoid missing extent

When doing qgroup rescan using the following script (modified from btrfs/017 test case), we can sometimes hit qgroup corruption.

------
umount $dev &> /dev/null
umount $mnt &> /dev/null

mkfs.btrfs -f -n 64k $dev
mount $dev $mnt

extent_size=8192

xfs_io -f -d -c "pwrite 0 $extent_size" $mnt/foo > /dev/null btrfs subvolume snapshot $mnt $mnt/snap

xfs_io -f -c "reflink $mnt/foo" $mnt/foo-reflink > /dev/null xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink > /dev/null xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink2 > /dev/unll btrfs quota enable $mnt

# -W is the new option to only wait rescan while not starting new one
btrfs quota rescan -W $mnt
btrfs qgroup show -prce $mnt

# Need to patch btrfs-progs to report qgroup mismatch as error
btrfs check $dev || _fail
------

For fast machine, we can hit some corruption which missed accounting tree blocks:
------

qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5           8.00KiB        0.00B         none         none ---     ---
0/257         8.00KiB        0.00B         none         none ---     ---

------

This is due to the fact that we're always searching commit root for btrfs_find_all_roots() at qgroup_rescan_leaf(), but the leaf we get is from current transaction, not commit root.

And if our tree blocks get modified in current transaction, we won't find any owner in commit root, thus causing the corruption.

Fix it by searching commit root for extent tree for qgroup_rescan_leaf().

Reported-by Nikolay Borisov <[email protected]>

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

---

Please keep in mind that it is possible to hit another type of race which double accounting tree blocks:
------

qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5          136.00KiB     128.00KiB         none         none ---     ---
0/257        136.00KiB     128.00KiB         none         none ---     ---

------
For this type of corruption, this patch could reduce the possibility, but the root cause is race between transaction commit and qgroup rescan, which needs to be addressed in another patch.

---
 fs/btrfs/qgroup.c | 5 +++++
 1 file changed, 5 insertions(+)

fs/btrfs/qgroup.c +5 -0

index 4baa4ba2d630..829e8fe5c97e 100644
@@ -2681,6 +2681,11 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
 	path = btrfs_alloc_path();
 	if (!path)
 		goto out;
+	/*
+	 * Rescan should only search for commit root, and any later difference
+	 * should be recorded by qgroup
+	 */
+	path->search_commit_root = 1;
 
 	err = 0;
 	while (!err && !btrfs_fs_closing(fs_info)) {

Signature · 1 line

2.17.0

David Sterba

2018-05-09 13:07 UTC #1 lore raw

Quoted 41 lines · Qu Wenruo

When doing qgroup rescan using the following script (modified from btrfs/017 test case), we can sometimes hit qgroup corruption.
------
umount $dev &> /dev/null
umount $mnt &> /dev/null
mkfs.btrfs -f -n 64k $dev
mount $dev $mnt
extent_size=8192
xfs_io -f -d -c "pwrite 0 $extent_size" $mnt/foo > /dev/null btrfs subvolume snapshot $mnt $mnt/snap
xfs_io -f -c "reflink $mnt/foo" $mnt/foo-reflink > /dev/null xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink > /dev/null xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink2 > /dev/unll btrfs quota enable $mnt
# -W is the new option to only wait rescan while not starting new one
btrfs quota rescan -W $mnt
btrfs qgroup show -prce $mnt
# Need to patch btrfs-progs to report qgroup mismatch as error
btrfs check $dev || _fail
------
For fast machine, we can hit some corruption which missed accounting tree blocks:
------
qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5           8.00KiB        0.00B         none         none ---     ---
0/257         8.00KiB        0.00B         none         none ---     ---
------
This is due to the fact that we're always searching commit root for btrfs_find_all_roots() at qgroup_rescan_leaf(), but the leaf we get is from current transaction, not commit root.

Quoted 8 lines

And if our tree blocks get modified in current transaction, we won't find any owner in commit root, thus causing the corruption.
Fix it by searching commit root for extent tree for qgroup_rescan_leaf().
Reported-by Nikolay Borisov <[email protected]>
Signed-off-by Qu Wenruo <[email protected]>

Added to misc-next, thanks.

Quoted 5 lines

---
Please keep in mind that it is possible to hit another type of race which double accounting tree blocks:
------

Quoted 8 lines

qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5          136.00KiB     128.00KiB         none         none ---     ---
0/257        136.00KiB     128.00KiB         none         none ---     ---
------
For this type of corruption, this patch could reduce the possibility, but the root cause is race between transaction commit and qgroup rescan, which needs to be addressed in another patch.

Both patches are now in misc-next, I saw the btrfs/017 failures occasionally so will watch if it's all ok now.

Jeff Mahoney

2018-05-11 17:08 UTC #2 lore raw

In reply to Qu Wenruo

Quoted 73 lines · Qu Wenruo

When doing qgroup rescan using the following script (modified from btrfs/017 test case), we can sometimes hit qgroup corruption.
------
umount $dev &> /dev/null
umount $mnt &> /dev/null
mkfs.btrfs -f -n 64k $dev
mount $dev $mnt
extent_size=8192
xfs_io -f -d -c "pwrite 0 $extent_size" $mnt/foo > /dev/null btrfs subvolume snapshot $mnt $mnt/snap
xfs_io -f -c "reflink $mnt/foo" $mnt/foo-reflink > /dev/null xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink > /dev/null xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink2 > /dev/unll btrfs quota enable $mnt
# -W is the new option to only wait rescan while not starting new one
btrfs quota rescan -W $mnt
btrfs qgroup show -prce $mnt
# Need to patch btrfs-progs to report qgroup mismatch as error
btrfs check $dev || _fail
------
For fast machine, we can hit some corruption which missed accounting tree blocks:
------
qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5           8.00KiB        0.00B         none         none ---     ---
0/257         8.00KiB        0.00B         none         none ---     ---
------
This is due to the fact that we're always searching commit root for btrfs_find_all_roots() at qgroup_rescan_leaf(), but the leaf we get is from current transaction, not commit root.
And if our tree blocks get modified in current transaction, we won't find any owner in commit root, thus causing the corruption.
Fix it by searching commit root for extent tree for qgroup_rescan_leaf().
Reported-by Nikolay Borisov <[email protected]>
Signed-off-by Qu Wenruo <[email protected]>
---
Please keep in mind that it is possible to hit another type of race which double accounting tree blocks:
------
qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5          136.00KiB     128.00KiB         none         none ---     ---
0/257        136.00KiB     128.00KiB         none         none ---     ---
------
For this type of corruption, this patch could reduce the possibility, but the root cause is race between transaction commit and qgroup rescan, which needs to be addressed in another patch.
---
 fs/btrfs/qgroup.c | 5 +++++
 1 file changed, 5 insertions(+)
fs/btrfs/qgroup.c +0 -0
index 4baa4ba2d630..829e8fe5c97e 100644
@@ -2681,6 +2681,11 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
 	path = btrfs_alloc_path();

fs/btrfs/qgroup.c static void btrfs_qgroup_rescan_worker(struct btrfs_work *work) · line 2691

Quoted 10 lines

fs/btrfs/qgroup.c +5 -0

 	if (!path)
 		goto out;
+	/*
+	 * Rescan should only search for commit root, and any later difference
+	 * should be recorded by qgroup
+	 */
+	path->search_commit_root = 1;
 
 	err = 0;
 	while (!err && !btrfs_fs_closing(fs_info)) {

If we're searching the commit root here, do we need the tree mod sequence number dance in qgroup_rescan_leaf anymore?

-Jeff

Signature · 2 lines

Jeff Mahoney
SUSE Labs

Qu Wenruo

2018-05-12 00:09 UTC #3 lore raw

Quoted 81 lines · Jeff Mahoney

Quoted 80 lines · Qu Wenruo
When doing qgroup rescan using the following script (modified from btrfs/017 test case), we can sometimes hit qgroup corruption.
------
umount $dev &> /dev/null
umount $mnt &> /dev/null
mkfs.btrfs -f -n 64k $dev
mount $dev $mnt
extent_size=8192
xfs_io -f -d -c "pwrite 0 $extent_size" $mnt/foo > /dev/null btrfs subvolume snapshot $mnt $mnt/snap
xfs_io -f -c "reflink $mnt/foo" $mnt/foo-reflink > /dev/null xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink > /dev/null xfs_io -f -c "reflink $mnt/foo" $mnt/snap/foo-reflink2 > /dev/unll btrfs quota enable $mnt
# -W is the new option to only wait rescan while not starting new one
btrfs quota rescan -W $mnt
btrfs qgroup show -prce $mnt
# Need to patch btrfs-progs to report qgroup mismatch as error
btrfs check $dev || _fail
------
For fast machine, we can hit some corruption which missed accounting tree blocks:
------
qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5           8.00KiB        0.00B         none         none ---     ---
0/257         8.00KiB        0.00B         none         none ---     ---
------
This is due to the fact that we're always searching commit root for btrfs_find_all_roots() at qgroup_rescan_leaf(), but the leaf we get is from current transaction, not commit root.
And if our tree blocks get modified in current transaction, we won't find any owner in commit root, thus causing the corruption.
Fix it by searching commit root for extent tree for qgroup_rescan_leaf().
Reported-by Nikolay Borisov <[email protected]>
Signed-off-by Qu Wenruo <[email protected]>
---
Please keep in mind that it is possible to hit another type of race which double accounting tree blocks:
------
qgroupid         rfer         excl     max_rfer     max_excl parent  child
--------         ----         ----     --------     -------- ------  -----
0/5          136.00KiB     128.00KiB         none         none ---     ---
0/257        136.00KiB     128.00KiB         none         none ---     ---
------
For this type of corruption, this patch could reduce the possibility, but the root cause is race between transaction commit and qgroup rescan, which needs to be addressed in another patch.
---
 fs/btrfs/qgroup.c | 5 +++++
 1 file changed, 5 insertions(+)
fs/btrfs/qgroup.c +5 -0
index 4baa4ba2d630..829e8fe5c97e 100644
@@ -2681,6 +2681,11 @@ static void btrfs_qgroup_rescan_worker(struct btrfs_work *work)
 	path = btrfs_alloc_path();
 	if (!path)
 		goto out;
+	/*
+	 * Rescan should only search for commit root, and any later difference
+	 * should be recorded by qgroup
+	 */
+	path->search_commit_root = 1;
 

Quoted 6 lines

fs/btrfs/qgroup.c static void btrfs_qgroup_rescan_worker(struct btrfs_work *work) · line 2691
Quoted 3 lines
fs/btrfs/qgroup.c +0 -0
 	err = 0;
 	while (!err && !btrfs_fs_closing(fs_info)) {
If we're searching the commit root here, do we need the tree mod sequence number dance in qgroup_rescan_leaf anymore?

No, so I'll remove it in next version.

Thanks for pointing this out,
Qu

Quoted 3 lines

-Jeff