Re: [PATCH v5 07/18] btrfs: function for recording simple quota deltas

David Sterba
2023-09-07T15:36:13Z

On Thu, Jul 27, 2023 at 03:12:54PM -0700, Boris Burkov wrote:
> Rather than re-computing shared/exclusive ownership based on backrefs
> and walking roots for implicit backrefs, simple quotas does an increment
> when creating an extent and a decrement when deleting it. Add the API
> for the extent item code to use to track those events.
>
> Also add a helper function to make collecting parent qgroups in a ulist
> easier for functions like this.
>
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
> fs/btrfs/qgroup.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
> fs/btrfs/qgroup.h | 11 ++++++-
> 2 files changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 8e3a4ced3077..dedc532669f4 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -332,6 +332,35 @@ static int del_relation_rb(struct btrfs_fs_info *fs_info,
> return -ENOENT;
> }
>
> +static int qgroup_collect_parents(struct btrfs_qgroup *qgroup,
> + struct ulist *ul)
> +{
> + struct ulist_iterator uiter;
> + struct ulist_node *unode;
> + struct btrfs_qgroup_list *glist;
> + struct btrfs_qgroup *qg;
> + int ret = 0;
> +
> + ulist_reinit(ul);
> + ret = ulist_add(ul, qgroup->qgroupid,
> + qgroup_to_aux(qgroup), GFP_ATOMIC);

Qu has sent a series to get rid of the GFP_ATOMIC allocations when
processing qgruops, so this would be good to port to the qgroup
iterators as well but it's a recent change and can be done later as an
optimization.

> + if (ret < 0)
> + goto out;
> + ULIST_ITER_INIT(&uiter);
> + while ((unode = ulist_next(ul, &uiter))) {
> + qg = unode_aux_to_qgroup(unode);
> + list_for_each_entry(glist, &qg->groups, next_group) {
> + ret = ulist_add(ul, glist->group->qgroupid,
> + qgroup_to_aux(glist->group), GFP_ATOMIC);
> + if (ret < 0)
> + goto out;
> + }
> + }
> + ret = 0;
> +out:
> + return ret;
> +}
> +
> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
> int btrfs_verify_qgroup_counts(struct btrfs_fs_info *fs_info, u64 qgroupid,
> u64 rfer, u64 excl)
> @@ -4535,3 +4564,47 @@ void btrfs_qgroup_destroy_extent_records(struct btrfs_transaction *trans)
> }
> *root = RB_ROOT;
> }
> +
> +int btrfs_record_simple_quota_delta(struct btrfs_fs_info *fs_info,

You can abbreviate all the 'simple_quota' in identifiers as 'squota'.

> + struct btrfs_simple_quota_delta *delta)
> +{
> + int ret;
> + struct ulist *ul = fs_info->qgroup_ulist;
> + struct btrfs_qgroup *qgroup;
> + struct ulist_iterator uiter;
> + struct ulist_node *unode;
> + struct btrfs_qgroup *qg;
> + u64 root = delta->root;
> + u64 num_bytes = delta->num_bytes;
> + int sign = delta->is_inc ? 1 : -1;

const int sign = (delta->is_inc ? 1 : -1);

> +
> + if (btrfs_qgroup_mode(fs_info) != BTRFS_QGROUP_MODE_SIMPLE)
> + return 0;
> +
> + if (!is_fstree(root))
> + return 0;
> +
> + spin_lock(&fs_info->qgroup_lock);
> + qgroup = find_qgroup_rb(fs_info, root);
> + if (!qgroup) {
> + ret = -ENOENT;
> + goto out;
> + }
> + ret = qgroup_collect_parents(qgroup, ul);
> + if (ret)
> + goto out;
> +
> + ULIST_ITER_INIT(&uiter);
> + while ((unode = ulist_next(ul, &uiter))) {
> + qg = unode_aux_to_qgroup(unode);
> + qg->excl += num_bytes * sign;
> + qg->rfer += num_bytes * sign;
> + qgroup_dirty(fs_info, qg);
> + }
> +
> +out:
> + spin_unlock(&fs_info->qgroup_lock);
> + if (!ret && delta->rsv_bytes)
> + btrfs_qgroup_free_refroot(fs_info, root, delta->rsv_bytes, BTRFS_QGROUP_RSV_DATA);
> + return ret;
> +}
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index d4c4d039585f..94d85b4fbebd 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -235,6 +235,14 @@ struct btrfs_qgroup {
> struct kobject kobj;
> };
>
> +struct btrfs_simple_quota_delta {

struct btrfs_squota_delta

> + u64 root; /* The fstree root this delta counts against */
> + u64 num_bytes; /* The number of bytes in the extent being counted */
> + u64 rsv_bytes; /* The number of bytes reserved for this extent */
> + bool is_inc; /* Whether we are using or freeing the extent */
> + bool is_data; /* Whether the extent is data or metadata */

Please put the comments on separate lines before the struct member definitions.

> +};
> +
> static inline u64 btrfs_qgroup_subvolid(u64 qgroupid)
> {
> return (qgroupid & ((1ULL << BTRFS_QGROUP_LEVEL_SHIFT) - 1));
> @@ -447,5 +455,6 @@ int btrfs_qgroup_trace_subtree_after_cow(struct btrfs_trans_handle *trans,
> struct btrfs_root *root, struct extent_buffer *eb);
> void btrfs_qgroup_destroy_extent_records(struct btrfs_transaction *trans);
> bool btrfs_check_quota_leak(struct btrfs_fs_info *fs_info);
> -
> +int btrfs_record_simple_quota_delta(struct btrfs_fs_info *fs_info,
> + struct btrfs_simple_quota_delta *delta);

Please keep the newline before the last #endif

> #endif
> --
> 2.41.0