From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net ([212.227.17.22]:56987 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728209AbeHIISZ (ORCPT ); Thu, 9 Aug 2018 04:18:25 -0400 Subject: Re: [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups From: Qu Wenruo To: Misono Tomohiro , linux-btrfs@vger.kernel.org References: <20180808060409.20048-1-wqu@suse.com> <82e8e5e4-7ada-433d-c380-82c534aac796@gmx.com> Message-ID: <13846684-4f34-ee9c-a816-a56e7de904ff@gmx.com> Date: Thu, 9 Aug 2018 13:55:01 +0800 MIME-Version: 1.0 In-Reply-To: <82e8e5e4-7ada-433d-c380-82c534aac796@gmx.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rMxaolGYLcAo7n1c9SV182QrXNMsggRvT" Sender: linux-btrfs-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --rMxaolGYLcAo7n1c9SV182QrXNMsggRvT Content-Type: multipart/mixed; boundary="X7xam7ztcVy7H5gulMTtPiAtoTpG6uNkH"; protected-headers="v1" From: Qu Wenruo To: Misono Tomohiro , linux-btrfs@vger.kernel.org Message-ID: <13846684-4f34-ee9c-a816-a56e7de904ff@gmx.com> Subject: Re: [PATCH] btrfs: qgroup: Don't populating excl numbers for snapshot src if it belongs to other qgroups References: <20180808060409.20048-1-wqu@suse.com> <82e8e5e4-7ada-433d-c380-82c534aac796@gmx.com> In-Reply-To: <82e8e5e4-7ada-433d-c380-82c534aac796@gmx.com> --X7xam7ztcVy7H5gulMTtPiAtoTpG6uNkH Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 8/8/18 3:48 PM, Qu Wenruo wrote: >=20 >=20 > On 2018=E5=B9=B408=E6=9C=8808=E6=97=A5 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 snapsh= ot >>> and it works if src doesn't belong to other qgroups. >>> >>> But if we have higher level qgroups, such behavior only works for lev= el >>> 0 qgroups, and higher level qgroups don't get update, thus making qgr= oup >>> 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_ha= ndle *trans, >>> if (!srcgroup) >>> goto unlock; >>> =20 >>> + /* >>> + * 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 |=3D >>> + 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=3D/dev/urandom of=3D/mnt/sub/file bs=3D1k count=3D1000 >> $ btrfs sub snap /mnt/sub /mnt/snap >> $ dmesg | tail -n 1 >> BTRFS info (device sdb7): src qgroup 0/257 belongs to higher level qgr= oup, >> 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 c= hild >> -------- ---- ---- -------- -------- ------ -= ---- >> 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 >> =20 >> 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 ch= ild >> -------- ---- ---- -------- -------- ------ -= ---- >> 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. >=20 > In this particular case, it's not hard to fix. >=20 > In fact for deleting/assigning qgroup with rfer =3D=3D excl case, it sh= ould > go through the quick accounting path. >=20 > But it looks like remove path doesn't go that path. My fault, in this case, since rfer !=3D excl, so we can't go quick updati= ng. 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 =3D 0 and excl =3D 0, and then remove qgroup relationship should work without the need to rescan. Thanks, Qu >=20 > I'll try to fix it soon. >=20 > Thanks, > Qu >=20 >> >> Thanks, >> Misono >> >> >=20 --X7xam7ztcVy7H5gulMTtPiAtoTpG6uNkH-- --rMxaolGYLcAo7n1c9SV182QrXNMsggRvT Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEELd9y5aWlW6idqkLhwj2R86El/qgFAltr1zUACgkQwj2R86El /qhBBAf8C4Gsvhvffll3c+h6Wi5rgDDalliYYHthO6rqQ3DruPtI1mDW7dJOeUH/ 7hzI3JMs5MNNxWLQgSd+gUupcpInMRgMpQAOSplYstaRLdbj1lhXDQss6RUmN0hH LMzTHCjPtx7dJosGF5jbX8+iAwV0Vw/a2aJlsSWF3mi2Rcnn09SSO6a5waGJ6CuN Y0vyfI/4DLZ7kelp0MySNcK24GOHht0cyf/EJ1WmM2/E2QtbtVF77s47AXcvGrGT Rx6mLM3lKXPLdsEmJ+VULstFzYoAnS1suXiCB+7QUpUWsD7J7g/uqrtBabr/015c Dl4KjkoKvBZmZ/EHINlmup74oxmZ8Q== =7Y2t -----END PGP SIGNATURE----- --rMxaolGYLcAo7n1c9SV182QrXNMsggRvT--