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 <quwen...@cn.fujitsu.com> > --- > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html