[Crash-utility] [PATCH 2/2] tree: add an option to dump the tree sorted

[Dave Anderson]

----- Original Message -----
> On Thu, Apr 12, 2018 at 3:57 PM, Dave Anderson <anderson at 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