RE: [RFC/PATCH 3/3] perf probe: Move print logic into cmd_probe()
> From: Namhyung Kim [mailto:namhy...@gmail.com] On Behalf Of Namhyung Kim
>
> 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
>
This change is good for me :)
And have a comment below,
[...]
> @@ -496,7 +499,41 @@ __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 = __add_perf_probe_events(params.events, params.nevents,
> + );
> + if (ret < 0)
> + goto out_cleanup;
> +
> + for (i = k = 0; i < params.nevents; i++)
> + k += pkgs[i].ntevs;
> +
> + pr_info("Added new event%s\n", (k > 1) ? "s:" : ":");
> + for (i = 0; i < params.nevents; i++) {
> + struct perf_probe_event *pev = pkgs[i].pev;
> +
> + for (k = 0; k < pkgs[i].ntevs; k++) {
> + struct probe_trace_event *tev =
> [i].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(pkgs, params.nevents);
I think this should be a separated function to avoid increasing the size of
__cmd_probe()
Thanks,
RE: [RFC/PATCH 3/3] perf probe: Move print logic into cmd_probe()
> From: Namhyung Kim [mailto:namhy...@gmail.com] On Behalf Of Namhyung Kim
>
> 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
>
This change is good for me :)
And have a comment below,
[...]
> @@ -496,7 +499,41 @@ __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 = __add_perf_probe_events(params.events, params.nevents,
> + );
> + if (ret < 0)
> + goto out_cleanup;
> +
> + for (i = k = 0; i < params.nevents; i++)
> + k += pkgs[i].ntevs;
> +
> + pr_info("Added new event%s\n", (k > 1) ? "s:" : ":");
> + for (i = 0; i < params.nevents; i++) {
> + struct perf_probe_event *pev = pkgs[i].pev;
> +
> + for (k = 0; k < pkgs[i].ntevs; k++) {
> + struct probe_trace_event *tev =
> [i].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(pkgs, params.nevents);
I think this should be a separated function to avoid increasing the size of
__cmd_probe()
Thanks,
