[Crash-utility] [PATCH 0/3] Add a "percpu" command

[Dave Anderson]

----- Original Message -----
> Hi all,
> 
> a few people complained to me that the crash utility lacks a
> user-friendly interface for percpu variables, especially if they are
> dynamically allocated. I implemented a new command which should make
> these people happy.
> 
> Tested on kernel 3.11, but I didn't really add anything
> version-dependent (instead I used the existing infrastructure).
> 
> Comments welcome!
> 
> Petr Tesarik


Hi Petr,

First let me preface this by saying that I like this concept a lot.

A couple comments...

First, a minor nit -- "make warn" shows this:

 $ make warn
 ...
 cc -c -g -DX86_64  -DGDB_7_6  symbols.c -I./gdb-7.6/bfd -I./gdb-7.6/include -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security 
 symbols.c: In function 'cmd_percpu':
 symbols.c:6071:2: warning: implicit declaration of function 'isdigit' [-Wimplicit-function-declaration]
 symbols.c:6143:6: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
 ...

As a quick test, I created a command input file that looks like this:

 $ cat input
 percpu -a irq_stack_union
 percpu -a gdt_page
 percpu -a exception_stacks
 percpu -a cpu_llc_shared_map
 percpu -a cpu_core_map
 percpu -a cpu_sibling_map
 percpu -a cpu_llc_id
 percpu -a cpu_number
 percpu -a x86_bios_cpu_apicid
 percpu -a x86_cpu_to_apicid
 percpu -a cpu_loops_per_jiffy
 percpu -a xen_vcpu_info
 percpu -a xen_vcpu
 percpu -a idt_desc
 percpu -a shadow_tls_desc
 ...
  
where I displayed all of the per-cpu variables that are seen in 
the kernel's per-cpu symbol list, and it worked nicely.

Your help page shows this:

 crash> help percpu

 NAME
   percpu - percpu variables

 SYNOPSIS
   percpu [-dx][-a] [-c cpu] [cpu]... [struct|union|*] [struct_name] [address|symbol]

So my test used the "[-a]" and "[symbol]" arguments.  

But it's not clear to me when you would use the "[struct|union|*]", 
"[struct_name]" and/or the "[address]" arguments?  The remainder of 
the help page give no explanation of what they are, nor does it 
contain any examples of how they would be used.  I'm presuming that 
it has something to do with dynamically-allocated per-cpu variables?

Now, as to the functionality of the command, currently the "p" command 
does this when presented with a percpu symbol:
 
 crash> p idle_threads
 PER-CPU DATA TYPE:
   struct task_struct *idle_threads;
 PER-CPU ADDRESSES:
   [0]: ffff88021e20e360
   [1]: ffff88021e24e360
   [2]: ffff88021e28e360
   [3]: ffff88021e2ce360
 crash> 

So then you can cast the address as whatever the type is, and display it.
 
Your command pulls out the calculated per-cpu address and prints 
it exactly as the "p" command does, and then follows it with the
symbolic display:
 
 crash> percpu -a idle_threads
   [0]: ffff88021e20e360
 (struct task_struct *) 0xffffffff81c13440 <init_task>
   [1]: ffff88021e24e360
 (struct task_struct *) 0xffff88021282d330
   [2]: ffff88021e28e360
 (struct task_struct *) 0xffff88021282dac0
   [3]: ffff88021e2ce360
 (struct task_struct *) 0xffff88021282e250
 crash>

Since this patch essentially adds a command that does the
same thing as the "p" command does with regular symbols, I 
wonder if it would be better suited to be merged with the 
"p" command?  Did you try doing something like that?  (As you
are probably aware, getting me to add a new command is like
like pulling teeth...)

Nice job, though, this is really going to be a nice addition.

Thanks,
 Dave