Em Tue, Nov 12, 2013 at 03:25:00PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung....@lge.com>
>
> When replaying a previous record session, it'll get a segfault since
> it doesn't initialize evsel->priv for finding syscall id. So fix it
> by initialize sys_enter/exit evsel manually.
>
> While at it, factor out perf_evsel__init_syscall_tp() to init a
> syscall evsel and remove unused perf_session__has_tp().
Try to avoid these "while at it" to combine multiple patches into one
:-)
I'll try to do it for you this time as it fixes a real problem, thanks
for fixing it!
More comments below:
- Arnaldo
> Cc: David Ahern <dsah...@gmail.com>
> Cc: Pekka Enberg <penb...@kernel.org>
> Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> ---
> tools/perf/builtin-trace.c | 63
> ++++++++++++++++++++++++++++------------------
> 1 file changed, 38 insertions(+), 25 deletions(-)
>
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> -static struct perf_evsel *perf_evsel__syscall_newtp(const char *direction,
> void *handler)
> +static int perf_evsel__init_syscall_tp(struct perf_evsel *evsel, void
> *handler)
> {
> - struct perf_evsel *evsel = perf_evsel__newtp("raw_syscalls", direction);
> + evsel->priv = malloc(sizeof(struct syscall_tp));
>
> - if (evsel) {
> - evsel->priv = malloc(sizeof(struct syscall_tp));
> + if (evsel->priv == NULL)
> + return -ENOMEM;
>
> - if (evsel->priv == NULL)
> - goto out_delete;
> + if (perf_evsel__init_sc_tp_uint_field(evsel, id))
> + return -1;
Try to be consistent here, if when allocating memory you return minus
ernno code, try to use that error reporting convention here as well.
Otherwise if perf_evsel__init_syscall_tp checks the error it will try to
figure out why it is returning -EPERM (-1) when
perf_evsel__init_sc_tp_uint_field fails :)
I think the right error to return here is -ENOENT, i.e. the field was
not found in this tracepoint.
>
> - if (perf_evsel__init_sc_tp_uint_field(evsel, id))
> - goto out_delete;
> + evsel->handler = handler;
> + return 0;
> +}
> +
> +static struct perf_evsel *perf_evsel__syscall_newtp(const char *direction,
> + void *handler)
> +{
> + struct perf_evsel *evsel = perf_evsel__newtp("raw_syscalls", direction);
>
> - evsel->handler = handler;
> + if (evsel) {
> + if (perf_evsel__init_syscall_tp(evsel, handler) < 0)
> + goto out_delete;
> }
>
> return evsel;
> @@ -1754,16 +1762,6 @@ static int trace__process_sample(struct perf_tool
> *tool,
> return err;
> }
>
> -static bool
> -perf_session__has_tp(struct perf_session *session, const char *name)
> -{
> - struct perf_evsel *evsel;
> -
> - evsel = perf_evlist__find_tracepoint_by_name(session->evlist, name);
> -
> - return evsel != NULL;
> -}
> -
> static int parse_target_str(struct trace *trace)
> {
> if (trace->opts.target.pid) {
> @@ -2000,8 +1998,6 @@ out_error:
> static int trace__replay(struct trace *trace)
> {
> const struct perf_evsel_str_handler handlers[] = {
> - { "raw_syscalls:sys_enter", trace__sys_enter, },
> - { "raw_syscalls:sys_exit", trace__sys_exit, },
> { "probe:vfs_getname", trace__vfs_getname, },
> };
> struct perf_data_file file = {
> @@ -2009,6 +2005,7 @@ static int trace__replay(struct trace *trace)
> .mode = PERF_DATA_MODE_READ,
> };
> struct perf_session *session;
> + struct perf_evsel *evsel;
> int err = -1;
>
> trace->tool.sample = trace__process_sample;
> @@ -2040,13 +2037,29 @@ static int trace__replay(struct trace *trace)
> if (err)
> goto out;
>
> - if (!perf_session__has_tp(session, "raw_syscalls:sys_enter")) {
> - pr_err("Data file does not have raw_syscalls:sys_enter
> events\n");
> + evsel = perf_evlist__find_tracepoint_by_name(session->evlist,
> + "raw_syscalls:sys_enter");
> + if (evsel == NULL) {
> + pr_err("Data file does not have raw_syscalls:sys_enter
> event\n");
> + goto out;
> + }
> +
> + if (perf_evsel__init_syscall_tp(evsel, trace__sys_enter) < 0 ||
> + perf_evsel__init_sc_tp_ptr_field(evsel, args)) {
> + pr_err("Error during initialize raw_syscalls:sys_enter
> event\n");
> + goto out;
> + }
> +
> + evsel = perf_evlist__find_tracepoint_by_name(session->evlist,
> + "raw_syscalls:sys_exit");
> + if (evsel == NULL) {
> + pr_err("Data file does not have raw_syscalls:sys_exit event\n");
> goto out;
> }
>
> - if (!perf_session__has_tp(session, "raw_syscalls:sys_exit")) {
> - pr_err("Data file does not have raw_syscalls:sys_exit
> events\n");
> + if (perf_evsel__init_syscall_tp(evsel, trace__sys_exit) < 0 ||
> + perf_evsel__init_sc_tp_uint_field(evsel, ret)) {
> + pr_err("Error during initialize raw_syscalls:sys_exit event\n");
> goto out;
> }
>
> --
> 1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
