>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.
Thanks,
Toshi.
>static long
>add_symbol_file_percpu(struct load_module *lm, struct gnu_request *req, long buflen)
>{
> char pbuf[BUFSIZE];
> int i;
> char *secname;
> long len;
>
> len = strlen(req->buf);
> for (i = 0; i < lm->mod_sections; i++) {
> secname = lm->mod_section_data[i].name;
> if ((lm->mod_section_data[i].flags & SEC_FOUND) &&
> (STREQ(secname, ".data.percpu") ||
> STREQ(secname, ".data..percpu"))) {
> sprintf(pbuf, " -s %s 0x%lx", secname, lm->mod_percpu);
> while ((len + strlen(pbuf)) >= buflen) {
> RESIZEBUF(req->buf, buflen, buflen * 2);
> buflen *= 2;
> }
> strcat(req->buf, pbuf);
> len += strlen(pbuf);
> }
> }
> return buflen;
>}
>
>The way you had it written, if RESIZEBUF() were to be called, it would never
>be seen by gdb, because req->buf would still point to the old buffer.
>
>Dave
>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
