Re: [PATCH 02/14] perf tools: Introduce struct add_entry_iter

2013-11-04 Thread Namhyung Kim 
On Fri, 1 Nov 2013 13:09:09 +0100, Jiri Olsa wrote:
> On Thu, Oct 31, 2013 at 03:56:04PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim 
>
> SNIP
>
>> +}
>> +
>> +static int
>> +iter_add_next_nop_entry(struct add_entry_iter *iter __maybe_unused,
>> +struct addr_location *al __maybe_unused)
>> +{
>> +return 0;
>> +}
>> +
>> +static int
>> +iter_prepare_mem_entry(struct add_entry_iter *iter, struct machine *machine,
>> +   struct perf_evsel *evsel, struct addr_location *al,
>> +   struct perf_sample *sample)
>> +{
>> +union perf_event *event = iter->priv;
>> +struct mem_info *mi;
>> +u8 cpumode;
>> +
>> +BUG_ON(event == NULL);
>
> the priv does not get assigned.. 'perf mem rep' aborts
>
> [jolsa@krava perf]$ ./perf mem rep --stdio
> Aborted
> perf: builtin-report.c:120: iter_prepare_mem_entry: Assertion `!(event == 
> ((void *)0))' failed.

Argh... forgot to set it for mem entries after code refactoring,
sorry. :-/  Will fix in v3.

Thanks,
Namhyung
--
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 02/14] perf tools: Introduce struct add_entry_iter

2013-11-04 Thread Namhyung Kim 
Hi Jiri,

On Fri, 1 Nov 2013 13:07:35 +0100, Jiri Olsa wrote:
> On Thu, Oct 31, 2013 at 03:56:04PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim 
>
> SNIP
>
>> +
>> +static int
>> +perf_evsel__add_entry(struct perf_evsel *evsel, struct addr_location *al,
>> +  struct perf_sample *sample, struct machine *machine,
>> +  struct add_entry_iter *iter)
>> +{
>> +int err;
>> +
>> +if ((sort__has_parent || symbol_conf.use_callchain) && 
>> sample->callchain) {
>> +err = machine__resolve_callchain(machine, evsel, al->thread,
>> + sample, >parent, al,
>> + iter->rep->max_stack);
>> +if (err)
>> +return err;
>> +}
>> +
>> +err = iter->prepare_entry(iter, machine, evsel, al, sample);
>> +if (err)
>> +return err;
>> +
>> +err = iter->add_single_entry(iter, al);
>> +if (err)
>> +return err;
>> +
>> +while (iter->next_entry(iter, al)) {
>> +err = iter->add_next_entry(iter, al);
>> +if (err)
>> +break;
>> +}
>> +
>> +err = iter->finish_entry(iter, al);
>> +
>> +return err;
>
>   return iter->finish_entry(iter, al);  ?

Right.  Will fix in v3.

Thanks,
Namhyung
--
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 02/14] perf tools: Introduce struct add_entry_iter

2013-11-04 Thread Namhyung Kim 
Hi Jiri,

On Fri, 1 Nov 2013 13:07:35 +0100, Jiri Olsa wrote:
 On Thu, Oct 31, 2013 at 03:56:04PM +0900, Namhyung Kim wrote:
 From: Namhyung Kim namhyung@lge.com

 SNIP

 +
 +static int
 +perf_evsel__add_entry(struct perf_evsel *evsel, struct addr_location *al,
 +  struct perf_sample *sample, struct machine *machine,
 +  struct add_entry_iter *iter)
 +{
 +int err;
 +
 +if ((sort__has_parent || symbol_conf.use_callchain)  
 sample-callchain) {
 +err = machine__resolve_callchain(machine, evsel, al-thread,
 + sample, iter-parent, al,
 + iter-rep-max_stack);
 +if (err)
 +return err;
 +}
 +
 +err = iter-prepare_entry(iter, machine, evsel, al, sample);
 +if (err)
 +return err;
 +
 +err = iter-add_single_entry(iter, al);
 +if (err)
 +return err;
 +
 +while (iter-next_entry(iter, al)) {
 +err = iter-add_next_entry(iter, al);
 +if (err)
 +break;
 +}
 +
 +err = iter-finish_entry(iter, al);
 +
 +return err;

   return iter-finish_entry(iter, al);  ?

Right.  Will fix in v3.

Thanks,
Namhyung
--
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 02/14] perf tools: Introduce struct add_entry_iter

2013-11-04 Thread Namhyung Kim 
On Fri, 1 Nov 2013 13:09:09 +0100, Jiri Olsa wrote:
 On Thu, Oct 31, 2013 at 03:56:04PM +0900, Namhyung Kim wrote:
 From: Namhyung Kim namhyung@lge.com

 SNIP

 +}
 +
 +static int
 +iter_add_next_nop_entry(struct add_entry_iter *iter __maybe_unused,
 +struct addr_location *al __maybe_unused)
 +{
 +return 0;
 +}
 +
 +static int
 +iter_prepare_mem_entry(struct add_entry_iter *iter, struct machine *machine,
 +   struct perf_evsel *evsel, struct addr_location *al,
 +   struct perf_sample *sample)
 +{
 +union perf_event *event = iter-priv;
 +struct mem_info *mi;
 +u8 cpumode;
 +
 +BUG_ON(event == NULL);

 the priv does not get assigned.. 'perf mem rep' aborts

 [jolsa@krava perf]$ ./perf mem rep --stdio
 Aborted
 perf: builtin-report.c:120: iter_prepare_mem_entry: Assertion `!(event == 
 ((void *)0))' failed.

Argh... forgot to set it for mem entries after code refactoring,
sorry. :-/  Will fix in v3.

Thanks,
Namhyung
--
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 02/14] perf tools: Introduce struct add_entry_iter

