Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Arnaldo Carvalho de Melo 
Em Tue, Oct 27, 2015 at 02:51:54PM +0100, Jiri Olsa escreveu:
> On Tue, Oct 27, 2015 at 10:44:40AM -0300, Arnaldo Carvalho de Melo wrote:
> > > I think we could easily add record specific event for this
> > > once it's needed.. there's plenty of free numbers in user
> > > events area ;-)

> > Shhh, don't ask PeterZ about it ;-)

> > But seriously, what makes this specific? Do you envision the stat code
> > needing 2^64-1 tags?

> nope ;-)

> it's just the read function perf_event__read_stat_config takes the
> 'struct perf_stat_config *' and updates it based on the event data.

> we would need to change the code to be more generic.. but as you said,
> it's future ;-)

Right, I applied the patch, I was just curious if I was missing some
detail that would prevent us from using this as a generic tag/value
perf.data storing facility.

- Arnaldo
--
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/

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Jiri Olsa 
On Tue, Oct 27, 2015 at 10:44:40AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 27, 2015 at 02:30:43PM +0100, Jiri Olsa escreveu:
> > On Tue, Oct 27, 2015 at 10:16:05AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Sun, Oct 25, 2015 at 03:51:27PM +0100, Jiri Olsa escreveu:
> > > > Adding stat config event to pass/store stat config data,
> > > > so report tools (report/script) know how to interpret
> > > > stat data.
> > > > 
> > > > The config data are stored in 'tag|value' way to allow
> > > > easy extension and backward compatibility.
> > > 
> > > I wonder if this couldn't be renamed 'PERF_RECORD_CONFIG' and just go
> > > on using one of those 2^64-1 tags for the 'stat config' needs, but then
> > > this is just a matter of changing the name of this event, which won't
> > > break anything when done.
> > 
> > we use this event to update 'struct perf_stat_config',
> > so it's kind of stat specific
> 
> What makes it specific? Isn't this just tag/value? If it was generic all
> it would take would be for us to register a range of values (or several)
> to be handled by the stat config reading routine.
>  
> > > Other stuff we may want to have stored like this include sysctl, sysfs
> > > values, kernel command line options used, etc.
> > 
> > I think we could easily add record specific event for this
> > once it's needed.. there's plenty of free numbers in user
> > events area ;-)
> 
> Shhh, don't ask PeterZ about it ;-)
> 
> But seriously, what makes this specific? Do you envision the stat code
> needing 2^64-1 tags?

nope ;-)

it's just the read function perf_event__read_stat_config takes the
'struct perf_stat_config *' and updates it based on the event data.

we would need to change the code to be more generic.. but as you said,
it's future ;-)

jirka
--
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/

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Arnaldo Carvalho de Melo 
Em Tue, Oct 27, 2015 at 02:30:43PM +0100, Jiri Olsa escreveu:
> On Tue, Oct 27, 2015 at 10:16:05AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sun, Oct 25, 2015 at 03:51:27PM +0100, Jiri Olsa escreveu:
> > > Adding stat config event to pass/store stat config data,
> > > so report tools (report/script) know how to interpret
> > > stat data.
> > > 
> > > The config data are stored in 'tag|value' way to allow
> > > easy extension and backward compatibility.
> > 
> > I wonder if this couldn't be renamed 'PERF_RECORD_CONFIG' and just go
> > on using one of those 2^64-1 tags for the 'stat config' needs, but then
> > this is just a matter of changing the name of this event, which won't
> > break anything when done.
> 
> we use this event to update 'struct perf_stat_config',
> so it's kind of stat specific

What makes it specific? Isn't this just tag/value? If it was generic all
it would take would be for us to register a range of values (or several)
to be handled by the stat config reading routine.
 
> > Other stuff we may want to have stored like this include sysctl, sysfs
> > values, kernel command line options used, etc.
> 
> I think we could easily add record specific event for this
> once it's needed.. there's plenty of free numbers in user
> events area ;-)

Shhh, don't ask PeterZ about it ;-)

But seriously, what makes this specific? Do you envision the stat code
needing 2^64-1 tags?

