[PATCH 0/2] couple of trivial tree patches

[PATCH] fix "ps -a" for tasks...

[PATCH] speed up "ps -r" by...

Daniel Vacek

Thursday, 12 April 2018 Thu, 12 Apr '18

6:39 a.m.

Daniel Vacek (2): tree: no need to bail out on tree corruption, skip the node and move on instead tree: add an option to dump the tree sorted defs.h | 1 + help.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools.c | 37 ++++++++++++++++++++---------- 3 files changed, 106 insertions(+), 12 deletions(-) -- 2.16.2

Show replies by date

Daniel Vacek

Thursday, 12 April Thu, 12 Apr

6:39 a.m.

New subject: [PATCH 1/2] tree: no need to bail out on tree corruption, skip the node and move on instead

--- tools.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tools.c b/tools.c index 186b703463a5..992f4776281a 100644 --- a/tools.c +++ b/tools.c @@ -4374,8 +4374,8 @@ rbtree_iteration(ulong node_p, struct tree_data *td, char *pos) { int i; uint print_radix; - ulong struct_p, left_p, right_p; - char left_pos[BUFSIZE], right_pos[BUFSIZE]; + ulong struct_p, new_p; + char new_pos[BUFSIZE]; static struct req_entry **e; if (!node_p) @@ -4430,16 +4430,17 @@ rbtree_iteration(ulong node_p, struct tree_data *td, char *pos) } } - readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &left_p, - sizeof(void *), "rb_node rb_left", FAULT_ON_ERROR); - readmem(node_p+OFFSET(rb_node_rb_right), KVADDR, &right_p, - sizeof(void *), "rb_node rb_right", FAULT_ON_ERROR); - - sprintf(left_pos, "%s/l", pos); - sprintf(right_pos, "%s/r", pos); + if ( readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) { + sprintf(new_pos, "%s/l", pos); + rbtree_iteration(new_p, td, new_pos); + } - rbtree_iteration(left_p, td, left_pos); - rbtree_iteration(right_p, td, right_pos); + if ( readmem(node_p+OFFSET(rb_node_rb_right), KVADDR, &new_p, + sizeof(void *), "rb_node rb_right", RETURN_ON_ERROR)) { + sprintf(new_pos, "%s/r", pos); + rbtree_iteration(new_p, td, new_pos); + } } void -- 2.16.2

Dave Anderson

8:32 a.m.

New subject: [PATCH 1/2] tree: no need to bail out on tree corruption, skip the node and move on instead

Hi Daniel, If the tree is corrupted, I would think that you would want to know the specifics, i.e., aside from the somewhat cryptic readmem() error message. Perhaps it would be worth printing a more meaningful error message, something like: tree: rb_node: <address>: corrupt rb_left pointer: <address> That way you could do the 2 readmem() calls w/RETURN_ON_ERROR|QUIET, and just have the concise error message. Thanks, Dave ----- Original Message -----

...

Daniel Vacek

9 a.m.

New subject: [PATCH 1/2] tree: no need to bail out on tree corruption, skip the node and move on instead

Hi Dave, On Thu, Apr 12, 2018 at 3:32 PM, Dave Anderson <anderson(a)redhat.com> wrote:

...

Yeah, that's a good point. I just tried a simple s/FAULT_ON_ERROR/RETURN_ON_ERROR/ and checked if that works, but keeping the former behaviour with regards to error reporting. But having a meaningful error output would be nice. Do you prefer a followup patch or amend to this one? --nX

...

That way you could do the 2 readmem() calls w/RETURN_ON_ERROR|QUIET, and just have the concise error message. Thanks, Dave ----- Original Message ----- > --- > tools.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/tools.c b/tools.c > index 186b703463a5..992f4776281a 100644 > --- a/tools.c > +++ b/tools.c > @@ -4374,8 +4374,8 @@ rbtree_iteration(ulong node_p, struct tree_data *td, > char *pos) > { > int i; > uint print_radix; > - ulong struct_p, left_p, right_p; > - char left_pos[BUFSIZE], right_pos[BUFSIZE]; > + ulong struct_p, new_p; > + char new_pos[BUFSIZE]; > static struct req_entry **e; > > if (!node_p) > @@ -4430,16 +4430,17 @@ rbtree_iteration(ulong node_p, struct tree_data *td, > char *pos) > } > } > > - readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &left_p, > - sizeof(void *), "rb_node rb_left", FAULT_ON_ERROR); > - readmem(node_p+OFFSET(rb_node_rb_right), KVADDR, &right_p, > - sizeof(void *), "rb_node rb_right", FAULT_ON_ERROR); > - > - sprintf(left_pos, "%s/l", pos); > - sprintf(right_pos, "%s/r", pos); > + if ( readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, > + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) { > + sprintf(new_pos, "%s/l", pos); > + rbtree_iteration(new_p, td, new_pos); > + } > > - rbtree_iteration(left_p, td, left_pos); > - rbtree_iteration(right_p, td, right_pos); > + if ( readmem(node_p+OFFSET(rb_node_rb_right), KVADDR, &new_p, > + sizeof(void *), "rb_node rb_right", RETURN_ON_ERROR)) { > + sprintf(new_pos, "%s/r", pos); > + rbtree_iteration(new_p, td, new_pos); > + } > } > > void > -- > 2.16.2 > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://www.redhat.com/mailman/listinfo/crash-utility >

Dave Anderson

9:02 a.m.

New subject: [PATCH 1/2] tree: no need to bail out on tree corruption, skip the node and move on instead

----- Original Message -----

...

Hi Dave, On Thu, Apr 12, 2018 at 3:32 PM, Dave Anderson <anderson(a)redhat.com> wrote: > > Hi Daniel, > > If the tree is corrupted, I would think that you would want to know the > specifics, i.e., aside from the somewhat cryptic readmem() error message. > > Perhaps it would be worth printing a more meaningful error message, > something like: > > tree: rb_node: <address>: corrupt rb_left pointer: <address> Yeah, that's a good point. I just tried a simple s/FAULT_ON_ERROR/RETURN_ON_ERROR/ and checked if that works, but keeping the former behaviour with regards to error reporting. But having a meaningful error output would be nice. Do you prefer a followup patch or amend to this one?

Given my subsequent bitching about 2/2, how about a v2 patchset with both patches updated? Thanks, Dave

...

--nX > That way you could do the 2 readmem() calls w/RETURN_ON_ERROR|QUIET, and > just have the concise error message. > > Thanks, > Dave > > ----- Original Message ----- >> --- >> tools.c | 23 ++++++++++++----------- >> 1 file changed, 12 insertions(+), 11 deletions(-) >> >> diff --git a/tools.c b/tools.c >> index 186b703463a5..992f4776281a 100644 >> --- a/tools.c >> +++ b/tools.c >> @@ -4374,8 +4374,8 @@ rbtree_iteration(ulong node_p, struct tree_data *td, >> char *pos) >> { >> int i; >> uint print_radix; >> - ulong struct_p, left_p, right_p; >> - char left_pos[BUFSIZE], right_pos[BUFSIZE]; >> + ulong struct_p, new_p; >> + char new_pos[BUFSIZE]; >> static struct req_entry **e; >> >> if (!node_p) >> @@ -4430,16 +4430,17 @@ rbtree_iteration(ulong node_p, struct tree_data >> *td, >> char *pos) >> } >> } >> >> - readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &left_p, >> - sizeof(void *), "rb_node rb_left", FAULT_ON_ERROR); >> - readmem(node_p+OFFSET(rb_node_rb_right), KVADDR, &right_p, >> - sizeof(void *), "rb_node rb_right", FAULT_ON_ERROR); >> - >> - sprintf(left_pos, "%s/l", pos); >> - sprintf(right_pos, "%s/r", pos); >> + if ( readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, >> + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) >> { >> + sprintf(new_pos, "%s/l", pos); >> + rbtree_iteration(new_p, td, new_pos); >> + } >> >> - rbtree_iteration(left_p, td, left_pos); >> - rbtree_iteration(right_p, td, right_pos); >> + if ( readmem(node_p+OFFSET(rb_node_rb_right), KVADDR, &new_p, >> + sizeof(void *), "rb_node rb_right", >> RETURN_ON_ERROR)) { >> + sprintf(new_pos, "%s/r", pos); >> + rbtree_iteration(new_p, td, new_pos); >> + } >> } >> >> void >> -- >> 2.16.2 >> >> -- >> Crash-utility mailing list >> Crash-utility(a)redhat.com >> https://www.redhat.com/mailman/listinfo/crash-utility >>

Daniel Vacek

6:39 a.m.

New subject: [PATCH 2/2] tree: add an option to dump the tree sorted

