* [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups @ 2018-08-08 6:04 Qu Wenruo 2018-08-08 7:41 ` Misono Tomohiro 0 siblings, 1 reply; 4+ messages in thread From: Qu Wenruo @ 2018-08-08 6:04 UTC (permalink / raw) To: linux-btrfs 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 <misono.tomohiro@jp.fujitsu.com> Signed-off-by: Qu Wenruo <wqu@suse.com> --- 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(&srcgroup->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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups 2018-08-08 6:04 [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups Qu Wenruo @ 2018-08-08 7:41 ` Misono Tomohiro 2018-08-08 7:48 ` Qu Wenruo 0 siblings, 1 reply; 4+ messages in thread From: Misono Tomohiro @ 2018-08-08 7:41 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs 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 <misono.tomohiro@jp.fujitsu.com> > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > 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(&srcgroup->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 <misono.tomohiro@jp.fujitsu.com> 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/0 1016.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/0 1016.00KiB 16.00KiB none none --- --- ^^^^^^^^^^ ^^^^^^^^ not cleared It seems some fix is needed for rescan too. Thanks, Misono ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups 2018-08-08 7:41 ` Misono Tomohiro @ 2018-08-08 7:48 ` Qu Wenruo 2018-08-09 5:55 ` Qu Wenruo 0 siblings, 1 reply; 4+ messages in thread From: Qu Wenruo @ 2018-08-08 7:48 UTC (permalink / raw) To: Misono Tomohiro, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 4695 bytes --] 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 <misono.tomohiro@jp.fujitsu.com> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> 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(&srcgroup->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 <misono.tomohiro@jp.fujitsu.com> > > 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/0 1016.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/0 1016.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 > > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups 2018-08-08 7:48 ` Qu Wenruo @ 2018-08-09 5:55 ` Qu Wenruo 0 siblings, 0 replies; 4+ messages in thread From: Qu Wenruo @ 2018-08-09 5:55 UTC (permalink / raw) To: Misono Tomohiro, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 5207 bytes --] 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 <misono.tomohiro@jp.fujitsu.com> >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>> --- >>> 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(&srcgroup->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 <misono.tomohiro@jp.fujitsu.com> >> >> 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/0 1016.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/0 1016.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 should update 0/257 to rfer = 0 and excl = 0, and then remove qgroup relationship should work without the need to rescan. Thanks, Qu > > I'll try to fix it soon. > > Thanks, > Qu > >> >> Thanks, >> Misono >> >> > [-- 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-08-09 8:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-08 6:04 [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups Qu Wenruo 2018-08-08 7:41 ` Misono Tomohiro 2018-08-08 7:48 ` Qu Wenruo 2018-08-09 5:55 ` Qu Wenruo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).