Anyway, we can revisit this when code needing to store tag/value
appears.

- Arnaldo
--
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/

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Jiri Olsa 
On Tue, Oct 27, 2015 at 10:16:05AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Oct 25, 2015 at 03:51:27PM +0100, Jiri Olsa escreveu:
> > Adding stat config event to pass/store stat config data,
> > so report tools (report/script) know how to interpret
> > stat data.
> > 
> > The config data are stored in 'tag|value' way to allow
> > easy extension and backward compatibility.
> 
> I wonder if this couldn't be renamed 'PERF_RECORD_CONFIG' and just go
> on using one of those 2^64-1 tags for the 'stat config' needs, but then
> this is just a matter of changing the name of this event, which won't
> break anything when done.

we use this event to update 'struct perf_stat_config',
so it's kind of stat specific

> 
> Other stuff we may want to have stored like this include sysctl, sysfs
> values, kernel command line options used, etc.

I think we could easily add record specific event for this
once it's needed.. there's plenty of free numbers in user
events area ;-)

jirka
--
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/

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Arnaldo Carvalho de Melo 
Em Sun, Oct 25, 2015 at 03:51:27PM +0100, Jiri Olsa escreveu:
> Adding stat config event to pass/store stat config data,
> so report tools (report/script) know how to interpret
> stat data.
> 
> The config data are stored in 'tag|value' way to allow
> easy extension and backward compatibility.

I wonder if this couldn't be renamed 'PERF_RECORD_CONFIG' and just go
on using one of those 2^64-1 tags for the 'stat config' needs, but then
this is just a matter of changing the name of this event, which won't
break anything when done.

Other stuff we may want to have stored like this include sysctl, sysfs
values, kernel command line options used, etc.

- Arnaldo

