Re: [PATCH v2 2/3] Support for perf to probe into SDT markers:

From: Hemant
Date: Tue Oct 08 2013 - 09:31:03 EST


On 10/08/2013 05:17 PM, Masami Hiramatsu wrote:
(2013/10/07 15:48), Hemant Kumar wrote:
[...]
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index cbd2383..6f09723 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -370,6 +370,17 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
pr_err("Error: Don't use --markers with --funcs.\n");
usage_with_options(probe_usage, options);
}
+ if (params.mod_events) {
+ ret = add_perf_probe_events(params.events,
+ params.nevents,
+ params.max_probe_points,
+ params.target,
+ params.force_add);
+ if (ret < 0) {
+ pr_err(" Error: Failed to add events. "
+ " (%d)\n", ret);
+ }
+ }
What is this code for? params.sdt is true only if "--markers" is set, and that
should not be used with --add and --del, because it's an action "query markers".
We should give an error and abort here.

Yeah, I see your point. We should not add an event in this case. Instead an error should be displayed. Thanks for pointing that. And we already have add_perf_probe_events() call. That should be called instead.


Other points are covered by Namhyung's review(thanks!).

Thank you!

Yeah, will make the required changes and post the next iteration ASAP.

--
Thanks
Hemant

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/