* [PATCH] btrfs: qgroup: Search commit root for rescan to avoid missing extent
@ 2018-05-03 7:20 Qu Wenruo
2018-05-09 13:04 ` David Sterba
2018-05-11 17:08 ` Jeff Mahoney
0 siblings, 2 replies; 4+ messages in thread
From: Qu Wenruo @ 2018-05-03 7:20 UTC (permalink / raw)
To: linux-btrfs
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 <nborisov@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
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(+)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 4baa4ba2d630..829e8fe5c97e 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -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)) {
--
2.17.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Search commit root for rescan to avoid missing extent
2018-05-03 7:20 [PATCH] btrfs: qgroup: Search commit root for rescan to avoid missing extent Qu Wenruo
@ 2018-05-09 13:04 ` David Sterba
2018-05-11 17:08 ` Jeff Mahoney
1 sibling, 0 replies; 4+ messages in thread
From: David Sterba @ 2018-05-09 13:04 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Thu, May 03, 2018 at 03:20:52PM +0800, Qu Wenruo wrote:
> 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 <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Added to misc-next, thanks.
> ---
>
> 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.
Both patches are now in misc-next, I saw the btrfs/017 failures
occasionally so will watch if it's all ok now.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Search commit root for rescan to avoid missing extent
2018-05-03 7:20 [PATCH] btrfs: qgroup: Search commit root for rescan to avoid missing extent Qu Wenruo
2018-05-09 13:04 ` David Sterba
@ 2018-05-11 17:08 ` Jeff Mahoney
2018-05-12 0:08 ` Qu Wenruo
1 sibling, 1 reply; 4+ messages in thread
From: Jeff Mahoney @ 2018-05-11 17:08 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 3289 bytes --]
On 5/3/18 3:20 AM, Qu Wenruo wrote:
> 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 <nborisov@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>
> 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(+)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 4baa4ba2d630..829e8fe5c97e 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -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)) {
>
If we're searching the commit root here, do we need the tree mod
sequence number dance in qgroup_rescan_leaf anymore?
-Jeff
--
Jeff Mahoney
SUSE Labs
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Search commit root for rescan to avoid missing extent
2018-05-11 17:08 ` Jeff Mahoney
@ 2018-05-12 0:08 ` Qu Wenruo
0 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2018-05-12 0:08 UTC (permalink / raw)
To: Jeff Mahoney, Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 3475 bytes --]
On 2018年05月12日 01:08, Jeff Mahoney wrote:
> On 5/3/18 3:20 AM, Qu Wenruo wrote:
>> 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 <nborisov@suse.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>
>> 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(+)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index 4baa4ba2d630..829e8fe5c97e 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -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)) {
>>
>
> 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
>
> -Jeff
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-05-12 0:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03 7:20 [PATCH] btrfs: qgroup: Search commit root for rescan to avoid missing extent Qu Wenruo
2018-05-09 13:04 ` David Sterba
2018-05-11 17:08 ` Jeff Mahoney
2018-05-12 0:08 ` Qu Wenruo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.