> Link: http://lkml.kernel.org/n/tip-1npdsfez8635vogthpqwt...@git.kernel.org
> Signed-off-by: Jiri Olsa 
> ---
>  tools/perf/util/event.c   |  1 +
>  tools/perf/util/event.h   | 20 
>  tools/perf/util/session.c | 24 
>  tools/perf/util/tool.h|  3 ++-
>  4 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 36dc992b072f..1165cb1d891f 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -39,6 +39,7 @@ static const char *perf_event__names[] = {
>   [PERF_RECORD_AUXTRACE_ERROR]= "AUXTRACE_ERROR",
>   [PERF_RECORD_THREAD_MAP]= "THREAD_MAP",
>   [PERF_RECORD_CPU_MAP]   = "CPU_MAP",
> + [PERF_RECORD_STAT_CONFIG]   = "STAT_CONFIG",
>  };
>  
>  const char *perf_event__name(unsigned int id)
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index e46f95285350..0bc3393dd28c 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -228,6 +228,7 @@ enum perf_user_event_type { /* above any possible kernel 
> type */
>   PERF_RECORD_AUXTRACE_ERROR  = 72,
>   PERF_RECORD_THREAD_MAP  = 73,
>   PERF_RECORD_CPU_MAP = 74,
> + PERF_RECORD_STAT_CONFIG = 75,
>   PERF_RECORD_HEADER_MAX
>  };
>  
> @@ -395,6 +396,24 @@ struct thread_map_event {
>   struct thread_map_data_eventdata[];
>  };
>  
> +enum {
> + PERF_STAT_CONFIG_TERM__AGGR_MODE= 0,
> + PERF_STAT_CONFIG_TERM__INTERVAL = 1,
> + PERF_STAT_CONFIG_TERM__SCALE= 2,
> + PERF_STAT_CONFIG_TERM__MAX  = 3,
> +};
> +
> +struct stat_config_term_event {
> + u64 tag;
> + u64 val;
> +};
> +
> +struct stat_config_event {
> + struct perf_event_headerheader;
> + u64 nr;
> + struct stat_config_term_event   data[];
> +};
> +
>  union perf_event {
>   struct perf_event_headerheader;
>   struct mmap_event   mmap;
> @@ -419,6 +438,7 @@ union perf_event {
>   struct context_switch_event context_switch;
>   struct thread_map_event thread_map;
>   struct cpu_map_eventcpu_map;
> + struct stat_config_eventstat_config;
>  };
>  
>  void perf_event__print_totals(void);
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 5126b18c671c..a550464925ea 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -315,6 +315,15 @@ int process_event_cpu_map_stub(struct perf_tool *tool 
> __maybe_unused,
>   return 0;
>  }
>  
> +static
> +int process_event_stat_config_stub(struct perf_tool *tool __maybe_unused,
> +union perf_event *event __maybe_unused,
> +struct perf_session *session __maybe_unused)
> +{
> + dump_printf(": unhandled!\n");
> + return 0;
> +}
> +
>  void perf_tool__fill_defaults(struct perf_tool *tool)
>  {
>   if (tool->sample == NULL)
> @@ -369,6 +378,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
>   tool->thread_map = process_event_thread_map_stub;
>   if (tool->cpu_map == NULL)
>   tool->cpu_map = process_event_cpu_map_stub;
> + if (tool->stat_config == NULL)
> + tool->stat_config = process_event_stat_config_stub;
>  }
>  
>  static void swap_sample_id_all(union perf_event *event, void *data)
> @@ -686,6 +697,16 @@ static void perf_event__cpu_map_swap(union perf_event 
> *event,
>   }
>  }
>  
> +static void perf_event__stat_config_swap(union perf_event *event,
> +  bool sample_id_all __maybe_unused)
> +{
> + u64 size;
> +
> + size  = event->stat_config.nr * sizeof(event->stat_config.data[0]);
> + size += 1; /* nr item itself */
> + mem_bswap_64(>stat_config.nr, size);
> +}
> +
>  typedef void (*perf_event__swap_op)(union perf_event *event,
>   bool sample_id_all);
>  
> @@ -715,6 +736,7 @@ static perf_event__swap_op perf_event__swap_ops[] = {
>   [PERF_RECORD_AUXTRACE_ERROR]  = perf_event__auxtrace_error_swap,
>

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Jiri Olsa 
On Tue, Oct 27, 2015 at 10:16:05AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Oct 25, 2015 at 03:51:27PM +0100, Jiri Olsa escreveu:
> > Adding stat config event to pass/store stat config data,
> > so report tools (report/script) know how to interpret
> > stat data.
> > 
> > The config data are stored in 'tag|value' way to allow
> > easy extension and backward compatibility.
> 
> I wonder if this couldn't be renamed 'PERF_RECORD_CONFIG' and just go
> on using one of those 2^64-1 tags for the 'stat config' needs, but then
> this is just a matter of changing the name of this event, which won't
> break anything when done.

we use this event to update 'struct perf_stat_config',
so it's kind of stat specific

> 
> Other stuff we may want to have stored like this include sysctl, sysfs
> values, kernel command line options used, etc.

I think we could easily add record specific event for this
once it's needed.. there's plenty of free numbers in user
events area ;-)

jirka
--
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/

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Arnaldo Carvalho de Melo 
Em Sun, Oct 25, 2015 at 03:51:27PM +0100, Jiri Olsa escreveu:
> Adding stat config event to pass/store stat config data,
> so report tools (report/script) know how to interpret
> stat data.
> 
> The config data are stored in 'tag|value' way to allow
> easy extension and backward compatibility.

I wonder if this couldn't be renamed 'PERF_RECORD_CONFIG' and just go
on using one of those 2^64-1 tags for the 'stat config' needs, but then
this is just a matter of changing the name of this event, which won't
break anything when done.

Other stuff we may want to have stored like this include sysctl, sysfs
values, kernel command line options used, etc.

- Arnaldo

