| Message ID | 20171024083941.21428-2-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 24.10.2017 11:39, Qu Wenruo wrote: > Instead of single qgroup->reserved, use a new structure btrfs_qgroup_rsv > to restore different types of reservation. > > This patch only updates the header and needed modification to pass > compile. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/qgroup.c | 16 ++++++++++------ > fs/btrfs/qgroup.h | 27 +++++++++++++++++++++++++-- > 2 files changed, 35 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index e172d4843eae..fe3adb996883 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -2444,7 +2444,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce) > } > > void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, > - u64 ref_root, u64 num_bytes) > + u64 ref_root, u64 num_bytes, > + enum btrfs_qgroup_rsv_type type) > { > struct btrfs_root *quota_root; > struct btrfs_qgroup *qgroup; > @@ -2936,7 +2937,8 @@ static int qgroup_free_reserved_data(struct inode *inode, > goto out; > freed += changeset.bytes_changed; > } > - btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed); > + btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed, > + BTRFS_QGROUP_RSV_DATA); > ret = freed; > out: > extent_changeset_release(&changeset); > @@ -2968,7 +2970,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, > if (free) > btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, > BTRFS_I(inode)->root->objectid, > - changeset.bytes_changed); > + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); > ret = changeset.bytes_changed; > out: > extent_changeset_release(&changeset); > @@ -3045,7 +3047,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root) > if (reserved == 0) > return; > trace_qgroup_meta_reserve(root, -(s64)reserved); > - btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved); > + btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved, > + BTRFS_QGROUP_RSV_META); > } > > void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) > @@ -3060,7 +3063,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) > WARN_ON(atomic64_read(&root->qgroup_meta_rsv) < num_bytes); > atomic64_sub(num_bytes, &root->qgroup_meta_rsv); > trace_qgroup_meta_reserve(root, -(s64)num_bytes); > - btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes); > + btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes, > + BTRFS_QGROUP_RSV_META); > } > > /* > @@ -3088,7 +3092,7 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode) > } > btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, > BTRFS_I(inode)->root->objectid, > - changeset.bytes_changed); > + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); > > } > extent_changeset_release(&changeset); > diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h > index d9984e87cddf..0b04cbc5b5ce 100644 > --- a/fs/btrfs/qgroup.h > +++ b/fs/btrfs/qgroup.h > @@ -61,6 +61,26 @@ struct btrfs_qgroup_extent_record { > struct ulist *old_roots; > }; > > +enum btrfs_qgroup_rsv_type { > + BTRFS_QGROUP_RSV_DATA = 0, > + BTRFS_QGROUP_RSV_META = 1, > + BTRFS_QGROUP_RSV_TYPES = 1, nit: Why not BTRFS_QGROUP_RSV_TYPES_MAX = 2; > +}; > + > +/* > + * Represents how many bytes we reserved for this qgroup. > + * > + * Each type should have different reservation behavior. > + * E.g, data follows its io_tree flag modification, while > + * *currently* meta is just reserve-and-clear during transcation. > + * > + * TODO: Add new type for delalloc, which can exist across several > + * transaction. > + */ > +struct btrfs_qgroup_rsv { > + u64 values[BTRFS_QGROUP_RSV_TYPES + 1]; nit: And here just BTRFS_QGROUP_RSV_TYPES_MAX rather than the +1 here, seems more idiomatic to me. > +}; > + > /* > * one struct for each qgroup, organized in fs_info->qgroup_tree. > */ > @@ -88,6 +108,7 @@ struct btrfs_qgroup { > * reservation tracking > */ > u64 reserved; > + struct btrfs_qgroup_rsv rsv; > > /* > * lists > @@ -228,12 +249,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, > struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid, > struct btrfs_qgroup_inherit *inherit); > void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, > - u64 ref_root, u64 num_bytes); > + u64 ref_root, u64 num_bytes, > + enum btrfs_qgroup_rsv_type type); > static inline void btrfs_qgroup_free_delayed_ref(struct btrfs_fs_info *fs_info, > u64 ref_root, u64 num_bytes) > { > trace_btrfs_qgroup_free_delayed_ref(fs_info, ref_root, num_bytes); > - btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes); > + btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes, > + BTRFS_QGROUP_RSV_DATA); > } > > #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017年10月24日 19:00, Nikolay Borisov wrote: > > > On 24.10.2017 11:39, Qu Wenruo wrote: >> Instead of single qgroup->reserved, use a new structure btrfs_qgroup_rsv >> to restore different types of reservation. >> >> This patch only updates the header and needed modification to pass >> compile. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/qgroup.c | 16 ++++++++++------ >> fs/btrfs/qgroup.h | 27 +++++++++++++++++++++++++-- >> 2 files changed, 35 insertions(+), 8 deletions(-) >> >> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c >> index e172d4843eae..fe3adb996883 100644 >> --- a/fs/btrfs/qgroup.c >> +++ b/fs/btrfs/qgroup.c >> @@ -2444,7 +2444,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce) >> } >> >> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >> - u64 ref_root, u64 num_bytes) >> + u64 ref_root, u64 num_bytes, >> + enum btrfs_qgroup_rsv_type type) >> { >> struct btrfs_root *quota_root; >> struct btrfs_qgroup *qgroup; >> @@ -2936,7 +2937,8 @@ static int qgroup_free_reserved_data(struct inode *inode, >> goto out; >> freed += changeset.bytes_changed; >> } >> - btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed); >> + btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed, >> + BTRFS_QGROUP_RSV_DATA); >> ret = freed; >> out: >> extent_changeset_release(&changeset); >> @@ -2968,7 +2970,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, >> if (free) >> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >> BTRFS_I(inode)->root->objectid, >> - changeset.bytes_changed); >> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >> ret = changeset.bytes_changed; >> out: >> extent_changeset_release(&changeset); >> @@ -3045,7 +3047,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root) >> if (reserved == 0) >> return; >> trace_qgroup_meta_reserve(root, -(s64)reserved); >> - btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved); >> + btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved, >> + BTRFS_QGROUP_RSV_META); >> } >> >> void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >> @@ -3060,7 +3063,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >> WARN_ON(atomic64_read(&root->qgroup_meta_rsv) < num_bytes); >> atomic64_sub(num_bytes, &root->qgroup_meta_rsv); >> trace_qgroup_meta_reserve(root, -(s64)num_bytes); >> - btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes); >> + btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes, >> + BTRFS_QGROUP_RSV_META); >> } >> >> /* >> @@ -3088,7 +3092,7 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode) >> } >> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >> BTRFS_I(inode)->root->objectid, >> - changeset.bytes_changed); >> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >> >> } >> extent_changeset_release(&changeset); >> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h >> index d9984e87cddf..0b04cbc5b5ce 100644 >> --- a/fs/btrfs/qgroup.h >> +++ b/fs/btrfs/qgroup.h >> @@ -61,6 +61,26 @@ struct btrfs_qgroup_extent_record { >> struct ulist *old_roots; >> }; >> >> +enum btrfs_qgroup_rsv_type { >> + BTRFS_QGROUP_RSV_DATA = 0, >> + BTRFS_QGROUP_RSV_META = 1, >> + BTRFS_QGROUP_RSV_TYPES = 1, > > nit: Why not BTRFS_QGROUP_RSV_TYPES_MAX = 2; My original plan is just as the same as yours. However I still remember I did it before and David fixed it by using TYPES, so I follow his naming schema here. Kernel is also using this naming schema else where: d91876496bcf ("btrfs: compress: put variables defined per compress type in struct to make cache friendly") > >> +}; >> + >> +/* >> + * Represents how many bytes we reserved for this qgroup. >> + * >> + * Each type should have different reservation behavior. >> + * E.g, data follows its io_tree flag modification, while >> + * *currently* meta is just reserve-and-clear during transcation. >> + * >> + * TODO: Add new type for delalloc, which can exist across several >> + * transaction. >> + */ >> +struct btrfs_qgroup_rsv { >> + u64 values[BTRFS_QGROUP_RSV_TYPES + 1]; > > nit: And here just BTRFS_QGROUP_RSV_TYPES_MAX rather than the +1 here, > seems more idiomatic to me. To follow same naming schema from David. (IIRC it was about tree-checker patchset, checking file extent type part) In fact, I crashed kernel several times due to the tiny +1, without even a clue for hours just testing blindly, until latest gcc gives warning about it. Thanks, Qu > >> +}; >> + >> /* >> * one struct for each qgroup, organized in fs_info->qgroup_tree. >> */ >> @@ -88,6 +108,7 @@ struct btrfs_qgroup { >> * reservation tracking >> */ >> u64 reserved; >> + struct btrfs_qgroup_rsv rsv; >> >> /* >> * lists >> @@ -228,12 +249,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, >> struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid, >> struct btrfs_qgroup_inherit *inherit); >> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >> - u64 ref_root, u64 num_bytes); >> + u64 ref_root, u64 num_bytes, >> + enum btrfs_qgroup_rsv_type type); >> static inline void btrfs_qgroup_free_delayed_ref(struct btrfs_fs_info *fs_info, >> u64 ref_root, u64 num_bytes) >> { >> trace_btrfs_qgroup_free_delayed_ref(fs_info, ref_root, num_bytes); >> - btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes); >> + btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes, >> + BTRFS_QGROUP_RSV_DATA); >> } >> >> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 24.10.2017 14:51, Qu Wenruo wrote: > > > On 2017年10月24日 19:00, Nikolay Borisov wrote: >> >> >> On 24.10.2017 11:39, Qu Wenruo wrote: >>> Instead of single qgroup->reserved, use a new structure btrfs_qgroup_rsv >>> to restore different types of reservation. >>> >>> This patch only updates the header and needed modification to pass >>> compile. >>> >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>> --- >>> fs/btrfs/qgroup.c | 16 ++++++++++------ >>> fs/btrfs/qgroup.h | 27 +++++++++++++++++++++++++-- >>> 2 files changed, 35 insertions(+), 8 deletions(-) >>> >>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c >>> index e172d4843eae..fe3adb996883 100644 >>> --- a/fs/btrfs/qgroup.c >>> +++ b/fs/btrfs/qgroup.c >>> @@ -2444,7 +2444,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce) >>> } >>> >>> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >>> - u64 ref_root, u64 num_bytes) >>> + u64 ref_root, u64 num_bytes, >>> + enum btrfs_qgroup_rsv_type type) >>> { >>> struct btrfs_root *quota_root; >>> struct btrfs_qgroup *qgroup; >>> @@ -2936,7 +2937,8 @@ static int qgroup_free_reserved_data(struct inode *inode, >>> goto out; >>> freed += changeset.bytes_changed; >>> } >>> - btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed); >>> + btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed, >>> + BTRFS_QGROUP_RSV_DATA); >>> ret = freed; >>> out: >>> extent_changeset_release(&changeset); >>> @@ -2968,7 +2970,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, >>> if (free) >>> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >>> BTRFS_I(inode)->root->objectid, >>> - changeset.bytes_changed); >>> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >>> ret = changeset.bytes_changed; >>> out: >>> extent_changeset_release(&changeset); >>> @@ -3045,7 +3047,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root) >>> if (reserved == 0) >>> return; >>> trace_qgroup_meta_reserve(root, -(s64)reserved); >>> - btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved); >>> + btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved, >>> + BTRFS_QGROUP_RSV_META); >>> } >>> >>> void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >>> @@ -3060,7 +3063,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >>> WARN_ON(atomic64_read(&root->qgroup_meta_rsv) < num_bytes); >>> atomic64_sub(num_bytes, &root->qgroup_meta_rsv); >>> trace_qgroup_meta_reserve(root, -(s64)num_bytes); >>> - btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes); >>> + btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes, >>> + BTRFS_QGROUP_RSV_META); >>> } >>> >>> /* >>> @@ -3088,7 +3092,7 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode) >>> } >>> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >>> BTRFS_I(inode)->root->objectid, >>> - changeset.bytes_changed); >>> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >>> >>> } >>> extent_changeset_release(&changeset); >>> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h >>> index d9984e87cddf..0b04cbc5b5ce 100644 >>> --- a/fs/btrfs/qgroup.h >>> +++ b/fs/btrfs/qgroup.h >>> @@ -61,6 +61,26 @@ struct btrfs_qgroup_extent_record { >>> struct ulist *old_roots; >>> }; >>> >>> +enum btrfs_qgroup_rsv_type { >>> + BTRFS_QGROUP_RSV_DATA = 0, >>> + BTRFS_QGROUP_RSV_META = 1, >>> + BTRFS_QGROUP_RSV_TYPES = 1, >> >> nit: Why not BTRFS_QGROUP_RSV_TYPES_MAX = 2; > > My original plan is just as the same as yours. > > However I still remember I did it before and David fixed it by using > TYPES, so I follow his naming schema here. I don't really care about whether it's called TYPES or _MAX. The more important thing is the fact that we have types set to the same value as the last valid value and thus when writing loops or checking ranges we need to use <= and + 1 respectively which IMO is a bit counter-intuitive. But as I said it's a nit so I don't have strong preference either way if this is the convention of btrfs so be it. > > Kernel is also using this naming schema else where: > d91876496bcf ("btrfs: compress: put variables defined per compress type > in struct to make cache friendly") > >> >>> +}; >>> + >>> +/* >>> + * Represents how many bytes we reserved for this qgroup. >>> + * >>> + * Each type should have different reservation behavior. >>> + * E.g, data follows its io_tree flag modification, while >>> + * *currently* meta is just reserve-and-clear during transcation. >>> + * >>> + * TODO: Add new type for delalloc, which can exist across several >>> + * transaction. >>> + */ >>> +struct btrfs_qgroup_rsv { >>> + u64 values[BTRFS_QGROUP_RSV_TYPES + 1]; >> >> nit: And here just BTRFS_QGROUP_RSV_TYPES_MAX rather than the +1 here, >> seems more idiomatic to me. > > To follow same naming schema from David. > (IIRC it was about tree-checker patchset, checking file extent type part) > > In fact, I crashed kernel several times due to the tiny +1, without even > a clue for hours just testing blindly, until latest gcc gives warning > about it. So you crashed the kernel because you were missing it or not ? > > Thanks, > Qu > >> >>> +}; >>> + >>> /* >>> * one struct for each qgroup, organized in fs_info->qgroup_tree. >>> */ >>> @@ -88,6 +108,7 @@ struct btrfs_qgroup { >>> * reservation tracking >>> */ >>> u64 reserved; >>> + struct btrfs_qgroup_rsv rsv; >>> >>> /* >>> * lists >>> @@ -228,12 +249,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, >>> struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid, >>> struct btrfs_qgroup_inherit *inherit); >>> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >>> - u64 ref_root, u64 num_bytes); >>> + u64 ref_root, u64 num_bytes, >>> + enum btrfs_qgroup_rsv_type type); >>> static inline void btrfs_qgroup_free_delayed_ref(struct btrfs_fs_info *fs_info, >>> u64 ref_root, u64 num_bytes) >>> { >>> trace_btrfs_qgroup_free_delayed_ref(fs_info, ref_root, num_bytes); >>> - btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes); >>> + btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes, >>> + BTRFS_QGROUP_RSV_DATA); >>> } >>> >>> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/24/17 7:51 AM, Qu Wenruo wrote: > > > On 2017年10月24日 19:00, Nikolay Borisov wrote: >> >> >> On 24.10.2017 11:39, Qu Wenruo wrote: >>> Instead of single qgroup->reserved, use a new structure btrfs_qgroup_rsv >>> to restore different types of reservation. >>> >>> This patch only updates the header and needed modification to pass >>> compile. >>> >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>> --- >>> fs/btrfs/qgroup.c | 16 ++++++++++------ >>> fs/btrfs/qgroup.h | 27 +++++++++++++++++++++++++-- >>> 2 files changed, 35 insertions(+), 8 deletions(-) >>> >>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c >>> index e172d4843eae..fe3adb996883 100644 >>> --- a/fs/btrfs/qgroup.c >>> +++ b/fs/btrfs/qgroup.c >>> @@ -2444,7 +2444,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce) >>> } >>> >>> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >>> - u64 ref_root, u64 num_bytes) >>> + u64 ref_root, u64 num_bytes, >>> + enum btrfs_qgroup_rsv_type type) >>> { >>> struct btrfs_root *quota_root; >>> struct btrfs_qgroup *qgroup; >>> @@ -2936,7 +2937,8 @@ static int qgroup_free_reserved_data(struct inode *inode, >>> goto out; >>> freed += changeset.bytes_changed; >>> } >>> - btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed); >>> + btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed, >>> + BTRFS_QGROUP_RSV_DATA); >>> ret = freed; >>> out: >>> extent_changeset_release(&changeset); >>> @@ -2968,7 +2970,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, >>> if (free) >>> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >>> BTRFS_I(inode)->root->objectid, >>> - changeset.bytes_changed); >>> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >>> ret = changeset.bytes_changed; >>> out: >>> extent_changeset_release(&changeset); >>> @@ -3045,7 +3047,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root) >>> if (reserved == 0) >>> return; >>> trace_qgroup_meta_reserve(root, -(s64)reserved); >>> - btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved); >>> + btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved, >>> + BTRFS_QGROUP_RSV_META); >>> } >>> >>> void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >>> @@ -3060,7 +3063,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >>> WARN_ON(atomic64_read(&root->qgroup_meta_rsv) < num_bytes); >>> atomic64_sub(num_bytes, &root->qgroup_meta_rsv); >>> trace_qgroup_meta_reserve(root, -(s64)num_bytes); >>> - btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes); >>> + btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes, >>> + BTRFS_QGROUP_RSV_META); >>> } >>> >>> /* >>> @@ -3088,7 +3092,7 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode) >>> } >>> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >>> BTRFS_I(inode)->root->objectid, >>> - changeset.bytes_changed); >>> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >>> >>> } >>> extent_changeset_release(&changeset); >>> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h >>> index d9984e87cddf..0b04cbc5b5ce 100644 >>> --- a/fs/btrfs/qgroup.h >>> +++ b/fs/btrfs/qgroup.h >>> @@ -61,6 +61,26 @@ struct btrfs_qgroup_extent_record { >>> struct ulist *old_roots; >>> }; >>> >>> +enum btrfs_qgroup_rsv_type { >>> + BTRFS_QGROUP_RSV_DATA = 0, >>> + BTRFS_QGROUP_RSV_META = 1, >>> + BTRFS_QGROUP_RSV_TYPES = 1, >> >> nit: Why not BTRFS_QGROUP_RSV_TYPES_MAX = 2; > > My original plan is just as the same as yours. > > However I still remember I did it before and David fixed it by using > TYPES, so I follow his naming schema here. > > Kernel is also using this naming schema else where: > d91876496bcf ("btrfs: compress: put variables defined per compress type > in struct to make cache friendly") The COMPRESS_TYPES pattern isn't the right pattern to follow here. That's a special case since there's a _NONE that doesn't have anything associated with it, so we don't need to take a slot in the array. We also don't care about any of the specific values, just that they start at 0. The BTRFS_COMPRESS_TYPES example also has a BTRFS_COMPRESS_LAST item in the enum, which serves the same purpose as MAX. I don't have a strong opinion on the naming, just that we don't play games with +1 when handling arrays since, as you say, that's just waiting for subtle bugs later. enum btrfs_qgroup_rsv_type { BTRFS_QGROUP_RSV_DATA = 0, BTRFS_QGROUP_RSV_META, BTRFS_QGROUP_RSV_LAST, }; >>> +}; >>> + >>> +/* >>> + * Represents how many bytes we reserved for this qgroup. >>> + * >>> + * Each type should have different reservation behavior. >>> + * E.g, data follows its io_tree flag modification, while >>> + * *currently* meta is just reserve-and-clear during transcation. >>> + * >>> + * TODO: Add new type for delalloc, which can exist across several >>> + * transaction. >>> + */ Minor nit: It's not just delalloc. Delayed items and inodes can as well. The general rule is that qgroup reservations aren't essentially different from block reservations and follow the same usage patterns when operating on leaf nodes. >>> +struct btrfs_qgroup_rsv { >>> + u64 values[BTRFS_QGROUP_RSV_TYPES + 1]; >> >> nit: And here just BTRFS_QGROUP_RSV_TYPES_MAX rather than the +1 here, >> seems more idiomatic to me. > > To follow same naming schema from David. > (IIRC it was about tree-checker patchset, checking file extent type part) > > In fact, I crashed kernel several times due to the tiny +1, without even > a clue for hours just testing blindly, until latest gcc gives warning > about it. BTRFS_QGROUP_RSV_LAST would do the job here. -Jeff >> >>> +}; >>> + >>> /* >>> * one struct for each qgroup, organized in fs_info->qgroup_tree. >>> */ >>> @@ -88,6 +108,7 @@ struct btrfs_qgroup { >>> * reservation tracking >>> */ >>> u64 reserved; >>> + struct btrfs_qgroup_rsv rsv; >>> >>> /* >>> * lists >>> @@ -228,12 +249,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, >>> struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid, >>> struct btrfs_qgroup_inherit *inherit); >>> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >>> - u64 ref_root, u64 num_bytes); >>> + u64 ref_root, u64 num_bytes, >>> + enum btrfs_qgroup_rsv_type type); >>> static inline void btrfs_qgroup_free_delayed_ref(struct btrfs_fs_info *fs_info, >>> u64 ref_root, u64 num_bytes) >>> { >>> trace_btrfs_qgroup_free_delayed_ref(fs_info, ref_root, num_bytes); >>> - btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes); >>> + btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes, >>> + BTRFS_QGROUP_RSV_DATA); >>> } >>> >>> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >
On 2017年10月24日 20:23, Nikolay Borisov wrote: > > > On 24.10.2017 14:51, Qu Wenruo wrote: >> >> >> On 2017年10月24日 19:00, Nikolay Borisov wrote: >>> >>> >>> On 24.10.2017 11:39, Qu Wenruo wrote: >>>> Instead of single qgroup->reserved, use a new structure btrfs_qgroup_rsv >>>> to restore different types of reservation. >>>> >>>> This patch only updates the header and needed modification to pass >>>> compile. >>>> >>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>> --- >>>> fs/btrfs/qgroup.c | 16 ++++++++++------ >>>> fs/btrfs/qgroup.h | 27 +++++++++++++++++++++++++-- >>>> 2 files changed, 35 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c >>>> index e172d4843eae..fe3adb996883 100644 >>>> --- a/fs/btrfs/qgroup.c >>>> +++ b/fs/btrfs/qgroup.c >>>> @@ -2444,7 +2444,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce) >>>> } >>>> >>>> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >>>> - u64 ref_root, u64 num_bytes) >>>> + u64 ref_root, u64 num_bytes, >>>> + enum btrfs_qgroup_rsv_type type) >>>> { >>>> struct btrfs_root *quota_root; >>>> struct btrfs_qgroup *qgroup; >>>> @@ -2936,7 +2937,8 @@ static int qgroup_free_reserved_data(struct inode *inode, >>>> goto out; >>>> freed += changeset.bytes_changed; >>>> } >>>> - btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed); >>>> + btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed, >>>> + BTRFS_QGROUP_RSV_DATA); >>>> ret = freed; >>>> out: >>>> extent_changeset_release(&changeset); >>>> @@ -2968,7 +2970,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, >>>> if (free) >>>> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >>>> BTRFS_I(inode)->root->objectid, >>>> - changeset.bytes_changed); >>>> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >>>> ret = changeset.bytes_changed; >>>> out: >>>> extent_changeset_release(&changeset); >>>> @@ -3045,7 +3047,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root) >>>> if (reserved == 0) >>>> return; >>>> trace_qgroup_meta_reserve(root, -(s64)reserved); >>>> - btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved); >>>> + btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved, >>>> + BTRFS_QGROUP_RSV_META); >>>> } >>>> >>>> void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >>>> @@ -3060,7 +3063,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >>>> WARN_ON(atomic64_read(&root->qgroup_meta_rsv) < num_bytes); >>>> atomic64_sub(num_bytes, &root->qgroup_meta_rsv); >>>> trace_qgroup_meta_reserve(root, -(s64)num_bytes); >>>> - btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes); >>>> + btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes, >>>> + BTRFS_QGROUP_RSV_META); >>>> } >>>> >>>> /* >>>> @@ -3088,7 +3092,7 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode) >>>> } >>>> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >>>> BTRFS_I(inode)->root->objectid, >>>> - changeset.bytes_changed); >>>> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >>>> >>>> } >>>> extent_changeset_release(&changeset); >>>> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h >>>> index d9984e87cddf..0b04cbc5b5ce 100644 >>>> --- a/fs/btrfs/qgroup.h >>>> +++ b/fs/btrfs/qgroup.h >>>> @@ -61,6 +61,26 @@ struct btrfs_qgroup_extent_record { >>>> struct ulist *old_roots; >>>> }; >>>> >>>> +enum btrfs_qgroup_rsv_type { >>>> + BTRFS_QGROUP_RSV_DATA = 0, >>>> + BTRFS_QGROUP_RSV_META = 1, >>>> + BTRFS_QGROUP_RSV_TYPES = 1, >>> >>> nit: Why not BTRFS_QGROUP_RSV_TYPES_MAX = 2; >> >> My original plan is just as the same as yours. >> >> However I still remember I did it before and David fixed it by using >> TYPES, so I follow his naming schema here. > > I don't really care about whether it's called TYPES or _MAX. The more > important thing is the fact that we have types set to the same value as > the last valid value and thus when writing loops or checking ranges we > need to use <= and + 1 respectively which IMO is a bit > counter-intuitive. But as I said it's a nit so I don't have strong > preference either way if this is the convention of btrfs so be it. >> >> Kernel is also using this naming schema else where: >> d91876496bcf ("btrfs: compress: put variables defined per compress type >> in struct to make cache friendly") >> >>> >>>> +}; >>>> + >>>> +/* >>>> + * Represents how many bytes we reserved for this qgroup. >>>> + * >>>> + * Each type should have different reservation behavior. >>>> + * E.g, data follows its io_tree flag modification, while >>>> + * *currently* meta is just reserve-and-clear during transcation. >>>> + * >>>> + * TODO: Add new type for delalloc, which can exist across several >>>> + * transaction. >>>> + */ >>>> +struct btrfs_qgroup_rsv { >>>> + u64 values[BTRFS_QGROUP_RSV_TYPES + 1]; >>> >>> nit: And here just BTRFS_QGROUP_RSV_TYPES_MAX rather than the +1 here, >>> seems more idiomatic to me. >> >> To follow same naming schema from David. >> (IIRC it was about tree-checker patchset, checking file extent type part) >> >> In fact, I crashed kernel several times due to the tiny +1, without even >> a clue for hours just testing blindly, until latest gcc gives warning >> about it. > > So you crashed the kernel because you were missing it or not ? Crashed it just because missing the +1. So I'd better go the _LAST solution. Thanks, Qu > >> >> Thanks, >> Qu >> >>> >>>> +}; >>>> + >>>> /* >>>> * one struct for each qgroup, organized in fs_info->qgroup_tree. >>>> */ >>>> @@ -88,6 +108,7 @@ struct btrfs_qgroup { >>>> * reservation tracking >>>> */ >>>> u64 reserved; >>>> + struct btrfs_qgroup_rsv rsv; >>>> >>>> /* >>>> * lists >>>> @@ -228,12 +249,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, >>>> struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid, >>>> struct btrfs_qgroup_inherit *inherit); >>>> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >>>> - u64 ref_root, u64 num_bytes); >>>> + u64 ref_root, u64 num_bytes, >>>> + enum btrfs_qgroup_rsv_type type); >>>> static inline void btrfs_qgroup_free_delayed_ref(struct btrfs_fs_info *fs_info, >>>> u64 ref_root, u64 num_bytes) >>>> { >>>> trace_btrfs_qgroup_free_delayed_ref(fs_info, ref_root, num_bytes); >>>> - btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes); >>>> + btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes, >>>> + BTRFS_QGROUP_RSV_DATA); >>>> } >>>> >>>> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS >>>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>
On 10/24/17 8:29 AM, Jeff Mahoney wrote: > On 10/24/17 7:51 AM, Qu Wenruo wrote: >> >> >> On 2017年10月24日 19:00, Nikolay Borisov wrote: >>> nit: Why not BTRFS_QGROUP_RSV_TYPES_MAX = 2; >> >> My original plan is just as the same as yours. >> >> However I still remember I did it before and David fixed it by using >> TYPES, so I follow his naming schema here. >> >> Kernel is also using this naming schema else where: >> d91876496bcf ("btrfs: compress: put variables defined per compress type >> in struct to make cache friendly") > > The COMPRESS_TYPES pattern isn't the right pattern to follow here. > That's a special case since there's a _NONE that doesn't have anything > associated with it, so we don't need to take a slot in the array. > > We also don't care about any of the specific values, just that they > start at 0. The BTRFS_COMPRESS_TYPES example also has a > BTRFS_COMPRESS_LAST item in the enum, which serves the same purpose as > MAX. I don't have a strong opinion on the naming, just that we don't > play games with +1 when handling arrays since, as you say, that's just > waiting for subtle bugs later. > > enum btrfs_qgroup_rsv_type { > BTRFS_QGROUP_RSV_DATA = 0, > BTRFS_QGROUP_RSV_META, > BTRFS_QGROUP_RSV_LAST, > }; It turns out that BTRFS_COMPRESS_LAST did exist until very recently: commit dc2f29212a2648164b054016dc5b948bf0fc92d5 Author: Anand Jain <anand.jain@oracle.com> Date: Sun Aug 13 12:02:41 2017 +0800 btrfs: remove unused BTRFS_COMPRESS_LAST ... so it was there, but just wasn't used. -Jeff
On 2017年10月24日 20:29, Jeff Mahoney wrote: > On 10/24/17 7:51 AM, Qu Wenruo wrote: >> >> >> On 2017年10月24日 19:00, Nikolay Borisov wrote: >>> >>> >>> On 24.10.2017 11:39, Qu Wenruo wrote: >>>> Instead of single qgroup->reserved, use a new structure btrfs_qgroup_rsv >>>> to restore different types of reservation. >>>> >>>> This patch only updates the header and needed modification to pass >>>> compile. >>>> >>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>> --- >>>> fs/btrfs/qgroup.c | 16 ++++++++++------ >>>> fs/btrfs/qgroup.h | 27 +++++++++++++++++++++++++-- >>>> 2 files changed, 35 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c >>>> index e172d4843eae..fe3adb996883 100644 >>>> --- a/fs/btrfs/qgroup.c >>>> +++ b/fs/btrfs/qgroup.c >>>> @@ -2444,7 +2444,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce) >>>> } >>>> >>>> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >>>> - u64 ref_root, u64 num_bytes) >>>> + u64 ref_root, u64 num_bytes, >>>> + enum btrfs_qgroup_rsv_type type) >>>> { >>>> struct btrfs_root *quota_root; >>>> struct btrfs_qgroup *qgroup; >>>> @@ -2936,7 +2937,8 @@ static int qgroup_free_reserved_data(struct inode *inode, >>>> goto out; >>>> freed += changeset.bytes_changed; >>>> } >>>> - btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed); >>>> + btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed, >>>> + BTRFS_QGROUP_RSV_DATA); >>>> ret = freed; >>>> out: >>>> extent_changeset_release(&changeset); >>>> @@ -2968,7 +2970,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, >>>> if (free) >>>> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >>>> BTRFS_I(inode)->root->objectid, >>>> - changeset.bytes_changed); >>>> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >>>> ret = changeset.bytes_changed; >>>> out: >>>> extent_changeset_release(&changeset); >>>> @@ -3045,7 +3047,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root) >>>> if (reserved == 0) >>>> return; >>>> trace_qgroup_meta_reserve(root, -(s64)reserved); >>>> - btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved); >>>> + btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved, >>>> + BTRFS_QGROUP_RSV_META); >>>> } >>>> >>>> void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >>>> @@ -3060,7 +3063,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) >>>> WARN_ON(atomic64_read(&root->qgroup_meta_rsv) < num_bytes); >>>> atomic64_sub(num_bytes, &root->qgroup_meta_rsv); >>>> trace_qgroup_meta_reserve(root, -(s64)num_bytes); >>>> - btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes); >>>> + btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes, >>>> + BTRFS_QGROUP_RSV_META); >>>> } >>>> >>>> /* >>>> @@ -3088,7 +3092,7 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode) >>>> } >>>> btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, >>>> BTRFS_I(inode)->root->objectid, >>>> - changeset.bytes_changed); >>>> + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); >>>> >>>> } >>>> extent_changeset_release(&changeset); >>>> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h >>>> index d9984e87cddf..0b04cbc5b5ce 100644 >>>> --- a/fs/btrfs/qgroup.h >>>> +++ b/fs/btrfs/qgroup.h >>>> @@ -61,6 +61,26 @@ struct btrfs_qgroup_extent_record { >>>> struct ulist *old_roots; >>>> }; >>>> >>>> +enum btrfs_qgroup_rsv_type { >>>> + BTRFS_QGROUP_RSV_DATA = 0, >>>> + BTRFS_QGROUP_RSV_META = 1, >>>> + BTRFS_QGROUP_RSV_TYPES = 1, >>> >>> nit: Why not BTRFS_QGROUP_RSV_TYPES_MAX = 2; >> >> My original plan is just as the same as yours. >> >> However I still remember I did it before and David fixed it by using >> TYPES, so I follow his naming schema here. >> >> Kernel is also using this naming schema else where: >> d91876496bcf ("btrfs: compress: put variables defined per compress type >> in struct to make cache friendly") > > The COMPRESS_TYPES pattern isn't the right pattern to follow here. > That's a special case since there's a _NONE that doesn't have anything > associated with it, so we don't need to take a slot in the array. > > We also don't care about any of the specific values, just that they > start at 0. The BTRFS_COMPRESS_TYPES example also has a > BTRFS_COMPRESS_LAST item in the enum, which serves the same purpose as > MAX. At least in latest mainline kernel, there is no COMPRESS_LAST now. --- enum btrfs_compression_type { BTRFS_COMPRESS_NONE = 0, BTRFS_COMPRESS_ZLIB = 1, BTRFS_COMPRESS_LZO = 2, BTRFS_COMPRESS_ZSTD = 3, BTRFS_COMPRESS_TYPES = 3, }; --- > I don't have a strong opinion on the naming, just that we don't > play games with +1 when handling arrays since, as you say, that's just > waiting for subtle bugs later. Not waiting, already experienced it during coding. > > enum btrfs_qgroup_rsv_type { > BTRFS_QGROUP_RSV_DATA = 0, > BTRFS_QGROUP_RSV_META, > BTRFS_QGROUP_RSV_LAST, > }; > >>>> +}; >>>> + >>>> +/* >>>> + * Represents how many bytes we reserved for this qgroup. >>>> + * >>>> + * Each type should have different reservation behavior. >>>> + * E.g, data follows its io_tree flag modification, while >>>> + * *currently* meta is just reserve-and-clear during transcation. >>>> + * >>>> + * TODO: Add new type for delalloc, which can exist across several >>>> + * transaction. >>>> + */ > > Minor nit: It's not just delalloc. Delayed items and inodes can as > well. The general rule is that qgroup reservations aren't essentially > different from block reservations and follow the same usage patterns > when operating on leaf nodes. Yep, any reserveration who can survive transaction commit should go here with a new reservation type. I'll change the TODO description here. > >>>> +struct btrfs_qgroup_rsv { >>>> + u64 values[BTRFS_QGROUP_RSV_TYPES + 1]; >>> >>> nit: And here just BTRFS_QGROUP_RSV_TYPES_MAX rather than the +1 here, >>> seems more idiomatic to me. >> >> To follow same naming schema from David. >> (IIRC it was about tree-checker patchset, checking file extent type part) >> >> In fact, I crashed kernel several times due to the tiny +1, without even >> a clue for hours just testing blindly, until latest gcc gives warning >> about it. > > BTRFS_QGROUP_RSV_LAST would do the job here. I'll go with this method. Thanks, Qu > > -Jeff > >>> >>>> +}; >>>> + >>>> /* >>>> * one struct for each qgroup, organized in fs_info->qgroup_tree. >>>> */ >>>> @@ -88,6 +108,7 @@ struct btrfs_qgroup { >>>> * reservation tracking >>>> */ >>>> u64 reserved; >>>> + struct btrfs_qgroup_rsv rsv; >>>> >>>> /* >>>> * lists >>>> @@ -228,12 +249,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, >>>> struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid, >>>> struct btrfs_qgroup_inherit *inherit); >>>> void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, >>>> - u64 ref_root, u64 num_bytes); >>>> + u64 ref_root, u64 num_bytes, >>>> + enum btrfs_qgroup_rsv_type type); >>>> static inline void btrfs_qgroup_free_delayed_ref(struct btrfs_fs_info *fs_info, >>>> u64 ref_root, u64 num_bytes) >>>> { >>>> trace_btrfs_qgroup_free_delayed_ref(fs_info, ref_root, num_bytes); >>>> - btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes); >>>> + btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes, >>>> + BTRFS_QGROUP_RSV_DATA); >>>> } >>>> >>>> #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS >>>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> > >
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index e172d4843eae..fe3adb996883 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -2444,7 +2444,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce) } void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, - u64 ref_root, u64 num_bytes) + u64 ref_root, u64 num_bytes, + enum btrfs_qgroup_rsv_type type) { struct btrfs_root *quota_root; struct btrfs_qgroup *qgroup; @@ -2936,7 +2937,8 @@ static int qgroup_free_reserved_data(struct inode *inode, goto out; freed += changeset.bytes_changed; } - btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed); + btrfs_qgroup_free_refroot(root->fs_info, root->objectid, freed, + BTRFS_QGROUP_RSV_DATA); ret = freed; out: extent_changeset_release(&changeset); @@ -2968,7 +2970,7 @@ static int __btrfs_qgroup_release_data(struct inode *inode, if (free) btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, BTRFS_I(inode)->root->objectid, - changeset.bytes_changed); + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); ret = changeset.bytes_changed; out: extent_changeset_release(&changeset); @@ -3045,7 +3047,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root) if (reserved == 0) return; trace_qgroup_meta_reserve(root, -(s64)reserved); - btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved); + btrfs_qgroup_free_refroot(fs_info, root->objectid, reserved, + BTRFS_QGROUP_RSV_META); } void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) @@ -3060,7 +3063,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes) WARN_ON(atomic64_read(&root->qgroup_meta_rsv) < num_bytes); atomic64_sub(num_bytes, &root->qgroup_meta_rsv); trace_qgroup_meta_reserve(root, -(s64)num_bytes); - btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes); + btrfs_qgroup_free_refroot(fs_info, root->objectid, num_bytes, + BTRFS_QGROUP_RSV_META); } /* @@ -3088,7 +3092,7 @@ void btrfs_qgroup_check_reserved_leak(struct inode *inode) } btrfs_qgroup_free_refroot(BTRFS_I(inode)->root->fs_info, BTRFS_I(inode)->root->objectid, - changeset.bytes_changed); + changeset.bytes_changed, BTRFS_QGROUP_RSV_DATA); } extent_changeset_release(&changeset); diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h index d9984e87cddf..0b04cbc5b5ce 100644 --- a/fs/btrfs/qgroup.h +++ b/fs/btrfs/qgroup.h @@ -61,6 +61,26 @@ struct btrfs_qgroup_extent_record { struct ulist *old_roots; }; +enum btrfs_qgroup_rsv_type { + BTRFS_QGROUP_RSV_DATA = 0, + BTRFS_QGROUP_RSV_META = 1, + BTRFS_QGROUP_RSV_TYPES = 1, +}; + +/* + * Represents how many bytes we reserved for this qgroup. + * + * Each type should have different reservation behavior. + * E.g, data follows its io_tree flag modification, while + * *currently* meta is just reserve-and-clear during transcation. + * + * TODO: Add new type for delalloc, which can exist across several + * transaction. + */ +struct btrfs_qgroup_rsv { + u64 values[BTRFS_QGROUP_RSV_TYPES + 1]; +}; + /* * one struct for each qgroup, organized in fs_info->qgroup_tree. */ @@ -88,6 +108,7 @@ struct btrfs_qgroup { * reservation tracking */ u64 reserved; + struct btrfs_qgroup_rsv rsv; /* * lists @@ -228,12 +249,14 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, u64 srcid, u64 objectid, struct btrfs_qgroup_inherit *inherit); void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info, - u64 ref_root, u64 num_bytes); + u64 ref_root, u64 num_bytes, + enum btrfs_qgroup_rsv_type type); static inline void btrfs_qgroup_free_delayed_ref(struct btrfs_fs_info *fs_info, u64 ref_root, u64 num_bytes) { trace_btrfs_qgroup_free_delayed_ref(fs_info, ref_root, num_bytes); - btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes); + btrfs_qgroup_free_refroot(fs_info, ref_root, num_bytes, + BTRFS_QGROUP_RSV_DATA); } #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
Instead of single qgroup->reserved, use a new structure btrfs_qgroup_rsv to restore different types of reservation. This patch only updates the header and needed modification to pass compile. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/qgroup.c | 16 ++++++++++------ fs/btrfs/qgroup.h | 27 +++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 8 deletions(-)