Messages in this thread | | | Date | Thu, 27 Jan 2022 21:19:29 +0800 | From | Changbin Du <> | Subject | Re: [PATCH] perf/ftrace: system_wide collection is not effective by default |
| |
On Wed, Jan 26, 2022 at 03:47:48PM -0800, Namhyung Kim wrote: > Hello, > > On Wed, Jan 26, 2022 at 7:19 AM Changbin Du <changbin.du@gmail.com> wrote: > > > > The ftrace.target.system_wide must be set before invoking > > evlist__create_maps(), otherwise it has no effect. > > Oh, right. Thanks for pointing that out. > > > > > Fixes: 53be50282269 ("perf ftrace: Add 'latency' subcommand") > > Signed-off-by: Changbin Du <changbin.du@gmail.com> > > --- > > tools/perf/builtin-ftrace.c | 45 ++++++++++++++++++++----------------- > > 1 file changed, 24 insertions(+), 21 deletions(-) > > > > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c > > index dec24dc0e767..26bff29ad277 100644 > > --- a/tools/perf/builtin-ftrace.c > > +++ b/tools/perf/builtin-ftrace.c > > @@ -1115,6 +1115,7 @@ enum perf_ftrace_subcommand { > > int cmd_ftrace(int argc, const char **argv) > > { > > int ret; > > + int (*cmd_func)(struct perf_ftrace *) = NULL; > > struct perf_ftrace ftrace = { > > .tracer = DEFAULT_TRACER, > > .target = { .uid = UINT_MAX, }, > > @@ -1221,6 +1222,28 @@ int cmd_ftrace(int argc, const char **argv) > > goto out_delete_filters; > > } > > > > + switch (subcmd) { > > + case PERF_FTRACE_TRACE: > > + if (!argc && target__none(&ftrace.target)) > > + ftrace.target.system_wide = true; > > We can move only this part after parse_options(). > I prefer that all parsing works get be done first. :)
> > + cmd_func = __cmd_ftrace; > > + break; > > + case PERF_FTRACE_LATENCY: > > + if (list_empty(&ftrace.filters)) { > > + pr_err("Should provide a function to measure\n"); > > + parse_options_usage(ftrace_usage, options, "T", 1); > > + ret = -EINVAL; > > + goto out_delete_evlist; > > Otherwise, it should go to the out_delete_filters. > yes, will fix it.
> > + } > > + cmd_func = __cmd_latency; > > + break; > > + case PERF_FTRACE_NONE: > > + default: > > + pr_err("Invalid subcommand\n"); > > + ret = -EINVAL; > > + goto out_delete_evlist; > > Ditto. > okay.
> Thanks, > Namhyung > > > > + } > > + > > ret = target__validate(&ftrace.target); > > if (ret) { > > char errbuf[512]; > > @@ -1248,27 +1271,7 @@ int cmd_ftrace(int argc, const char **argv) > > goto out_delete_evlist; > > } > > > > - switch (subcmd) { > > - case PERF_FTRACE_TRACE: > > - if (!argc && target__none(&ftrace.target)) > > - ftrace.target.system_wide = true; > > - ret = __cmd_ftrace(&ftrace); > > - break; > > - case PERF_FTRACE_LATENCY: > > - if (list_empty(&ftrace.filters)) { > > - pr_err("Should provide a function to measure\n"); > > - parse_options_usage(ftrace_usage, options, "T", 1); > > - ret = -EINVAL; > > - goto out_delete_evlist; > > - } > > - ret = __cmd_latency(&ftrace); > > - break; > > - case PERF_FTRACE_NONE: > > - default: > > - pr_err("Invalid subcommand\n"); > > - ret = -EINVAL; > > - break; > > - } > > + ret = cmd_func(&ftrace); > > > > out_delete_evlist: > > evlist__delete(ftrace.evlist); > > -- > > 2.32.0 > >
-- Cheers, Changbin Du
| |