perf diff: Fix to work with new hists design

The perf diff command is broken since:
  perf hists: Threaded addition and sorting of entries
  commit 1980c2ebd7020d82c024b8c4046849b38e78e7da

Several places were broken:
  - hists data need to be collected into opened sessions instead
    of into events
  - session's hists data need to be initialized properly when the
    session is created
  - hist_entry__pcnt_snprintf: the percentage and displacement
    buffer preparation must not use 'ret' because it's used
    as a pointer to the final buffer

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20120322133726.GB1601@m.brq.redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 4f19513d7dda068eed48141ee4b3b300381cdc3d..d29d350fb2b731187d92233d6c0ddb5ef2a368a6 100644 (file)
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -24,6 +24,11 @@ static char    diff__default_sort_order[] = "dso,symbol";
 static bool  force;
 static bool show_displacement;
 
+struct perf_diff {
+       struct perf_tool tool;
+       struct perf_session *session;
+};
+
 static int hists__add_entry(struct hists *self,
                            struct addr_location *al, u64 period)
 {
@@ -32,12 +37,14 @@ static int hists__add_entry(struct hists *self,
        return -ENOMEM;
 }
 
-static int diff__process_sample_event(struct perf_tool *tool __used,
+static int diff__process_sample_event(struct perf_tool *tool,
                                      union perf_event *event,
                                      struct perf_sample *sample,
                                      struct perf_evsel *evsel __used,
                                      struct machine *machine)
 {
+       struct perf_diff *_diff = container_of(tool, struct perf_diff, tool);
+       struct perf_session *session = _diff->session;
        struct addr_location al;
 
        if (perf_event__preprocess_sample(event, machine, &al, sample, NULL) < 0) {
@@ -49,24 +56,26 @@ static int diff__process_sample_event(struct perf_tool *tool __used,
        if (al.filtered || al.sym == NULL)
                return 0;
 
-       if (hists__add_entry(&evsel->hists, &al, sample->period)) {
+       if (hists__add_entry(&session->hists, &al, sample->period)) {
                pr_warning("problem incrementing symbol period, skipping event\n");
                return -1;
        }
 
-       evsel->hists.stats.total_period += sample->period;
+       session->hists.stats.total_period += sample->period;
        return 0;
 }
 
-static struct perf_tool perf_diff = {
-       .sample = diff__process_sample_event,
-       .mmap   = perf_event__process_mmap,
-       .comm   = perf_event__process_comm,
-       .exit   = perf_event__process_task,
-       .fork   = perf_event__process_task,
-       .lost   = perf_event__process_lost,
-       .ordered_samples = true,
-       .ordering_requires_timestamps = true,
+static struct perf_diff diff = {
+       .tool = {
+               .sample = diff__process_sample_event,
+               .mmap   = perf_event__process_mmap,
+               .comm   = perf_event__process_comm,
+               .exit   = perf_event__process_task,
+               .fork   = perf_event__process_task,
+               .lost   = perf_event__process_lost,
+               .ordered_samples = true,
+               .ordering_requires_timestamps = true,
+       },
 };
 
 static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
@@ -107,12 +116,6 @@ static void hists__resort_entries(struct hists *self)
        self->entries = tmp;
 }
 
-static void hists__set_positions(struct hists *self)
-{
-       hists__output_resort(self);
-       hists__resort_entries(self);
-}
-
 static struct hist_entry *hists__find_entry(struct hists *self,
                                            struct hist_entry *he)
 {
@@ -146,30 +149,37 @@ static void hists__match(struct hists *older, struct hists *newer)
 static int __cmd_diff(void)
 {
        int ret, i;
+#define older (session[0])
+#define newer (session[1])
        struct perf_session *session[2];
 
-       session[0] = perf_session__new(input_old, O_RDONLY, force, false, &perf_diff);
-       session[1] = perf_session__new(input_new, O_RDONLY, force, false, &perf_diff);
+       older = perf_session__new(input_old, O_RDONLY, force, false,
+                                 &diff.tool);
+       newer = perf_session__new(input_new, O_RDONLY, force, false,
+                                 &diff.tool);
        if (session[0] == NULL || session[1] == NULL)
                return -ENOMEM;
 
        for (i = 0; i < 2; ++i) {
-               ret = perf_session__process_events(session[i], &perf_diff);
+               diff.session = session[i];
+               ret = perf_session__process_events(session[i], &diff.tool);
                if (ret)
                        goto out_delete;
+               hists__output_resort(&session[i]->hists);
        }
 
-       hists__output_resort(&session[1]->hists);
        if (show_displacement)
-               hists__set_positions(&session[0]->hists);
+               hists__resort_entries(&older->hists);
 
-       hists__match(&session[0]->hists, &session[1]->hists);
-       hists__fprintf(&session[1]->hists, &session[0]->hists,
+       hists__match(&older->hists, &newer->hists);
+       hists__fprintf(&newer->hists, &older->hists,
                       show_displacement, true, 0, 0, stdout);
 out_delete:
        for (i = 0; i < 2; ++i)
                perf_session__delete(session[i]);
        return ret;
+#undef older
+#undef newer
 }
 
 static const char * const diff_usage[] = {

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0221700075c54c20c916089877fc1e4aa88a0330..d9da62a7234fd3c81389549cbb478eb371a2183c 100644 (file)
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -34,7 +34,7 @@ int __perf_evsel__sample_size(u64 sample_type)
        return size;
 }
 
-static void hists__init(struct hists *hists)
+void hists__init(struct hists *hists)
 {
        memset(hists, 0, sizeof(*hists));
        hists->entries_in_array[0] = hists->entries_in_array[1] = RB_ROOT;

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 3158ca3d69a14bee73036472f03899244fe9c4ad..3d6b3e4cb66bb9bfbb32b75958ef1f23a4880163 100644 (file)
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -170,4 +170,6 @@ static inline int perf_evsel__sample_size(struct perf_evsel *evsel)
        return __perf_evsel__sample_size(evsel->attr.sample_type);
 }
 
+void hists__init(struct hists *hists);
+
 #endif /* __PERF_EVSEL_H */

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 5fb19013ca0c21b3970c93b49a79565dc029a200..c61235f81260a0b63eca35ec6b9cf19ce799b77e 100644 (file)
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -891,9 +891,9 @@ static int hist_entry__pcnt_snprintf(struct hist_entry *he, char *s,
                diff = new_percent - old_percent;
 
                if (fabs(diff) >= 0.01)
-                       ret += scnprintf(bf, sizeof(bf), "%+4.2F%%", diff);
+                       scnprintf(bf, sizeof(bf), "%+4.2F%%", diff);
                else
-                       ret += scnprintf(bf, sizeof(bf), " ");
+                       scnprintf(bf, sizeof(bf), " ");
 
                if (sep)
                        ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
@@ -902,9 +902,9 @@ static int hist_entry__pcnt_snprintf(struct hist_entry *he, char *s,
 
                if (show_displacement) {
                        if (displacement)
-                               ret += scnprintf(bf, sizeof(bf), "%+4ld", displacement);
+                               scnprintf(bf, sizeof(bf), "%+4ld", displacement);
                        else
-                               ret += scnprintf(bf, sizeof(bf), " ");
+                               scnprintf(bf, sizeof(bf), " ");
 
                        if (sep)
                                ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 002ebbf59f48e84aa69898de786b6227e2402ad7..9412e3b05f6888c9504c7ab6b3dd0effe73de627 100644 (file)
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -140,6 +140,7 @@ struct perf_session *perf_session__new(const char *filename, int mode,
        INIT_LIST_HEAD(&self->ordered_samples.sample_cache);
        INIT_LIST_HEAD(&self->ordered_samples.to_free);
        machine__init(&self->host_machine, "", HOST_KERNEL_ID);
+       hists__init(&self->hists);
 
        if (mode == O_RDONLY) {
                if (perf_session__open(self, force) < 0)