2013-11-01 Thread Jiri Olsa 
On Thu, Oct 31, 2013 at 03:56:04PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim 

SNIP

> +}
> +
> +static int
> +iter_add_next_nop_entry(struct add_entry_iter *iter __maybe_unused,
> + struct addr_location *al __maybe_unused)
> +{
> + return 0;
> +}
> +
> +static int
> +iter_prepare_mem_entry(struct add_entry_iter *iter, struct machine *machine,
> +struct perf_evsel *evsel, struct addr_location *al,
> +struct perf_sample *sample)
> +{
> + union perf_event *event = iter->priv;
> + struct mem_info *mi;
> + u8 cpumode;
> +
> + BUG_ON(event == NULL);

the priv does not get assigned.. 'perf mem rep' aborts

[jolsa@krava perf]$ ./perf mem rep --stdio
Aborted
perf: builtin-report.c:120: iter_prepare_mem_entry: Assertion `!(event == 
((void *)0))' failed.

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 02/14] perf tools: Introduce struct add_entry_iter

2013-11-01 Thread Jiri Olsa 
On Thu, Oct 31, 2013 at 03:56:04PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim 

SNIP

> +
> +static int
> +perf_evsel__add_entry(struct perf_evsel *evsel, struct addr_location *al,
> +   struct perf_sample *sample, struct machine *machine,
> +   struct add_entry_iter *iter)
> +{
> + int err;
> +
> + if ((sort__has_parent || symbol_conf.use_callchain) && 
> sample->callchain) {
> + err = machine__resolve_callchain(machine, evsel, al->thread,
> +  sample, >parent, al,
> +  iter->rep->max_stack);
> + if (err)
> + return err;
> + }
> +
> + err = iter->prepare_entry(iter, machine, evsel, al, sample);
> + if (err)
> + return err;
> +
> + err = iter->add_single_entry(iter, al);
> + if (err)
> + return err;
> +
> + while (iter->next_entry(iter, al)) {
> + err = iter->add_next_entry(iter, al);
> + if (err)
> + break;
> + }
> +
> + err = iter->finish_entry(iter, al);
> +
> + return err;

return iter->finish_entry(iter, al);  ?

--
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 02/14] perf tools: Introduce struct add_entry_iter

2013-11-01 Thread Jiri Olsa 
On Thu, Oct 31, 2013 at 03:56:04PM +0900, Namhyung Kim wrote:
 From: Namhyung Kim namhyung@lge.com

SNIP

 +
 +static int
 +perf_evsel__add_entry(struct perf_evsel *evsel, struct addr_location *al,
 +   struct perf_sample *sample, struct machine *machine,
 +   struct add_entry_iter *iter)
 +{
 + int err;
 +
 + if ((sort__has_parent || symbol_conf.use_callchain)  
 sample-callchain) {
 + err = machine__resolve_callchain(machine, evsel, al-thread,
 +  sample, iter-parent, al,
 +  iter-rep-max_stack);
 + if (err)
 + return err;
 + }
 +
 + err = iter-prepare_entry(iter, machine, evsel, al, sample);
 + if (err)
 + return err;
 +
 + err = iter-add_single_entry(iter, al);
 + if (err)
 + return err;
 +
 + while (iter-next_entry(iter, al)) {
 + err = iter-add_next_entry(iter, al);
 + if (err)
 + break;
 + }
 +
 + err = iter-finish_entry(iter, al);
 +
 + return err;

return iter-finish_entry(iter, al);  ?

--
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 02/14] perf tools: Introduce struct add_entry_iter

2013-11-01 Thread Jiri Olsa 
On Thu, Oct 31, 2013 at 03:56:04PM +0900, Namhyung Kim wrote:
 From: Namhyung Kim namhyung@lge.com

SNIP

 +}
 +
 +static int
 +iter_add_next_nop_entry(struct add_entry_iter *iter __maybe_unused,
 + struct addr_location *al __maybe_unused)
 +{
 + return 0;
 +}
 +
 +static int
 +iter_prepare_mem_entry(struct add_entry_iter *iter, struct machine *machine,
 +struct perf_evsel *evsel, struct addr_location *al,
 +struct perf_sample *sample)
 +{
 + union perf_event *event = iter-priv;
 + struct mem_info *mi;
 + u8 cpumode;
 +
 + BUG_ON(event == NULL);

the priv does not get assigned.. 'perf mem rep' aborts

[jolsa@krava perf]$ ./perf mem rep --stdio
Aborted
perf: builtin-report.c:120: iter_prepare_mem_entry: Assertion `!(event == 
((void *)0))' failed.

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/

[PATCH 02/14] perf tools: Introduce struct add_entry_iter

2013-10-31 Thread Namhyung Kim 
From: Namhyung Kim 

There're some duplicate code when adding hist entries.  They are
different in that some have branch info or mem info but generally do
same thing.  So introduce new struct add_entry_iter and add callbacks
to customize each case in general way.

The new perf_evsel__add_entry() function will look like:

  iter->prepare_entry();
  iter->add_single_entry();

  while (iter->next_entry())
iter->add_next_entry();

  iter->finish_entry();

This will help further work like the cumulative callchain patchset.

Cc: Jiri Olsa 
Cc: Stephane Eranian 
Cc: Frederic Weisbecker 
Signed-off-by: Namhyung Kim 
---
 tools/perf/builtin-report.c | 435 +---
 1 file changed, 284 insertions(+), 151 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index a7a8f7769629..f18cd43687d9 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -73,38 +73,74 @@ static int perf_report_config(const char *var, const char 
*value, void *cb)
return perf_default_config(var, value, cb);
 }
 