> Link: http://lkml.kernel.org/n/tip-1npdsfez8635vogthpqwt...@git.kernel.org
> Signed-off-by: Jiri Olsa 
> ---
>  tools/perf/util/event.c   |  1 +
>  tools/perf/util/event.h   | 20 
>  tools/perf/util/session.c | 24 
>  tools/perf/util/tool.h|  3 ++-
>  4 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 36dc992b072f..1165cb1d891f 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -39,6 +39,7 @@ static const char *perf_event__names[] = {
>   [PERF_RECORD_AUXTRACE_ERROR]= "AUXTRACE_ERROR",
>   [PERF_RECORD_THREAD_MAP]= "THREAD_MAP",
>   [PERF_RECORD_CPU_MAP]   = "CPU_MAP",
> + [PERF_RECORD_STAT_CONFIG]   = "STAT_CONFIG",
>  };
>  
>  const char *perf_event__name(unsigned int id)
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index e46f95285350..0bc3393dd28c 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -228,6 +228,7 @@ enum perf_user_event_type { /* above any possible kernel 
> type */
>   PERF_RECORD_AUXTRACE_ERROR  = 72,
>   PERF_RECORD_THREAD_MAP  = 73,
>   PERF_RECORD_CPU_MAP = 74,
> + PERF_RECORD_STAT_CONFIG = 75,
>   PERF_RECORD_HEADER_MAX
>  };
>  
> @@ -395,6 +396,24 @@ struct thread_map_event {
>   struct thread_map_data_eventdata[];
>  };
>  
> +enum {
> + PERF_STAT_CONFIG_TERM__AGGR_MODE= 0,
> + PERF_STAT_CONFIG_TERM__INTERVAL = 1,
> + PERF_STAT_CONFIG_TERM__SCALE= 2,
> + PERF_STAT_CONFIG_TERM__MAX  = 3,
> +};
> +
> +struct stat_config_term_event {
> + u64 tag;
> + u64 val;
> +};
> +
> +struct stat_config_event {
> + struct perf_event_headerheader;
> + u64 nr;
> + struct stat_config_term_event   data[];
> +};
> +
>  union perf_event {
>   struct perf_event_headerheader;
>   struct mmap_event   mmap;
> @@ -419,6 +438,7 @@ union perf_event {
>   struct context_switch_event context_switch;
>   struct thread_map_event thread_map;
>   struct cpu_map_eventcpu_map;
> + struct stat_config_eventstat_config;
>  };
>  
>  void perf_event__print_totals(void);
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 5126b18c671c..a550464925ea 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -315,6 +315,15 @@ int process_event_cpu_map_stub(struct perf_tool *tool 
> __maybe_unused,
>   return 0;
>  }
>  
> +static
> +int process_event_stat_config_stub(struct perf_tool *tool __maybe_unused,
> +union perf_event *event __maybe_unused,
> +struct perf_session *session __maybe_unused)
> +{
> + dump_printf(": unhandled!\n");
> + return 0;
> +}
> +
>  void perf_tool__fill_defaults(struct perf_tool *tool)
>  {
>   if (tool->sample == NULL)
> @@ -369,6 +378,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
>   tool->thread_map = process_event_thread_map_stub;
>   if (tool->cpu_map == NULL)
>   tool->cpu_map = process_event_cpu_map_stub;
> + if (tool->stat_config == NULL)
> + tool->stat_config = process_event_stat_config_stub;
>  }
>  
>  static void swap_sample_id_all(union perf_event *event, void *data)
> @@ -686,6 +697,16 @@ static void perf_event__cpu_map_swap(union perf_event 
> *event,
>   }
>  }
>  
> +static void perf_event__stat_config_swap(union perf_event *event,
> +  bool sample_id_all __maybe_unused)
> +{
> + u64 size;
> +
> + size  = event->stat_config.nr * sizeof(event->stat_config.data[0]);
> + size += 1; /* nr item itself */
> + mem_bswap_64(>stat_config.nr, size);
> +}
> +
>  typedef void (*perf_event__swap_op)(union perf_event *event,
>   bool sample_id_all);
>  
> @@ -715,6 +736,7 @@ static perf_event__swap_op perf_event__swap_ops[] = {
>   [PERF_RECORD_AUXTRACE_ERROR]  = perf_event__auxtrace_error_swap,
>

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Arnaldo Carvalho de Melo 
Em Tue, Oct 27, 2015 at 02:30:43PM +0100, Jiri Olsa escreveu:
> On Tue, Oct 27, 2015 at 10:16:05AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sun, Oct 25, 2015 at 03:51:27PM +0100, Jiri Olsa escreveu:
> > > Adding stat config event to pass/store stat config data,
> > > so report tools (report/script) know how to interpret
> > > stat data.
> > > 
> > > The config data are stored in 'tag|value' way to allow
> > > easy extension and backward compatibility.
> > 
> > I wonder if this couldn't be renamed 'PERF_RECORD_CONFIG' and just go
> > on using one of those 2^64-1 tags for the 'stat config' needs, but then
> > this is just a matter of changing the name of this event, which won't
> > break anything when done.
> 
> we use this event to update 'struct perf_stat_config',
> so it's kind of stat specific

What makes it specific? Isn't this just tag/value? If it was generic all
it would take would be for us to register a range of values (or several)
to be handled by the stat config reading routine.
 
> > Other stuff we may want to have stored like this include sysctl, sysfs
> > values, kernel command line options used, etc.
> 
> I think we could easily add record specific event for this
> once it's needed.. there's plenty of free numbers in user
> events area ;-)

