On 3/8/18 12:54 AM, Qu Wenruo wrote: > > > On 2018年03月08日 10:40, je...@suse.com wrote: >> From: Jeff Mahoney <je...@suse.com> >> >> The only mechanism we have in the progs for searching qgroups is to load >> all of them and filter the results. This works for qgroup show but >> to add quota information to 'btrfs subvoluem show' it's pretty wasteful. >> >> This patch splits out setting up the search and performing the search so >> we can search for a single qgroupid more easily. Since TREE_SEARCH >> will give results that don't strictly match the search terms, we add >> a filter to match only the results we care about. >> >> Signed-off-by: Jeff Mahoney <je...@suse.com> >> --- >> qgroup.c | 143 >> ++++++++++++++++++++++++++++++++++++++++++++++++--------------- >> qgroup.h | 7 ++++ >> 2 files changed, 116 insertions(+), 34 deletions(-) >> >> diff --git a/qgroup.c b/qgroup.c >> index 57815718..d076b1de 100644 >> --- a/qgroup.c >> +++ b/qgroup.c >> @@ -1165,11 +1165,30 @@ static inline void print_status_flag_warning(u64 >> flags) >> warning("qgroup data inconsistent, rescan recommended"); >> } >> >> -static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup) >> +static bool key_in_range(const struct btrfs_key *key, >> + const struct btrfs_ioctl_search_key *sk) >> +{ >> + if (key->objectid < sk->min_objectid || >> + key->objectid > sk->max_objectid) >> + return false; >> + >> + if (key->type < sk->min_type || >> + key->type > sk->max_type) >> + return false; >> + >> + if (key->offset < sk->min_offset || >> + key->offset > sk->max_offset) >> + return false; >> + >> + return true; >> +} > > Even with the key_in_range() check here, we are still following the tree > search slice behavior: > > tree search will still gives us all the items in key range from > (min_objectid, min_type, min_offset) to > (max_objectid, max_type, max_offset). > > I don't see much different between the tree search ioctl and this one.
It's fundamentally different. The one in the kernel has a silly interface. It should be min_key and max_key since the components aren't evaluated independently. It effectively treats min_key and max_key as 136-bit values and returns everything between them, inclusive. That's the slice behavior, as you call it. This key_in_range treats each component separately and acts as a filter on the slice returned from the kernel. If we request min/max_offset = 259, the caller will not get anything without offset = 259. I suppose, ultimately, this could also be done using a filter on the rbtree using the existing interface but that seems even more wasteful. -Jeff >> + >> +static int __qgroups_search(int fd, struct btrfs_ioctl_search_args *args, >> + struct qgroup_lookup *qgroup_lookup) >> { >> int ret; >> - struct btrfs_ioctl_search_args args; >> - struct btrfs_ioctl_search_key *sk = &args.key; >> + struct btrfs_ioctl_search_key *sk = &args->key; >> + struct btrfs_ioctl_search_key filter_key = args->key; >> struct btrfs_ioctl_search_header *sh; >> unsigned long off = 0; >> unsigned int i; >> @@ -1180,30 +1199,15 @@ static int __qgroups_search(int fd, struct >> qgroup_lookup *qgroup_lookup) >> u64 qgroupid; >> u64 qgroupid1; >> >> - memset(&args, 0, sizeof(args)); >> - >> - sk->tree_id = BTRFS_QUOTA_TREE_OBJECTID; >> - sk->max_type = BTRFS_QGROUP_RELATION_KEY; >> - sk->min_type = BTRFS_QGROUP_STATUS_KEY; >> - sk->max_objectid = (u64)-1; >> - sk->max_offset = (u64)-1; >> - sk->max_transid = (u64)-1; >> - sk->nr_items = 4096; >> - >> qgroup_lookup_init(qgroup_lookup); >> >> while (1) { >> - ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args); >> + ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, args); >> if (ret < 0) { >> - if (errno == ENOENT) { >> - error("can't list qgroups: quotas not enabled"); >> + if (errno == ENOENT) >> ret = -ENOTTY; >> - } else { >> - error("can't list qgroups: %s", >> - strerror(errno)); >> + else >> ret = -errno; >> - } >> - >> break; >> } >> >> @@ -1217,37 +1221,46 @@ static int __qgroups_search(int fd, struct >> qgroup_lookup *qgroup_lookup) >> * read the root_ref item it contains >> */ >> for (i = 0; i < sk->nr_items; i++) { >> - sh = (struct btrfs_ioctl_search_header *)(args.buf + >> + struct btrfs_key key; >> + >> + sh = (struct btrfs_ioctl_search_header *)(args->buf + >> off); >> off += sizeof(*sh); >> >> - switch (btrfs_search_header_type(sh)) { >> + key.objectid = btrfs_search_header_objectid(sh); >> + key.type = btrfs_search_header_type(sh); >> + key.offset = btrfs_search_header_offset(sh); >> + >> + if (!key_in_range(&key, &filter_key)) >> + goto next; > > It still looks like that most other qgroup info will get calculated. > >> + >> + switch (key.type) { >> case BTRFS_QGROUP_STATUS_KEY: >> si = (struct btrfs_qgroup_status_item *) >> - (args.buf + off); >> + (args->buf + off); >> flags = btrfs_stack_qgroup_status_flags(si); >> >> print_status_flag_warning(flags); >> break; >> case BTRFS_QGROUP_INFO_KEY: >> - qgroupid = btrfs_search_header_offset(sh); >> + qgroupid = key.offset; >> info = (struct btrfs_qgroup_info_item *) >> - (args.buf + off); >> + (args->buf + off); >> >> ret = update_qgroup_info(fd, qgroup_lookup, >> qgroupid, info); >> break; >> case BTRFS_QGROUP_LIMIT_KEY: >> - qgroupid = btrfs_search_header_offset(sh); >> + qgroupid = key.offset; >> limit = (struct btrfs_qgroup_limit_item *) >> - (args.buf + off); >> + (args->buf + off); >> >> ret = update_qgroup_limit(fd, qgroup_lookup, >> qgroupid, limit); >> break; >> case BTRFS_QGROUP_RELATION_KEY: >> - qgroupid = btrfs_search_header_offset(sh); >> - qgroupid1 = btrfs_search_header_objectid(sh); >> + qgroupid = key.offset; >> + qgroupid1 = key.objectid; >> >> if (qgroupid < qgroupid1) >> break; >> @@ -1262,15 +1275,16 @@ static int __qgroups_search(int fd, struct >> qgroup_lookup *qgroup_lookup) >> if (ret) >> return ret; >> >> +next: >> off += btrfs_search_header_len(sh); >> >> /* >> * record the mins in sk so we can make sure the >> * next search doesn't repeat this root >> */ >> - sk->min_type = btrfs_search_header_type(sh); >> - sk->min_offset = btrfs_search_header_offset(sh); >> - sk->min_objectid = btrfs_search_header_objectid(sh); >> + sk->min_type = key.type; >> + sk->min_offset = key.offset; >> + sk->min_objectid = key.objectid; >> } >> sk->nr_items = 4096; >> /* >> @@ -1286,6 +1300,67 @@ static int __qgroups_search(int fd, struct >> qgroup_lookup *qgroup_lookup) >> return ret; >> } >> >> +static int qgroups_search_all(int fd, struct qgroup_lookup *qgroup_lookup) >> +{ >> + struct btrfs_ioctl_search_args args = { >> + .key = { >> + .tree_id = BTRFS_QUOTA_TREE_OBJECTID, >> + .max_type = BTRFS_QGROUP_RELATION_KEY, >> + .min_type = BTRFS_QGROUP_STATUS_KEY, >> + .max_objectid = (u64)-1, >> + .max_offset = (u64)-1, >> + .max_transid = (u64)-1, >> + .nr_items = 4096, >> + }, >> + }; >> + int ret; >> + >> + ret = __qgroups_search(fd, &args, qgroup_lookup); >> + if (ret == -ENOTTY) >> + error("can't list qgroups: quotas not enabled"); >> + else if (ret < 0) >> + error("can't list qgroups: %s", strerror(-ret)); >> + return ret; >> +} >> + >> +int btrfs_qgroup_query(int fd, u64 qgroupid, struct btrfs_qgroup_stats >> *stats) >> +{ >> + struct btrfs_ioctl_search_args args = { >> + .key = { >> + .tree_id = BTRFS_QUOTA_TREE_OBJECTID, >> + .min_type = BTRFS_QGROUP_INFO_KEY, >> + .max_type = BTRFS_QGROUP_LIMIT_KEY, >> + .max_objectid = 0, >> + .min_offset = qgroupid, >> + .max_offset = qgroupid, > > Just keep in mind that such search could include unrelated qgroup info > into the result, key_in_range() check won't help much due to the limit > of how tree search works. > >> + .max_transid = (u64)-1, >> + .nr_items = 4096, >> + }, >> + }; >> + struct qgroup_lookup qgroup_lookup; >> + struct btrfs_qgroup *qgroup; >> + struct rb_node *n; >> + int ret; >> + >> + ret = __qgroups_search(fd, &args, &qgroup_lookup); >> + if (ret < 0) >> + return ret; >> + >> + ret = -ENODATA; >> + n = rb_first(&qgroup_lookup.root); > > And in worst case, if we're query some qgroup which doesn't exist (maybe > manually deleted), then due to limit of tree search, @qgroup_lookup > could contain unrelated qgroup info. > > In that case, rb_first() could gives us some unrelated qgroup. > > For example, we have qgroups info for 5, 258, 259, 1/1, and we're trying > to query qgroup 257 (deleted manually), then we will get the following > result from tree search: > > item 3 key (0 QGROUP_INFO 0/258) itemoff 16131 itemsize 40 > item 4 key (0 QGROUP_INFO 0/259) itemoff 16131 itemsize 40 > item 5 key (0 QGROUP_INFO 1/1) itemoff 16091 itemsize 40 > item 6 key (0 QGROUP_LIMIT 0/5) itemoff 16051 itemsize 40 > > So in @qgroup_lookup, the first item will be qgroup 258 instead of the > non-exist qgroup 257. > > Either caller needs extra check or we need to do extra check in this > function. > > Thanks, > Qu > >> + if (n) { >> + qgroup = rb_entry(n, struct btrfs_qgroup, rb_node); >> + stats->qgroupid = qgroup->qgroupid; >> + stats->info = qgroup->info; >> + stats->limit = qgroup->limit; >> + >> + ret = 0; >> + } >> + >> + __free_all_qgroups(&qgroup_lookup); >> + return ret; >> +} >> + >> static void print_all_qgroups(struct qgroup_lookup *qgroup_lookup, bool >> verbose) >> { >> >> @@ -1312,7 +1387,7 @@ int btrfs_show_qgroups(int fd, >> struct qgroup_lookup sort_tree; >> int ret; >> >> - ret = __qgroups_search(fd, &qgroup_lookup); >> + ret = qgroups_search_all(fd, &qgroup_lookup); >> if (ret) >> return ret; >> __filter_and_sort_qgroups(&qgroup_lookup, &sort_tree, >> diff --git a/qgroup.h b/qgroup.h >> index 5e71349c..688f92b2 100644 >> --- a/qgroup.h >> +++ b/qgroup.h >> @@ -87,6 +87,12 @@ struct btrfs_qgroup_info { >> u64 exclusive_compressed; >> }; >> >> +struct btrfs_qgroup_stats { >> + u64 qgroupid; >> + struct btrfs_qgroup_info info; >> + struct btrfs_qgroup_limit limit; >> +}; >> + >> int btrfs_qgroup_parse_sort_string(const char *opt_arg, >> struct btrfs_qgroup_comparer_set **comps); >> int btrfs_show_qgroups(int fd, struct btrfs_qgroup_filter_set *, >> @@ -105,4 +111,5 @@ int qgroup_inherit_add_group(struct btrfs_qgroup_inherit >> **inherit, char *arg); >> int qgroup_inherit_add_copy(struct btrfs_qgroup_inherit **inherit, char >> *arg, >> int type); >> >> +int btrfs_qgroup_query(int fd, u64 qgroupid, struct btrfs_qgroup_stats >> *stats); >> #endif >> > -- Jeff Mahoney SUSE Labs
signature.asc
Description: OpenPGP digital signature