* [PATCH 1/2] perf: Fix checking of duplicate probe to give proper hint @ 2020-02-25 14:41 zhe.he 2020-02-25 14:41 ` [PATCH 2/2] perf: probe-file: Check return value of strlist__add zhe.he 2020-02-25 22:34 ` [PATCH 1/2] perf: Fix checking of duplicate probe to give proper hint Masami Hiramatsu 0 siblings, 2 replies; 6+ messages in thread From: zhe.he @ 2020-02-25 14:41 UTC (permalink / raw) To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mhiramat, kstewart, tglx, linux-kernel, zhe.he From: He Zhe <zhe.he@windriver.com> Since commit 72363540c009 ("perf probe: Support multiprobe event") and its series, if there are multiple probes for one event, __probe_file__get_namelist would return -EEXIST and cause the following failure without proper hint, due to adding existing entry to output list. root@qemuarm64:~# perf probe -x /lib64/libc.so.6 free Added new events: probe_libc:free (on free in /lib64/libc-2.31.so) probe_libc:free (on free in /lib64/libc-2.31.so) You can now use it in all perf tools, such as: perf record -e probe_libc:free -aR sleep 1 root@qemuarm64:~# perf probe -l probe_libc:free (on free@plt in /lib64/libc-2.31.so) probe_libc:free (on cfree in /lib64/libc-2.31.so) root@qemuarm64:~# perf probe -x /lib64/libc.so.6 free Error: Failed to add events. As we just want to check if there is any probe with the same name, -EEXIST can be ignored, so we can have the right hint as before. root@qemuarm64:~# perf probe -x /lib64/libc.so.6 free Error: event "free" already exists. Hint: Remove existing event by 'perf probe -d' or force duplicates by 'perf probe -f' or set 'force=yes' in BPF source. Error: Failed to add events. Signed-off-by: He Zhe <zhe.he@windriver.com> --- tools/perf/util/probe-file.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c index 5003ba403345..cf44c05f89c1 100644 --- a/tools/perf/util/probe-file.c +++ b/tools/perf/util/probe-file.c @@ -201,10 +201,16 @@ static struct strlist *__probe_file__get_namelist(int fd, bool include_group) if (include_group) { ret = e_snprintf(buf, 128, "%s:%s", tev.group, tev.event); - if (ret >= 0) + if (ret >= 0) { ret = strlist__add(sl, buf); - } else + if (ret == -EEXIST) + ret = 0; + } + } else { ret = strlist__add(sl, tev.event); + if (ret == -EEXIST) + ret = 0; + } clear_probe_trace_event(&tev); if (ret < 0) break; -- 2.24.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] perf: probe-file: Check return value of strlist__add 2020-02-25 14:41 [PATCH 1/2] perf: Fix checking of duplicate probe to give proper hint zhe.he @ 2020-02-25 14:41 ` zhe.he 2020-02-25 22:49 ` Masami Hiramatsu 2020-02-25 22:34 ` [PATCH 1/2] perf: Fix checking of duplicate probe to give proper hint Masami Hiramatsu 1 sibling, 1 reply; 6+ messages in thread From: zhe.he @ 2020-02-25 14:41 UTC (permalink / raw) To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, mhiramat, kstewart, tglx, linux-kernel, zhe.he From: He Zhe <zhe.he@windriver.com> strlist__add may fail with -ENOMEM or -EEXIST. Check it and give debugging hint when necessary. Signed-off-by: He Zhe <zhe.he@windriver.com> --- tools/perf/builtin-probe.c | 30 ++++++++++++++++-------------- tools/perf/util/probe-file.c | 26 +++++++++++++++++++++----- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c index 26bc5923e6b5..8b4511c70fed 100644 --- a/tools/perf/builtin-probe.c +++ b/tools/perf/builtin-probe.c @@ -442,24 +442,26 @@ static int perf_del_probe_events(struct strfilter *filter) } ret = probe_file__get_events(kfd, filter, klist); - if (ret == 0) { - strlist__for_each_entry(ent, klist) - pr_info("Removed event: %s\n", ent->s); + if (ret < 0) + goto out; - ret = probe_file__del_strlist(kfd, klist); - if (ret < 0) - goto error; - } + strlist__for_each_entry(ent, klist) + pr_info("Removed event: %s\n", ent->s); + + ret = probe_file__del_strlist(kfd, klist); + if (ret < 0) + goto error; ret2 = probe_file__get_events(ufd, filter, ulist); - if (ret2 == 0) { - strlist__for_each_entry(ent, ulist) - pr_info("Removed event: %s\n", ent->s); + if (ret2 < 0) + goto out; - ret2 = probe_file__del_strlist(ufd, ulist); - if (ret2 < 0) - goto error; - } + strlist__for_each_entry(ent, ulist) + pr_info("Removed event: %s\n", ent->s); + + ret2 = probe_file__del_strlist(ufd, ulist); + if (ret2 < 0) + goto error; if (ret == -ENOENT && ret2 == -ENOENT) pr_warning("\"%s\" does not hit any event.\n", str); diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c index cf44c05f89c1..00f086cba88f 100644 --- a/tools/perf/util/probe-file.c +++ b/tools/perf/util/probe-file.c @@ -307,10 +307,14 @@ int probe_file__get_events(int fd, struct strfilter *filter, p = strchr(ent->s, ':'); if ((p && strfilter__compare(filter, p + 1)) || strfilter__compare(filter, ent->s)) { - strlist__add(plist, ent->s); - ret = 0; + ret = strlist__add(plist, ent->s); + if (ret < 0) { + pr_debug("strlist__add failed (%d)\n", ret); + goto out; + } } } +out: strlist__delete(namelist); return ret; @@ -517,7 +521,11 @@ static int probe_cache__load(struct probe_cache *pcache) ret = -EINVAL; goto out; } - strlist__add(entry->tevlist, buf); + ret = strlist__add(entry->tevlist, buf); + if (ret < 0) { + pr_debug("strlist__add failed (%d)\n", ret); + goto out; + } } } out: @@ -678,7 +686,12 @@ int probe_cache__add_entry(struct probe_cache *pcache, command = synthesize_probe_trace_command(&tevs[i]); if (!command) goto out_err; - strlist__add(entry->tevlist, command); + ret = strlist__add(entry->tevlist, command); + if (ret < 0) { + pr_debug("strlist__add failed (%d)\n", ret); + goto out_err; + } + free(command); } list_add_tail(&entry->node, &pcache->entries); @@ -859,7 +872,10 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname) break; } - strlist__add(entry->tevlist, buf); + ret = strlist__add(entry->tevlist, buf); + if (ret < 0) + pr_debug("strlist__add failed (%d)\n", ret); + free(buf); entry = NULL; } -- 2.24.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf: probe-file: Check return value of strlist__add 2020-02-25 14:41 ` [PATCH 2/2] perf: probe-file: Check return value of strlist__add zhe.he @ 2020-02-25 22:49 ` Masami Hiramatsu 2020-02-26 2:49 ` He Zhe 0 siblings, 1 reply; 6+ messages in thread From: Masami Hiramatsu @ 2020-02-25 22:49 UTC (permalink / raw) To: zhe.he Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, kstewart, tglx, linux-kernel On Tue, 25 Feb 2020 22:41:43 +0800 <zhe.he@windriver.com> wrote: > From: He Zhe <zhe.he@windriver.com> > > strlist__add may fail with -ENOMEM or -EEXIST. Check it and give debugging > hint when necessary. > > Signed-off-by: He Zhe <zhe.he@windriver.com> > --- > tools/perf/builtin-probe.c | 30 ++++++++++++++++-------------- > tools/perf/util/probe-file.c | 26 +++++++++++++++++++++----- > 2 files changed, 37 insertions(+), 19 deletions(-) > > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c > index 26bc5923e6b5..8b4511c70fed 100644 > --- a/tools/perf/builtin-probe.c > +++ b/tools/perf/builtin-probe.c > @@ -442,24 +442,26 @@ static int perf_del_probe_events(struct strfilter *filter) > } > > ret = probe_file__get_events(kfd, filter, klist); > - if (ret == 0) { > - strlist__for_each_entry(ent, klist) > - pr_info("Removed event: %s\n", ent->s); > + if (ret < 0) > + goto out; No, this is ignored by design. Since probe_file__get_events() returns -ENOENT when no event is matched, this should be just ignored, and goto uprobe event matching. > > - ret = probe_file__del_strlist(kfd, klist); > - if (ret < 0) > - goto error; > - } > + strlist__for_each_entry(ent, klist) > + pr_info("Removed event: %s\n", ent->s); > + > + ret = probe_file__del_strlist(kfd, klist); > + if (ret < 0) > + goto error; > > ret2 = probe_file__get_events(ufd, filter, ulist); > - if (ret2 == 0) { > - strlist__for_each_entry(ent, ulist) > - pr_info("Removed event: %s\n", ent->s); > + if (ret2 < 0) > + goto out; Ditto. Thank you, > > - ret2 = probe_file__del_strlist(ufd, ulist); > - if (ret2 < 0) > - goto error; > - } > + strlist__for_each_entry(ent, ulist) > + pr_info("Removed event: %s\n", ent->s); > + > + ret2 = probe_file__del_strlist(ufd, ulist); > + if (ret2 < 0) > + goto error; > > if (ret == -ENOENT && ret2 == -ENOENT) > pr_warning("\"%s\" does not hit any event.\n", str); > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c > index cf44c05f89c1..00f086cba88f 100644 > --- a/tools/perf/util/probe-file.c > +++ b/tools/perf/util/probe-file.c > @@ -307,10 +307,14 @@ int probe_file__get_events(int fd, struct strfilter *filter, > p = strchr(ent->s, ':'); > if ((p && strfilter__compare(filter, p + 1)) || > strfilter__compare(filter, ent->s)) { > - strlist__add(plist, ent->s); > - ret = 0; > + ret = strlist__add(plist, ent->s); > + if (ret < 0) { > + pr_debug("strlist__add failed (%d)\n", ret); > + goto out; > + } > } > } > +out: > strlist__delete(namelist); > > return ret; > @@ -517,7 +521,11 @@ static int probe_cache__load(struct probe_cache *pcache) > ret = -EINVAL; > goto out; > } > - strlist__add(entry->tevlist, buf); > + ret = strlist__add(entry->tevlist, buf); > + if (ret < 0) { > + pr_debug("strlist__add failed (%d)\n", ret); > + goto out; > + } > } > } > out: > @@ -678,7 +686,12 @@ int probe_cache__add_entry(struct probe_cache *pcache, > command = synthesize_probe_trace_command(&tevs[i]); > if (!command) > goto out_err; > - strlist__add(entry->tevlist, command); > + ret = strlist__add(entry->tevlist, command); > + if (ret < 0) { > + pr_debug("strlist__add failed (%d)\n", ret); > + goto out_err; > + } > + > free(command); > } > list_add_tail(&entry->node, &pcache->entries); > @@ -859,7 +872,10 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname) > break; > } > > - strlist__add(entry->tevlist, buf); > + ret = strlist__add(entry->tevlist, buf); > + if (ret < 0) > + pr_debug("strlist__add failed (%d)\n", ret); > + > free(buf); > entry = NULL; > } > -- > 2.24.1 > -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf: probe-file: Check return value of strlist__add 2020-02-25 22:49 ` Masami Hiramatsu @ 2020-02-26 2:49 ` He Zhe 2020-02-26 7:11 ` Masami Hiramatsu 0 siblings, 1 reply; 6+ messages in thread From: He Zhe @ 2020-02-26 2:49 UTC (permalink / raw) To: Masami Hiramatsu Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, kstewart, tglx, linux-kernel On 2/26/20 6:49 AM, Masami Hiramatsu wrote: > On Tue, 25 Feb 2020 22:41:43 +0800 > <zhe.he@windriver.com> wrote: > >> From: He Zhe <zhe.he@windriver.com> >> >> strlist__add may fail with -ENOMEM or -EEXIST. Check it and give debugging >> hint when necessary. >> >> Signed-off-by: He Zhe <zhe.he@windriver.com> >> --- >> tools/perf/builtin-probe.c | 30 ++++++++++++++++-------------- >> tools/perf/util/probe-file.c | 26 +++++++++++++++++++++----- >> 2 files changed, 37 insertions(+), 19 deletions(-) >> >> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c >> index 26bc5923e6b5..8b4511c70fed 100644 >> --- a/tools/perf/builtin-probe.c >> +++ b/tools/perf/builtin-probe.c >> @@ -442,24 +442,26 @@ static int perf_del_probe_events(struct strfilter *filter) >> } >> >> ret = probe_file__get_events(kfd, filter, klist); >> - if (ret == 0) { >> - strlist__for_each_entry(ent, klist) >> - pr_info("Removed event: %s\n", ent->s); >> + if (ret < 0) >> + goto out; > No, this is ignored by design. > Since probe_file__get_events() returns -ENOENT when no event is matched, > this should be just ignored, and goto uprobe event matching. Thanks for pointing it out. However when strlist__add in probe_file__get_events returns a -ENOMEM and we ignore that, though it happens not very likely, we would miss some entries. So I add checks here and in probe_file__get_events to give a heads-up in advance. And the same reason is for the checks below for probe_cache__load, probe_cache__add_entry and probe_cache__scan_sdt. Regards, Zhe > >> >> - ret = probe_file__del_strlist(kfd, klist); >> - if (ret < 0) >> - goto error; >> - } >> + strlist__for_each_entry(ent, klist) >> + pr_info("Removed event: %s\n", ent->s); >> + >> + ret = probe_file__del_strlist(kfd, klist); >> + if (ret < 0) >> + goto error; >> >> ret2 = probe_file__get_events(ufd, filter, ulist); >> - if (ret2 == 0) { >> - strlist__for_each_entry(ent, ulist) >> - pr_info("Removed event: %s\n", ent->s); >> + if (ret2 < 0) >> + goto out; > Ditto. > > Thank you, > >> >> - ret2 = probe_file__del_strlist(ufd, ulist); >> - if (ret2 < 0) >> - goto error; >> - } >> + strlist__for_each_entry(ent, ulist) >> + pr_info("Removed event: %s\n", ent->s); >> + >> + ret2 = probe_file__del_strlist(ufd, ulist); >> + if (ret2 < 0) >> + goto error; >> >> if (ret == -ENOENT && ret2 == -ENOENT) >> pr_warning("\"%s\" does not hit any event.\n", str); >> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c >> index cf44c05f89c1..00f086cba88f 100644 >> --- a/tools/perf/util/probe-file.c >> +++ b/tools/perf/util/probe-file.c >> @@ -307,10 +307,14 @@ int probe_file__get_events(int fd, struct strfilter *filter, >> p = strchr(ent->s, ':'); >> if ((p && strfilter__compare(filter, p + 1)) || >> strfilter__compare(filter, ent->s)) { >> - strlist__add(plist, ent->s); >> - ret = 0; >> + ret = strlist__add(plist, ent->s); >> + if (ret < 0) { >> + pr_debug("strlist__add failed (%d)\n", ret); >> + goto out; >> + } >> } >> } >> +out: >> strlist__delete(namelist); >> >> return ret; >> @@ -517,7 +521,11 @@ static int probe_cache__load(struct probe_cache *pcache) >> ret = -EINVAL; >> goto out; >> } >> - strlist__add(entry->tevlist, buf); >> + ret = strlist__add(entry->tevlist, buf); >> + if (ret < 0) { >> + pr_debug("strlist__add failed (%d)\n", ret); >> + goto out; >> + } >> } >> } >> out: >> @@ -678,7 +686,12 @@ int probe_cache__add_entry(struct probe_cache *pcache, >> command = synthesize_probe_trace_command(&tevs[i]); >> if (!command) >> goto out_err; >> - strlist__add(entry->tevlist, command); >> + ret = strlist__add(entry->tevlist, command); >> + if (ret < 0) { >> + pr_debug("strlist__add failed (%d)\n", ret); >> + goto out_err; >> + } >> + >> free(command); >> } >> list_add_tail(&entry->node, &pcache->entries); >> @@ -859,7 +872,10 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname) >> break; >> } >> >> - strlist__add(entry->tevlist, buf); >> + ret = strlist__add(entry->tevlist, buf); >> + if (ret < 0) >> + pr_debug("strlist__add failed (%d)\n", ret); >> + >> free(buf); >> entry = NULL; >> } >> -- >> 2.24.1 >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf: probe-file: Check return value of strlist__add 2020-02-26 2:49 ` He Zhe @ 2020-02-26 7:11 ` Masami Hiramatsu 0 siblings, 0 replies; 6+ messages in thread From: Masami Hiramatsu @ 2020-02-26 7:11 UTC (permalink / raw) To: He Zhe Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, kstewart, tglx, linux-kernel On Wed, 26 Feb 2020 10:49:19 +0800 He Zhe <zhe.he@windriver.com> wrote: > > > On 2/26/20 6:49 AM, Masami Hiramatsu wrote: > > On Tue, 25 Feb 2020 22:41:43 +0800 > > <zhe.he@windriver.com> wrote: > > > >> From: He Zhe <zhe.he@windriver.com> > >> > >> strlist__add may fail with -ENOMEM or -EEXIST. Check it and give debugging > >> hint when necessary. > >> > >> Signed-off-by: He Zhe <zhe.he@windriver.com> > >> --- > >> tools/perf/builtin-probe.c | 30 ++++++++++++++++-------------- > >> tools/perf/util/probe-file.c | 26 +++++++++++++++++++++----- > >> 2 files changed, 37 insertions(+), 19 deletions(-) > >> > >> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c > >> index 26bc5923e6b5..8b4511c70fed 100644 > >> --- a/tools/perf/builtin-probe.c > >> +++ b/tools/perf/builtin-probe.c > >> @@ -442,24 +442,26 @@ static int perf_del_probe_events(struct strfilter *filter) > >> } > >> > >> ret = probe_file__get_events(kfd, filter, klist); > >> - if (ret == 0) { > >> - strlist__for_each_entry(ent, klist) > >> - pr_info("Removed event: %s\n", ent->s); > >> + if (ret < 0) > >> + goto out; > > No, this is ignored by design. > > Since probe_file__get_events() returns -ENOENT when no event is matched, > > this should be just ignored, and goto uprobe event matching. > > Thanks for pointing it out. However when strlist__add in probe_file__get_events > returns a -ENOMEM and we ignore that, though it happens not very likely, we > would miss some entries. So I add checks here and in probe_file__get_events to > give a heads-up in advance. If you are aware of -ENOMEM ( I guess in such case you'll see OOM killer sooner or later ), please just catch it. I mean if (ret == -ENOMEM) goto out; will be good. Thank you, > > And the same reason is for the checks below for probe_cache__load, > probe_cache__add_entry and probe_cache__scan_sdt. > > > Regards, > Zhe > > > > >> > >> - ret = probe_file__del_strlist(kfd, klist); > >> - if (ret < 0) > >> - goto error; > >> - } > >> + strlist__for_each_entry(ent, klist) > >> + pr_info("Removed event: %s\n", ent->s); > >> + > >> + ret = probe_file__del_strlist(kfd, klist); > >> + if (ret < 0) > >> + goto error; > >> > >> ret2 = probe_file__get_events(ufd, filter, ulist); > >> - if (ret2 == 0) { > >> - strlist__for_each_entry(ent, ulist) > >> - pr_info("Removed event: %s\n", ent->s); > >> + if (ret2 < 0) > >> + goto out; > > Ditto. > > > > Thank you, > > > >> > >> - ret2 = probe_file__del_strlist(ufd, ulist); > >> - if (ret2 < 0) > >> - goto error; > >> - } > >> + strlist__for_each_entry(ent, ulist) > >> + pr_info("Removed event: %s\n", ent->s); > >> + > >> + ret2 = probe_file__del_strlist(ufd, ulist); > >> + if (ret2 < 0) > >> + goto error; > >> > >> if (ret == -ENOENT && ret2 == -ENOENT) > >> pr_warning("\"%s\" does not hit any event.\n", str); > >> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c > >> index cf44c05f89c1..00f086cba88f 100644 > >> --- a/tools/perf/util/probe-file.c > >> +++ b/tools/perf/util/probe-file.c > >> @@ -307,10 +307,14 @@ int probe_file__get_events(int fd, struct strfilter *filter, > >> p = strchr(ent->s, ':'); > >> if ((p && strfilter__compare(filter, p + 1)) || > >> strfilter__compare(filter, ent->s)) { > >> - strlist__add(plist, ent->s); > >> - ret = 0; > >> + ret = strlist__add(plist, ent->s); > >> + if (ret < 0) { > >> + pr_debug("strlist__add failed (%d)\n", ret); > >> + goto out; > >> + } > >> } > >> } > >> +out: > >> strlist__delete(namelist); > >> > >> return ret; > >> @@ -517,7 +521,11 @@ static int probe_cache__load(struct probe_cache *pcache) > >> ret = -EINVAL; > >> goto out; > >> } > >> - strlist__add(entry->tevlist, buf); > >> + ret = strlist__add(entry->tevlist, buf); > >> + if (ret < 0) { > >> + pr_debug("strlist__add failed (%d)\n", ret); > >> + goto out; > >> + } > >> } > >> } > >> out: > >> @@ -678,7 +686,12 @@ int probe_cache__add_entry(struct probe_cache *pcache, > >> command = synthesize_probe_trace_command(&tevs[i]); > >> if (!command) > >> goto out_err; > >> - strlist__add(entry->tevlist, command); > >> + ret = strlist__add(entry->tevlist, command); > >> + if (ret < 0) { > >> + pr_debug("strlist__add failed (%d)\n", ret); > >> + goto out_err; > >> + } > >> + > >> free(command); > >> } > >> list_add_tail(&entry->node, &pcache->entries); > >> @@ -859,7 +872,10 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname) > >> break; > >> } > >> > >> - strlist__add(entry->tevlist, buf); > >> + ret = strlist__add(entry->tevlist, buf); > >> + if (ret < 0) > >> + pr_debug("strlist__add failed (%d)\n", ret); > >> + > >> free(buf); > >> entry = NULL; > >> } > >> -- > >> 2.24.1 > >> > > > -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] perf: Fix checking of duplicate probe to give proper hint 2020-02-25 14:41 [PATCH 1/2] perf: Fix checking of duplicate probe to give proper hint zhe.he 2020-02-25 14:41 ` [PATCH 2/2] perf: probe-file: Check return value of strlist__add zhe.he @ 2020-02-25 22:34 ` Masami Hiramatsu 1 sibling, 0 replies; 6+ messages in thread From: Masami Hiramatsu @ 2020-02-25 22:34 UTC (permalink / raw) To: zhe.he Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung, kstewart, tglx, linux-kernel Hi, Thanks for reporting. This bug has been reported and I should fixed last year... https://lkml.org/lkml/2019/12/3/136 Arnaldo, haven't you picked it yet? Thank you, On Tue, 25 Feb 2020 22:41:42 +0800 <zhe.he@windriver.com> wrote: > From: He Zhe <zhe.he@windriver.com> > > Since commit 72363540c009 ("perf probe: Support multiprobe event") and its > series, if there are multiple probes for one event, > __probe_file__get_namelist would return -EEXIST and cause the following > failure without proper hint, due to adding existing entry to output list. > > root@qemuarm64:~# perf probe -x /lib64/libc.so.6 free > Added new events: > probe_libc:free (on free in /lib64/libc-2.31.so) > probe_libc:free (on free in /lib64/libc-2.31.so) > > You can now use it in all perf tools, such as: > > perf record -e probe_libc:free -aR sleep 1 > > root@qemuarm64:~# perf probe -l > probe_libc:free (on free@plt in /lib64/libc-2.31.so) > probe_libc:free (on cfree in /lib64/libc-2.31.so) > root@qemuarm64:~# perf probe -x /lib64/libc.so.6 free > Error: Failed to add events. > > As we just want to check if there is any probe with the same name, -EEXIST > can be ignored, so we can have the right hint as before. > > root@qemuarm64:~# perf probe -x /lib64/libc.so.6 free > Error: event "free" already exists. > Hint: Remove existing event by 'perf probe -d' > or force duplicates by 'perf probe -f' > or set 'force=yes' in BPF source. > Error: Failed to add events. > > Signed-off-by: He Zhe <zhe.he@windriver.com> > --- > tools/perf/util/probe-file.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c > index 5003ba403345..cf44c05f89c1 100644 > --- a/tools/perf/util/probe-file.c > +++ b/tools/perf/util/probe-file.c > @@ -201,10 +201,16 @@ static struct strlist *__probe_file__get_namelist(int fd, bool include_group) > if (include_group) { > ret = e_snprintf(buf, 128, "%s:%s", tev.group, > tev.event); > - if (ret >= 0) > + if (ret >= 0) { > ret = strlist__add(sl, buf); > - } else > + if (ret == -EEXIST) > + ret = 0; > + } > + } else { > ret = strlist__add(sl, tev.event); > + if (ret == -EEXIST) > + ret = 0; > + } > clear_probe_trace_event(&tev); > if (ret < 0) > break; > -- > 2.24.1 > -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-26 7:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-25 14:41 [PATCH 1/2] perf: Fix checking of duplicate probe to give proper hint zhe.he 2020-02-25 14:41 ` [PATCH 2/2] perf: probe-file: Check return value of strlist__add zhe.he 2020-02-25 22:49 ` Masami Hiramatsu 2020-02-26 2:49 ` He Zhe 2020-02-26 7:11 ` Masami Hiramatsu 2020-02-25 22:34 ` [PATCH 1/2] perf: Fix checking of duplicate probe to give proper hint Masami Hiramatsu
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.