--- defs.h | 1 + help.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools.c | 16 +++++++++++-- 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/defs.h b/defs.h index adddb9f2748d..ec298cbd70be 100644 --- a/defs.h +++ b/defs.h @@ -2480,6 +2480,7 @@ struct tree_data { #define TREE_STRUCT_RADIX_16 (VERBOSE << 6) #define TREE_PARSE_MEMBER (VERBOSE << 7) #define TREE_READ_MEMBER (VERBOSE << 8) +#define TREE_LINEAR_ORDER (VERBOSE << 9) #define ALIAS_RUNTIME (1) #define ALIAS_RCLOCAL (2) diff --git a/help.c b/help.c index c5cec5365962..e7df50fa2ef4 100644 --- a/help.c +++ b/help.c @@ -5698,6 +5698,8 @@ char *help_tree[] = { " indicates \"root/l/r\" means that the node is the right child", " of the left child of the root node. For radix trees, the height", " and slot index values are shown with respect to the root.", +" -l Dump the tree sorted in linear order starting with the leftmost", +" node and progressing to the right.", " ", " The meaning of the \"start\" argument, which can be expressed either in", " hexadecimal format or symbolically, depends upon whether the -N option", @@ -5823,6 +5825,84 @@ char *help_tree[] = { " ffffea000407de58", " position: root/3/28", "", +" List the tree in linear order from the leftmost node progressing", +" to the right using the -l option:\n", +" %s> tree -lps vm_area_struct.vm_start,vm_end -o vm_area_struct.vm_rb -r mm_struct.mm_rb 0xffff8805dee5e400 | sed 's/^ //' | paste - - - - | column -ts' '", +" ffff8805e108f008 position: root/l/l/l/l/l/l/l/l/l vm_start = 0x7f6f9ca5f000 vm_end = 0x7f6f9d44c000", +" ffff880540d35d88 position: root/l/l/l/l/l/l/l/l vm_start = 0x7f6f9d44c000 vm_end = 0x7f6f9d454000", +" ffff8805def64d80 position: root/l/l/l/l/l/l/l vm_start = 0x7f6f9d454000 vm_end = 0x7f6f9d653000", +" ffff8805e0b46510 position: root/l/l/l/l/l/l/l/r/l vm_start = 0x7f6f9d653000 vm_end = 0x7f6f9d654000", +" ffff8805e108e288 position: root/l/l/l/l/l/l/l/r vm_start = 0x7f6f9d654000 vm_end = 0x7f6f9d655000", +" ffff8805def65bd8 position: root/l/l/l/l/l/l/l/r/r vm_start = 0x7f6f9d655000 vm_end = 0x7f6f9d661000", +" ffff8805def64ca8 position: root/l/l/l/l/l/l vm_start = 0x7f6f9d661000 vm_end = 0x7f6f9d860000", +" ffff8805def641b0 position: root/l/l/l/l/l/l/r/l vm_start = 0x7f6f9d860000 vm_end = 0x7f6f9d861000", +" ffff8805def64438 position: root/l/l/l/l/l/l/r/l/r vm_start = 0x7f6f9d861000 vm_end = 0x7f6f9d862000", +" ffff8805def65368 position: root/l/l/l/l/l/l/r vm_start = 0x7f6f9d862000 vm_end = 0x7f6f9d868000", +" ffff880540ec2e58 position: root/l/l/l/l/l/l/r/r vm_start = 0x7f6f9d868000 vm_end = 0x7f6f9d88c000", +" ...", +" ffff8805dfc08e58 position: root/l/r/r/l/r/l vm_start = 0x7f6fa020f000 vm_end = 0x7f6fa0216000", +" ffff880540ec35f0 position: root/l/r/r/l/r vm_start = 0x7f6fa0216000 vm_end = 0x7f6fa0217000", +" ffff8805dfc09cb0 position: root/l/r/r/l/r/r vm_start = 0x7f6fa0217000 vm_end = 0x7f6fa0338000", +" ffff8805dfc08360 position: root/l/r/r vm_start = 0x7f6fa0338000 vm_end = 0x7f6fa0538000", +" ffff8805dfc08bd0 position: root vm_start = 0x7f6fa07af000 vm_end = 0x7f6fa09ae000", +" ffff8805dfc08288 position: root/r/l/l/l/l/l vm_start = 0x7f6fa09ae000 vm_end = 0x7f6fa09b2000", +" ffff880540ec31b8 position: root/r/l/l/l/l/l/r vm_start = 0x7f6fa09b2000 vm_end = 0x7f6fa09b3000", +" ffff8805dfc08ca8 position: root/r/l/l/l/l vm_start = 0x7f6fa09b3000 vm_end = 0x7f6fa09b4000", +" ffff8800f78c15f0 position: root/r/l/l/l/l/r vm_start = 0x7f6fa09b4000 vm_end = 0x7f6fa0b6c000", +" ...", +" ffff8805def651b8 position: root/r/r/r/r/l/l vm_start = 0x7f6fa2f42000 vm_end = 0x7f6fa2f43000", +" ffff880540d35e60 position: root/r/r/r/r/l vm_start = 0x7f6fa2f43000 vm_end = 0x7f6fa2f44000", +" ffff880540d35518 position: root/r/r/r/r/l/r vm_start = 0x7f6fa2f44000 vm_end = 0x7f6fa2f85000", +" ffff880540d356c8 position: root/r/r/r/r vm_start = 0x7f6fa3185000 vm_end = 0x7f6fa3187000", +" ffff8805def64000 position: root/r/r/r/r/r/l/l vm_start = 0x7f6fa3187000 vm_end = 0x7f6fa3188000", +" ffff880540d35cb0 position: root/r/r/r/r/r/l vm_start = 0x7f6fa3188000 vm_end = 0x7f6fa3189000", +" ffff8805def64798 position: root/r/r/r/r/r/l/r vm_start = 0x7f6fa4bf9000 vm_end = 0x7f6fa4c1a000", +" ffff880540d340d8 position: root/r/r/r/r/r vm_start = 0x7ffc2b386000 vm_end = 0x7ffc2b3a8000", +" ffff880540d35950 position: root/r/r/r/r/r/r vm_start = 0x7ffc2b3c2000 vm_end = 0x7ffc2b3c4000", +"", +" Compared to the top/down order:\n", +" %s> tree -ps vm_area_struct.vm_start,vm_end -o vm_area_struct.vm_rb -r mm_struct.mm_rb 0xffff8805dee5e400 | sed 's/^ //' | paste - - - - | column -ts' '", +" ffff8805dfc08bd0 position: root vm_start = 0x7f6fa07af000 vm_end = 0x7f6fa09ae000", +" ffff8805e0b79b00 position: root/l vm_start = 0x7f6f9f585000 vm_end = 0x7f6f9f785000", +" ffff8805e0b79e60 position: root/l/l vm_start = 0x7f6f9ec2b000 vm_end = 0x7f6f9ee2b000", +" ffff8805e0b78bd0 position: root/l/l/l vm_start = 0x7f6f9e2c1000 vm_end = 0x7f6f9e4c0000", +" ffff8805e07f0bd0 position: root/l/l/l/l vm_start = 0x7f6f9da92000 vm_end = 0x7f6f9dc91000", +" ffff880540ec20d8 position: root/l/l/l/l/l vm_start = 0x7f6f9d88c000 vm_end = 0x7f6f9da8b000", +" ffff8805def64ca8 position: root/l/l/l/l/l/l vm_start = 0x7f6f9d661000 vm_end = 0x7f6f9d860000", +" ffff8805def64d80 position: root/l/l/l/l/l/l/l vm_start = 0x7f6f9d454000 vm_end = 0x7f6f9d653000", +" ffff880540d35d88 position: root/l/l/l/l/l/l/l/l vm_start = 0x7f6f9d44c000 vm_end = 0x7f6f9d454000", +" ffff8805e108f008 position: root/l/l/l/l/l/l/l/l/l vm_start = 0x7f6f9ca5f000 vm_end = 0x7f6f9d44c000", +" ffff8805e108e288 position: root/l/l/l/l/l/l/l/r vm_start = 0x7f6f9d654000 vm_end = 0x7f6f9d655000", +" ffff8805e0b46510 position: root/l/l/l/l/l/l/l/r/l vm_start = 0x7f6f9d653000 vm_end = 0x7f6f9d654000", +" ffff8805def65bd8 position: root/l/l/l/l/l/l/l/r/r vm_start = 0x7f6f9d655000 vm_end = 0x7f6f9d661000", +" ...", +" ffff8805dfc08360 position: root/l/r/r vm_start = 0x7f6fa0338000 vm_end = 0x7f6fa0538000", +" ffff8805dfc096c8 position: root/l/r/r/l vm_start = 0x7f6fa0010000 vm_end = 0x7f6fa020f000", +" ffff880540ec2870 position: root/l/r/r/l/l vm_start = 0x7f6f9ffe9000 vm_end = 0x7f6f9ffea000", +" ffff8805dfc09440 position: root/l/r/r/l/l/l vm_start = 0x7f6f9ffe8000 vm_end = 0x7f6f9ffe9000", +" ffff8805dfc09290 position: root/l/r/r/l/l/r vm_start = 0x7f6f9ffea000 vm_end = 0x7f6fa0010000", +" ffff880540ec35f0 position: root/l/r/r/l/r vm_start = 0x7f6fa0216000 vm_end = 0x7f6fa0217000", +" ffff8805dfc08e58 position: root/l/r/r/l/r/l vm_start = 0x7f6fa020f000 vm_end = 0x7f6fa0216000", +" ffff8805dfc09cb0 position: root/l/r/r/l/r/r vm_start = 0x7f6fa0217000 vm_end = 0x7f6fa0338000", +" ffff880540d346c0 position: root/r vm_start = 0x7f6fa1f5a000 vm_end = 0x7f6fa2159000", +" ffff8800f78c10e0 position: root/r/l vm_start = 0x7f6fa135f000 vm_end = 0x7f6fa155f000", +" ffff8800f78c1440 position: root/r/l/l vm_start = 0x7f6fa0d8d000 vm_end = 0x7f6fa0f8d000", +" ffff8800f78c1950 position: root/r/l/l/l vm_start = 0x7f6fa0b6c000 vm_end = 0x7f6fa0d6c000", +" ffff8805dfc08ca8 position: root/r/l/l/l/l vm_start = 0x7f6fa09b3000 vm_end = 0x7f6fa09b4000", +" ffff8805dfc08288 position: root/r/l/l/l/l/l vm_start = 0x7f6fa09ae000 vm_end = 0x7f6fa09b2000", +" ffff880540ec31b8 position: root/r/l/l/l/l/l/r vm_start = 0x7f6fa09b2000 vm_end = 0x7f6fa09b3000", +" ffff8800f78c15f0 position: root/r/l/l/l/l/r vm_start = 0x7f6fa09b4000 vm_end = 0x7f6fa0b6c000", +" ...", +" ffff880540d356c8 position: root/r/r/r/r vm_start = 0x7f6fa3185000 vm_end = 0x7f6fa3187000", +" ffff880540d35e60 position: root/r/r/r/r/l vm_start = 0x7f6fa2f43000 vm_end = 0x7f6fa2f44000", +" ffff8805def651b8 position: root/r/r/r/r/l/l vm_start = 0x7f6fa2f42000 vm_end = 0x7f6fa2f43000", +" ffff880540d35518 position: root/r/r/r/r/l/r vm_start = 0x7f6fa2f44000 vm_end = 0x7f6fa2f85000", +" ffff880540d340d8 position: root/r/r/r/r/r vm_start = 0x7ffc2b386000 vm_end = 0x7ffc2b3a8000", +" ffff880540d35cb0 position: root/r/r/r/r/r/l vm_start = 0x7f6fa3188000 vm_end = 0x7f6fa3189000", +" ffff8805def64000 position: root/r/r/r/r/r/l/l vm_start = 0x7f6fa3187000 vm_end = 0x7f6fa3188000", +" ffff8805def64798 position: root/r/r/r/r/r/l/r vm_start = 0x7f6fa4bf9000 vm_end = 0x7f6fa4c1a000", +" ffff880540d35950 position: root/r/r/r/r/r/r vm_start = 0x7ffc2b3c2000 vm_end = 0x7ffc2b3c4000", +"", " Alternatively, take the address of the radix_tree_node from the", " radix_tree_root structure in the address_space structure above,", " and display the tree with the -N option:\n", diff --git a/tools.c b/tools.c index 992f4776281a..1970e8bb136b 100644 --- a/tools.c +++ b/tools.c @@ -3926,7 +3926,7 @@ cmd_tree() td = &tree_data; BZERO(td, sizeof(struct tree_data)); - while ((c = getopt(argcnt, args, "xdt:r:o:s:S:pN")) != EOF) { + while ((c = getopt(argcnt, args, "xdt:r:o:s:S:plN")) != EOF) { switch (c) { case 't': @@ -3993,6 +3993,10 @@ cmd_tree() td->flags |= TREE_POSITION_DISPLAY; break; + case 'l': + td->flags |= TREE_LINEAR_ORDER; + break; + case 'N': td->flags |= TREE_NODE_POINTER; break; @@ -4399,6 +4403,13 @@ rbtree_iteration(ulong node_p, struct tree_data *td, char *pos) struct_p = node_p - td->node_member_offset; + if (td->flags & TREE_LINEAR_ORDER && + readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) { + sprintf(new_pos, "%s/l", pos); + rbtree_iteration(new_p, td, new_pos); + } + if (td->flags & VERBOSE) fprintf(fp, "%lx\n", struct_p); @@ -4430,7 +4441,8 @@ rbtree_iteration(ulong node_p, struct tree_data *td, char *pos) } } - if ( readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, + if (!(td->flags & TREE_LINEAR_ORDER) && + readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) { sprintf(new_pos, "%s/l", pos); rbtree_iteration(new_p, td, new_pos); -- 2.16.2

Dave Anderson

8:57 a.m.

New subject: [PATCH 2/2] tree: add an option to dump the tree sorted

Daniel, Hmmm, when looking at your example, I just noticed it didn't use the -t argument, but then looking at the code realized that the command defaults to rbtrees. I actually don't think that was the original intent, but maybe that fact should should be indicated in the help page? Although I can't come up with a compelling argument why one should be the default. Either that, or -t should be enforced. Anyway, you probably didn't notice that all help page output is constrained to 80 columns. Your example command alone is 162 bytes long, making it way too confusing to figure out what the command is doing. Can you just show the command output without piping it to an external command, or make a much simpler example? And also keep it to 80 columns or less, and put the "l" argument in the synopsis line at the top. Thanks, Dave ----- Original Message -----

...

Daniel Vacek

9:51 a.m.

New subject: [PATCH 2/2] tree: add an option to dump the tree sorted

On Thu, Apr 12, 2018 at 3:57 PM, Dave Anderson <anderson(a)redhat.com> wrote:

...

Wow. I didn't know this was not intended. I thought that red-black is default as radix trees are only used in address_space of mapped files (page cache), AFAIK. I usually do not need to specify the type as all the major trees are red-black - being it cgroups, scheduler entities, memory mappings, inodes, you name it. Again, I didn't change anything with regards to this behavior.

...

Anyway, you probably didn't notice that all help page output is constrained to 80 columns. Your example command alone is 162 bytes long, making it way too confusing to figure out what the command is doing. Can you just show the command output without piping it to an external command, or make a much simpler example? And also keep it to 80 columns or less,

I was thinking about wrapping the command to a second line but that's what terminal will do anyways if narrower so I left it that way. The rest of the pipe is just formatting/pretty printing the table. Otherwise the output would be waaaay toooo loooong and not really human readable. I do not think it can get any simpler than that. I can get rid of one of the columns though, it does not need to contain both the start and the end of the VMA range. As an added value it also teaches eventual readers some advanced usage techniques in general so I believe it can be useful. There is enough simple examples already. Would you rather prefer something like this? crash> task -R mm PID: 28682 TASK: ffff880036d4af10 CPU: 0 COMMAND: "crash" mm = 0xffff880074b5be80, crash> tree -lp -o vm_area_struct.vm_rb -r mm_struct.mm_rb 0xffff880074b5be80 ffff88001f2c50e0 position: root/l/l/l/l/l ffff88001f2c5290 position: root/l/l/l/l ffff880074bfc6c0 position: root/l/l/l/l/r ffff88001f2c4bd0 position: root/l/l/l ffff880074bfc948 position: root/l/l/l/r/l/l ffff880036e54510 position: root/l/l/l/r/l/l/r ffff88001f2c5bd8 position: root/l/l/l/r/l ffff880036e54af8 position: root/l/l/l/r/l/r/l ffff880036e54f30 position: root/l/l/l/r/l/r ffff88000e06aa20 position: root/l/l/l/r/l/r/r ffff88000e06b368 position: root/l/l/l/r ffff88000e06ba28 position: root/l/l/l/r/r/l ffff88000e06ae58 position: root/l/l/l/r/r ffff88000e06a6c0 position: root/l/l/l/r/r/r ... ffff88001f2c51b8 position: root/l/r/r/r/l ffff88001f2c4d80 position: root/l/r/r/r ffff880074bfd878 position: root/l/r/r/r/r ffff88001f2c5a28 position: root ffff88001f2c4a20 position: root/r/l/l/l ffff88001f2c4360 position: root/r/l/l ffff880074bfcaf8 position: root/r/l/l/r ... ffff88001f2c5e60 position: root/r/r/l/l/l ffff88001f2c4ca8 position: root/r/r/l/l ffff88001f2c5008 position: root/r/r/l ffff88001f2c5d88 position: root/r/r/l/r ffff880074bfd6c8 position: root/r/r/l/r/r ffff88001f2c4288 position: root/r/r ffff88001f2c4510 position: root/r/r/r ffff88001f2c5b00 position: root/r/r/r/r This is not as readable in my opinion. And also it does not show why would one care about the order in the first place. See? I do not believe a hard limit of 80 columns is useful for any serious kernel debugging. For developing and maintaining a source code, yes. For dumping a lot of debugging data in a human readable form, no. For example if I type 'timer -r' I already get an output which is over eighty. How would you copy/paste this to help? Or how do you feel about the output itself? CLOCK: 1 HRTIMER_CLOCK_BASE: ffff8805ed82d9a0 [ktime_get_real] CURRENT 1522436617507394900 SOFTEXPIRES EXPIRES HRTIMER FUNCTION 1522436624101007000 1522436624101057000 ffff8805e08ebd60 ffffffff810a95c0 <hrtimer_wakeup> 1522436628203647900 1522436628203697900 ffff8805e33fbd60 ffffffff810a95c0 <hrtimer_wakeup> 1522436635368413000 1522436635368463000 ffff8805e07cfd60 ffffffff810a95c0 <hrtimer_wakeup> 1522436652248574300 1522436652248624300 ffff8805e1acfd60 ffffffff810a95c0 <hrtimer_wakeup> 1522436652257417000 1522436652257467000 ffff8805e07a3d60 ffffffff810a95c0 <hrtimer_wakeup> 1522436664228364000 1522436664228414000 ffff8805e1347d60 ffffffff810a95c0 <hrtimer_wakeup> That's about the same width as my example. What do you suggest?

...

and put the "l" argument in the synopsis line at the top.

Nah. I missed that one. Will fix it. Or you can just amend it yourself. --nX

...

Thanks, Dave ----- Original Message ----- > --- > defs.h | 1 + > help.c | 80 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > tools.c | 16 +++++++++++-- > 3 files changed, 95 insertions(+), 2 deletions(-) > > diff --git a/defs.h b/defs.h > index adddb9f2748d..ec298cbd70be 100644 > --- a/defs.h > +++ b/defs.h > @@ -2480,6 +2480,7 @@ struct tree_data { > #define TREE_STRUCT_RADIX_16 (VERBOSE << 6) > #define TREE_PARSE_MEMBER (VERBOSE << 7) > #define TREE_READ_MEMBER (VERBOSE << 8) > +#define TREE_LINEAR_ORDER (VERBOSE << 9) > > #define ALIAS_RUNTIME (1) > #define ALIAS_RCLOCAL (2) > diff --git a/help.c b/help.c > index c5cec5365962..e7df50fa2ef4 100644 > --- a/help.c > +++ b/help.c > @@ -5698,6 +5698,8 @@ char *help_tree[] = { > " indicates \"root/l/r\" means that the node is the right > child", > " of the left child of the root node. For radix trees, the > height", > " and slot index values are shown with respect to the root.", > +" -l Dump the tree sorted in linear order starting with the > leftmost", > +" node and progressing to the right.", > " ", > " The meaning of the \"start\" argument, which can be expressed either in", > " hexadecimal format or symbolically, depends upon whether the -N option", > @@ -5823,6 +5825,84 @@ char *help_tree[] = { > " ffffea000407de58", > " position: root/3/28", > "", > +" List the tree in linear order from the leftmost node progressing", > +" to the right using the -l option:\n", > +" %s> tree -lps vm_area_struct.vm_start,vm_end -o vm_area_struct.vm_rb -r > mm_struct.mm_rb 0xffff8805dee5e400 | sed 's/^ //' | paste - - - - | column > -ts' '", > +" ffff8805e108f008 position: root/l/l/l/l/l/l/l/l/l vm_start = > 0x7f6f9ca5f000 vm_end = 0x7f6f9d44c000", > +" ffff880540d35d88 position: root/l/l/l/l/l/l/l/l vm_start = > 0x7f6f9d44c000 vm_end = 0x7f6f9d454000", > +" ffff8805def64d80 position: root/l/l/l/l/l/l/l vm_start = > 0x7f6f9d454000 vm_end = 0x7f6f9d653000", > +" ffff8805e0b46510 position: root/l/l/l/l/l/l/l/r/l vm_start = > 0x7f6f9d653000 vm_end = 0x7f6f9d654000", > +" ffff8805e108e288 position: root/l/l/l/l/l/l/l/r vm_start = > 0x7f6f9d654000 vm_end = 0x7f6f9d655000", > +" ffff8805def65bd8 position: root/l/l/l/l/l/l/l/r/r vm_start = > 0x7f6f9d655000 vm_end = 0x7f6f9d661000", > +" ffff8805def64ca8 position: root/l/l/l/l/l/l vm_start = > 0x7f6f9d661000 vm_end = 0x7f6f9d860000", > +" ffff8805def641b0 position: root/l/l/l/l/l/l/r/l vm_start = > 0x7f6f9d860000 vm_end = 0x7f6f9d861000", > +" ffff8805def64438 position: root/l/l/l/l/l/l/r/l/r vm_start = > 0x7f6f9d861000 vm_end = 0x7f6f9d862000", > +" ffff8805def65368 position: root/l/l/l/l/l/l/r vm_start = > 0x7f6f9d862000 vm_end = 0x7f6f9d868000", > +" ffff880540ec2e58 position: root/l/l/l/l/l/l/r/r vm_start = > 0x7f6f9d868000 vm_end = 0x7f6f9d88c000", > +" ...", > +" ffff8805dfc08e58 position: root/l/r/r/l/r/l vm_start = > 0x7f6fa020f000 vm_end = 0x7f6fa0216000", > +" ffff880540ec35f0 position: root/l/r/r/l/r vm_start = > 0x7f6fa0216000 vm_end = 0x7f6fa0217000", > +" ffff8805dfc09cb0 position: root/l/r/r/l/r/r vm_start = > 0x7f6fa0217000 vm_end = 0x7f6fa0338000", > +" ffff8805dfc08360 position: root/l/r/r vm_start = > 0x7f6fa0338000 vm_end = 0x7f6fa0538000", > +" ffff8805dfc08bd0 position: root vm_start = > 0x7f6fa07af000 vm_end = 0x7f6fa09ae000", > +" ffff8805dfc08288 position: root/r/l/l/l/l/l vm_start = > 0x7f6fa09ae000 vm_end = 0x7f6fa09b2000", > +" ffff880540ec31b8 position: root/r/l/l/l/l/l/r vm_start = > 0x7f6fa09b2000 vm_end = 0x7f6fa09b3000", > +" ffff8805dfc08ca8 position: root/r/l/l/l/l vm_start = > 0x7f6fa09b3000 vm_end = 0x7f6fa09b4000", > +" ffff8800f78c15f0 position: root/r/l/l/l/l/r vm_start = > 0x7f6fa09b4000 vm_end = 0x7f6fa0b6c000", > +" ...", > +" ffff8805def651b8 position: root/r/r/r/r/l/l vm_start = > 0x7f6fa2f42000 vm_end = 0x7f6fa2f43000", > +" ffff880540d35e60 position: root/r/r/r/r/l vm_start = > 0x7f6fa2f43000 vm_end = 0x7f6fa2f44000", > +" ffff880540d35518 position: root/r/r/r/r/l/r vm_start = > 0x7f6fa2f44000 vm_end = 0x7f6fa2f85000", > +" ffff880540d356c8 position: root/r/r/r/r vm_start = > 0x7f6fa3185000 vm_end = 0x7f6fa3187000", > +" ffff8805def64000 position: root/r/r/r/r/r/l/l vm_start = > 0x7f6fa3187000 vm_end = 0x7f6fa3188000", > +" ffff880540d35cb0 position: root/r/r/r/r/r/l vm_start = > 0x7f6fa3188000 vm_end = 0x7f6fa3189000", > +" ffff8805def64798 position: root/r/r/r/r/r/l/r vm_start = > 0x7f6fa4bf9000 vm_end = 0x7f6fa4c1a000", > +" ffff880540d340d8 position: root/r/r/r/r/r vm_start = > 0x7ffc2b386000 vm_end = 0x7ffc2b3a8000", > +" ffff880540d35950 position: root/r/r/r/r/r/r vm_start = > 0x7ffc2b3c2000 vm_end = 0x7ffc2b3c4000", > +"", > +" Compared to the top/down order:\n", > +" %s> tree -ps vm_area_struct.vm_start,vm_end -o vm_area_struct.vm_rb -r > mm_struct.mm_rb 0xffff8805dee5e400 | sed 's/^ //' | paste - - - - | column > -ts' '", > +" ffff8805dfc08bd0 position: root vm_start = > 0x7f6fa07af000 vm_end = 0x7f6fa09ae000", > +" ffff8805e0b79b00 position: root/l vm_start = > 0x7f6f9f585000 vm_end = 0x7f6f9f785000", > +" ffff8805e0b79e60 position: root/l/l vm_start = > 0x7f6f9ec2b000 vm_end = 0x7f6f9ee2b000", > +" ffff8805e0b78bd0 position: root/l/l/l vm_start = > 0x7f6f9e2c1000 vm_end = 0x7f6f9e4c0000", > +" ffff8805e07f0bd0 position: root/l/l/l/l vm_start = > 0x7f6f9da92000 vm_end = 0x7f6f9dc91000", > +" ffff880540ec20d8 position: root/l/l/l/l/l vm_start = > 0x7f6f9d88c000 vm_end = 0x7f6f9da8b000", > +" ffff8805def64ca8 position: root/l/l/l/l/l/l vm_start = > 0x7f6f9d661000 vm_end = 0x7f6f9d860000", > +" ffff8805def64d80 position: root/l/l/l/l/l/l/l vm_start = > 0x7f6f9d454000 vm_end = 0x7f6f9d653000", > +" ffff880540d35d88 position: root/l/l/l/l/l/l/l/l vm_start = > 0x7f6f9d44c000 vm_end = 0x7f6f9d454000", > +" ffff8805e108f008 position: root/l/l/l/l/l/l/l/l/l vm_start = > 0x7f6f9ca5f000 vm_end = 0x7f6f9d44c000", > +" ffff8805e108e288 position: root/l/l/l/l/l/l/l/r vm_start = > 0x7f6f9d654000 vm_end = 0x7f6f9d655000", > +" ffff8805e0b46510 position: root/l/l/l/l/l/l/l/r/l vm_start = > 0x7f6f9d653000 vm_end = 0x7f6f9d654000", > +" ffff8805def65bd8 position: root/l/l/l/l/l/l/l/r/r vm_start = > 0x7f6f9d655000 vm_end = 0x7f6f9d661000", > +" ...", > +" ffff8805dfc08360 position: root/l/r/r vm_start = > 0x7f6fa0338000 vm_end = 0x7f6fa0538000", > +" ffff8805dfc096c8 position: root/l/r/r/l vm_start = > 0x7f6fa0010000 vm_end = 0x7f6fa020f000", > +" ffff880540ec2870 position: root/l/r/r/l/l vm_start = > 0x7f6f9ffe9000 vm_end = 0x7f6f9ffea000", > +" ffff8805dfc09440 position: root/l/r/r/l/l/l vm_start = > 0x7f6f9ffe8000 vm_end = 0x7f6f9ffe9000", > +" ffff8805dfc09290 position: root/l/r/r/l/l/r vm_start = > 0x7f6f9ffea000 vm_end = 0x7f6fa0010000", > +" ffff880540ec35f0 position: root/l/r/r/l/r vm_start = > 0x7f6fa0216000 vm_end = 0x7f6fa0217000", > +" ffff8805dfc08e58 position: root/l/r/r/l/r/l vm_start = > 0x7f6fa020f000 vm_end = 0x7f6fa0216000", > +" ffff8805dfc09cb0 position: root/l/r/r/l/r/r vm_start = > 0x7f6fa0217000 vm_end = 0x7f6fa0338000", > +" ffff880540d346c0 position: root/r vm_start = > 0x7f6fa1f5a000 vm_end = 0x7f6fa2159000", > +" ffff8800f78c10e0 position: root/r/l vm_start = > 0x7f6fa135f000 vm_end = 0x7f6fa155f000", > +" ffff8800f78c1440 position: root/r/l/l vm_start = > 0x7f6fa0d8d000 vm_end = 0x7f6fa0f8d000", > +" ffff8800f78c1950 position: root/r/l/l/l vm_start = > 0x7f6fa0b6c000 vm_end = 0x7f6fa0d6c000", > +" ffff8805dfc08ca8 position: root/r/l/l/l/l vm_start = > 0x7f6fa09b3000 vm_end = 0x7f6fa09b4000", > +" ffff8805dfc08288 position: root/r/l/l/l/l/l vm_start = > 0x7f6fa09ae000 vm_end = 0x7f6fa09b2000", > +" ffff880540ec31b8 position: root/r/l/l/l/l/l/r vm_start = > 0x7f6fa09b2000 vm_end = 0x7f6fa09b3000", > +" ffff8800f78c15f0 position: root/r/l/l/l/l/r vm_start = > 0x7f6fa09b4000 vm_end = 0x7f6fa0b6c000", > +" ...", > +" ffff880540d356c8 position: root/r/r/r/r vm_start = > 0x7f6fa3185000 vm_end = 0x7f6fa3187000", > +" ffff880540d35e60 position: root/r/r/r/r/l vm_start = > 0x7f6fa2f43000 vm_end = 0x7f6fa2f44000", > +" ffff8805def651b8 position: root/r/r/r/r/l/l vm_start = > 0x7f6fa2f42000 vm_end = 0x7f6fa2f43000", > +" ffff880540d35518 position: root/r/r/r/r/l/r vm_start = > 0x7f6fa2f44000 vm_end = 0x7f6fa2f85000", > +" ffff880540d340d8 position: root/r/r/r/r/r vm_start = > 0x7ffc2b386000 vm_end = 0x7ffc2b3a8000", > +" ffff880540d35cb0 position: root/r/r/r/r/r/l vm_start = > 0x7f6fa3188000 vm_end = 0x7f6fa3189000", > +" ffff8805def64000 position: root/r/r/r/r/r/l/l vm_start = > 0x7f6fa3187000 vm_end = 0x7f6fa3188000", > +" ffff8805def64798 position: root/r/r/r/r/r/l/r vm_start = > 0x7f6fa4bf9000 vm_end = 0x7f6fa4c1a000", > +" ffff880540d35950 position: root/r/r/r/r/r/r vm_start = > 0x7ffc2b3c2000 vm_end = 0x7ffc2b3c4000", > +"", > " Alternatively, take the address of the radix_tree_node from the", > " radix_tree_root structure in the address_space structure above,", > " and display the tree with the -N option:\n", > diff --git a/tools.c b/tools.c > index 992f4776281a..1970e8bb136b 100644 > --- a/tools.c > +++ b/tools.c > @@ -3926,7 +3926,7 @@ cmd_tree() > td = &tree_data; > BZERO(td, sizeof(struct tree_data)); > > - while ((c = getopt(argcnt, args, "xdt:r:o:s:S:pN")) != EOF) { > + while ((c = getopt(argcnt, args, "xdt:r:o:s:S:plN")) != EOF) { > switch (c) > { > case 't': > @@ -3993,6 +3993,10 @@ cmd_tree() > td->flags |= TREE_POSITION_DISPLAY; > break; > > + case 'l': > + td->flags |= TREE_LINEAR_ORDER; > + break; > + > case 'N': > td->flags |= TREE_NODE_POINTER; > break; > @@ -4399,6 +4403,13 @@ rbtree_iteration(ulong node_p, struct tree_data *td, > char *pos) > > struct_p = node_p - td->node_member_offset; > > + if (td->flags & TREE_LINEAR_ORDER && > + readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, > + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) { > + sprintf(new_pos, "%s/l", pos); > + rbtree_iteration(new_p, td, new_pos); > + } > + > if (td->flags & VERBOSE) > fprintf(fp, "%lx\n", struct_p); > > @@ -4430,7 +4441,8 @@ rbtree_iteration(ulong node_p, struct tree_data *td, > char *pos) > } > } > > - if ( readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, > + if (!(td->flags & TREE_LINEAR_ORDER) && > + readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, > sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) { > sprintf(new_pos, "%s/l", pos); > rbtree_iteration(new_p, td, new_pos); > -- > 2.16.2 > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://www.redhat.com/mailman/listinfo/crash-utility >

Dave Anderson

10:11 a.m.

New subject: [PATCH 2/2] tree: add an option to dump the tree sorted

----- Original Message -----

...

On Thu, Apr 12, 2018 at 3:57 PM, Dave Anderson <anderson(a)redhat.com> wrote: > > Daniel, > > Hmmm, when looking at your example, I just noticed it didn't use the -t > argument, > but then looking at the code realized that the command defaults to rbtrees. > I > actually don't think that was the original intent, but maybe that fact > should > should be indicated in the help page? Although I can't come up with a > compelling > argument why one should be the default. Either that, or -t should be > enforced. Wow. I didn't know this was not intended. I thought that red-black is default as radix trees are only used in address_space of mapped files (page cache), AFAIK. I usually do not need to specify the type as all the major trees are red-black - being it cgroups, scheduler entities, memory mappings, inodes, you name it. Again, I didn't change anything with regards to this behavior.

OK, that's a good point. So let's declare the default at top of the help page "-t type" segment.

...

> Anyway, you probably didn't notice that all help page output is constrained > to 80 columns. Your example command alone is 162 bytes long, making it way > too confusing to figure out what the command is doing. Can you just show > the command output without piping it to an external command, or make a much > simpler example? And also keep it to 80 columns or less, I was thinking about wrapping the command to a second line but that's what terminal will do anyways if narrower so I left it that way. The rest of the pipe is just formatting/pretty printing the table. Otherwise the output would be waaaay toooo loooong and not really human readable. I do not think it can get any simpler than that. I can get rid of one of the columns though, it does not need to contain both the start and the end of the VMA range. As an added value it also teaches eventual readers some advanced usage techniques in general so I believe it can be useful. There is enough simple examples already. Would you rather prefer something like this? crash> task -R mm PID: 28682 TASK: ffff880036d4af10 CPU: 0 COMMAND: "crash" mm = 0xffff880074b5be80, crash> tree -lp -o vm_area_struct.vm_rb -r mm_struct.mm_rb 0xffff880074b5be80 ffff88001f2c50e0 position: root/l/l/l/l/l ffff88001f2c5290 position: root/l/l/l/l ffff880074bfc6c0 position: root/l/l/l/l/r ffff88001f2c4bd0 position: root/l/l/l ffff880074bfc948 position: root/l/l/l/r/l/l ffff880036e54510 position: root/l/l/l/r/l/l/r ffff88001f2c5bd8 position: root/l/l/l/r/l ffff880036e54af8 position: root/l/l/l/r/l/r/l ffff880036e54f30 position: root/l/l/l/r/l/r ffff88000e06aa20 position: root/l/l/l/r/l/r/r ffff88000e06b368 position: root/l/l/l/r ffff88000e06ba28 position: root/l/l/l/r/r/l ffff88000e06ae58 position: root/l/l/l/r/r ffff88000e06a6c0 position: root/l/l/l/r/r/r ... ffff88001f2c51b8 position: root/l/r/r/r/l ffff88001f2c4d80 position: root/l/r/r/r ffff880074bfd878 position: root/l/r/r/r/r ffff88001f2c5a28 position: root ffff88001f2c4a20 position: root/r/l/l/l ffff88001f2c4360 position: root/r/l/l ffff880074bfcaf8 position: root/r/l/l/r ... ffff88001f2c5e60 position: root/r/r/l/l/l ffff88001f2c4ca8 position: root/r/r/l/l ffff88001f2c5008 position: root/r/r/l ffff88001f2c5d88 position: root/r/r/l/r ffff880074bfd6c8 position: root/r/r/l/r/r ffff88001f2c4288 position: root/r/r ffff88001f2c4510 position: root/r/r/r ffff88001f2c5b00 position: root/r/r/r/r This is not as readable in my opinion. And also it does not show why would one care about the order in the first place. See?

Yes, that's what I'm looking for. You can also remove the "task" command, and just say something in the leading description like, "Given an mm_struct address of 0xffff880074b5be80, ..." As far as caring about the order, you could add the "-s vm_area_struct.vm_start" option, and indicate that the -l option would display the vm_start addresses in an ascending value. That would obviously overflow the command line, but you could use a numeric argument for the -o argument as is done in a few other examples above that.

...

I do not believe a hard limit of 80 columns is useful for any serious kernel debugging. For developing and maintaining a source code, yes. For dumping a lot of debugging data in a human readable form, no.

I don't care about the length of any command output, I'm just talking about the output examples in the help page data. Thanks, Dave

Daniel Vacek

10:37 a.m.

New subject: [PATCH 2/2] tree: add an option to dump the tree sorted

On Thu, Apr 12, 2018 at 5:11 PM, Dave Anderson <anderson(a)redhat.com> wrote:

...

----- Original Message ----- > On Thu, Apr 12, 2018 at 3:57 PM, Dave Anderson <anderson(a)redhat.com> wrote: > > > > Daniel, > > > > Hmmm, when looking at your example, I just noticed it didn't use the -t > > argument, > > but then looking at the code realized that the command defaults to rbtrees. > > I > > actually don't think that was the original intent, but maybe that fact > > should > > should be indicated in the help page? Although I can't come up with a > > compelling > > argument why one should be the default. Either that, or -t should be > > enforced. > > Wow. I didn't know this was not intended. I thought that red-black is > default as radix trees are only used in address_space of mapped files > (page cache), AFAIK. I usually do not need to specify the type as all > the major trees are red-black - being it cgroups, scheduler entities, > memory mappings, inodes, you name it. > > Again, I didn't change anything with regards to this behavior. OK, that's a good point. So let's declare the default at top of the help page "-t type" segment. > > > Anyway, you probably didn't notice that all help page output is constrained > > to 80 columns. Your example command alone is 162 bytes long, making it way > > too confusing to figure out what the command is doing. Can you just show > > the command output without piping it to an external command, or make a much > > simpler example? And also keep it to 80 columns or less, > > I was thinking about wrapping the command to a second line but that's > what terminal will do anyways if narrower so I left it that way. The > rest of the pipe is just formatting/pretty printing the table. > Otherwise the output would be waaaay toooo loooong and not really > human readable. I do not think it can get any simpler than that. I can > get rid of one of the columns though, it does not need to contain both > the start and the end of the VMA range. > > As an added value it also teaches eventual readers some advanced usage > techniques in general so I believe it can be useful. There is enough > simple examples already. Would you rather prefer something like this? > > crash> task -R mm > PID: 28682 TASK: ffff880036d4af10 CPU: 0 COMMAND: "crash" > mm = 0xffff880074b5be80, > > crash> tree -lp -o vm_area_struct.vm_rb -r mm_struct.mm_rb 0xffff880074b5be80 > ffff88001f2c50e0 > position: root/l/l/l/l/l > ffff88001f2c5290 > position: root/l/l/l/l > ffff880074bfc6c0 > position: root/l/l/l/l/r > ffff88001f2c4bd0 > position: root/l/l/l > ffff880074bfc948 > position: root/l/l/l/r/l/l > ffff880036e54510 > position: root/l/l/l/r/l/l/r > ffff88001f2c5bd8 > position: root/l/l/l/r/l > ffff880036e54af8 > position: root/l/l/l/r/l/r/l > ffff880036e54f30 > position: root/l/l/l/r/l/r > ffff88000e06aa20 > position: root/l/l/l/r/l/r/r > ffff88000e06b368 > position: root/l/l/l/r > ffff88000e06ba28 > position: root/l/l/l/r/r/l > ffff88000e06ae58 > position: root/l/l/l/r/r > ffff88000e06a6c0 > position: root/l/l/l/r/r/r > ... > ffff88001f2c51b8 > position: root/l/r/r/r/l > ffff88001f2c4d80 > position: root/l/r/r/r > ffff880074bfd878 > position: root/l/r/r/r/r > ffff88001f2c5a28 > position: root > ffff88001f2c4a20 > position: root/r/l/l/l > ffff88001f2c4360 > position: root/r/l/l > ffff880074bfcaf8 > position: root/r/l/l/r > ... > ffff88001f2c5e60 > position: root/r/r/l/l/l > ffff88001f2c4ca8 > position: root/r/r/l/l > ffff88001f2c5008 > position: root/r/r/l > ffff88001f2c5d88 > position: root/r/r/l/r > ffff880074bfd6c8 > position: root/r/r/l/r/r > ffff88001f2c4288 > position: root/r/r > ffff88001f2c4510 > position: root/r/r/r > ffff88001f2c5b00 > position: root/r/r/r/r > > This is not as readable in my opinion. And also it does not show why > would one care about the order in the first place. See? Yes, that's what I'm looking for. You can also remove the "task" command, and just say something in the leading description like, "Given an mm_struct address of 0xffff880074b5be80, ..." As far as caring about the order, you could add the "-s vm_area_struct.vm_start" option, and indicate that the -l option would display the vm_start addresses in an ascending value. That would obviously overflow the command line, but you could use a numeric argument for the -o argument as is done in a few other examples above that. > I do not believe a hard limit of 80 columns is useful for any serious > kernel debugging. For developing and maintaining a source code, yes. > For dumping a lot of debugging data in a human readable form, no. I don't care about the length of any command output, I'm just talking about the output examples in the help page data.

I see. I'm gonna use the example from my last email. That's simple enough and clean. Will send v2 tomorrow. --nX

...

Thanks, Dave

Dave Anderson

Monday, 16 April Mon, 16 Apr

3:18 p.m.

New subject: [PATCH] speed up "ps -r" by storing the length of rlim array

----- Original Message -----

...

Without this patch, the "ps -r" command takes one minute or more per 1,000 tasks. The cause is that getting the length of {task,signal}_struct.rlim array takes some time and it is done for each task. This patch stores the value, and it will take only about 0.5 seconds per 1,000 tasks. Signed-off-by: Kazuhito Hagio <k-hagio ab jp nec com>

Nice -- this is a huge performance improvement! Queued for crash-7.2.2: https://github.com/crash-utility/crash/commit/6588de928a111dd10f74af2a624... Thanks, Dave

...

---> defs.h | 2 ++ symbols.c | 8 ++++++++ task.c | 6 ++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/defs.h b/defs.h index adddb9f..4b6ebc2 100644 --- a/defs.h +++ b/defs.h @@ -2191,6 +2191,8 @@ struct array_table { int kmem_cache_cpu_slab; int rt_prio_array_queue; int height_to_maxnodes; + int task_struct_rlim; + int signal_struct_rlim; }; /* diff --git a/symbols.c b/symbols.c index 638800a..e934e7f 100644 --- a/symbols.c +++ b/symbols.c @@ -8440,6 +8440,10 @@ builtin_array_length(char *s, int len, int *two_dim) lenptr = &array_table.kmem_cache_cpu_slab; else if (STREQ(s, "rt_prio_array.queue")) lenptr = &array_table.rt_prio_array_queue; + else if (STREQ(s, "task_struct.rlim")) + lenptr = &array_table.task_struct_rlim; + else if (STREQ(s, "signal_struct.rlim")) + lenptr = &array_table.signal_struct_rlim; if (!lenptr) /* not stored */ return(len); @@ -10520,6 +10524,10 @@ dump_offset_table(char *spec, ulong makestruct) ARRAY_LENGTH(kmem_cache_cpu_slab)); fprintf(fp, " rt_prio_array_queue: %d\n", ARRAY_LENGTH(rt_prio_array_queue)); + fprintf(fp, " task_struct_rlim: %d\n", + ARRAY_LENGTH(task_struct_rlim)); + fprintf(fp, " signal_struct_rlim: %d\n", + ARRAY_LENGTH(signal_struct_rlim)); if (spec) { int in_size_table, in_array_table, arrays, offsets, sizes; diff --git a/task.c b/task.c index 560adfa..2418e4c 100644 --- a/task.c +++ b/task.c @@ -4027,12 +4027,14 @@ show_task_rlimit(struct task_context *tc) in_task_struct = in_signal_struct = FALSE; if (VALID_MEMBER(task_struct_rlim)) { - rlimit_index = get_array_length("task_struct.rlim", NULL, 0); + rlimit_index = (i = ARRAY_LENGTH(task_struct_rlim)) ? + i : get_array_length("task_struct.rlim", NULL, 0); in_task_struct = TRUE; } else if (VALID_MEMBER(signal_struct_rlim)) { if (!VALID_MEMBER(task_struct_signal)) error(FATAL, "cannot determine rlimit array location\n"); - rlimit_index = get_array_length("signal_struct.rlim", NULL, 0); + rlimit_index = (i = ARRAY_LENGTH(signal_struct_rlim)) ? + i : get_array_length("signal_struct.rlim", NULL, 0); in_signal_struct = TRUE; } -- 1.8.3.1

Daniel Vacek

Thursday, 12 April Thu, 12 Apr

10:15 a.m.

New subject: [PATCH 2/2] tree: add an option to dump the tree sorted

On Thu, Apr 12, 2018 at 4:51 PM, Daniel Vacek <neelx(a)redhat.com> wrote:

...

On Thu, Apr 12, 2018 at 3:57 PM, Dave Anderson <anderson(a)redhat.com> wrote: > > Daniel, > > Hmmm, when looking at your example, I just noticed it didn't use the -t argument, > but then looking at the code realized that the command defaults to rbtrees. I > actually don't think that was the original intent, but maybe that fact should > should be indicated in the help page? Although I can't come up with a compelling > argument why one should be the default. Either that, or -t should be enforced. Wow. I didn't know this was not intended. I thought that red-black is default as radix trees are only used in address_space of mapped files (page cache), AFAIK. I usually do not need to specify the type as all the major trees are red-black - being it cgroups, scheduler entities, memory mappings, inodes, you name it. Again, I didn't change anything with regards to this behavior. > Anyway, you probably didn't notice that all help page output is constrained > to 80 columns. Your example command alone is 162 bytes long, making it way > too confusing to figure out what the command is doing. Can you just show > the command output without piping it to an external command, or make a much > simpler example? And also keep it to 80 columns or less, I was thinking about wrapping the command to a second line but that's what terminal will do anyways if narrower so I left it that way. The rest of the pipe is just formatting/pretty printing the table. Otherwise the output would be waaaay toooo loooong and not really human readable. I do not think it can get any simpler than that. I can get rid of one of the columns though, it does not need to contain both the start and the end of the VMA range.

Or even get rid of the position? Something as simple as this: crash> tree -ls vm_area_struct.vm_start -o vm_area_struct.vm_rb -r mm_struct.mm_rb 0xffff880074b5be80 | paste - - ffff88001f2c50e0 vm_start = 0x400000 ffff88001f2c5290 vm_start = 0xceb000 ffff880074bfc6c0 vm_start = 0xcec000 ffff88001f2c4bd0 vm_start = 0xd10000 ffff880074bfc948 vm_start = 0x1fe9000 ffff880036e54510 vm_start = 0x7ff6aa296000 ffff88001f2c5bd8 vm_start = 0x7ff6aa298000 ffff880036e54af8 vm_start = 0x7ff6aa497000 ffff880036e54f30 vm_start = 0x7ff6aa498000 ffff88000e06aa20 vm_start = 0x7ff6aa499000 ffff88000e06b368 vm_start = 0x7ff6ab95f000 ... ffff88001f2c5e60 vm_start = 0x7ff6bc1af000 ffff88001f2c4ca8 vm_start = 0x7ff6bc1b6000 ffff88001f2c5008 vm_start = 0x7ff6bc200000 ffff88001f2c5d88 vm_start = 0x7ff6bc205000 ffff880074bfd6c8 vm_start = 0x7ff6bc206000 ffff88001f2c4288 vm_start = 0x7ff6bc207000 ffff88001f2c4510 vm_start = 0x7ffc7a5fc000 ffff88001f2c5b00 vm_start = 0x7ffc7a6d1000 vs. crash> tree -s vm_area_struct.vm_start -o vm_area_struct.vm_rb -r mm_struct.mm_rb 0xffff880074b5be80 | paste - - ffff88001f2c5a28 vm_start = 0x7ff6bbbb9000 ffff88001f2c55f0 vm_start = 0x7ff6bb252000 ffff88000e06a360 vm_start = 0x7ff6ac6c3000 ffff88001f2c4bd0 vm_start = 0xd10000 ffff88001f2c5290 vm_start = 0xceb000 ffff88001f2c50e0 vm_start = 0x400000 ffff880074bfc6c0 vm_start = 0xcec000 ffff88000e06b368 vm_start = 0x7ff6ab95f000 ffff88001f2c5bd8 vm_start = 0x7ff6aa298000 ffff880074bfc948 vm_start = 0x1fe9000 ffff880036e54510 vm_start = 0x7ff6aa296000 ffff880036e54f30 vm_start = 0x7ff6aa498000 ffff880036e54af8 vm_start = 0x7ff6aa497000 ffff88000e06aa20 vm_start = 0x7ff6aa499000 ffff88000e06ae58 vm_start = 0x7ff6ac1df000 ffff88000e06ba28 vm_start = 0x7ff6abefc000 ffff88000e06a6c0 vm_start = 0x7ff6ac41b000 ffff88001f2c4000 vm_start = 0x7ff6bac75000 ffff88000e06bd88 vm_start = 0x7ff6b2d00000 ffff88000e06b440 vm_start = 0x7ff6b28de000 ... ffff880074bfd6c8 vm_start = 0x7ff6bc206000 ffff88001f2c4510 vm_start = 0x7ffc7a5fc000 ffff88001f2c5b00 vm_start = 0x7ffc7a6d1000 That would work, right? Though even with the position it may fit to 80... Still I do not see the point. --nX

...

As an added value it also teaches eventual readers some advanced usage techniques in general so I believe it can be useful. There is enough simple examples already. Would you rather prefer something like this? crash> task -R mm PID: 28682 TASK: ffff880036d4af10 CPU: 0 COMMAND: "crash" mm = 0xffff880074b5be80, crash> tree -lp -o vm_area_struct.vm_rb -r mm_struct.mm_rb 0xffff880074b5be80 ffff88001f2c50e0 position: root/l/l/l/l/l ffff88001f2c5290 position: root/l/l/l/l ffff880074bfc6c0 position: root/l/l/l/l/r ffff88001f2c4bd0 position: root/l/l/l ffff880074bfc948 position: root/l/l/l/r/l/l ffff880036e54510 position: root/l/l/l/r/l/l/r ffff88001f2c5bd8 position: root/l/l/l/r/l ffff880036e54af8 position: root/l/l/l/r/l/r/l ffff880036e54f30 position: root/l/l/l/r/l/r ffff88000e06aa20 position: root/l/l/l/r/l/r/r ffff88000e06b368 position: root/l/l/l/r ffff88000e06ba28 position: root/l/l/l/r/r/l ffff88000e06ae58 position: root/l/l/l/r/r ffff88000e06a6c0 position: root/l/l/l/r/r/r ... ffff88001f2c51b8 position: root/l/r/r/r/l ffff88001f2c4d80 position: root/l/r/r/r ffff880074bfd878 position: root/l/r/r/r/r ffff88001f2c5a28 position: root ffff88001f2c4a20 position: root/r/l/l/l ffff88001f2c4360 position: root/r/l/l ffff880074bfcaf8 position: root/r/l/l/r ... ffff88001f2c5e60 position: root/r/r/l/l/l ffff88001f2c4ca8 position: root/r/r/l/l ffff88001f2c5008 position: root/r/r/l ffff88001f2c5d88 position: root/r/r/l/r ffff880074bfd6c8 position: root/r/r/l/r/r ffff88001f2c4288 position: root/r/r ffff88001f2c4510 position: root/r/r/r ffff88001f2c5b00 position: root/r/r/r/r This is not as readable in my opinion. And also it does not show why would one care about the order in the first place. See? I do not believe a hard limit of 80 columns is useful for any serious kernel debugging. For developing and maintaining a source code, yes. For dumping a lot of debugging data in a human readable form, no. For example if I type 'timer -r' I already get an output which is over eighty. How would you copy/paste this to help? Or how do you feel about the output itself? CLOCK: 1 HRTIMER_CLOCK_BASE: ffff8805ed82d9a0 [ktime_get_real] CURRENT 1522436617507394900 SOFTEXPIRES EXPIRES HRTIMER FUNCTION 1522436624101007000 1522436624101057000 ffff8805e08ebd60 ffffffff810a95c0 <hrtimer_wakeup> 1522436628203647900 1522436628203697900 ffff8805e33fbd60 ffffffff810a95c0 <hrtimer_wakeup> 1522436635368413000 1522436635368463000 ffff8805e07cfd60 ffffffff810a95c0 <hrtimer_wakeup> 1522436652248574300 1522436652248624300 ffff8805e1acfd60 ffffffff810a95c0 <hrtimer_wakeup> 1522436652257417000 1522436652257467000 ffff8805e07a3d60 ffffffff810a95c0 <hrtimer_wakeup> 1522436664228364000 1522436664228414000 ffff8805e1347d60 ffffffff810a95c0 <hrtimer_wakeup> That's about the same width as my example. What do you suggest? > and put the "l" argument in the synopsis line at the top. Nah. I missed that one. Will fix it. Or you can just amend it yourself. --nX > Thanks, > Dave > > > > ----- Original Message ----- >> --- >> defs.h | 1 + >> help.c | 80 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> tools.c | 16 +++++++++++-- >> 3 files changed, 95 insertions(+), 2 deletions(-) >> >> diff --git a/defs.h b/defs.h >> index adddb9f2748d..ec298cbd70be 100644 >> --- a/defs.h >> +++ b/defs.h >> @@ -2480,6 +2480,7 @@ struct tree_data { >> #define TREE_STRUCT_RADIX_16 (VERBOSE << 6) >> #define TREE_PARSE_MEMBER (VERBOSE << 7) >> #define TREE_READ_MEMBER (VERBOSE << 8) >> +#define TREE_LINEAR_ORDER (VERBOSE << 9) >> >> #define ALIAS_RUNTIME (1) >> #define ALIAS_RCLOCAL (2) >> diff --git a/help.c b/help.c >> index c5cec5365962..e7df50fa2ef4 100644 >> --- a/help.c >> +++ b/help.c >> @@ -5698,6 +5698,8 @@ char *help_tree[] = { >> " indicates \"root/l/r\" means that the node is the right >> child", >> " of the left child of the root node. For radix trees, the >> height", >> " and slot index values are shown with respect to the root.", >> +" -l Dump the tree sorted in linear order starting with the >> leftmost", >> +" node and progressing to the right.", >> " ", >> " The meaning of the \"start\" argument, which can be expressed either in", >> " hexadecimal format or symbolically, depends upon whether the -N option", >> @@ -5823,6 +5825,84 @@ char *help_tree[] = { >> " ffffea000407de58", >> " position: root/3/28", >> "", >> +" List the tree in linear order from the leftmost node progressing", >> +" to the right using the -l option:\n", >> +" %s> tree -lps vm_area_struct.vm_start,vm_end -o vm_area_struct.vm_rb -r >> mm_struct.mm_rb 0xffff8805dee5e400 | sed 's/^ //' | paste - - - - | column >> -ts' '", >> +" ffff8805e108f008 position: root/l/l/l/l/l/l/l/l/l vm_start = >> 0x7f6f9ca5f000 vm_end = 0x7f6f9d44c000", >> +" ffff880540d35d88 position: root/l/l/l/l/l/l/l/l vm_start = >> 0x7f6f9d44c000 vm_end = 0x7f6f9d454000", >> +" ffff8805def64d80 position: root/l/l/l/l/l/l/l vm_start = >> 0x7f6f9d454000 vm_end = 0x7f6f9d653000", >> +" ffff8805e0b46510 position: root/l/l/l/l/l/l/l/r/l vm_start = >> 0x7f6f9d653000 vm_end = 0x7f6f9d654000", >> +" ffff8805e108e288 position: root/l/l/l/l/l/l/l/r vm_start = >> 0x7f6f9d654000 vm_end = 0x7f6f9d655000", >> +" ffff8805def65bd8 position: root/l/l/l/l/l/l/l/r/r vm_start = >> 0x7f6f9d655000 vm_end = 0x7f6f9d661000", >> +" ffff8805def64ca8 position: root/l/l/l/l/l/l vm_start = >> 0x7f6f9d661000 vm_end = 0x7f6f9d860000", >> +" ffff8805def641b0 position: root/l/l/l/l/l/l/r/l vm_start = >> 0x7f6f9d860000 vm_end = 0x7f6f9d861000", >> +" ffff8805def64438 position: root/l/l/l/l/l/l/r/l/r vm_start = >> 0x7f6f9d861000 vm_end = 0x7f6f9d862000", >> +" ffff8805def65368 position: root/l/l/l/l/l/l/r vm_start = >> 0x7f6f9d862000 vm_end = 0x7f6f9d868000", >> +" ffff880540ec2e58 position: root/l/l/l/l/l/l/r/r vm_start = >> 0x7f6f9d868000 vm_end = 0x7f6f9d88c000", >> +" ...", >> +" ffff8805dfc08e58 position: root/l/r/r/l/r/l vm_start = >> 0x7f6fa020f000 vm_end = 0x7f6fa0216000", >> +" ffff880540ec35f0 position: root/l/r/r/l/r vm_start = >> 0x7f6fa0216000 vm_end = 0x7f6fa0217000", >> +" ffff8805dfc09cb0 position: root/l/r/r/l/r/r vm_start = >> 0x7f6fa0217000 vm_end = 0x7f6fa0338000", >> +" ffff8805dfc08360 position: root/l/r/r vm_start = >> 0x7f6fa0338000 vm_end = 0x7f6fa0538000", >> +" ffff8805dfc08bd0 position: root vm_start = >> 0x7f6fa07af000 vm_end = 0x7f6fa09ae000", >> +" ffff8805dfc08288 position: root/r/l/l/l/l/l vm_start = >> 0x7f6fa09ae000 vm_end = 0x7f6fa09b2000", >> +" ffff880540ec31b8 position: root/r/l/l/l/l/l/r vm_start = >> 0x7f6fa09b2000 vm_end = 0x7f6fa09b3000", >> +" ffff8805dfc08ca8 position: root/r/l/l/l/l vm_start = >> 0x7f6fa09b3000 vm_end = 0x7f6fa09b4000", >> +" ffff8800f78c15f0 position: root/r/l/l/l/l/r vm_start = >> 0x7f6fa09b4000 vm_end = 0x7f6fa0b6c000", >> +" ...", >> +" ffff8805def651b8 position: root/r/r/r/r/l/l vm_start = >> 0x7f6fa2f42000 vm_end = 0x7f6fa2f43000", >> +" ffff880540d35e60 position: root/r/r/r/r/l vm_start = >> 0x7f6fa2f43000 vm_end = 0x7f6fa2f44000", >> +" ffff880540d35518 position: root/r/r/r/r/l/r vm_start = >> 0x7f6fa2f44000 vm_end = 0x7f6fa2f85000", >> +" ffff880540d356c8 position: root/r/r/r/r vm_start = >> 0x7f6fa3185000 vm_end = 0x7f6fa3187000", >> +" ffff8805def64000 position: root/r/r/r/r/r/l/l vm_start = >> 0x7f6fa3187000 vm_end = 0x7f6fa3188000", >> +" ffff880540d35cb0 position: root/r/r/r/r/r/l vm_start = >> 0x7f6fa3188000 vm_end = 0x7f6fa3189000", >> +" ffff8805def64798 position: root/r/r/r/r/r/l/r vm_start = >> 0x7f6fa4bf9000 vm_end = 0x7f6fa4c1a000", >> +" ffff880540d340d8 position: root/r/r/r/r/r vm_start = >> 0x7ffc2b386000 vm_end = 0x7ffc2b3a8000", >> +" ffff880540d35950 position: root/r/r/r/r/r/r vm_start = >> 0x7ffc2b3c2000 vm_end = 0x7ffc2b3c4000", >> +"", >> +" Compared to the top/down order:\n", >> +" %s> tree -ps vm_area_struct.vm_start,vm_end -o vm_area_struct.vm_rb -r >> mm_struct.mm_rb 0xffff8805dee5e400 | sed 's/^ //' | paste - - - - | column >> -ts' '", >> +" ffff8805dfc08bd0 position: root vm_start = >> 0x7f6fa07af000 vm_end = 0x7f6fa09ae000", >> +" ffff8805e0b79b00 position: root/l vm_start = >> 0x7f6f9f585000 vm_end = 0x7f6f9f785000", >> +" ffff8805e0b79e60 position: root/l/l vm_start = >> 0x7f6f9ec2b000 vm_end = 0x7f6f9ee2b000", >> +" ffff8805e0b78bd0 position: root/l/l/l vm_start = >> 0x7f6f9e2c1000 vm_end = 0x7f6f9e4c0000", >> +" ffff8805e07f0bd0 position: root/l/l/l/l vm_start = >> 0x7f6f9da92000 vm_end = 0x7f6f9dc91000", >> +" ffff880540ec20d8 position: root/l/l/l/l/l vm_start = >> 0x7f6f9d88c000 vm_end = 0x7f6f9da8b000", >> +" ffff8805def64ca8 position: root/l/l/l/l/l/l vm_start = >> 0x7f6f9d661000 vm_end = 0x7f6f9d860000", >> +" ffff8805def64d80 position: root/l/l/l/l/l/l/l vm_start = >> 0x7f6f9d454000 vm_end = 0x7f6f9d653000", >> +" ffff880540d35d88 position: root/l/l/l/l/l/l/l/l vm_start = >> 0x7f6f9d44c000 vm_end = 0x7f6f9d454000", >> +" ffff8805e108f008 position: root/l/l/l/l/l/l/l/l/l vm_start = >> 0x7f6f9ca5f000 vm_end = 0x7f6f9d44c000", >> +" ffff8805e108e288 position: root/l/l/l/l/l/l/l/r vm_start = >> 0x7f6f9d654000 vm_end = 0x7f6f9d655000", >> +" ffff8805e0b46510 position: root/l/l/l/l/l/l/l/r/l vm_start = >> 0x7f6f9d653000 vm_end = 0x7f6f9d654000", >> +" ffff8805def65bd8 position: root/l/l/l/l/l/l/l/r/r vm_start = >> 0x7f6f9d655000 vm_end = 0x7f6f9d661000", >> +" ...", >> +" ffff8805dfc08360 position: root/l/r/r vm_start = >> 0x7f6fa0338000 vm_end = 0x7f6fa0538000", >> +" ffff8805dfc096c8 position: root/l/r/r/l vm_start = >> 0x7f6fa0010000 vm_end = 0x7f6fa020f000", >> +" ffff880540ec2870 position: root/l/r/r/l/l vm_start = >> 0x7f6f9ffe9000 vm_end = 0x7f6f9ffea000", >> +" ffff8805dfc09440 position: root/l/r/r/l/l/l vm_start = >> 0x7f6f9ffe8000 vm_end = 0x7f6f9ffe9000", >> +" ffff8805dfc09290 position: root/l/r/r/l/l/r vm_start = >> 0x7f6f9ffea000 vm_end = 0x7f6fa0010000", >> +" ffff880540ec35f0 position: root/l/r/r/l/r vm_start = >> 0x7f6fa0216000 vm_end = 0x7f6fa0217000", >> +" ffff8805dfc08e58 position: root/l/r/r/l/r/l vm_start = >> 0x7f6fa020f000 vm_end = 0x7f6fa0216000", >> +" ffff8805dfc09cb0 position: root/l/r/r/l/r/r vm_start = >> 0x7f6fa0217000 vm_end = 0x7f6fa0338000", >> +" ffff880540d346c0 position: root/r vm_start = >> 0x7f6fa1f5a000 vm_end = 0x7f6fa2159000", >> +" ffff8800f78c10e0 position: root/r/l vm_start = >> 0x7f6fa135f000 vm_end = 0x7f6fa155f000", >> +" ffff8800f78c1440 position: root/r/l/l vm_start = >> 0x7f6fa0d8d000 vm_end = 0x7f6fa0f8d000", >> +" ffff8800f78c1950 position: root/r/l/l/l vm_start = >> 0x7f6fa0b6c000 vm_end = 0x7f6fa0d6c000", >> +" ffff8805dfc08ca8 position: root/r/l/l/l/l vm_start = >> 0x7f6fa09b3000 vm_end = 0x7f6fa09b4000", >> +" ffff8805dfc08288 position: root/r/l/l/l/l/l vm_start = >> 0x7f6fa09ae000 vm_end = 0x7f6fa09b2000", >> +" ffff880540ec31b8 position: root/r/l/l/l/l/l/r vm_start = >> 0x7f6fa09b2000 vm_end = 0x7f6fa09b3000", >> +" ffff8800f78c15f0 position: root/r/l/l/l/l/r vm_start = >> 0x7f6fa09b4000 vm_end = 0x7f6fa0b6c000", >> +" ...", >> +" ffff880540d356c8 position: root/r/r/r/r vm_start = >> 0x7f6fa3185000 vm_end = 0x7f6fa3187000", >> +" ffff880540d35e60 position: root/r/r/r/r/l vm_start = >> 0x7f6fa2f43000 vm_end = 0x7f6fa2f44000", >> +" ffff8805def651b8 position: root/r/r/r/r/l/l vm_start = >> 0x7f6fa2f42000 vm_end = 0x7f6fa2f43000", >> +" ffff880540d35518 position: root/r/r/r/r/l/r vm_start = >> 0x7f6fa2f44000 vm_end = 0x7f6fa2f85000", >> +" ffff880540d340d8 position: root/r/r/r/r/r vm_start = >> 0x7ffc2b386000 vm_end = 0x7ffc2b3a8000", >> +" ffff880540d35cb0 position: root/r/r/r/r/r/l vm_start = >> 0x7f6fa3188000 vm_end = 0x7f6fa3189000", >> +" ffff8805def64000 position: root/r/r/r/r/r/l/l vm_start = >> 0x7f6fa3187000 vm_end = 0x7f6fa3188000", >> +" ffff8805def64798 position: root/r/r/r/r/r/l/r vm_start = >> 0x7f6fa4bf9000 vm_end = 0x7f6fa4c1a000", >> +" ffff880540d35950 position: root/r/r/r/r/r/r vm_start = >> 0x7ffc2b3c2000 vm_end = 0x7ffc2b3c4000", >> +"", >> " Alternatively, take the address of the radix_tree_node from the", >> " radix_tree_root structure in the address_space structure above,", >> " and display the tree with the -N option:\n", >> diff --git a/tools.c b/tools.c >> index 992f4776281a..1970e8bb136b 100644 >> --- a/tools.c >> +++ b/tools.c >> @@ -3926,7 +3926,7 @@ cmd_tree() >> td = &tree_data; >> BZERO(td, sizeof(struct tree_data)); >> >> - while ((c = getopt(argcnt, args, "xdt:r:o:s:S:pN")) != EOF) { >> + while ((c = getopt(argcnt, args, "xdt:r:o:s:S:plN")) != EOF) { >> switch (c) >> { >> case 't': >> @@ -3993,6 +3993,10 @@ cmd_tree() >> td->flags |= TREE_POSITION_DISPLAY; >> break; >> >> + case 'l': >> + td->flags |= TREE_LINEAR_ORDER; >> + break; >> + >> case 'N': >> td->flags |= TREE_NODE_POINTER; >> break; >> @@ -4399,6 +4403,13 @@ rbtree_iteration(ulong node_p, struct tree_data *td, >> char *pos) >> >> struct_p = node_p - td->node_member_offset; >> >> + if (td->flags & TREE_LINEAR_ORDER && >> + readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, >> + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) { >> + sprintf(new_pos, "%s/l", pos); >> + rbtree_iteration(new_p, td, new_pos); >> + } >> + >> if (td->flags & VERBOSE) >> fprintf(fp, "%lx\n", struct_p); >> >> @@ -4430,7 +4441,8 @@ rbtree_iteration(ulong node_p, struct tree_data *td, >> char *pos) >> } >> } >> >> - if ( readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, >> + if (!(td->flags & TREE_LINEAR_ORDER) && >> + readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, >> sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR)) { >> sprintf(new_pos, "%s/l", pos); >> rbtree_iteration(new_p, td, new_pos); >> -- >> 2.16.2 >> >> -- >> Crash-utility mailing list >> Crash-utility(a)redhat.com >> https://www.redhat.com/mailman/listinfo/crash-utility >>

Daniel Vacek

Monday, 16 April Mon, 16 Apr

12:59 p.m.

New subject: [PATCH v2 0/3] couple of trivial tree patches

Daniel Vacek (3): tree: no need to bail out on tree corruption tree: add an option to dump the tree sorted tree: document that type defaults to rbtree defs.h | 1 + help.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- tools.c | 51 ++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 97 insertions(+), 15 deletions(-) -- 2.17.0

Daniel Vacek

12:59 p.m.

New subject: [PATCH v2 1/3] tree: no need to bail out on tree corruption

instead skip the node reporting a proper error and move on v2: add friendly error messages instead of generic readmem failures --- tools.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/tools.c b/tools.c index 186b703463a5..7c7f65ac558b 100644 --- a/tools.c +++ b/tools.c @@ -4374,8 +4374,8 @@ rbtree_iteration(ulong node_p, struct tree_data *td, char *pos) { int i; uint print_radix; - ulong struct_p, left_p, right_p; - char left_pos[BUFSIZE], right_pos[BUFSIZE]; + ulong struct_p, new_p, test_p; + char new_pos[BUFSIZE]; static struct req_entry **e; if (!node_p) @@ -4430,16 +4430,25 @@ rbtree_iteration(ulong node_p, struct tree_data *td, char *pos) } } - readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &left_p, - sizeof(void *), "rb_node rb_left", FAULT_ON_ERROR); - readmem(node_p+OFFSET(rb_node_rb_right), KVADDR, &right_p, - sizeof(void *), "rb_node rb_right", FAULT_ON_ERROR); - - sprintf(left_pos, "%s/l", pos); - sprintf(right_pos, "%s/r", pos); - - rbtree_iteration(left_p, td, left_pos); - rbtree_iteration(right_p, td, right_pos); + if ( readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR) && new_p) + if (readmem(new_p+OFFSET(rb_node_rb_left), KVADDR, &test_p, + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR|QUIET)) { + sprintf(new_pos, "%s/l", pos); + rbtree_iteration(new_p, td, new_pos); + } else + error(INFO, "rb_node: %lx: corrupted rb_left pointer: %lx\n", + node_p, new_p); + + if ( readmem(node_p+OFFSET(rb_node_rb_right), KVADDR, &new_p, + sizeof(void *), "rb_node rb_right", RETURN_ON_ERROR) && new_p) + if (readmem(new_p+OFFSET(rb_node_rb_left), KVADDR, &test_p, + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR|QUIET)) { + sprintf(new_pos, "%s/r", pos); + rbtree_iteration(new_p, td, new_pos); + } else + error(INFO, "rb_node: %lx: corrupted rb_right pointer: %lx\n", + node_p, new_p); } void -- 2.17.0

