RE: [PATCH 3/5] perf probe: Move print logic into cmd_probe()

From: åæéå / HIRAMATUïMASAMI
Date: Fri Sep 04 2015 - 07:49:04 EST

Next message: Borislav Petkov: "Re: [Patch V0] x86, mce: Don't clear global error reporting banks during cpu_offline"
Previous message: Ulf Hansson: "Re: [PATCH] ARM: ux500: fix typo in regulator names"
In reply to: Namhyung Kim: "[PATCH 3/5] perf probe: Move print logic into cmd_probe()"
Next in thread: Namhyung Kim: "[PATCH 2/5] perf probe: Attach trace_probe_event with perf_probe_event"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

> From: Namhyung Kim [mailto:namhyung@xxxxxxxxxx]
>
> Showing actual trace event when adding perf events is only needed in
> perf probe command. But the add functionality itself can be used by
> other places. So move the printing code into the cmd_probe().
>
> Also it combines the output if more than one event is added.
>
> Before:
> $ sudo perf probe -a do_fork -a do_exit
> Added new event:
> probe:do_fork (on do_fork)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:do_fork -aR sleep 1
>
> Added new events:
> probe:do_exit (on do_exit)
> probe:do_exit_1 (on do_exit)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:do_exit_1 -aR sleep 1
>
> After:
> $ sudo perf probe -a do_fork -a do_exit
> Added new events:
> probe:do_fork (on do_fork)
> probe:do_exit (on do_exit)
> probe:do_exit_1 (on do_exit)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:do_exit_1 -aR sleep 1
>

Looks good to me :)

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>

Thanks!

> Cc: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
> tools/perf/builtin-probe.c | 48 ++++++++++++++++++++++++++++++++++++++++++-
> tools/perf/util/probe-event.c | 22 +++-----------------
> tools/perf/util/probe-event.h | 3 +++
> 3 files changed, 53 insertions(+), 20 deletions(-)
>
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index b81cec33b4b2..b8cf6cb7e1bf 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -311,6 +311,52 @@ static void pr_err_with_code(const char *msg, int err)
> pr_err("\n");
> }
>
> +static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
> +{
> + int ret;
> + int i, k;
> + const char *event = NULL, *group = NULL;
> +
> + ret = convert_perf_probe_events(pevs, npevs);
> + if (ret < 0)
> + goto out_cleanup;
> +
> + ret = apply_perf_probe_events(pevs, npevs);
> + if (ret < 0)
> + goto out_cleanup;
> +
> + for (i = k = 0; i < npevs; i++)
> + k += pevs[i].ntevs;
> +
> + pr_info("Added new event%s\n", (k > 1) ? "s:" : ":");
> + for (i = 0; i < npevs; i++) {
> + struct perf_probe_event *pev = &pevs[i];
> +
> + for (k = 0; k < pev->ntevs; k++) {
> + struct probe_trace_event *tev = &pev->tevs[k];
> +
> + /* We use tev's name for showing new events */
> + show_perf_probe_event(tev->group, tev->event, pev,
> + tev->point.module, false);
> +
> + /* Save the last valid name */
> + event = tev->event;
> + group = tev->group;
> + }
> + }
> +
> + /* Note that it is possible to skip all events because of blacklist */
> + if (event) {
> + /* Show how to use the event. */
> + pr_info("\nYou can now use it in all perf tools, such as:\n\n");
> + pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
> + }
> +
> +out_cleanup:
> + cleanup_perf_probe_events(pevs, npevs);
> + return ret;
> +}
> +
> static int
> __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> {
> @@ -496,7 +542,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> usage_with_options(probe_usage, options);
> }
>
> - ret = add_perf_probe_events(params.events, params.nevents);
> + ret = perf_add_probe_events(params.events, params.nevents);
> if (ret < 0) {
> pr_err_with_code(" Error: Failed to add events.", ret);
> return ret;
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 0d3a051b9202..01b9a5bd9449 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2180,9 +2180,9 @@ static int perf_probe_event__sprintf(const char *group, const char *event,
> }
>
> /* Show an event */
> -static int show_perf_probe_event(const char *group, const char *event,
> - struct perf_probe_event *pev,
> - const char *module, bool use_stdout)
> +int show_perf_probe_event(const char *group, const char *event,
> + struct perf_probe_event *pev,
> + const char *module, bool use_stdout)
> {
> struct strbuf buf = STRBUF_INIT;
> int ret;
> @@ -2399,7 +2399,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
> {
> int i, fd, ret;
> struct probe_trace_event *tev = NULL;
> - const char *event = NULL, *group = NULL;
> struct strlist *namelist;
>
> fd = probe_file__open(PF_FL_RW | (pev->uprobes ? PF_FL_UPROBE : 0));
> @@ -2415,7 +2414,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
> }
>
> ret = 0;
> - pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
> for (i = 0; i < ntevs; i++) {
> tev = &tevs[i];
> /* Skip if the symbol is out of .text or blacklisted */
> @@ -2432,13 +2430,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
> if (ret < 0)
> break;
>
> - /* We use tev's name for showing new events */
> - show_perf_probe_event(tev->group, tev->event, pev,
> - tev->point.module, false);
> - /* Save the last valid name */
> - event = tev->event;
> - group = tev->group;
> -
> /*
> * Probes after the first probe which comes from same
> * user input are always allowed to add suffix, because
> @@ -2450,13 +2441,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
> if (ret == -EINVAL && pev->uprobes)
> warn_uprobe_event_compat(tev);
>
> - /* Note that it is possible to skip all events because of blacklist */
> - if (ret >= 0 && event) {
> - /* Show how to use the event. */
> - pr_info("\nYou can now use it in all perf tools, such as:\n\n");
> - pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
> - }
> -
> strlist__delete(namelist);
> close_out:
> close(fd);
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 70c327bd61de..610f743671e1 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -144,6 +144,9 @@ extern int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> extern int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> extern void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> extern int del_perf_probe_events(struct strfilter *filter);
> +extern int show_perf_probe_event(const char *group, const char *event,
> + struct perf_probe_event *pev,
> + const char *module, bool use_stdout);
> extern int show_perf_probe_events(struct strfilter *filter);
> extern int show_line_range(struct line_range *lr, const char *module,
> bool user);
> --
> 2.5.0

N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nÇ‰·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~††Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå

Next message: Borislav Petkov: "Re: [Patch V0] x86, mce: Don't clear global error reporting banks during cpu_offline"
Previous message: Ulf Hansson: "Re: [PATCH] ARM: ux500: fix typo in regulator names"
In reply to: Namhyung Kim: "[PATCH 3/5] perf probe: Move print logic into cmd_probe()"
Next in thread: Namhyung Kim: "[PATCH 2/5] perf probe: Attach trace_probe_event with perf_probe_event"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]