-static int perf_report__add_mem_hist_entry(struct perf_tool *tool,
-  struct addr_location *al,
-  struct perf_sample *sample,
-  struct perf_evsel *evsel,
-  struct machine *machine,
-  union perf_event *event)
-{
-   struct perf_report *rep = container_of(tool, struct perf_report, tool);
-   struct symbol *parent = NULL;
-   u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
-   int err = 0;
+struct add_entry_iter {
+   int total;
+   int curr;
+
+   struct perf_report *rep;
+   struct perf_evsel *evsel;
+   struct perf_sample *sample;
struct hist_entry *he;
-   struct mem_info *mi, *mx;
-   uint64_t cost;
+   struct symbol *parent;
+   void *priv;
+
+   int (*prepare_entry)(struct add_entry_iter *, struct machine *,
+struct perf_evsel *, struct addr_location *,
+struct perf_sample *);
+   int (*add_single_entry)(struct add_entry_iter *, struct addr_location 
*);
+   int (*next_entry)(struct add_entry_iter *, struct addr_location *);
+   int (*add_next_entry)(struct add_entry_iter *, struct addr_location *);
+   int (*finish_entry)(struct add_entry_iter *, struct addr_location *);
+};
 
-   if ((sort__has_parent || symbol_conf.use_callchain) &&
-   sample->callchain) {
-   err = machine__resolve_callchain(machine, evsel, al->thread,
-sample, , al,
-rep->max_stack);
-   if (err)
-   return err;
-   }
+static int
+iter_next_nop_entry(struct add_entry_iter *iter __maybe_unused,
+   struct addr_location *al __maybe_unused)
+{
+   return 0;
+}
+
+static int
+iter_add_next_nop_entry(struct add_entry_iter *iter __maybe_unused,
+   struct addr_location *al __maybe_unused)
+{
+   return 0;
+}
+
+static int
+iter_prepare_mem_entry(struct add_entry_iter *iter, struct machine *machine,
+  struct perf_evsel *evsel, struct addr_location *al,
+  struct perf_sample *sample)
+{
+   union perf_event *event = iter->priv;
+   struct mem_info *mi;
+   u8 cpumode;
+
+   BUG_ON(event == NULL);
+
+   cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
 
mi = machine__resolve_mem(machine, al->thread, sample, cpumode);
-   if (!mi)
+   if (mi == NULL)
return -ENOMEM;
 
-   if (rep->hide_unresolved && !al->sym)
+   iter->evsel = evsel;
+   iter->sample = sample;
+   iter->priv = mi;
+   return 0;
+}
+
+static int
+iter_add_single_mem_entry(struct add_entry_iter *iter, struct addr_location 
*al)
+{
+   u64 cost;
+   struct mem_info *mi = iter->priv;
+   struct hist_entry *he;
+
+   if (iter->rep->hide_unresolved && !al->sym)
return 0;
 
-   cost = sample->weight;
+   cost = iter->sample->weight;
if (!cost)
cost = 1;
 
@@ -115,11 +151,24 @@ static int perf_report__add_mem_hist_entry(struct 
perf_tool *tool,
 * and this is indirectly achieved by passing period=weight here
 * and the he_stat__add_period() function.
 */
-   he = __hists__add_entry(>hists, al, parent, NULL, mi,
+   he = __hists__add_entry(>evsel->hists, al, iter->parent, NULL, mi,
cost, cost, 0);
if (!he)
return -ENOMEM;
 
+   iter->he = he;
+   return 0;
+}
+
+static int
+iter_finish_mem_entry(struct add_entry_iter *iter, struct addr_location *al)
+{
+   struct perf_evsel

[PATCH 02/14] perf tools: Introduce struct add_entry_iter

2013-10-31 Thread Namhyung Kim 
From: Namhyung Kim namhyung@lge.com

There're some duplicate code when adding hist entries.  They are
different in that some have branch info or mem info but generally do
same thing.  So introduce new struct add_entry_iter and add callbacks
to customize each case in general way.

The new perf_evsel__add_entry() function will look like:

  iter-prepare_entry();
  iter-add_single_entry();

  while (iter-next_entry())
iter-add_next_entry();

  iter-finish_entry();

This will help further work like the cumulative callchain patchset.

Cc: Jiri Olsa jo...@redhat.com
Cc: Stephane Eranian eran...@google.com
Cc: Frederic Weisbecker fweis...@gmail.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
 tools/perf/builtin-report.c | 435 +---
 1 file changed, 284 insertions(+), 151 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index a7a8f7769629..f18cd43687d9 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -73,38 +73,74 @@ static int perf_report_config(const char *var, const char 
*value, void *cb)
return perf_default_config(var, value, cb);
 }
 
-static int perf_report__add_mem_hist_entry(struct perf_tool *tool,
-  struct addr_location *al,
-  struct perf_sample *sample,
-  struct perf_evsel *evsel,
-  struct machine *machine,
-  union perf_event *event)
-{
-   struct perf_report *rep = container_of(tool, struct perf_report, tool);
-   struct symbol *parent = NULL;
-   u8 cpumode = event-header.misc  PERF_RECORD_MISC_CPUMODE_MASK;
-   int err = 0;
+struct add_entry_iter {
+   int total;
+   int curr;
+
+   struct perf_report *rep;
+   struct perf_evsel *evsel;
+   struct perf_sample *sample;
struct hist_entry *he;
-   struct mem_info *mi, *mx;
-   uint64_t cost;
+   struct symbol *parent;
+   void *priv;
+
+   int (*prepare_entry)(struct add_entry_iter *, struct machine *,
+struct perf_evsel *, struct addr_location *,
+struct perf_sample *);
+   int (*add_single_entry)(struct add_entry_iter *, struct addr_location 
*);
+   int (*next_entry)(struct add_entry_iter *, struct addr_location *);
+   int (*add_next_entry)(struct add_entry_iter *, struct addr_location *);
+   int (*finish_entry)(struct add_entry_iter *, struct addr_location *);
+};
 
-   if ((sort__has_parent || symbol_conf.use_callchain) 
-   sample-callchain) {
-   err = machine__resolve_callchain(machine, evsel, al-thread,
-sample, parent, al,
-rep-max_stack);
-   if (err)
-   return err;
-   }
+static int
+iter_next_nop_entry(struct add_entry_iter *iter __maybe_unused,
+   struct addr_location *al __maybe_unused)
+{
+   return 0;
+}
+
+static int
+iter_add_next_nop_entry(struct add_entry_iter *iter __maybe_unused,
+   struct addr_location *al __maybe_unused)
+{
+   return 0;
+}
+
+static int
+iter_prepare_mem_entry(struct add_entry_iter *iter, struct machine *machine,
+  struct perf_evsel *evsel, struct addr_location *al,
+  struct perf_sample *sample)
+{
+   union perf_event *event = iter-priv;
+   struct mem_info *mi;
+   u8 cpumode;
+
+   BUG_ON(event == NULL);
+
+   cpumode = event-header.misc  PERF_RECORD_MISC_CPUMODE_MASK;
 
mi = machine__resolve_mem(machine, al-thread, sample, cpumode);
-   if (!mi)
+   if (mi == NULL)
return -ENOMEM;
 
-   if (rep-hide_unresolved  !al-sym)
+   iter-evsel = evsel;
+   iter-sample = sample;
+   iter-priv = mi;
+   return 0;
+}
+
+static int
+iter_add_single_mem_entry(struct add_entry_iter *iter, struct addr_location 
*al)
+{
+   u64 cost;
+   struct mem_info *mi = iter-priv;
+   struct hist_entry *he;
+
+   if (iter-rep-hide_unresolved  !al-sym)
return 0;
 
-   cost = sample-weight;
+   cost = iter-sample-weight;
if (!cost)
cost = 1;
 
@@ -115,11 +151,24 @@ static int perf_report__add_mem_hist_entry(struct 
perf_tool *tool,
 * and this is indirectly achieved by passing period=weight here
 * and the he_stat__add_period() function.
 */
-   he = __hists__add_entry(evsel-hists, al, parent, NULL, mi,
+   he = __hists__add_entry(iter-evsel-hists, al, iter-parent, NULL, mi,
cost, cost, 0);
if (!he)
return -ENOMEM;
 
+   iter-he = he;
+   return 0;
+}
+
+static int
+iter_finish_mem_entry(struct