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

[Hemant <hkshaw at linux dot vnet dot ibm dot com>]

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