Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753561AbcD0PfR (ORCPT ); Wed, 27 Apr 2016 11:35:17 -0400 Received: from e28smtp09.in.ibm.com ([125.16.236.9]:34801 "EHLO e28smtp09.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753087AbcD0PfL (ORCPT ); Wed, 27 Apr 2016 11:35:11 -0400 X-IBM-Helo: d28relay10.in.ibm.com X-IBM-MailFrom: hemant@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Message-ID: <5720DC1D.5000800@linux.vnet.ibm.com> Date: Wed, 27 Apr 2016 21:04:53 +0530 From: Hemant Kumar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Masami Hiramatsu CC: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Namhyung Kim , Peter Zijlstra , Ingo Molnar , Ananth N Mavinakayanahalli Subject: Re: [PATCH perf/core v4 18/19] perf probe: Allow wildcard for cached events References: <20160426090200.11891.43944.stgit@devbox> <20160426090459.11891.41553.stgit@devbox> In-Reply-To: <20160426090459.11891.41553.stgit@devbox> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable x-cbid: 16042715-0041-0000-0000-00000C664524 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10911 Lines: 354 On 04/26/2016 02:34 PM, Masami Hiramatsu wrote: > Allo glob wildcard for reusing cached/SDT events. This also > automatically find the target binaries, e.g. > > # perf probe -a %sdt_libc:\* > > This example adds probes for all SDT in libc. > Note that the SDTs must have been scanned by perf buildid-cache. There is a segfault after applying this patch : # perf probe -x /home/hemant/test %marker1 Segmentation fault (core dumped) The problem is there is no group name with this marker and strglobmatch tries to match entry->pev.group with pev->group (which is NULL). > Signed-off-by: Masami Hiramatsu > --- > tools/perf/util/probe-event.c | 153 ++++++++++++++++++++++++++++++++++++++++- > tools/perf/util/probe-event.h | 1 > tools/perf/util/probe-file.c | 33 ++++++++- > tools/perf/util/probe-file.h | 5 + > 4 files changed, 183 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 1128631..3bd81f0 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -236,7 +236,7 @@ static void clear_perf_probe_point(struct perf_probe_point *pp) > free(pp->lazy_line); > } > > -static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs) > +void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs) > { > int i; > > @@ -1151,7 +1151,7 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev) > ptr = strchr(*arg, ':'); > if (ptr) { > *ptr = '\0'; > - if (!is_c_func_name(*arg)) > + if (!pev->sdt && !is_c_func_name(*arg)) > goto ng_name; > pev->group = strdup(*arg); > if (!pev->group) > @@ -1159,7 +1159,7 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev) > *arg = ptr + 1; > } else > pev->group = NULL; > - if (!is_c_func_name(*arg)) { > + if (!pev->sdt && !is_c_func_name(*arg)) { > ng_name: > semantic_error("%s is bad for event name -it must " > "follow C symbol-naming rule.\n", *arg); > @@ -1585,6 +1585,11 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev) > p = strchr(argv[1], ':'); > if (p) { > tp->module = strndup(argv[1], p - argv[1]); > + if (!tp->module) { > + ret = -ENOMEM; > + goto out; > + } > + tev->uprobes = (tp->module[0] == '/'); > p++; > } else > p = argv[1]; > @@ -2430,7 +2435,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev, > int ret; > > /* If probe_event or trace_event already have the name, reuse it */ > - if (pev->event) > + if (pev->event && !pev->sdt) > event = pev->event; > else if (tev->event) > event = tev->event; > @@ -2443,7 +2448,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev, > else > event = tev->point.realname; > } > - if (pev->group) > + if (pev->group && !pev->sdt) > group = pev->group; > else if (tev->group) > group = tev->group; > @@ -2794,6 +2799,137 @@ errout: > > bool __weak arch__prefers_symtab(void) { return false; } > > +/* Concatinate two arrays */ > +static void *memcat(void *a, size_t sz_a, void *b, size_t sz_b) > +{ > + void *ret; > + > + ret = malloc(sz_a + sz_b); > + if (ret) { > + memcpy(ret, a, sz_a); > + memcpy(ret + sz_a, b, sz_b); > + } > + return ret; > +} > + > +static int > +concat_probe_trace_events(struct probe_trace_event **tevs, int *ntevs, > + struct probe_trace_event **tevs2, int ntevs2) > +{ > + struct probe_trace_event *new_tevs; > + int ret = 0; > + > + if (ntevs == 0) { > + *tevs = *tevs2; > + *ntevs = ntevs2; > + *tevs2 = NULL; > + return 0; > + } > + > + if (*ntevs + ntevs2 > probe_conf.max_probes) > + ret = -E2BIG; > + else { > + /* Concatinate the array of probe_trace_event */ > + new_tevs = memcat(*tevs, (*ntevs) * sizeof(**tevs), > + *tevs2, ntevs2 * sizeof(**tevs2)); > + if (!new_tevs) > + ret = -ENOMEM; > + else { > + free(*tevs); > + *tevs = new_tevs; > + *ntevs += ntevs2; > + } > + } > + if (ret < 0) > + clear_probe_trace_events(*tevs2, ntevs2); > + zfree(tevs2); > + > + return ret; > +} > + > +/* Try to find probe_trace_event from given probe caches */ > +static int find_cached_events(struct perf_probe_event *pev, > + struct probe_trace_event **tevs, > + const char *target) > +{ > + struct probe_cache *cache; > + struct probe_cache_entry *entry; > + struct probe_trace_event *tmp_tevs = NULL; > + int ntevs = 0; > + int ret = 0; > + > + cache = probe_cache__new(target); > + if (!cache) > + return -ENOENT; > + > + for_each_probe_cache_entry(entry, cache) { > + /* Skip the cache entry which has no name */ > + if (!entry->pev.event || !entry->pev.group) > + continue; > + if (strglobmatch(entry->pev.group, pev->group) && > + strglobmatch(entry->pev.event, pev->event)) { Due to the above issue, we can change the above condition to : if (strglobmatch(entry->pev.event, pev->event) { if (!pev->group || (pev->group && strglobmatch(entry->pev.group, pev->group))) // Do stuff } > + ret = probe_cache_entry__get_event(entry, &tmp_tevs); > + if (ret > 0) > + ret = concat_probe_trace_events(tevs, &ntevs, > + &tmp_tevs, ret); > + if (ret < 0) > + break; > + } > + } > + probe_cache__delete(cache); > + if (ret < 0) { > + clear_probe_trace_events(*tevs, ntevs); > + zfree(tevs); > + } else { > + ret = ntevs; > + if (target[0] == '/') > + pev->uprobes = true; > + } > + > + return ret; > +} > + > +/* Try to find probe_trace_event from all probe caches */ > +static int find_cached_events_all(struct perf_probe_event *pev, > + struct probe_trace_event **tevs) > +{ > + struct probe_trace_event *tmp_tevs = NULL; > + struct strlist *bidlist; > + struct str_node *nd; > + char *pathname; > + int ntevs = 0; > + int ret; > + > + /* Get the buildid list of all valid caches */ > + ret = build_id_cache__list_all(&bidlist, true); > + if (ret < 0) { > + pr_debug("Failed to get buildids: %d\n", ret); > + return ret; > + } > + > + ret = 0; > + strlist__for_each(nd, bidlist) { > + pathname = build_id_cache__origname(nd->s); > + ret = find_cached_events(pev, &tmp_tevs, pathname); > + if (ret > 0) > + ret = concat_probe_trace_events(tevs, &ntevs, > + &tmp_tevs, ret); > + /* In the case of cnt == 0, we just skip it */ > + free(pathname); > + if (ret < 0) > + break; > + } > + strlist__delete(bidlist); > + > + if (ret < 0) { > + clear_probe_trace_events(*tevs, ntevs); > + zfree(tevs); > + } else > + ret = ntevs; > + > + return ret; > +} > + > static int find_probe_trace_events_from_cache(struct perf_probe_event *pev, > struct probe_trace_event **tevs) > { > @@ -2803,6 +2939,13 @@ static int find_probe_trace_events_from_cache(struct perf_probe_event *pev, > struct str_node *node; > int ret, i; > > + if (pev->sdt) { > + /* For SDT/cached events, we use special search functions */ > + if (!pev->target) > + return find_cached_events_all(pev, tevs); > + else > + return find_cached_events(pev, tevs, pev->target); > + } > cache = probe_cache__new(pev->target); > if (!cache) > return 0; > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h > index 2a23efe..39b5a35 100644 > --- a/tools/perf/util/probe-event.h > +++ b/tools/perf/util/probe-event.h > @@ -134,6 +134,7 @@ bool perf_probe_event_need_dwarf(struct perf_probe_event *pev); > /* Release event contents */ > void clear_perf_probe_event(struct perf_probe_event *pev); > void clear_probe_trace_event(struct probe_trace_event *tev); > +void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs); > > /* Command string to line-range */ > int parse_line_range_desc(const char *cmd, struct line_range *lr); > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c > index 2437b48..896d645 100644 > --- a/tools/perf/util/probe-file.c > +++ b/tools/perf/util/probe-file.c > @@ -360,6 +360,31 @@ probe_cache_entry__new(struct perf_probe_event *pev) > return ret; > } > > +int probe_cache_entry__get_event(struct probe_cache_entry *entry, > + struct probe_trace_event **tevs) > +{ > + struct probe_trace_event *tev; > + struct str_node *node; > + int ret, i; > + > + ret = strlist__nr_entries(entry->tevlist); > + if (ret > probe_conf.max_probes) > + return -E2BIG; > + > + *tevs = zalloc(ret * sizeof(*tev)); > + if (!*tevs) > + return -ENOMEM; > + > + i = 0; > + strlist__for_each(node, entry->tevlist) { > + tev = &(*tevs)[i++]; > + ret = parse_probe_trace_command(node->s, tev); > + if (ret < 0) > + break; > + } > + return i; > +} > + > /* For the kernel probe caches, pass target = NULL */ > static int probe_cache__open(struct probe_cache *pcache, const char *target) > { > @@ -528,7 +553,7 @@ probe_cache__find(struct probe_cache *pcache, struct perf_probe_event *pev) > if (!cmd) > return NULL; > > - list_for_each_entry(entry, &pcache->list, list) { > + for_each_probe_cache_entry(entry, pcache) { > if (pev->sdt) { > if (entry->pev.event && > streql(entry->pev.event, pev->event) && > @@ -558,7 +583,7 @@ probe_cache__find_by_name(struct probe_cache *pcache, > { > struct probe_cache_entry *entry = NULL; > > - list_for_each_entry(entry, &pcache->list, list) { > + for_each_probe_cache_entry(entry, pcache) { > /* Hit if same event name or same command-string */ > if (streql(entry->pev.group, group) && > streql(entry->pev.event, event)) > @@ -711,7 +736,7 @@ int probe_cache__commit(struct probe_cache *pcache) > if (ret < 0) > goto out; > > - list_for_each_entry(entry, &pcache->list, list) { > + for_each_probe_cache_entry(entry, pcache) { > ret = probe_cache_entry__write(entry, pcache->fd); > pr_debug("Cache committed: %d\n", ret); > if (ret < 0) > @@ -750,7 +775,7 @@ static int probe_cache__show_entries(struct probe_cache *pcache, > { > struct probe_cache_entry *entry; > > - list_for_each_entry(entry, &pcache->list, list) { > + for_each_probe_cache_entry(entry, pcache) { > if (probe_cache_entry__compare(entry, filter)) > printf("%s\n", entry->spev); > } > diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h > index 55322f1..a02bbbd 100644 > --- a/tools/perf/util/probe-file.h > +++ b/tools/perf/util/probe-file.h > @@ -32,6 +32,11 @@ struct probe_cache { > struct list_head list; > }; > > +int probe_cache_entry__get_event(struct probe_cache_entry *entry, > + struct probe_trace_event **tevs); > +#define for_each_probe_cache_entry(entry, pcache) \ > + list_for_each_entry(entry, &pcache->list, list) > + > struct probe_cache *probe_cache__new(const char *target); > int probe_cache__add_entry(struct probe_cache *pcache, > struct perf_probe_event *pev, > -- Thanks, Hemant Kumar