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
