----- 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 -- Crash-utility mailing list [email protected] https://www.redhat.com/mailman/listinfo/crash-utility
