(2015/01/14 10:45), Namhyung Kim wrote: > On Mon, Jan 12, 2015 at 09:31:30PM +0900, Masami Hiramatsu wrote: >> (2015/01/10 19:33), Namhyung Kim wrote: >>> The find_probe_trace_events_from_map() searches matching symbol from a >>> map (so from a backing dso). For uprobes, it'll create a new map (and >>> dso) and loads it using a filter. It's a little bit inefficient in that >>> it'll read out the symbol table everytime but works well anyway. >>> >>> For kprobes however, it'll reuse existing kernel map which might be >>> loaded before. In this case map__load() just returns with no result. >>> It makes kprobes always failed to find symbol even if it exists in the >>> map (dso). >>> >>> To fix it, use map__find_symbol_by_name() instead. It'll load a map >>> with full symbols and sorts them by name. It needs to search sibing >>> nodes since there can be multiple (local) symbols with same name. Now >>> resulting symbol references are saved in the funcs list. >>> >>> Cc: Masami Hiramatsu <[email protected]> >>> Signed-off-by: Namhyung Kim <[email protected]> >>> --- >>> tools/perf/util/probe-event.c | 101 >>> ++++++++++++++++++++++++++++++++++++------ >>> 1 file changed, 87 insertions(+), 14 deletions(-) >>> >>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >>> index 7f9b8632e433..e5af16988791 100644 >>> --- a/tools/perf/util/probe-event.c >>> +++ b/tools/perf/util/probe-event.c >>> @@ -2191,20 +2191,86 @@ static int __add_probe_trace_events(struct >>> perf_probe_event *pev, >>> return ret; >>> } >>> >>> -static char *looking_function_name; >>> -static int num_matched_functions; >>> +struct symbol_entry { >>> + struct list_head node; >>> + struct symbol *sym; >>> +}; >>> >>> -static int probe_function_filter(struct map *map __maybe_unused, >>> - struct symbol *sym) >>> +/* returns 1 if symbol was added, 0 if symbol was skipped, -1 if error */ >>> +static int add_symbol_entry(struct symbol *sym, struct list_head *head) >>> { >>> - if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) && >>> - strcmp(looking_function_name, sym->name) == 0) { >>> - num_matched_functions++; >>> + struct symbol_entry *ent; >>> + >>> + if (sym->binding != STB_GLOBAL && sym->binding != STB_LOCAL) >>> return 0; >>> - } >>> + >>> + ent = malloc(sizeof(*ent)); >>> + if (ent == NULL) >>> + return -1; >> >> return -ENOMEM; ? > > Okay, will change. > > >> >>> + >>> + ent->sym = sym; >>> + list_add(&ent->node, head); >>> return 1; >>> } >>> >>> +static int find_probe_functions(struct map *map, char *name, struct >>> list_head *head) >>> +{ >>> + struct symbol *sym, *orig_sym; >>> + struct symbol_entry *ent; >>> + struct rb_node *node; >>> + int found = 0; >>> + int ret; >>> + >>> + sym = map__find_symbol_by_name(map, name, NULL); >>> + if (sym == NULL) >>> + return 0; >>> + >>> + ret = add_symbol_entry(sym, head); >>> + if (ret < 0) >>> + goto err; >>> + >>> + found += ret; >> >> If ret always be 1 in successful case, we'd better do "found++" here. >> And it also means we can do it shorter as below. >> >> if (add_symbol_entry(sym, head) < 0) >> goto err; >> found++; > > But it can return 0 in successful case like STB_WEAK.. I'm not sure > how we can handle the weak functions properly, but anyway the original > code already ignored the weak functions.
Ah, I see... OK, it should not be changed. >>> + >>> + /* search back and forth to find symbols that have same name */ >> >> Hmm, I see. but this code looks no-good sign... Can we have any generic >> synonym handling routine? > > Like what? I guess we can change map__find_symbol_by_name() to return > a list of symbols or add a new function to do it. Is it okay to you? Yeah, one possible solution is introducing map__find_all_symbols_by_name() for looking up all the symbols who have same name. Or, map__find_symbol_by_name() does forward search on rbtree to find the first symbol and returns it, so that caller just do backward search. Thank you, -- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
