Hi Lianbo,
On Wed, Jan 11, 2023 at 5:53 PM lijiang <lijiang@xxxxxxxxxx> wrote:
>
> Thank you for the great job, Tao.
>
> On Tue, Jan 10, 2023 at 2:57 PM <crash-utility-request@xxxxxxxxxx> wrote:
>>
>> Date: Tue, 10 Jan 2023 14:56:28 +0800
>> From: Tao Liu <ltao@xxxxxxxxxx>
>> To: crash-utility@xxxxxxxxxx
>> Subject: [PATCH v5 2/6] Add tree cmd support for maple
>> tree
>> Message-ID: <20230110065632.12530-3-ltao@xxxxxxxxxx>
>> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>>
>> Maple tree is a new data structure for crash, so cmd_tree support is
>> needed for users to dump and view the content of maple tree. This patch
>> achieves this by porting mt_dump() and its related functions from kernel,
>> and adapting them with tree cmd.
>>
>> We introduced a new -v arg specifically for dumping the complete
>> content of maple tree:
>>
>> crash> tree -t maple 0xffff9034c006aec0 -v
>>
>> maple_tree(ffff9034c006aec0) flags 309, height 2 root 0xffff9034de70041e
>>
>> 0-18446744073709551615: node 0xffff9034de700400 depth 0 type 3 parent 0xffff9034c006aec1 contents:...
>> 0-140112331583487: node 0xffff9034c01e8800 depth 1 type 1 parent 0xffff9034de700406 contents:...
>> 0-94643156942847: (nil)
>> 94643156942848-94643158024191: 0xffff9035131754c0
>> 94643158024192-94643160117247: (nil)
>> ...
>>
>> The old tree args can work as well:
>>
>> crash> tree -t maple -r mm_struct.mm_mt 0xffff9034c006aec0 -p
>> ffff9035131754c0
>> index: 1 position: root/0/1
>> ffff9035131751c8
>> index: 2 position: root/0/3
>> ffff9035131757b8
>> index: 3 position: root/0/4
>> ...
>>
>> crash> tree -t maple 0xffff9034c006aec0 -p -x -s vm_area_struct.vm_start,vm_end
>> ffff9035131754c0
>> index: 1 position: root/0/1
>> vm_start = 0x5613d3c00000,
>> vm_end = 0x5613d3d08000,
>> ffff9035131751c8
>> index: 2 position: root/0/3
>> vm_start = 0x5613d3f07000,
>> vm_end = 0x5613d3f0b000,
>> ffff9035131757b8
>> index: 3 position: root/0/4
>> vm_start = 0x5613d3f0b000,
>> vm_end = 0x5613d3f14000,
>> ....
>>
>> Signed-off-by: Tao Liu <ltao@xxxxxxxxxx>
>> ---
>> defs.h | 1 +
>> maple_tree.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>> tools.c | 67 +++++++++++++++++++++++++++++++++++++++-------------
>> 3 files changed, 112 insertions(+), 17 deletions(-)
>>
>> diff --git a/defs.h b/defs.h
>> index 46bfd4a..f98e189 100644
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -2713,6 +2713,7 @@ struct tree_data {
>> #define TREE_PARSE_MEMBER (VERBOSE << 7)
>> #define TREE_READ_MEMBER (VERBOSE << 8)
>> #define TREE_LINEAR_ORDER (VERBOSE << 9)
>> +#define TREE_STRUCT_VERBOSE (VERBOSE << 10)
>>
>> #define ALIAS_RUNTIME (1)
>> #define ALIAS_RCLOCAL (2)
>> diff --git a/maple_tree.c b/maple_tree.c
>> index 502172f..d9d1f6a 100644
>> --- a/maple_tree.c
>> +++ b/maple_tree.c
>> @@ -173,6 +173,10 @@ static void do_mt_range64(ulong entry, ulong min, ulong max,
>>
>> mr64_buf = node_buf + OFFSET(maple_node_mr64);
>>
>> + if (td && td->flags & TREE_STRUCT_VERBOSE) {
>> + dump_mt_range64(mr64_buf);
>> + }
>> +
>> for (i = 0; i < mt_slots[maple_range_64]; i++) {
>> last = max;
>>
>> @@ -230,6 +234,10 @@ static void do_mt_arange64(ulong entry, ulong min, ulong max,
>>
>> ma64_buf = node_buf + OFFSET(maple_node_ma64);
>>
>> + if (td && td->flags & TREE_STRUCT_VERBOSE) {
>> + dump_mt_arange64(ma64_buf);
>> + }
>> +
>> for (i = 0; i < mt_slots[maple_arange_64]; i++) {
>> last = max;
>>
>> @@ -275,6 +283,49 @@ static void do_mt_entry(ulong entry, ulong min, ulong max, uint depth,
>>
>> if (!td)
>> return;
>> +
>> + if (!td->count && td->structname_args) {
>> + /*
>> + * Retrieve all members' info only once (count == 0)
>> + * After last iteration all memory will be freed up
>> + */
>> + e = (struct req_entry **)GETBUF(sizeof(*e) * td->structname_args);
>
>
> I would suggest freeing the allocated memory for "e" at the end of this function.
I think free "e" at the end of the function is reasonable. By a quick
glimpse, it seems that do_xarray_entry() and do_rdtree_entry() etc.
have the similar issue?
Thanks,
Tao Liu
>
>> + for (i = 0; i < td->structname_args; i++)
>> + e[i] = fill_member_offsets(td->structname[i]);
>> + }
>> +
>> + td->count++;
>> +
>> + if (td->flags & TREE_STRUCT_VERBOSE) {
>> + dump_mt_entry(entry, min, max, depth);
>> + } else if (td->flags & VERBOSE && entry)
>> + fprintf(fp, "%lx\n", entry);
>> + if (td->flags & TREE_POSITION_DISPLAY && entry)
>> + fprintf(fp, " index: %ld position: %s/%u\n",
>> + ++(*global_index), path, index);
>> +
>> + if (td->structname) {
>> + if (td->flags & TREE_STRUCT_RADIX_10)
>> + print_radix = 10;
>> + else if (td->flags & TREE_STRUCT_RADIX_16)
>> + print_radix = 16;
>> + else
>> + print_radix = 0;
>> +
>> + for (i = 0; i < td->structname_args; i++) {
>> + switch (count_chars(td->structname[i], '.')) {
>> + case 0:
>> + dump_struct(td->structname[i],
>> + entry, print_radix);
>> + break;
>> + default:
>> + if (td->flags & TREE_PARSE_MEMBER)
>> + dump_struct_members_for_tree(td, i, entry);
>> + else if (td->flags & TREE_READ_MEMBER)
>> + dump_struct_members_fast(e[i], print_radix, entry);
>> + }
>> + }
>> + }
>
>
> Can you help to add the FREEBUF() as below? Kazu.
> + if (e)
> + FREEBUF(e);
>
> Other changes look good to me, for the v5 with the minor modification: Ack.
>
> Thanks.
> Lianbo
>
>> }
>>
>> static void do_mt_node(ulong entry, ulong min, ulong max,
>> @@ -293,6 +344,10 @@ static void do_mt_node(ulong entry, ulong min, ulong max,
>> readmem(maple_node, KVADDR, node_buf, SIZE(maple_node),
>> "mt_dump_node read maple_node", FAULT_ON_ERROR);
>>
>> + if (td && td->flags & TREE_STRUCT_VERBOSE) {
>> + dump_mt_node(maple_node, node_buf, type, min, max, depth);
>> + }
>> +
>> switch (type) {
>> case maple_dense:
>> for (i = 0; i < mt_slots[maple_dense]; i++) {
>> @@ -335,6 +390,12 @@ static int do_maple_tree_traverse(ulong ptr, int is_root,
>> "mt_dump read maple_tree", FAULT_ON_ERROR);
>> entry = ULONG(tree_buf + OFFSET(maple_tree_ma_root));
>>
>> + if (td && td->flags & TREE_STRUCT_VERBOSE) {
>> + fprintf(fp, "maple_tree(%lx) flags %X, height %u root 0x%lx\n\n",
>> + ptr, UINT(tree_buf + OFFSET(maple_tree_ma_flags)),
>> + mt_height(tree_buf), entry);
>> + }
>> +
>> if (!xa_is_node(entry))
>> do_mt_entry(entry, 0, 0, 0, 0, path, &global_index, ops);
>> else if (entry) {
>> diff --git a/tools.c b/tools.c
>> index 5f86771..c2cfa7e 100644
>> --- a/tools.c
>> +++ b/tools.c
>> @@ -30,7 +30,7 @@ static void dealloc_hq_entry(struct hq_entry *);
>> static void show_options(void);
>> static void dump_struct_members(struct list_data *, int, ulong);
>> static void rbtree_iteration(ulong, struct tree_data *, char *);
>> -static void dump_struct_members_for_tree(struct tree_data *, int, ulong);
>> +void dump_struct_members_for_tree(struct tree_data *, int, ulong);
>>
>> struct req_entry {
>> char *arg, *name, **member;
>> @@ -40,8 +40,8 @@ struct req_entry {
>> };
>>
>> static void print_value(struct req_entry *, unsigned int, ulong, unsigned int);
>> -static struct req_entry *fill_member_offsets(char *);
>> -static void dump_struct_members_fast(struct req_entry *, int, ulong);
>> +struct req_entry *fill_member_offsets(char *);
>> +void dump_struct_members_fast(struct req_entry *, int, ulong);
>>
>> FILE *
>> set_error(char *target)
>> @@ -3666,7 +3666,7 @@ dump_struct_members_fast(struct req_entry *e, int radix, ulong p)
>> }
>> }
>>
>> -static struct req_entry *
>> +struct req_entry *
>> fill_member_offsets(char *arg)
>> {
>> int j;
>> @@ -4307,6 +4307,7 @@ dump_struct_members(struct list_data *ld, int idx, ulong next)
>> #define RADIXTREE_REQUEST (0x1)
>> #define RBTREE_REQUEST (0x2)
>> #define XARRAY_REQUEST (0x4)
>> +#define MAPLE_REQUEST (0x8)
>>
>> void
>> cmd_tree()
>> @@ -4317,6 +4318,7 @@ cmd_tree()
>> struct datatype_member struct_member, *sm;
>> struct syment *sp;
>> ulong value;
>> + char *type_name = NULL;
>>
>> type_flag = 0;
>> root_offset = 0;
>> @@ -4324,25 +4326,33 @@ cmd_tree()
>> td = &tree_data;
>> BZERO(td, sizeof(struct tree_data));
>>
>> - while ((c = getopt(argcnt, args, "xdt:r:o:s:S:plN")) != EOF) {
>> + while ((c = getopt(argcnt, args, "xdt:r:o:s:S:plNv")) != EOF) {
>> switch (c)
>> {
>> case 't':
>> - if (type_flag & (RADIXTREE_REQUEST|RBTREE_REQUEST|XARRAY_REQUEST)) {
>> + if (type_flag & (RADIXTREE_REQUEST|RBTREE_REQUEST|XARRAY_REQUEST|MAPLE_REQUEST)) {
>> error(INFO, "multiple tree types may not be entered\n");
>> cmd_usage(pc->curcmd, SYNOPSIS);
>> }
>>
>> if (STRNEQ(optarg, "ra"))
>> - if (MEMBER_EXISTS("radix_tree_root", "xa_head"))
>> + if (MEMBER_EXISTS("radix_tree_root", "xa_head")) {
>> type_flag = XARRAY_REQUEST;
>> - else
>> + type_name = "Xarrays";
>> + } else {
>> type_flag = RADIXTREE_REQUEST;
>> - else if (STRNEQ(optarg, "rb"))
>> + type_name = "radix trees";
>> + }
>> + else if (STRNEQ(optarg, "rb")) {
>> type_flag = RBTREE_REQUEST;
>> - else if (STRNEQ(optarg, "x"))
>> + type_name = "rbtrees";
>> + } else if (STRNEQ(optarg, "x")) {
>> type_flag = XARRAY_REQUEST;
>> - else {
>> + type_name = "Xarrays";
>> + } else if (STRNEQ(optarg, "m")) {
>> + type_flag = MAPLE_REQUEST;
>> + type_name = "maple trees";
>> + } else {
>> error(INFO, "invalid tree type: %s\n", optarg);
>> cmd_usage(pc->curcmd, SYNOPSIS);
>> }
>> @@ -4417,6 +4427,9 @@ cmd_tree()
>> "-d and -x are mutually exclusive\n");
>> td->flags |= TREE_STRUCT_RADIX_10;
>> break;
>> + case 'v':
>> + td->flags |= TREE_STRUCT_VERBOSE;
>> + break;
>> default:
>> argerrs++;
>> break;
>> @@ -4426,13 +4439,17 @@ cmd_tree()
>> if (argerrs)
>> cmd_usage(pc->curcmd, SYNOPSIS);
>>
>> - if ((type_flag & (XARRAY_REQUEST|RADIXTREE_REQUEST)) && (td->flags & TREE_LINEAR_ORDER))
>> - error(FATAL, "-l option is not applicable to %s\n",
>> - type_flag & RADIXTREE_REQUEST ? "radix trees" : "Xarrays");
>> + if ((type_flag & (XARRAY_REQUEST|RADIXTREE_REQUEST|MAPLE_REQUEST)) &&
>> + (td->flags & TREE_LINEAR_ORDER))
>> + error(FATAL, "-l option is not applicable to %s\n", type_name);
>>
>> - if ((type_flag & (XARRAY_REQUEST|RADIXTREE_REQUEST)) && (td->flags & TREE_NODE_OFFSET_ENTERED))
>> - error(FATAL, "-o option is not applicable to %s\n",
>> - type_flag & RADIXTREE_REQUEST ? "radix trees" : "Xarrays");
>> + if ((type_flag & (XARRAY_REQUEST|RADIXTREE_REQUEST|MAPLE_REQUEST)) &&
>> + (td->flags & TREE_NODE_OFFSET_ENTERED))
>> + error(FATAL, "-o option is not applicable to %s\n", type_name);
>> +
>> + if ((type_flag & (RBTREE_REQUEST|XARRAY_REQUEST|RADIXTREE_REQUEST)) &&
>> + (td->flags & TREE_STRUCT_VERBOSE))
>> + error(FATAL, "-v option is not applicable to %s\n", type_name);
>>
>> if ((td->flags & TREE_ROOT_OFFSET_ENTERED) &&
>> (td->flags & TREE_NODE_POINTER))
>> @@ -4506,12 +4523,26 @@ next_arg:
>> if (td->flags & TREE_STRUCT_RADIX_16)
>> fprintf(fp, "%sTREE_STRUCT_RADIX_16",
>> others++ ? "|" : "");
>> + if (td->flags & TREE_PARSE_MEMBER)
>> + fprintf(fp, "%sTREE_PARSE_MEMBER",
>> + others++ ? "|" : "");
>> + if (td->flags & TREE_READ_MEMBER)
>> + fprintf(fp, "%sTREE_READ_MEMBER",
>> + others++ ? "|" : "");
>> + if (td->flags & TREE_LINEAR_ORDER)
>> + fprintf(fp, "%sTREE_LINEAR_ORDER",
>> + others++ ? "|" : "");
>> + if (td->flags & TREE_STRUCT_VERBOSE)
>> + fprintf(fp, "%sTREE_STRUCT_VERBOSE",
>> + others++ ? "|" : "");
>> fprintf(fp, ")\n");
>> fprintf(fp, " type: ");
>> if (type_flag & RADIXTREE_REQUEST)
>> fprintf(fp, "radix\n");
>> else if (type_flag & XARRAY_REQUEST)
>> fprintf(fp, "xarray\n");
>> + else if (type_flag & MAPLE_REQUEST)
>> + fprintf(fp, "maple\n");
>> else
>> fprintf(fp, "red-black%s",
>> type_flag & RBTREE_REQUEST ?
>> @@ -4532,6 +4563,8 @@ next_arg:
>> do_rdtree(td);
>> else if (type_flag & XARRAY_REQUEST)
>> do_xatree(td);
>> + else if (type_flag & MAPLE_REQUEST)
>> + do_mptree(td);
>> else
>> do_rbtree(td);
>> hq_close();
>> --
>> 2.33.1
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