Shhh, don't ask PeterZ about it ;-)

But seriously, what makes this specific? Do you envision the stat code
needing 2^64-1 tags?

Anyway, we can revisit this when code needing to store tag/value
appears.

- Arnaldo
--
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/

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Jiri Olsa 
On Tue, Oct 27, 2015 at 10:44:40AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 27, 2015 at 02:30:43PM +0100, Jiri Olsa escreveu:
> > On Tue, Oct 27, 2015 at 10:16:05AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Sun, Oct 25, 2015 at 03:51:27PM +0100, Jiri Olsa escreveu:
> > > > Adding stat config event to pass/store stat config data,
> > > > so report tools (report/script) know how to interpret
> > > > stat data.
> > > > 
> > > > The config data are stored in 'tag|value' way to allow
> > > > easy extension and backward compatibility.
> > > 
> > > I wonder if this couldn't be renamed 'PERF_RECORD_CONFIG' and just go
> > > on using one of those 2^64-1 tags for the 'stat config' needs, but then
> > > this is just a matter of changing the name of this event, which won't
> > > break anything when done.
> > 
> > we use this event to update 'struct perf_stat_config',
> > so it's kind of stat specific
> 
> What makes it specific? Isn't this just tag/value? If it was generic all
> it would take would be for us to register a range of values (or several)
> to be handled by the stat config reading routine.
>  
> > > Other stuff we may want to have stored like this include sysctl, sysfs
> > > values, kernel command line options used, etc.
> > 
> > I think we could easily add record specific event for this
> > once it's needed.. there's plenty of free numbers in user
> > events area ;-)
> 
> Shhh, don't ask PeterZ about it ;-)
> 
> But seriously, what makes this specific? Do you envision the stat code
> needing 2^64-1 tags?

nope ;-)

it's just the read function perf_event__read_stat_config takes the
'struct perf_stat_config *' and updates it based on the event data.

we would need to change the code to be more generic.. but as you said,
it's future ;-)

jirka
--
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/

Re: [PATCH 11/52] perf tools: Add stat config event

2015-10-27 Thread Arnaldo Carvalho de Melo 
Em Tue, Oct 27, 2015 at 02:51:54PM +0100, Jiri Olsa escreveu:
> On Tue, Oct 27, 2015 at 10:44:40AM -0300, Arnaldo Carvalho de Melo wrote:
> > > I think we could easily add record specific event for this
> > > once it's needed.. there's plenty of free numbers in user
> > > events area ;-)

> > Shhh, don't ask PeterZ about it ;-)

> > But seriously, what makes this specific? Do you envision the stat code
> > needing 2^64-1 tags?

> nope ;-)

> it's just the read function perf_event__read_stat_config takes the
> 'struct perf_stat_config *' and updates it based on the event data.

> we would need to change the code to be more generic.. but as you said,
> it's future ;-)

Right, I applied the patch, I was just curious if I was missing some
detail that would prevent us from using this as a generic tag/value
perf.data storing facility.

- Arnaldo
--
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/