Re: [Crash-utility] [PATCH 0/4] To support module percpu symbol

> To fix that, perhaps the syment structures for all module symbols could > have an indication that they are module-only symbols? There are a couple > of unused fields in the structure -- perhaps "pad1" could be changed to be > a "flags" field that could have a "MODULE_SYMBOL" bit set in it: > > struct syment { > ulong value; > char *name; > struct syment *val_hash_next; > struct syment *name_hash_next; > char type; > unsigned char cnt; > unsigned char pad1; > unsigned char pad2; > }; > > If that were true, then get_mod_percpu_sym_owner() could reject > any syment that doesn't have the MODULE_SYMBOL flag set, and then > there should be no problem with "symbol overlap". What do you > think about doing something like that? Sounds fine. pad1 is already used by patch_kernel_symbol(). I will rename pad2.

>Also, I don't quite understand why it was necessary in your patch to >modify cmd_p() like this: > > --- a/symbols.c > +++ b/symbols.c > @@ -5636,7 +5636,10 @@ cmd_p(void) > leader = strlen(buf2); > if (module_symbol(sp->value, NULL, NULL, NULL, *gdb_output_radix)) > do_load_module_filter = TRUE; > - } else if (st->flags & LOAD_MODULE_SYMS) > + } else if ((percpu_sp = per_cpu_symbol_search(args[optind])) && > + display_per_cpu_info(percpu_sp)) > + return; > + else if (st->flags & LOAD_MODULE_SYMS) > do_load_module_filter = TRUE; > > if (leader || do_load_module_filter) > > If the symbol could not be found by symbol_search() in line 5669, > then how could it be found later by the second per_cpu_symbol_search() > added above? Because per_cpu_symbol_search() can search percpu symbol with per_cpu__ prefix which is automatically renamed. sprintf(old, "per_cpu__%s", symbol); if ((sp = symbol_search(old))) return sp; This retry is valid for old kernel percpu symbol naming model. DEFINE_PER_CPU(struct kernel_stat, kstat); We try to enter "p per_cpu__kstat" in old kernel or "p kstat" in new kernel and further, enter "struct kernel_stat <virtual address>". Someone who does not know real DEFINE_PER_CPU(X) declared symbol name force to try "sym X -> possible alternatives" or check kernel code, System.map. I thought that per_cpu_symbol_search() aims to resolve UI gap between tow kernel percpu model from above implementation but present cmd_p() is not worked well because of symbol_search() failed. ("p kstat" works well in the old kernel with this patch)

> And for for that matter, in the very few modules that have percpu sections > in my test machines/dumpfiles, I don't see any actual per-cpu symbols listed > by "sym -[mM]". How do you actually "see" the name of a module's percpu symbol? Truth, I've also been touched first one when I tried to make KVM command (based on linux-2.6.35 & CONFIG_KVM=m) over private extension module. So I actually "see" the name of KVM modules. Old kernel dose not have such a module, so I made module for test. Although extension module want to obtain KVM percpu symbol addresses via crash symbol API, it can not answer. My work is blocked by module's per-cpu (CONFIG_KVM=y is not prefer solution). I send patch to crash utility with this background issue.

>Not really. But I don't see a problem with the way that you did it -- which >seems to work just fine. The only thing that needs to be changed is where >add_symbol_file_percpu() calls RESIZEBUF() -- it needs to have "req" passed >as an argument instead of "req->buf", like this: Oh, entire bug! I'll fix remained problems and send next patch with updated portion only. I would like to follow your advisement.