| Message ID | 5dea98d4f3749f895402164c3cba61b176ff3b2e.1708919464.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: qgroup: allow quick inherit if a snapshot if created and added to the same parent | expand |
On Mon, Feb 26, 2024 at 02:21:13PM +1030, Qu Wenruo wrote: > Currently "btrfs subvolume snapshot -i <qgroupid>" would always mark the > qgroup inconsistent. > > This can be annoying if the fs has a lot of snapshots, and needs qgroup > to get the accounting for the amount of bytes it can free for each > snapshot. > > Although we have the new simple quote as a solution, there is also a > case where we can skip the full scan, if all the following conditions > are met: > > - The source subvolume belongs to a higher level parent qgroup > - The parent qgroup already owns all its bytes exclusively > - The new snapshot is also added to the same parent qgroup > > In that case, we only need to add nodesize to the parent qgroup and > avoid a full rescan. Sounds reasonable and safe. > > This patch would add the extra quick accounting update for such inherit. "Add th extra ..." > Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: David Sterba <dsterba@suse.com> > fs/btrfs/qgroup.c | 74 ++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 67 insertions(+), 7 deletions(-) > > +static int qgroup_snapshot_quick_inherit(struct btrfs_fs_info *fs_info, > + u64 srcid, u64 parentid) > +{ > + struct btrfs_qgroup *src; > + struct btrfs_qgroup *parent; > + struct btrfs_qgroup_list *list; > + int nr_parents = 0; > + > + src = find_qgroup_rb(fs_info, srcid); > + if (!src) > + return -ENOENT; > + parent = find_qgroup_rb(fs_info, parentid); > + if (!parent) > + return -ENOENT; > + > + list_for_each_entry(list, &src->groups, next_group) { > + /* The parent is not the same, quick update not possible. */ > + if (list->group->qgroupid != parentid) > + return 1; > + nr_parents++; > + } > + /* > + * The source has multiple parents, do not bother it and require a > + * full rescan. > + */ > + if (nr_parents != 1) > + return 1; You can put the check to the list_for_each above so it does not continue once it's known that there are more parent qgroups.
在 2024/2/29 00:31, David Sterba 写道: > On Mon, Feb 26, 2024 at 02:21:13PM +1030, Qu Wenruo wrote: >> Currently "btrfs subvolume snapshot -i <qgroupid>" would always mark the >> qgroup inconsistent. >> >> This can be annoying if the fs has a lot of snapshots, and needs qgroup >> to get the accounting for the amount of bytes it can free for each >> snapshot. >> >> Although we have the new simple quote as a solution, there is also a >> case where we can skip the full scan, if all the following conditions >> are met: >> >> - The source subvolume belongs to a higher level parent qgroup >> - The parent qgroup already owns all its bytes exclusively >> - The new snapshot is also added to the same parent qgroup >> >> In that case, we only need to add nodesize to the parent qgroup and >> avoid a full rescan. > > Sounds reasonable and safe. >> >> This patch would add the extra quick accounting update for such inherit. > > "Add th extra ..." > >> Signed-off-by: Qu Wenruo <wqu@suse.com> > > Reviewed-by: David Sterba <dsterba@suse.com> > >> fs/btrfs/qgroup.c | 74 ++++++++++++++++++++++++++++++++++++++++++----- >> 1 file changed, 67 insertions(+), 7 deletions(-) >> >> +static int qgroup_snapshot_quick_inherit(struct btrfs_fs_info *fs_info, >> + u64 srcid, u64 parentid) >> +{ >> + struct btrfs_qgroup *src; >> + struct btrfs_qgroup *parent; >> + struct btrfs_qgroup_list *list; >> + int nr_parents = 0; >> + >> + src = find_qgroup_rb(fs_info, srcid); >> + if (!src) >> + return -ENOENT; >> + parent = find_qgroup_rb(fs_info, parentid); >> + if (!parent) >> + return -ENOENT; >> + >> + list_for_each_entry(list, &src->groups, next_group) { >> + /* The parent is not the same, quick update not possible. */ >> + if (list->group->qgroupid != parentid) >> + return 1; >> + nr_parents++; >> + } >> + /* >> + * The source has multiple parents, do not bother it and require a >> + * full rescan. >> + */ >> + if (nr_parents != 1) >> + return 1; > > You can put the check to the list_for_each above so it does not continue > once it's known that there are more parent qgroups. Let me check if I can do a single line check to make sure a list only contains one entry. Thanks, Qu
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index b3bf08fc2a39..4fa83c76b37b 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -3087,6 +3087,56 @@ static int qgroup_auto_inherit(struct btrfs_fs_info *fs_info, return 0; } +/* + * If @src has a single @parent, and that @parent is owning all its bytes + * exclusively, we can skip the full rescan, by just adding nodesize to + * the @parent's excl/rfer. + * + * Return <0 for fatal errors (like srcid/parentid has no qgroup). + * Return 0 if a quick inherit is done. + * Return >0 if a quick inherit is not possible, and a full rescan is needed. + */ +static int qgroup_snapshot_quick_inherit(struct btrfs_fs_info *fs_info, + u64 srcid, u64 parentid) +{ + struct btrfs_qgroup *src; + struct btrfs_qgroup *parent; + struct btrfs_qgroup_list *list; + int nr_parents = 0; + + src = find_qgroup_rb(fs_info, srcid); + if (!src) + return -ENOENT; + parent = find_qgroup_rb(fs_info, parentid); + if (!parent) + return -ENOENT; + + list_for_each_entry(list, &src->groups, next_group) { + /* The parent is not the same, quick update not possible. */ + if (list->group->qgroupid != parentid) + return 1; + nr_parents++; + } + /* + * The source has multiple parents, do not bother it and require a + * full rescan. + */ + if (nr_parents != 1) + return 1; + + /* + * The parent is not exclusively owning all its bytes. + * We're not sure if the source has any bytes not fully owned + * by the parent. + */ + if (parent->excl != parent->rfer) + return 1; + + parent->excl += fs_info->nodesize; + parent->rfer += fs_info->nodesize; + return 0; +} + /* * Copy the accounting information between qgroups. This is necessary * when a snapshot or a subvolume is created. Throwing an error will @@ -3255,6 +3305,13 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid, qgroup_dirty(fs_info, dstgroup); qgroup_dirty(fs_info, srcgroup); + + /* + * If the source qgroup has parent but the new one doesn't, + * we need a full rescan. + */ + if (!inherit && !list_empty(&srcgroup->groups)) + need_rescan = true; } if (!inherit) @@ -3269,14 +3326,17 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid, if (ret) goto unlock; } + if (srcid) { + /* Check if we can do a quick inherit. */ + ret = qgroup_snapshot_quick_inherit(fs_info, srcid, + *i_qgroups); + if (ret < 0) + goto unlock; + if (ret > 0) + need_rescan = true; + ret = 0; + } ++i_qgroups; - - /* - * If we're doing a snapshot, and adding the snapshot to a new - * qgroup, the numbers are guaranteed to be incorrect. - */ - if (srcid) - need_rescan = true; } for (i = 0; i < inherit->num_ref_copies; ++i, i_qgroups += 2) {
Currently "btrfs subvolume snapshot -i <qgroupid>" would always mark the qgroup inconsistent. This can be annoying if the fs has a lot of snapshots, and needs qgroup to get the accounting for the amount of bytes it can free for each snapshot. Although we have the new simple quote as a solution, there is also a case where we can skip the full scan, if all the following conditions are met: - The source subvolume belongs to a higher level parent qgroup - The parent qgroup already owns all its bytes exclusively - The new snapshot is also added to the same parent qgroup In that case, we only need to add nodesize to the parent qgroup and avoid a full rescan. This patch would add the extra quick accounting update for such inherit. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/qgroup.c | 74 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 67 insertions(+), 7 deletions(-)