From: Hemant Kumar <hemant@linux.vnet.ibm.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
linux-kernel@vger.kernel.org, Namhyung Kim <namhyung@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Subject: Re: [PATCH perf/core v4 18/19] perf probe: Allow wildcard for cached events
Date: Wed, 27 Apr 2016 21:04:53 +0530 [thread overview]
Message-ID: <5720DC1D.5000800@linux.vnet.ibm.com> (raw)
In-Reply-To: <20160426090459.11891.41553.stgit@devbox>
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 <mhiramat@kernel.org>
> ---
> 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
next prev parent reply other threads:[~2016-04-27 15:35 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-26 9:02 [PATCH perf/core v4 00/19] perf-probe --cache and SDT support Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings Masami Hiramatsu
2016-04-26 13:36 ` Arnaldo Carvalho de Melo
2016-04-26 14:40 ` Masami Hiramatsu
2016-04-26 14:59 ` Arnaldo Carvalho de Melo
2016-04-27 18:44 ` Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid Masami Hiramatsu
2016-04-26 13:45 ` Arnaldo Carvalho de Melo
2016-04-26 14:47 ` Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache Masami Hiramatsu
2016-04-26 13:47 ` Arnaldo Carvalho de Melo
2016-04-26 14:42 ` Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory Masami Hiramatsu
2016-04-26 13:40 ` Arnaldo Carvalho de Melo
2016-04-26 14:07 ` Arnaldo Carvalho de Melo
2016-04-26 14:52 ` Masami Hiramatsu
2016-04-26 15:00 ` Arnaldo Carvalho de Melo
2016-04-27 15:35 ` [tip:perf/core] perf tools: Add lsdir() helper " tip-bot for Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 05/19] perf-buildid-cache: Use lsdir for looking up buildid caches Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 06/19] perf-probe: Let probe_file__add_event return 0 if succeeded Masami Hiramatsu
2016-04-26 13:49 ` Arnaldo Carvalho de Melo
2016-04-27 15:35 ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 07/19] perf probe: Add --cache option to cache the probe definitions Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 08/19] perf probe: Use cache entry if possible Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 09/19] perf probe: Show all cached probes Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 10/19] perf probe: Remove caches when --cache is given Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 11/19] perf/sdt: ELF support for SDT Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 12/19] perf probe: Add group name support Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 13/19] perf-probe: Set default kprobe group name if it is not given Masami Hiramatsu
2016-04-26 13:50 ` Arnaldo Carvalho de Melo
2016-04-27 15:35 ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 14/19] perf buildid-cache: Scan and import user SDT events to probe cache Masami Hiramatsu
2016-04-27 15:19 ` Hemant Kumar
2016-04-27 15:28 ` Arnaldo Carvalho de Melo
2016-04-27 19:36 ` Masami Hiramatsu
2016-04-27 20:23 ` Hemant Kumar
2016-04-27 20:16 ` Hemant Kumar
2016-04-26 9:04 ` [PATCH perf/core v4 15/19] perf probe: Accept %sdt and %cached event name Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 16/19] perf-list: Show SDT and pre-cached events Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 17/19] perf-list: Skip SDTs placed in invalid binaries Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 18/19] perf probe: Allow wildcard for cached events Masami Hiramatsu
2016-04-27 15:34 ` Hemant Kumar [this message]
2016-04-27 18:51 ` Masami Hiramatsu
2016-04-26 9:05 ` [PATCH perf/core v4 19/19] perf probe: Support @BUILDID or @FILE suffix for SDT events Masami Hiramatsu
2016-04-27 15:36 ` [PATCH perf/core v4 00/19] perf-probe --cache and SDT support Hemant Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5720DC1D.5000800@linux.vnet.ibm.com \
--to=hemant@linux.vnet.ibm.com \
--cc=acme@kernel.org \
--cc=ananth@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.