Re: [Crash-utility] [PATCH v2 5/6] Add do_maple_tree support for maple tree

On Tue, Oct 25, 2022 at 8:39 PM <crash-utility-request(a)redhat.com&gt; wrote: > Date: Tue, 25 Oct 2022 20:38:24 +0800 > From: Tao Liu <ltao(a)redhat.com&gt; > To: crash-utility(a)redhat.com > Subject: [Crash-utility] [PATCH v2 5/6] Add do_maple_tree support for > maple tree > Message-ID: <20221025123825.36421-6-ltao(a)redhat.com&gt; > Content-Type: text/plain; charset="US-ASCII"; x-default=true > > do_maple_tree() is similar to do_radix_tree() and do_xarray(), which > takes the same do_maple_tree_traverse entry as tree cmd. Currently > do_maple_tree() is not called by any other functions, we reserve it > for future use. > > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > --- > defs.h | 6 +++ > maple_tree.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 155 insertions(+) > > diff --git a/defs.h b/defs.h > index 3f2453e..827e561 100644 > --- a/defs.h > +++ b/defs.h > @@ -5581,6 +5581,12 @@ int cleanup_memory_driver(void); > > void maple_init(void); > int do_mptree(struct tree_data *); > +ulong do_maple_tree(ulong, int, struct list_pair *); > +#define MAPLE_TREE_COUNT (1) > +#define MAPLE_TREE_SEARCH (2) > +#define MAPLE_TREE_DUMP (3) > +#define MAPLE_TREE_GATHER (4) > +#define MAPLE_TREE_DUMP_CB (5) > > /* > * help.c > diff --git a/maple_tree.c b/maple_tree.c > index 33903cb..e8a287a 100644 > --- a/maple_tree.c > +++ b/maple_tree.c > @@ -805,6 +805,13 @@ struct cmd_tree_info { > struct tree_data *td; > } cmd_tree_info; > > +struct maple_tree_ops { > + void (*entry)(ulong node, ulong slot, const char *path, > + ulong index, void *private); > + uint radix; > + void *private; > +} maple_tree_ops; > + > static const char spaces[] = " "; > > static void do_mt_range64(const struct maple_tree *, void *, > @@ -1028,6 +1035,10 @@ static void do_mt_entry(void *entry, unsigned long min, unsigned long max, > int print_radix = 0, i; > static struct req_entry **e = NULL; > > + if (maple_tree_ops.entry) > + maple_tree_ops.entry((ulong)entry, (ulong)entry, path, max, > + maple_tree_ops.private); > + > if (!cmd_tree_info.td) > return; > > @@ -1162,12 +1173,150 @@ int do_mptree(struct tree_data *td) > int is_root = !(td->flags & TREE_NODE_POINTER); > > memset(&cmd_tree_info, 0, sizeof(cmd_tree_info)); > + memset(&maple_tree_ops, 0, sizeof(maple_tree_ops)); See comments below. > cmd_tree_info.td = td; > do_maple_tree_traverse(td->start, is_root); > > return 0; > } > > +/*************For do_maple_tree*****************/ > +static void do_maple_tree_count(ulong node, ulong slot, const char *path, > + ulong index, void *private) > +{ > + struct do_maple_tree_info *info = private; > + info->count++; > +} > + > +static void do_maple_tree_search(ulong node, ulong slot, const char *path, > + ulong index, void *private) > +{ > + struct do_maple_tree_info *info = private; > + struct list_pair *rtp = info->data; > + > + if (rtp->index == index) { > + rtp->value = (void *)slot; > + info->count = 1; > + } > +} > + > +static void do_maple_tree_dump(ulong node, ulong slot, const char *path, > + ulong index, void *private) > +{ > + struct do_maple_tree_info *info = private; > + fprintf(fp, "[%lu] %lx\n", index, slot); > + info->count++; > +} > + > +static void do_maple_tree_gather(ulong node, ulong slot, const char *path, > + ulong index, void *private) > +{ > + struct do_maple_tree_info *info = private; > + struct list_pair *rtp = info->data; > + > + if (info->maxcount) { > + rtp[info->count].index = index; > + rtp[info->count].value = (void *)slot; > + > + info->count++; > + info->maxcount--; > + } > +} > + > +static void do_maple_tree_dump_cb(ulong node, ulong slot, const char *path, > + ulong index, void *private) > +{ > + struct do_maple_tree_info *info = private; > + struct list_pair *rtp = info->data; > + int (*cb)(ulong) = rtp->value; > + > + /* Caller defined operation */ > + if (!cb(slot)) { > + error(FATAL, "do_maple_tree: callback " > + "operation failed: entry: %ld item: %lx\n", > + info->count, slot); > + } > + info->count++; > +} > + > +/* > + * do_maple_tree argument usage: > + * > + * root: Address of a maple_tree_root structure > + * > + * flag: MAPLE_TREE_COUNT - Return the number of entries in the tree. > + * MAPLE_TREE_SEARCH - Search for an entry at rtp->index; if found, > + * store the entry in rtp->value and return a count of 1; otherwise > + * return a count of 0. > + * MAPLE_TREE_DUMP - Dump all existing index/value pairs. > + * MAPLE_TREE_GATHER - Store all existing index/value pairs in the > + * passed-in array of list_pair structs starting at rtp, > + * returning the count of entries stored; the caller can/should > + * limit the number of returned entries by putting the array size > + * (max count) in the rtp->index field of the first structure > + * in the passed-in array. > + * MAPLE_TREE_DUMP_CB - Similar with MAPLE_TREE_DUMP, but for each > + * maple tree entry, a user defined callback at rtp->value will > + * be invoked. > + * > + * rtp: Unused by MAPLE_TREE_COUNT and MAPLE_TREE_DUMP. ^^^ > + * A pointer to a list_pair structure for MAPLE_TREE_SEARCH. > + * A pointer to an array of list_pair structures for > + * MAPLE_TREE_GATHER; the dimension (max count) of the array may > + * be stored in the index field of the first structure to avoid > + * any chance of an overrun. > + * For MAPLE_TREE_DUMP_CB, the rtp->value must be initialized as a > + * callback function. The callback prototype must be: int (*)(ulong); > + */ > +ulong > +do_maple_tree(ulong root, int flag, struct list_pair *rtp) The parameter name "rtp" looks a bit weird, could you please replace the "rtp" with the "lp" and update the above comment?

