From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: changbin.du@intel.com, jolsa@redhat.com, peterz@infradead.org,
mingo@redhat.com, namhyung@kernel.org, yan.y.zhao@intel.com,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] perf trace: __print_array should print hex format instead of dec
Date: Tue, 13 Mar 2018 11:27:32 -0300 [thread overview]
Message-ID: <20180313142732.GA29837@kernel.org> (raw)
In-Reply-To: <1520937557-24895-1-git-send-email-changbin.du@intel.com>
Em Tue, Mar 13, 2018 at 06:39:17PM +0800, changbin.du@intel.com escreveu:
> From: Changbin Du <changbin.du@intel.com>
>
> The token '__print_array' is to print the array in hex format, but not
> decimal numbers. The implementation of __print_array in kernel side is:
> __print_array()->trace_print_array_seq()
>
> This patch align the perf's behavior with kernel so we have a consistent
> event format.
Look ok, i.e. makes the userspace formatting do what the equivalent
kernel code does, Rostedt, Ack?
- Arnaldo
> Reported-by: Zhao Yan <yan.y.zhao@intel.com>
> Signed-off-by: Changbin Du <changbin.du@intel.com>
> ---
> tools/lib/traceevent/event-parse.c | 60 +++++++++++-----------
> tools/lib/traceevent/event-parse.h | 6 +--
> .../perf/util/scripting-engines/trace-event-perl.c | 8 +--
> .../util/scripting-engines/trace-event-python.c | 8 +--
> 4 files changed, 42 insertions(+), 40 deletions(-)
>
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index e5f2acb..d9f770c 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -835,10 +835,10 @@ static void free_arg(struct print_arg *arg)
> free_arg(arg->hex.field);
> free_arg(arg->hex.size);
> break;
> - case PRINT_INT_ARRAY:
> - free_arg(arg->int_array.field);
> - free_arg(arg->int_array.count);
> - free_arg(arg->int_array.el_size);
> + case PRINT_ARRAY:
> + free_arg(arg->array.field);
> + free_arg(arg->array.count);
> + free_arg(arg->array.el_size);
> break;
> case PRINT_TYPE:
> free(arg->typecast.type);
> @@ -2669,25 +2669,25 @@ static enum event_type
> process_int_array(struct event_format *event, struct print_arg *arg, char **tok)
> {
> memset(arg, 0, sizeof(*arg));
> - arg->type = PRINT_INT_ARRAY;
> + arg->type = PRINT_ARRAY;
>
> - if (alloc_and_process_delim(event, ",", &arg->int_array.field))
> + if (alloc_and_process_delim(event, ",", &arg->array.field))
> goto out;
>
> - if (alloc_and_process_delim(event, ",", &arg->int_array.count))
> + if (alloc_and_process_delim(event, ",", &arg->array.count))
> goto free_field;
>
> - if (alloc_and_process_delim(event, ")", &arg->int_array.el_size))
> + if (alloc_and_process_delim(event, ")", &arg->array.el_size))
> goto free_size;
>
> return read_token_item(tok);
>
> free_size:
> - free_arg(arg->int_array.count);
> - arg->int_array.count = NULL;
> + free_arg(arg->array.count);
> + arg->array.count = NULL;
> free_field:
> - free_arg(arg->int_array.field);
> - arg->int_array.field = NULL;
> + free_arg(arg->array.field);
> + arg->array.field = NULL;
> out:
> *tok = NULL;
> return EVENT_ERROR;
> @@ -3564,7 +3564,7 @@ eval_num_arg(void *data, int size, struct event_format *event, struct print_arg
> break;
> case PRINT_FLAGS:
> case PRINT_SYMBOL:
> - case PRINT_INT_ARRAY:
> + case PRINT_ARRAY:
> case PRINT_HEX:
> case PRINT_HEX_STR:
> break;
> @@ -4015,44 +4015,45 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
> }
> break;
>
> - case PRINT_INT_ARRAY: {
> + case PRINT_ARRAY: {
> void *num;
> int el_size;
>
> - if (arg->int_array.field->type == PRINT_DYNAMIC_ARRAY) {
> + if (arg->array.field->type == PRINT_DYNAMIC_ARRAY) {
> unsigned long offset;
> struct format_field *field =
> - arg->int_array.field->dynarray.field;
> + arg->array.field->dynarray.field;
> offset = pevent_read_number(pevent,
> data + field->offset,
> field->size);
> num = data + (offset & 0xffff);
> } else {
> - field = arg->int_array.field->field.field;
> + field = arg->array.field->field.field;
> if (!field) {
> - str = arg->int_array.field->field.name;
> + str = arg->array.field->field.name;
> field = pevent_find_any_field(event, str);
> if (!field)
> goto out_warning_field;
> - arg->int_array.field->field.field = field;
> + arg->array.field->field.field = field;
> }
> num = data + field->offset;
> }
> - len = eval_num_arg(data, size, event, arg->int_array.count);
> + len = eval_num_arg(data, size, event, arg->array.count);
> el_size = eval_num_arg(data, size, event,
> - arg->int_array.el_size);
> + arg->array.el_size);
> + trace_seq_putc(s, '{');
> for (i = 0; i < len; i++) {
> if (i)
> trace_seq_putc(s, ' ');
>
> if (el_size == 1) {
> - trace_seq_printf(s, "%u", *(uint8_t *)num);
> + trace_seq_printf(s, "0x%x", *(uint8_t *)num);
> } else if (el_size == 2) {
> - trace_seq_printf(s, "%u", *(uint16_t *)num);
> + trace_seq_printf(s, "0x%x", *(uint16_t *)num);
> } else if (el_size == 4) {
> - trace_seq_printf(s, "%u", *(uint32_t *)num);
> + trace_seq_printf(s, "0x%x", *(uint32_t *)num);
> } else if (el_size == 8) {
> - trace_seq_printf(s, "%"PRIu64, *(uint64_t *)num);
> + trace_seq_printf(s, "0x%"PRIx64, *(uint64_t *)num);
> } else {
> trace_seq_printf(s, "BAD SIZE:%d 0x%x",
> el_size, *(uint8_t *)num);
> @@ -4061,6 +4062,7 @@ static void print_str_arg(struct trace_seq *s, void *data, int size,
>
> num += el_size;
> }
> + trace_seq_putc(s, '}');
> break;
> }
> case PRINT_TYPE:
> @@ -5795,13 +5797,13 @@ static void print_args(struct print_arg *args)
> print_args(args->hex.size);
> printf(")");
> break;
> - case PRINT_INT_ARRAY:
> + case PRINT_ARRAY:
> printf("__print_array(");
> - print_args(args->int_array.field);
> + print_args(args->array.field);
> printf(", ");
> - print_args(args->int_array.count);
> + print_args(args->array.count);
> printf(", ");
> - print_args(args->int_array.el_size);
> + print_args(args->array.el_size);
> printf(")");
> break;
> case PRINT_STRING:
> diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
> index 0c03538..6ae2e8b 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -249,7 +249,7 @@ struct print_arg_hex {
> struct print_arg *size;
> };
>
> -struct print_arg_int_array {
> +struct print_arg_array {
> struct print_arg *field;
> struct print_arg *count;
> struct print_arg *el_size;
> @@ -283,7 +283,7 @@ enum print_arg_type {
> PRINT_FLAGS,
> PRINT_SYMBOL,
> PRINT_HEX,
> - PRINT_INT_ARRAY,
> + PRINT_ARRAY,
> PRINT_TYPE,
> PRINT_STRING,
> PRINT_BSTRING,
> @@ -305,7 +305,7 @@ struct print_arg {
> struct print_arg_flags flags;
> struct print_arg_symbol symbol;
> struct print_arg_hex hex;
> - struct print_arg_int_array int_array;
> + struct print_arg_array array;
> struct print_arg_func func;
> struct print_arg_string string;
> struct print_arg_bitmask bitmask;
> diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
> index 7b79c41..112d627 100644
> --- a/tools/perf/util/scripting-engines/trace-event-perl.c
> +++ b/tools/perf/util/scripting-engines/trace-event-perl.c
> @@ -224,10 +224,10 @@ static void define_event_symbols(struct event_format *event,
> define_event_symbols(event, ev_name, args->hex.field);
> define_event_symbols(event, ev_name, args->hex.size);
> break;
> - case PRINT_INT_ARRAY:
> - define_event_symbols(event, ev_name, args->int_array.field);
> - define_event_symbols(event, ev_name, args->int_array.count);
> - define_event_symbols(event, ev_name, args->int_array.el_size);
> + case PRINT_ARRAY:
> + define_event_symbols(event, ev_name, args->array.field);
> + define_event_symbols(event, ev_name, args->array.count);
> + define_event_symbols(event, ev_name, args->array.el_size);
> break;
> case PRINT_BSTRING:
> case PRINT_DYNAMIC_ARRAY:
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index ea07088..fb24f83 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -270,10 +270,10 @@ static void define_event_symbols(struct event_format *event,
> define_event_symbols(event, ev_name, args->hex.field);
> define_event_symbols(event, ev_name, args->hex.size);
> break;
> - case PRINT_INT_ARRAY:
> - define_event_symbols(event, ev_name, args->int_array.field);
> - define_event_symbols(event, ev_name, args->int_array.count);
> - define_event_symbols(event, ev_name, args->int_array.el_size);
> + case PRINT_ARRAY:
> + define_event_symbols(event, ev_name, args->array.field);
> + define_event_symbols(event, ev_name, args->array.count);
> + define_event_symbols(event, ev_name, args->array.el_size);
> break;
> case PRINT_STRING:
> break;
> --
> 2.7.4
next prev parent reply other threads:[~2018-03-13 14:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-13 10:39 [PATCH] perf trace: __print_array should print hex format instead of dec changbin.du
2018-03-13 14:27 ` Arnaldo Carvalho de Melo [this message]
2018-03-15 17:20 ` Steven Rostedt
2018-03-15 17:31 ` Steven Rostedt
2018-03-15 17:49 ` Arnaldo Carvalho de Melo
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=20180313142732.GA29837@kernel.org \
--to=acme@kernel.org \
--cc=changbin.du@intel.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=yan.y.zhao@intel.com \
/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.