Re: [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups
On 8/8/18 3:48 PM, Qu Wenruo wrote:
>
>
> On 2018年08月08日 15:41, Misono Tomohiro wrote:
>> On 2018/08/08 15:04, Qu Wenruo wrote:
>>> When quota is enabled and we do a snapshot, we just update the 'excl'
>>> number of both snapshot src and dst to src's 'rfer' - nodesize.
>>>
>>> It's a quick hack to avoid quota rescan every time we create a snapshot
>>> and it works if src doesn't belong to other qgroups.
>>>
>>> But if we have higher level qgroups, such behavior only works for level
>>> 0 qgroups, and higher level qgroups don't get update, thus making qgroup
>>> number inconsistent.
>>>
>>> The problem of updating higher level qgroup numbers is, it's way to
>>> complex.
>>>
>>> Under the following case, it's pretty simple: (src is 257, dst is 258)
>>> 0/257 - 1/0, 0/258.
>>>
>>> In this case, we only need to modify 1/0 to reduce its 'excl'
>>>
>>> But under the following case, it will go out of control:
>>>
>>> 0/257 - 1/0, 0/258 - 1/1 (using -i option), 1/0 - 2/0, 1/1 - 2/0.
>>>
>>> So to make it simple, if snapshot src has higher level qgroups, just
>>> mark qgroup inconsistent and let later rescan to do its job.
>>>
>>> Reported-by: Misono Tomohiro
>>> Signed-off-by: Qu Wenruo
>>> ---
>>> fs/btrfs/qgroup.c | 16
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>>> index ec4351fd7537..2b3d2dd1b735 100644
>>> --- a/fs/btrfs/qgroup.c
>>> +++ b/fs/btrfs/qgroup.c
>>> @@ -2298,6 +2298,22 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle
>>> *trans,
>>> if (!srcgroup)
>>> goto unlock;
>>>
>>> + /*
>>> +* If snapshot source is belonging to high level qgroups, it
>>> +* will be a more complex to hack the numbers.
>>> +* E.g. source is 257, snapshot is 258:
>>> +* 0/257 - 1/0, creating snapshot 258 will need to update 1/0
>>> +* It's too complex when higher level qgroup is involved.
>>> +* Mark qgroup inconsistent for later rescan
>>> +*/
>>> + if (!list_empty(>groups)) {
>>> + btrfs_info_rl(fs_info,
>>> +"src qgroup 0/%llu belongs to higher level qgroup, creating snapshot for
>>> it need qgroup rescan",
>>> + srcid);
>>> + fs_info->qgroup_flags |=
>>> + BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
>>> + goto unlock;
>>> + }
>>> /*
>>> * We call inherit after we clone the root in order to make sure
>>> * our counts don't go crazy, so at this point the only
>>>
>>
>> Thanks for the quick fix.
>> Tested-by/Reviewed-by: Misono Tomohiro
>>
>> However there is still another problem about removing relation:
>>
>> (4.18-rc7 with above patch)
>> $ mkfs.btrfs -fq $DEV
>> $ mount $DEV /mnt
>>
>> $ btrfs quota enable /mnt
>> $ btrfs qgroup create 1/0 /mnt
>> $ btrfs sub create /mnt/sub
>> $ btrfs qgroup assign 0/257 1/0 /mnt
>>
>> $ dd if=/dev/urandom of=/mnt/sub/file bs=1k count=1000
>> $ btrfs sub snap /mnt/sub /mnt/snap
>> $ dmesg | tail -n 1
>> BTRFS info (device sdb7): src qgroup 0/257 belongs to higher level qgroup,
>> creating snapshot for it need qgroup rescan
>>
>> $ btrfs quota rescan -w /mnt
>> $ btrfs qgroup show -pcre /mnt
>> qgroupid rfer excl max_rfer max_excl parent child
>> -- -
>> 0/5 16.00KiB 16.00KiB none none --- ---
>> 0/257 1016.00KiB 16.00KiB none none 1/0 ---
>> 0/258 1016.00KiB 16.00KiB none none --- ---
>> 1/01016.00KiB 16.00KiB none none --- 0/257
>>
>> so far so good, but:
>>
>> $ btrfs qgroup remove 0/257 1/0 /mnt
>> WARNING: quotas may be inconsistent, rescan needed
>> $ btrfs quota rescan -w /mnt
>> $ btrfs qgroup show -pcre /mnt
>> qgoupid rfer excl max_rfer max_excl parent child
>> -- -
>> 0/5 16.00KiB 16.00KiB none none --- ---
>> 0/257 1016.00KiB 16.00KiB none none --- ---
>> 0/258 1016.00KiB 16.00KiB none none --- ---
>> 1/01016.00KiB 16.00KiB none none --- ---
>>^^ not cleared
>>
>> It seems some fix is needed for rescan too.
>
> In this particular case, it's not hard to fix.
>
> In fact for deleting/assigning qgroup with rfer == excl case, it should
> go through the quick accounting path.
>
> But it looks like remove path doesn't go that path.
My fault, in this case, since rfer != excl, so we can't go quick updating.
But on the other hand, if you don't remove the 0 level qgroup 0/257
directly, but using subvolume delete, qgroup
Re: [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups
On 2018年08月08日 15:41, Misono Tomohiro wrote:
> On 2018/08/08 15:04, Qu Wenruo wrote:
>> When quota is enabled and we do a snapshot, we just update the 'excl'
>> number of both snapshot src and dst to src's 'rfer' - nodesize.
>>
>> It's a quick hack to avoid quota rescan every time we create a snapshot
>> and it works if src doesn't belong to other qgroups.
>>
>> But if we have higher level qgroups, such behavior only works for level
>> 0 qgroups, and higher level qgroups don't get update, thus making qgroup
>> number inconsistent.
>>
>> The problem of updating higher level qgroup numbers is, it's way to
>> complex.
>>
>> Under the following case, it's pretty simple: (src is 257, dst is 258)
>> 0/257 - 1/0, 0/258.
>>
>> In this case, we only need to modify 1/0 to reduce its 'excl'
>>
>> But under the following case, it will go out of control:
>>
>> 0/257 - 1/0, 0/258 - 1/1 (using -i option), 1/0 - 2/0, 1/1 - 2/0.
>>
>> So to make it simple, if snapshot src has higher level qgroups, just
>> mark qgroup inconsistent and let later rescan to do its job.
>>
>> Reported-by: Misono Tomohiro
>> Signed-off-by: Qu Wenruo
>> ---
>> fs/btrfs/qgroup.c | 16
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
>> index ec4351fd7537..2b3d2dd1b735 100644
>> --- a/fs/btrfs/qgroup.c
>> +++ b/fs/btrfs/qgroup.c
>> @@ -2298,6 +2298,22 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle
>> *trans,
>> if (!srcgroup)
>> goto unlock;
>>
>> +/*
>> + * If snapshot source is belonging to high level qgroups, it
>> + * will be a more complex to hack the numbers.
>> + * E.g. source is 257, snapshot is 258:
>> + * 0/257 - 1/0, creating snapshot 258 will need to update 1/0
>> + * It's too complex when higher level qgroup is involved.
>> + * Mark qgroup inconsistent for later rescan
>> + */
>> +if (!list_empty(>groups)) {
>> +btrfs_info_rl(fs_info,
>> +"src qgroup 0/%llu belongs to higher level qgroup, creating snapshot for it
>> need qgroup rescan",
>> + srcid);
>> +fs_info->qgroup_flags |=
>> +BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
>> +goto unlock;
>> +}
>> /*
>> * We call inherit after we clone the root in order to make sure
>> * our counts don't go crazy, so at this point the only
>>
>
> Thanks for the quick fix.
> Tested-by/Reviewed-by: Misono Tomohiro
>
> However there is still another problem about removing relation:
>
> (4.18-rc7 with above patch)
> $ mkfs.btrfs -fq $DEV
> $ mount $DEV /mnt
>
> $ btrfs quota enable /mnt
> $ btrfs qgroup create 1/0 /mnt
> $ btrfs sub create /mnt/sub
> $ btrfs qgroup assign 0/257 1/0 /mnt
>
> $ dd if=/dev/urandom of=/mnt/sub/file bs=1k count=1000
> $ btrfs sub snap /mnt/sub /mnt/snap
> $ dmesg | tail -n 1
> BTRFS info (device sdb7): src qgroup 0/257 belongs to higher level qgroup,
> creating snapshot for it need qgroup rescan
>
> $ btrfs quota rescan -w /mnt
> $ btrfs qgroup show -pcre /mnt
> qgroupid rfer excl max_rfer max_excl parent child
> -- -
> 0/5 16.00KiB 16.00KiB none none --- ---
> 0/257 1016.00KiB 16.00KiB none none 1/0 ---
> 0/258 1016.00KiB 16.00KiB none none --- ---
> 1/01016.00KiB 16.00KiB none none --- 0/257
>
> so far so good, but:
>
> $ btrfs qgroup remove 0/257 1/0 /mnt
> WARNING: quotas may be inconsistent, rescan needed
> $ btrfs quota rescan -w /mnt
> $ btrfs qgroup show -pcre /mnt
> qgoupid rfer excl max_rfer max_excl parent child
> -- -
> 0/5 16.00KiB 16.00KiB none none --- ---
> 0/257 1016.00KiB 16.00KiB none none --- ---
> 0/258 1016.00KiB 16.00KiB none none --- ---
> 1/01016.00KiB 16.00KiB none none --- ---
>^^ not cleared
>
> It seems some fix is needed for rescan too.
In this particular case, it's not hard to fix.
In fact for deleting/assigning qgroup with rfer == excl case, it should
go through the quick accounting path.
But it looks like remove path doesn't go that path.
I'll try to fix it soon.
Thanks,
Qu
>
> Thanks,
> Misono
>
>
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups
On 2018/08/08 15:04, Qu Wenruo wrote:
> When quota is enabled and we do a snapshot, we just update the 'excl'
> number of both snapshot src and dst to src's 'rfer' - nodesize.
>
> It's a quick hack to avoid quota rescan every time we create a snapshot
> and it works if src doesn't belong to other qgroups.
>
> But if we have higher level qgroups, such behavior only works for level
> 0 qgroups, and higher level qgroups don't get update, thus making qgroup
> number inconsistent.
>
> The problem of updating higher level qgroup numbers is, it's way to
> complex.
>
> Under the following case, it's pretty simple: (src is 257, dst is 258)
> 0/257 - 1/0, 0/258.
>
> In this case, we only need to modify 1/0 to reduce its 'excl'
>
> But under the following case, it will go out of control:
>
> 0/257 - 1/0, 0/258 - 1/1 (using -i option), 1/0 - 2/0, 1/1 - 2/0.
>
> So to make it simple, if snapshot src has higher level qgroups, just
> mark qgroup inconsistent and let later rescan to do its job.
>
> Reported-by: Misono Tomohiro
> Signed-off-by: Qu Wenruo
> ---
> fs/btrfs/qgroup.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index ec4351fd7537..2b3d2dd1b735 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2298,6 +2298,22 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle
> *trans,
> if (!srcgroup)
> goto unlock;
>
> + /*
> + * If snapshot source is belonging to high level qgroups, it
> + * will be a more complex to hack the numbers.
> + * E.g. source is 257, snapshot is 258:
> + * 0/257 - 1/0, creating snapshot 258 will need to update 1/0
> + * It's too complex when higher level qgroup is involved.
> + * Mark qgroup inconsistent for later rescan
> + */
> + if (!list_empty(>groups)) {
> + btrfs_info_rl(fs_info,
> +"src qgroup 0/%llu belongs to higher level qgroup, creating snapshot for it
> need qgroup rescan",
> + srcid);
> + fs_info->qgroup_flags |=
> + BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> + goto unlock;
> + }
> /*
>* We call inherit after we clone the root in order to make sure
>* our counts don't go crazy, so at this point the only
>
Thanks for the quick fix.
Tested-by/Reviewed-by: Misono Tomohiro
However there is still another problem about removing relation:
(4.18-rc7 with above patch)
$ mkfs.btrfs -fq $DEV
$ mount $DEV /mnt
$ btrfs quota enable /mnt
$ btrfs qgroup create 1/0 /mnt
$ btrfs sub create /mnt/sub
$ btrfs qgroup assign 0/257 1/0 /mnt
$ dd if=/dev/urandom of=/mnt/sub/file bs=1k count=1000
$ btrfs sub snap /mnt/sub /mnt/snap
$ dmesg | tail -n 1
BTRFS info (device sdb7): src qgroup 0/257 belongs to higher level qgroup,
creating snapshot for it need qgroup rescan
$ btrfs quota rescan -w /mnt
$ btrfs qgroup show -pcre /mnt
qgroupid rfer excl max_rfer max_excl parent child
-- -
0/5 16.00KiB 16.00KiB none none --- ---
0/257 1016.00KiB 16.00KiB none none 1/0 ---
0/258 1016.00KiB 16.00KiB none none --- ---
1/01016.00KiB 16.00KiB none none --- 0/257
so far so good, but:
$ btrfs qgroup remove 0/257 1/0 /mnt
WARNING: quotas may be inconsistent, rescan needed
$ btrfs quota rescan -w /mnt
$ btrfs qgroup show -pcre /mnt
qgoupid rfer excl max_rfer max_excl parent child
-- -
0/5 16.00KiB 16.00KiB none none --- ---
0/257 1016.00KiB 16.00KiB none none --- ---
0/258 1016.00KiB 16.00KiB none none --- ---
1/01016.00KiB 16.00KiB none none --- ---
^^ not cleared
It seems some fix is needed for rescan too.
Thanks,
Misono
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups
When quota is enabled and we do a snapshot, we just update the 'excl'
number of both snapshot src and dst to src's 'rfer' - nodesize.
It's a quick hack to avoid quota rescan every time we create a snapshot
and it works if src doesn't belong to other qgroups.
But if we have higher level qgroups, such behavior only works for level
0 qgroups, and higher level qgroups don't get update, thus making qgroup
number inconsistent.
The problem of updating higher level qgroup numbers is, it's way to
complex.
Under the following case, it's pretty simple: (src is 257, dst is 258)
0/257 - 1/0, 0/258.
In this case, we only need to modify 1/0 to reduce its 'excl'
But under the following case, it will go out of control:
0/257 - 1/0, 0/258 - 1/1 (using -i option), 1/0 - 2/0, 1/1 - 2/0.
So to make it simple, if snapshot src has higher level qgroups, just
mark qgroup inconsistent and let later rescan to do its job.
Reported-by: Misono Tomohiro
Signed-off-by: Qu Wenruo
---
fs/btrfs/qgroup.c | 16
1 file changed, 16 insertions(+)
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index ec4351fd7537..2b3d2dd1b735 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -2298,6 +2298,22 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle
*trans,
if (!srcgroup)
goto unlock;
+ /*
+* If snapshot source is belonging to high level qgroups, it
+* will be a more complex to hack the numbers.
+* E.g. source is 257, snapshot is 258:
+* 0/257 - 1/0, creating snapshot 258 will need to update 1/0
+* It's too complex when higher level qgroup is involved.
+* Mark qgroup inconsistent for later rescan
+*/
+ if (!list_empty(>groups)) {
+ btrfs_info_rl(fs_info,
+"src qgroup 0/%llu belongs to higher level qgroup, creating snapshot for it
need qgroup rescan",
+ srcid);
+ fs_info->qgroup_flags |=
+ BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
+ goto unlock;
+ }
/*
* We call inherit after we clone the root in order to make sure
* our counts don't go crazy, so at this point the only
--
2.18.0
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