> +{ > + struct do_maple_tree_info info = { > + .count = 0, > + .data = rtp, Ditto. > + }; > + > + memset(&maple_tree_ops, 0, sizeof(maple_tree_ops)); > + memset(&cmd_tree_info, 0, sizeof(cmd_tree_info)); > + maple_tree_ops.private = &info; Think about it again. It could be good to use a local variable and eventually pass the ops parameter to do_maple_tree_traverse(). Just like:

//filesys.c struct do_radix_tree_info info = { .count = 0, .data = rtp, }; struct radix_tree_ops ops = { .radix = 16, .private = &info, }; > + > + switch (flag) > + { > + case MAPLE_TREE_COUNT: > + maple_tree_ops.entry = do_maple_tree_count; > + break; > + > + case MAPLE_TREE_SEARCH: > + maple_tree_ops.entry = do_maple_tree_search; > + break; > + > + case MAPLE_TREE_DUMP: > + maple_tree_ops.entry = do_maple_tree_dump; > + break; > + > + case MAPLE_TREE_GATHER: > + if (!(info.maxcount = rtp->index)) > + info.maxcount = (ulong)(-1); /* caller beware */ > + > + maple_tree_ops.entry = do_maple_tree_gather; > + break; > + > + case MAPLE_TREE_DUMP_CB: > + if (rtp->value == NULL) { > + error(FATAL, "do_maple_tree: need set callback function"); > + return -EINVAL; The above "return" seems to be redundant.

Thanks. Lianbo > + } > + maple_tree_ops.entry = do_maple_tree_dump_cb; > + break; > + > + default: > + error(FATAL, "do_maple_tree: invalid flag: %lx\n", flag); > + } > + > + do_maple_tree_traverse(root, true); > + return info.count; > +} > + > /***********************************************/ > void maple_init(void) > { > -- > 2.33.1