Daniel Vacek

12:59 p.m.

New subject: [PATCH v2 2/3] tree: add an option to dump the tree sorted

v2: limit example to 80 columns and fill -l in synopsis as well --- defs.h | 1 + help.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- tools.c | 20 ++++++++++++++++++-- 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/defs.h b/defs.h index adddb9f2748d..ec298cbd70be 100644 --- a/defs.h +++ b/defs.h @@ -2480,6 +2480,7 @@ struct tree_data { #define TREE_STRUCT_RADIX_16 (VERBOSE << 6) #define TREE_PARSE_MEMBER (VERBOSE << 7) #define TREE_READ_MEMBER (VERBOSE << 8) +#define TREE_LINEAR_ORDER (VERBOSE << 9) #define ALIAS_RUNTIME (1) #define ALIAS_RCLOCAL (2) diff --git a/help.c b/help.c index c5cec5365962..3f9a404ee3c0 100644 --- a/help.c +++ b/help.c @@ -5658,7 +5658,7 @@ NULL char *help_tree[] = { "tree", "display radix tree or red-black tree", -"-t [radix|rbtree] [-r offset] [-[s|S] struct[.member[,member]] -[x|d]]\n [-o offset] [-p] [-N] start", +"-t [radix|rbtree] [-r offset] [-[s|S] struct[.member[,member]] -[x|d]]\n [-o offset] [-p] [-l] [-N] start", " This command dumps the contents of a radix tree or a red-black tree.", " The arguments are as follows:\n", " -t type The type of tree to dump; the type string can be either ", @@ -5698,6 +5699,8 @@ char *help_tree[] = { " indicates \"root/l/r\" means that the node is the right child", " of the left child of the root node. For radix trees, the height", " and slot index values are shown with respect to the root.", +" -l Dump the tree sorted in linear order starting with the leftmost", +" node and progressing to the right.", " ", " The meaning of the \"start\" argument, which can be expressed either in", " hexadecimal format or symbolically, depends upon whether the -N option", @@ -5823,6 +5826,59 @@ char *help_tree[] = { " ffffea000407de58", " position: root/3/28", "", +" Given the mm_struct address of 0xffff880074b5be80 list the VMA tree in linear", +" order from the leftmost node progressing to the right using the -l option:\n", +" %s> tree -ls vm_area_struct.vm_start -o vm_area_struct.vm_rb \\", +" -r mm_struct.mm_rb 0xffff880074b5be80 | paste - -", +" ffff88001f2c50e0 vm_start = 0x400000", +" ffff88001f2c5290 vm_start = 0xceb000", +" ffff880074bfc6c0 vm_start = 0xcec000", +" ffff88001f2c4bd0 vm_start = 0xd10000", +" ffff880074bfc948 vm_start = 0x1fe9000", +" ffff880036e54510 vm_start = 0x7ff6aa296000", +" ffff88001f2c5bd8 vm_start = 0x7ff6aa298000", +" ffff880036e54af8 vm_start = 0x7ff6aa497000", +" ffff880036e54f30 vm_start = 0x7ff6aa498000", +" ffff88000e06aa20 vm_start = 0x7ff6aa499000", +" ffff88000e06b368 vm_start = 0x7ff6ab95f000", +" ...", +" ffff88001f2c5e60 vm_start = 0x7ff6bc1af000", +" ffff88001f2c4ca8 vm_start = 0x7ff6bc1b6000", +" ffff88001f2c5008 vm_start = 0x7ff6bc200000", +" ffff88001f2c5d88 vm_start = 0x7ff6bc205000", +" ffff880074bfd6c8 vm_start = 0x7ff6bc206000", +" ffff88001f2c4288 vm_start = 0x7ff6bc207000", +" ffff88001f2c4510 vm_start = 0x7ffc7a5fc000", +" ffff88001f2c5b00 vm_start = 0x7ffc7a6d1000", +"", +" Compared to the top/down root/leaves order:\n", +" %s> tree -s vm_area_struct.vm_start -o vm_area_struct.vm_rb \\", +" -r mm_struct.mm_rb 0xffff880074b5be80 | paste - -", +" ffff88001f2c5a28 vm_start = 0x7ff6bbbb9000", +" ffff88001f2c55f0 vm_start = 0x7ff6bb252000", +" ffff88000e06a360 vm_start = 0x7ff6ac6c3000", +" ffff88001f2c4bd0 vm_start = 0xd10000", +" ffff88001f2c5290 vm_start = 0xceb000", +" ffff88001f2c50e0 vm_start = 0x400000", +" ffff880074bfc6c0 vm_start = 0xcec000", +" ffff88000e06b368 vm_start = 0x7ff6ab95f000", +" ffff88001f2c5bd8 vm_start = 0x7ff6aa298000", +" ffff880074bfc948 vm_start = 0x1fe9000", +" ffff880036e54510 vm_start = 0x7ff6aa296000", +" ffff880036e54f30 vm_start = 0x7ff6aa498000", +" ffff880036e54af8 vm_start = 0x7ff6aa497000", +" ffff88000e06aa20 vm_start = 0x7ff6aa499000", +" ffff88000e06ae58 vm_start = 0x7ff6ac1df000", +" ffff88000e06ba28 vm_start = 0x7ff6abefc000", +" ffff88000e06a6c0 vm_start = 0x7ff6ac41b000", +" ffff88001f2c4000 vm_start = 0x7ff6bac75000", +" ffff88000e06bd88 vm_start = 0x7ff6b2d00000", +" ffff88000e06b440 vm_start = 0x7ff6b28de000", +" ...", +" ffff880074bfd6c8 vm_start = 0x7ff6bc206000", +" ffff88001f2c4510 vm_start = 0x7ffc7a5fc000", +" ffff88001f2c5b00 vm_start = 0x7ffc7a6d1000", +"", " Alternatively, take the address of the radix_tree_node from the", " radix_tree_root structure in the address_space structure above,", " and display the tree with the -N option:\n", diff --git a/tools.c b/tools.c index 7c7f65ac558b..b8e19e4284c5 100644 --- a/tools.c +++ b/tools.c @@ -3926,7 +3926,7 @@ cmd_tree() td = &tree_data; BZERO(td, sizeof(struct tree_data)); - while ((c = getopt(argcnt, args, "xdt:r:o:s:S:pN")) != EOF) { + while ((c = getopt(argcnt, args, "xdt:r:o:s:S:plN")) != EOF) { switch (c) { case 't': @@ -3993,6 +3993,10 @@ cmd_tree() td->flags |= TREE_POSITION_DISPLAY; break; + case 'l': + td->flags |= TREE_LINEAR_ORDER; + break; + case 'N': td->flags |= TREE_NODE_POINTER; break; @@ -4397,6 +4401,17 @@ rbtree_iteration(ulong node_p, struct tree_data *td, char *pos) else error(FATAL, "\nduplicate tree entry: %lx\n", node_p); + if (td->flags & TREE_LINEAR_ORDER && + readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR) && new_p) + if (readmem(new_p+OFFSET(rb_node_rb_left), KVADDR, &test_p, + sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR|QUIET)) { + sprintf(new_pos, "%s/l", pos); + rbtree_iteration(new_p, td, new_pos); + } else + error(INFO, "rb_node: %lx: corrupted rb_left pointer: %lx\n", + node_p, new_p); + struct_p = node_p - td->node_member_offset; if (td->flags & VERBOSE) @@ -4430,7 +4445,8 @@ rbtree_iteration(ulong node_p, struct tree_data *td, char *pos) } } - if ( readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, + if (!(td->flags & TREE_LINEAR_ORDER) && + readmem(node_p+OFFSET(rb_node_rb_left), KVADDR, &new_p, sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR) && new_p) if (readmem(new_p+OFFSET(rb_node_rb_left), KVADDR, &test_p, sizeof(void *), "rb_node rb_left", RETURN_ON_ERROR|QUIET)) { -- 2.17.0

Daniel Vacek

12:59 p.m.

New subject: [PATCH v2 3/3] tree: document that type defaults to rbtree

--- help.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/help.c b/help.c index 3f9a404ee3c0..2016b42f50c3 100644 --- a/help.c +++ b/help.c @@ -5664,7 +5664,7 @@ char *help_tree[] = { " The arguments are as follows:\n", " -t type The type of tree to dump; the type string can be either ", " \"radix\" or \"rbtree\", although only the first two characters", -" are required.", +" are required. If not specified, rbtree is the default type.", " -r offset If the \"start\" argument is the address of a data structure that", " contains the radix_tree_root or rb_root structure, then this is", " the offset to that structure member. If the offset is non-zero,", -- 2.17.0

Dave Anderson

Tuesday, 17 April Tue, 17 Apr

9:15 a.m.

New subject: [PATCH v2 0/3] couple of trivial tree patches

----- Original Message -----

...

Hi Daniel, This looks good. I fixed up the "make warn" issues for tools.c, and moved the new rbtree help page info up into the rbtree section. Queued for crash-7.2.2: https://github.com/crash-utility/crash/commit/90642e6ffa32aaa5aacd24871c8... Thanks, Dave

Daniel Vacek

Wednesday, 18 April Wed, 18 Apr

3:45 a.m.

New subject: [PATCH v2 0/3] couple of trivial tree patches

On Tue, Apr 17, 2018 at 4:15 PM, Dave Anderson <anderson(a)redhat.com> wrote:

...

----- Original Message ----- > Daniel Vacek (3): > tree: no need to bail out on tree corruption > tree: add an option to dump the tree sorted > tree: document that type defaults to rbtree > > defs.h | 1 + > help.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- > tools.c | 51 ++++++++++++++++++++++++++++++++++++++------------- > 3 files changed, 97 insertions(+), 15 deletions(-) > Hi Daniel, This looks good. I fixed up the "make warn" issues for tools.c, and moved the new rbtree help page info up into the rbtree section. Queued for crash-7.2.2: https://github.com/crash-utility/crash/commit/90642e6ffa32aaa5aacd24871c8...

Checking it I just realized that the help does not state that '-l' option actually only applies to redblack trees and the code also does not check and report misuse when '-l' is specified together with '-t ra'. --nX

...

Thanks, Dave

Daniel Vacek

6:19 a.m.

New subject: [PATCH] tree: help fixes and -t radix vs. -l mutual exclusivity check

--- help.c | 7 ++++--- tools.c | 11 +++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/help.c b/help.c index 5061c48dcdec..24700d05f1d6 100644 --- a/help.c +++ b/help.c @@ -5658,12 +5658,15 @@ NULL char *help_tree[] = { "tree", "display radix tree or red-black tree", -"-t [radix|rbtree] [-r offset] [-[s|S] struct[.member[,member]] -[x|d]]\n [-o offset] [-p] [-l] [-N] start", +"[-t {radix|rbtree}] [-l] [-p] [{-s|-S} struct[.member[,member]] [-x|-d]]\n [-o offset] [-r offset|-N] start", " This command dumps the contents of a radix tree or a red-black tree.", " The arguments are as follows:\n", " -t type The type of tree to dump; the type string can be either ", " \"radix\" or \"rbtree\", although only the first two characters", " are required. If not specified, rbtree is the default type.", +" -l For red-black trees, dump the tree sorted in linear order starting", +" with the leftmost node and progressing to the right. This option", +" does not apply to radix trees.", " -r offset If the \"start\" argument is the address of a data structure that", " contains the radix_tree_root or rb_root structure, then this is", " the offset to that structure member. If the offset is non-zero,", @@ -5698,8 +5701,6 @@ char *help_tree[] = { " indicates \"root/l/r\" means that the node is the right child", " of the left child of the root node. For radix trees, the height", " and slot index values are shown with respect to the root.", -" -l Dump the tree sorted in linear order starting with the leftmost", -" node and progressing to the right.", " ", " The meaning of the \"start\" argument, which can be expressed either in", " hexadecimal format or symbolically, depends upon whether the -N option", diff --git a/tools.c b/tools.c index cd8947147480..b09e564fd3cc 100644 --- a/tools.c +++ b/tools.c @@ -3946,6 +3946,10 @@ cmd_tree() break; + case 'l': + td->flags |= TREE_LINEAR_ORDER; + break; + case 'r': if (td->flags & TREE_ROOT_OFFSET_ENTERED) error(FATAL, @@ -3993,10 +3997,6 @@ cmd_tree() td->flags |= TREE_POSITION_DISPLAY; break; - case 'l': - td->flags |= TREE_LINEAR_ORDER; - break; - case 'N': td->flags |= TREE_NODE_POINTER; break; @@ -4023,6 +4023,9 @@ cmd_tree() if (argerrs) cmd_usage(pc->curcmd, SYNOPSIS); + if ((type_flag & RADIXTREE_REQUEST) && (td->flags |= TREE_LINEAR_ORDER)) + error(INFO, "-l option does not apply for radix trees, ignoring\n"); + if ((type_flag & RADIXTREE_REQUEST) && (td->flags & TREE_NODE_OFFSET_ENTERED)) error(FATAL, "-o option is not applicable to radix trees\n"); -- 2.17.0

Dave Anderson

8:41 a.m.

New subject: [PATCH] tree: help fixes and -t radix vs. -l mutual exclusivity check

----- Original Message ----- ... [ cut ] ...

...

diff --git a/tools.c b/tools.c index cd8947147480..b09e564fd3cc 100644 --- a/tools.c +++ b/tools.c @@ -3946,6 +3946,10 @@ cmd_tree() break; + case 'l': + td->flags |= TREE_LINEAR_ORDER; + break; + case 'r': if (td->flags & TREE_ROOT_OFFSET_ENTERED) error(FATAL, @@ -3993,10 +3997,6 @@ cmd_tree() td->flags |= TREE_POSITION_DISPLAY; break; - case 'l': - td->flags |= TREE_LINEAR_ORDER; - break; - case 'N': td->flags |= TREE_NODE_POINTER; break; @@ -4023,6 +4023,9 @@ cmd_tree() if (argerrs) cmd_usage(pc->curcmd, SYNOPSIS); + if ((type_flag & RADIXTREE_REQUEST) && (td->flags |= TREE_LINEAR_ORDER)) + error(INFO, "-l option does not apply for radix trees, ignoring\n"); +

Cut-and-paste error above, should be "& TREE_LINEAR_ORDER". Instead of ignoring it, I also made it FATAL like the following one:

...

if ((type_flag & RADIXTREE_REQUEST) && (td->flags & TREE_NODE_OFFSET_ENTERED)) error(FATAL, "-o option is not applicable to radix trees\n");

Queued for crash-7.2.2: https://github.com/crash-utility/crash/commit/9aa345148cc67ddaefb256e2c29... Thanks, Dave

Daniel Vacek

11:37 a.m.

New subject: [PATCH] tree: help fixes and -t radix vs. -l mutual exclusivity check

On Wed, Apr 18, 2018 at 3:41 PM, Dave Anderson <anderson(a)redhat.com> wrote:

...

----- Original Message ----- ... [ cut ] ... > diff --git a/tools.c b/tools.c > index cd8947147480..b09e564fd3cc 100644 > --- a/tools.c > +++ b/tools.c > @@ -3946,6 +3946,10 @@ cmd_tree() > > break; > > + case 'l': > + td->flags |= TREE_LINEAR_ORDER; > + break; > + > case 'r': > if (td->flags & TREE_ROOT_OFFSET_ENTERED) > error(FATAL, > @@ -3993,10 +3997,6 @@ cmd_tree() > td->flags |= TREE_POSITION_DISPLAY; > break; > > - case 'l': > - td->flags |= TREE_LINEAR_ORDER; > - break; > - > case 'N': > td->flags |= TREE_NODE_POINTER; > break; > @@ -4023,6 +4023,9 @@ cmd_tree() > if (argerrs) > cmd_usage(pc->curcmd, SYNOPSIS); > > + if ((type_flag & RADIXTREE_REQUEST) && (td->flags |= TREE_LINEAR_ORDER)) > + error(INFO, "-l option does not apply for radix trees, ignoring\n"); > + Cut-and-paste error above, should be "& TREE_LINEAR_ORDER". Instead of ignoring it, I also made it FATAL like the following one:

Oops, true. Sorry about that. Funny enough it did pass my testing.

...

> if ((type_flag & RADIXTREE_REQUEST) && (td->flags & TREE_NODE_OFFSET_ENTERED)) > error(FATAL, "-o option is not applicable to radix trees\n"); Queued for crash-7.2.2: https://github.com/crash-utility/crash/commit/9aa345148cc67ddaefb256e2c29...

Thanks --nX

...

Thanks, Dave

3048

days inactive

3054

days old

devel@lists.crash-utility.osci.io

Manage subscription

20 comments

2 participants

tags (0)

participants (2)

Daniel Vacek
Dave Anderson

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[PATCH 0/2] couple of trivial tree patches