All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Arnaldo Carvalho de Melo <acme@infradead.org>,
	Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] lib tools traceevent: Add back pevent assignment in __pevent_parse_format()
Date: Mon, 01 Oct 2012 22:42:09 +0900	[thread overview]
Message-ID: <1349098929.1613.11.camel@leonhard> (raw)
In-Reply-To: <1348885245.22822.104.camel@gandalf.local.home>

2012-09-28 (금), 22:20 -0400, Steven Rostedt:
> Even though with the change of commit commit 2b29175 "tools lib traceevent:
> Carve out events format parsing routine", allowed __pevent_parse_format() to
> parse an event without the need of a pevent handler, the event still needs to
> assign the pevent handed to it.
> 
> There's no problem with assigning it if the pevent is NULL, as the event->pevent
> would be NULL without the assignment. But function parsing handlers may be
> assigned to the pevent handler to help in parsing the event. If there's no
> pevent then there would not be any function handlers, but if the pevent
> isn't assigned first before parsing the event, it wont honor the function
> handlers that were assigned.
> 
> Worse yet, the current code crashes if an event has a function that it tries
> to parse. For example:
> 
>  # perf record -e scsi:scsi_dispatch_cmd_timeout
>  Segmentation fault (core dumped)
> 
> This happens because the scsi_dispatch_cmd_timeout event format has the following:
> 
>   __print_hex(__get_dynamic_array(cmnd), REC->cmd_len)
> 
> which hasn't been defined by the pevent code.

???  We have both of __print_hex() and __get_dynamic_array() handler,
please see process_function.  In my case, the offending function was
"scsi_trace_parse_cdb".

Other than that, looks good to me.

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung





  reply	other threads:[~2012-10-01 13:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-29  2:20 [PATCH] lib tools traceevent: Add back pevent assignment in __pevent_parse_format() Steven Rostedt
2012-10-01 13:42 ` Namhyung Kim [this message]
2012-10-01 14:02   ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1349098929.1613.11.camel@leonhard \
    --to=namhyung@kernel.org \
    --cc=acme@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.