Hi, Qu,
On 2015/09/08 18:08, Qu Wenruo wrote:
> Qgroup reserved space needs to be released from inode dirty map and get
> freed at different timing:
>
> 1) Release when the metadata is written into tree
> After corresponding metadata is written into tree, any newer write will
> be COWed(don't include NOCOW case yet).
> So we must release its range from inode dirty range map, or we will
> forget to reserve needed range, causing accounting exceeding the limit.
>
> 2) Free reserved bytes when delayed ref is run
> When delayed refs are run, qgroup accounting will follow soon and turn
> the reserved bytes into rfer/excl numbers.
> As run_delayed_refs and qgroup accounting are all done at
> commit_transaction() time, we are safe to free reserved space in
> run_delayed_ref time().
>
> With these timing to release/free reserved space, we should be able to
> resolve the long existing qgroup reserve space leak problem.
>
> Signed-off-by: Qu Wenruo <quwenruo@xxxxxxxxxxxxxx>
> ---
> fs/btrfs/extent-tree.c | 4 ++++
> fs/btrfs/inode.c | 10 ++++++++++
> fs/btrfs/qgroup.c | 5 ++---
> fs/btrfs/qgroup.h | 8 +++++++-
> 4 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 5411f0a..65e60eb 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2345,6 +2345,10 @@ static int run_one_delayed_ref(struct btrfs_trans_handle *trans,
> node->num_bytes);
> }
> }
> +
> + /* Also free its reserved qgroup space */
> + btrfs_qgroup_free_refroot(root->fs_info, head->qgroup_ref_root,
> + head->qgroup_reserved);
> return ret;
> }
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 61b2c17..1f7cac0 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2112,6 +2112,16 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,
> ret = btrfs_alloc_reserved_file_extent(trans, root,
> root->root_key.objectid,
> btrfs_ino(inode), file_pos, &ins);
> + if (ret < 0)
> + goto out;
> + /*
> + * Release the reserved range from inode dirty range map, and
> + * move it to delayed ref codes, as now accounting only happens at
> + * commit_transaction() time.
> + */
> + btrfs_qgroup_release_data(inode, file_pos, ram_bytes);
> + ret = btrfs_add_delayed_qgroup_reserve(root->fs_info, trans,
> + root->objectid, disk_bytenr, ram_bytes);
> out:
> btrfs_free_path(path);
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index ba7888f..5a69a2d 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2169,14 +2169,13 @@ out:
> return ret;
> }
>
> -void btrfs_qgroup_free(struct btrfs_root *root, u64 num_bytes)
> +void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
> + u64 ref_root, u64 num_bytes)
> {
> struct btrfs_root *quota_root;
> struct btrfs_qgroup *qgroup;
> - struct btrfs_fs_info *fs_info = root->fs_info;
> struct ulist_node *unode;
> struct ulist_iterator uiter;
> - u64 ref_root = root->root_key.objectid;
> int ret = 0;
>
> if (!is_fstree(ref_root))
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index 8e69dc1..49fa15e 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -75,7 +75,13 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans,
> struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid,
> struct btrfs_qgroup_inherit *inherit);
> int btrfs_qgroup_reserve(struct btrfs_root *root, u64 num_bytes);
> -void btrfs_qgroup_free(struct btrfs_root *root, u64 num_bytes);
> +void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
> + u64 ref_root, u64 num_bytes);
> +static inline void btrfs_qgroup_free(struct btrfs_root *root, u64 num_bytes)
> +{
> + return btrfs_qgroup_free_refroot(root->fs_info, root->objectid,
> + num_bytes);
Is 'return' necessary?
Thanks,
Tsutomu
> +}
>
> void assert_qgroups_uptodate(struct btrfs_trans_handle *trans);
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html