* [PATCH] perf report: fix handling of memory sampling sort orders
@ 2013-02-22 14:28 Stephane Eranian
2013-02-22 18:43 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 3+ messages in thread
From: Stephane Eranian @ 2013-02-22 14:28 UTC (permalink / raw)
To: linux-kernel; +Cc: acme, jolsa, peterz, mingo, namhyung
When the memory sampling sort orders were used on perf.data
files without memory sampling data, it would crash perf. This
patch fixes this by handling the lack of memory information
gracefully, printing N/A and formatting columns correctly
whenever necessary.
This patch is to be applied on top of the memory sampling
patchset. It is relative to Arnaldo's perf/mem branch.
Reported-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Stephane Eranian <eranian@google.com>
---
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 4008b7f..98572ca 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -99,6 +99,13 @@ struct perf_sample {
struct stack_dump user_stack;
};
+#define PERF_MEM_DATA_SRC_NONE \
+ (PERF_MEM_S(OP, NA) |\
+ PERF_MEM_S(LVL, NA) |\
+ PERF_MEM_S(SNOOP, NA) |\
+ PERF_MEM_S(LOCK, NA) |\
+ PERF_MEM_S(TLB, NA))
+
struct build_id_event {
struct perf_event_header header;
pid_t pid;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index bace0cc..d1799a4 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1175,7 +1175,7 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
array++;
}
- data->dsrc = 0;
+ data->dsrc = PERF_MEM_DATA_SRC_NONE;
if (type & PERF_SAMPLE_DATA_SRC) {
data->dsrc = *array;
array++;
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 043593d..454d7f1 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -118,7 +118,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
hists__new_col_len(hists, HISTC_SYMBOL_TO, symlen);
hists__set_unres_dso_col_len(hists, HISTC_DSO_TO);
}
- } else if (h->mem_info) {
+ }
+
+ if (h->mem_info) {
/*
* +4 accounts for '[x] ' priv level info
* +2 account of 0x prefix on raw addresses
@@ -141,11 +143,15 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
symlen = unresolved_col_width + 4 + 2;
hists__set_unres_dso_col_len(hists, HISTC_MEM_DADDR_DSO);
}
- hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
- hists__new_col_len(hists, HISTC_MEM_TLB, 22);
- hists__new_col_len(hists, HISTC_MEM_SNOOP, 12);
- hists__new_col_len(hists, HISTC_MEM_LVL, 21+3);
+ } else {
+ symlen = unresolved_col_width + 4 + 2;
+ hists__new_col_len(hists, HISTC_MEM_DADDR_SYMBOL, symlen);
+ hists__set_unres_dso_col_len(hists, HISTC_MEM_DADDR_DSO);
}
+ hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
+ hists__new_col_len(hists, HISTC_MEM_TLB, 22);
+ hists__new_col_len(hists, HISTC_MEM_SNOOP, 12);
+ hists__new_col_len(hists, HISTC_MEM_LVL, 21+3);
}
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 1dc3860..0fdaac7 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -198,7 +198,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
}
ret += repsep_snprintf(bf + ret, size - ret, "[%c] ", level);
- if (sym) {
+ if (sym && map) {
if (map->type == MAP__VARIABLE) {
ret += repsep_snprintf(bf + ret, size - ret, "%s", sym->name);
ret += repsep_snprintf(bf + ret, size - ret, "+0x%llx",
@@ -469,39 +469,72 @@ static int hist_entry__mispredict_snprintf(struct hist_entry *self, char *bf,
static int64_t
sort__daddr_cmp(struct hist_entry *left, struct hist_entry *right)
{
- struct addr_map_symbol *l = &left->mem_info->daddr;
- struct addr_map_symbol *r = &right->mem_info->daddr;
+ uint64_t l = 0, r = 0;
- return (int64_t)(r->addr - l->addr);
+ if (left->mem_info)
+ l = left->mem_info->daddr.addr;
+ if (right->mem_info)
+ r = right->mem_info->daddr.addr;
+
+ return (int64_t)(r - l);
}
static int hist_entry__daddr_snprintf(struct hist_entry *self, char *bf,
size_t size, unsigned int width)
{
- return _hist_entry__sym_snprintf(self->mem_info->daddr.map,
- self->mem_info->daddr.sym,
- self->mem_info->daddr.addr,
- self->level, bf, size, width);
+ uint64_t addr = 0;
+ struct map *map = NULL;
+ struct symbol *sym = NULL;
+
+ if (self->mem_info) {
+ addr = self->mem_info->daddr.addr;
+ map = self->mem_info->daddr.map;
+ sym = self->mem_info->daddr.sym;
+ }
+ return _hist_entry__sym_snprintf(map, sym, addr, self->level, bf, size,
+ width);
}
static int64_t
sort__dso_daddr_cmp(struct hist_entry *left, struct hist_entry *right)
{
- return _sort__dso_cmp(left->mem_info->daddr.map, right->mem_info->daddr.map);
+ struct map *map_l = NULL;
+ struct map *map_r = NULL;
+
+ if (left->mem_info)
+ map_l = left->mem_info->daddr.map;
+ if (right->mem_info)
+ map_r = right->mem_info->daddr.map;
+
+ return _sort__dso_cmp(map_l, map_r);
}
static int hist_entry__dso_daddr_snprintf(struct hist_entry *self, char *bf,
size_t size, unsigned int width)
{
- return _hist_entry__dso_snprintf(self->mem_info->daddr.map, bf, size,
- width);
+ struct map *map = NULL;
+
+ if (self->mem_info)
+ map = self->mem_info->daddr.map;
+
+ return _hist_entry__dso_snprintf(map, bf, size, width);
}
static int64_t
sort__locked_cmp(struct hist_entry *left, struct hist_entry *right)
{
- union perf_mem_data_src dsrc_l = left->mem_info->dsrc;
- union perf_mem_data_src dsrc_r = right->mem_info->dsrc;
+ union perf_mem_data_src dsrc_l;
+ union perf_mem_data_src dsrc_r;
+
+ if (left->mem_info)
+ dsrc_l = left->mem_info->dsrc;
+ else
+ dsrc_l.mem_lock = PERF_MEM_LOCK_NA;
+
+ if (right->mem_info)
+ dsrc_r = right->mem_info->dsrc;
+ else
+ dsrc_r.mem_lock = PERF_MEM_LOCK_NA;
return (int64_t)(dsrc_r.mem_lock - dsrc_l.mem_lock);
}
@@ -509,8 +542,11 @@ sort__locked_cmp(struct hist_entry *left, struct hist_entry *right)
static int hist_entry__locked_snprintf(struct hist_entry *self, char *bf,
size_t size, unsigned int width)
{
- const char *out = "??";
- u64 mask = self->mem_info->dsrc.mem_lock;
+ const char *out;
+ u64 mask = PERF_MEM_LOCK_NA;
+
+ if (self->mem_info)
+ mask = self->mem_info->dsrc.mem_lock;
if (mask & PERF_MEM_LOCK_NA)
out = "N/A";
@@ -525,8 +561,18 @@ static int hist_entry__locked_snprintf(struct hist_entry *self, char *bf,
static int64_t
sort__tlb_cmp(struct hist_entry *left, struct hist_entry *right)
{
- union perf_mem_data_src dsrc_l = left->mem_info->dsrc;
- union perf_mem_data_src dsrc_r = right->mem_info->dsrc;
+ union perf_mem_data_src dsrc_l;
+ union perf_mem_data_src dsrc_r;
+
+ if (left->mem_info)
+ dsrc_l = left->mem_info->dsrc;
+ else
+ dsrc_l.mem_dtlb = PERF_MEM_TLB_NA;
+
+ if (right->mem_info)
+ dsrc_r = right->mem_info->dsrc;
+ else
+ dsrc_r.mem_dtlb = PERF_MEM_TLB_NA;
return (int64_t)(dsrc_r.mem_dtlb - dsrc_l.mem_dtlb);
}
@@ -548,11 +594,14 @@ static int hist_entry__tlb_snprintf(struct hist_entry *self, char *bf,
char out[64];
size_t sz = sizeof(out) - 1; /* -1 for null termination */
size_t l = 0, i;
- u64 m = self->mem_info->dsrc.mem_dtlb;
+ u64 m = PERF_MEM_TLB_NA;
u64 hit, miss;
out[0] = '\0';
+ if (self->mem_info)
+ m = self->mem_info->dsrc.mem_dtlb;
+
hit = m & PERF_MEM_TLB_HIT;
miss = m & PERF_MEM_TLB_MISS;
@@ -569,6 +618,8 @@ static int hist_entry__tlb_snprintf(struct hist_entry *self, char *bf,
strncat(out, tlb_access[i], sz - l);
l += strlen(tlb_access[i]);
}
+ if (*out == '\0')
+ strcpy(out, "N/A");
if (hit)
strncat(out, " hit", sz - l);
if (miss)
@@ -580,8 +631,18 @@ static int hist_entry__tlb_snprintf(struct hist_entry *self, char *bf,
static int64_t
sort__lvl_cmp(struct hist_entry *left, struct hist_entry *right)
{
- union perf_mem_data_src dsrc_l = left->mem_info->dsrc;
- union perf_mem_data_src dsrc_r = right->mem_info->dsrc;
+ union perf_mem_data_src dsrc_l;
+ union perf_mem_data_src dsrc_r;
+
+ if (left->mem_info)
+ dsrc_l = left->mem_info->dsrc;
+ else
+ dsrc_l.mem_lvl = PERF_MEM_LVL_NA;
+
+ if (right->mem_info)
+ dsrc_r = right->mem_info->dsrc;
+ else
+ dsrc_r.mem_lvl = PERF_MEM_LVL_NA;
return (int64_t)(dsrc_r.mem_lvl - dsrc_l.mem_lvl);
}
@@ -610,9 +671,12 @@ static int hist_entry__lvl_snprintf(struct hist_entry *self, char *bf,
char out[64];
size_t sz = sizeof(out) - 1; /* -1 for null termination */
size_t i, l = 0;
- u64 m = self->mem_info->dsrc.mem_lvl;
+ u64 m = PERF_MEM_LVL_NA;
u64 hit, miss;
+ if (self->mem_info)
+ m = self->mem_info->dsrc.mem_lvl;
+
out[0] = '\0';
hit = m & PERF_MEM_LVL_HIT;
@@ -631,6 +695,8 @@ static int hist_entry__lvl_snprintf(struct hist_entry *self, char *bf,
strncat(out, mem_lvl[i], sz - l);
l += strlen(mem_lvl[i]);
}
+ if (*out == '\0')
+ strcpy(out, "N/A");
if (hit)
strncat(out, " hit", sz - l);
if (miss)
@@ -642,8 +708,18 @@ static int hist_entry__lvl_snprintf(struct hist_entry *self, char *bf,
static int64_t
sort__snoop_cmp(struct hist_entry *left, struct hist_entry *right)
{
- union perf_mem_data_src dsrc_l = left->mem_info->dsrc;
- union perf_mem_data_src dsrc_r = right->mem_info->dsrc;
+ union perf_mem_data_src dsrc_l;
+ union perf_mem_data_src dsrc_r;
+
+ if (left->mem_info)
+ dsrc_l = left->mem_info->dsrc;
+ else
+ dsrc_l.mem_snoop = PERF_MEM_SNOOP_NA;
+
+ if (right->mem_info)
+ dsrc_r = right->mem_info->dsrc;
+ else
+ dsrc_r.mem_snoop = PERF_MEM_SNOOP_NA;
return (int64_t)(dsrc_r.mem_snoop - dsrc_l.mem_snoop);
}
@@ -663,10 +739,13 @@ static int hist_entry__snoop_snprintf(struct hist_entry *self, char *bf,
char out[64];
size_t sz = sizeof(out) - 1; /* -1 for null termination */
size_t i, l = 0;
- u64 m = self->mem_info->dsrc.mem_snoop;
+ u64 m = PERF_MEM_SNOOP_NA;
out[0] = '\0';
+ if (self->mem_info)
+ m = self->mem_info->dsrc.mem_snoop;
+
for (i = 0; m && i < NUM_SNOOP_ACCESS; i++, m >>= 1) {
if (!(m & 0x1))
continue;
@@ -678,6 +757,9 @@ static int hist_entry__snoop_snprintf(struct hist_entry *self, char *bf,
l += strlen(snoop_access[i]);
}
+ if (*out == '\0')
+ strcpy(out, "N/A");
+
return repsep_snprintf(bf, size, "%-*s", width, out);
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] perf report: fix handling of memory sampling sort orders
2013-02-22 14:28 [PATCH] perf report: fix handling of memory sampling sort orders Stephane Eranian
@ 2013-02-22 18:43 ` Arnaldo Carvalho de Melo
2013-02-26 5:46 ` Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-02-22 18:43 UTC (permalink / raw)
To: Stephane Eranian; +Cc: linux-kernel, jolsa, peterz, mingo, namhyung
Em Fri, Feb 22, 2013 at 03:28:38PM +0100, Stephane Eranian escreveu:
>
> When the memory sampling sort orders were used on perf.data
> files without memory sampling data, it would crash perf. This
> patch fixes this by handling the lack of memory information
> gracefully, printing N/A and formatting columns correctly
> whenever necessary.
>
> This patch is to be applied on top of the memory sampling
> patchset. It is relative to Arnaldo's perf/mem branch.
Ok, folded that into the original patch, so that we can bisect things in
this area.
Updating the perf/mem branch with this, i.e. force pushed.
- Arnaldo
> Reported-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
>
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index 4008b7f..98572ca 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -99,6 +99,13 @@ struct perf_sample {
> struct stack_dump user_stack;
> };
>
> +#define PERF_MEM_DATA_SRC_NONE \
> + (PERF_MEM_S(OP, NA) |\
> + PERF_MEM_S(LVL, NA) |\
> + PERF_MEM_S(SNOOP, NA) |\
> + PERF_MEM_S(LOCK, NA) |\
> + PERF_MEM_S(TLB, NA))
> +
> struct build_id_event {
> struct perf_event_header header;
> pid_t pid;
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index bace0cc..d1799a4 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1175,7 +1175,7 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
> array++;
> }
>
> - data->dsrc = 0;
> + data->dsrc = PERF_MEM_DATA_SRC_NONE;
> if (type & PERF_SAMPLE_DATA_SRC) {
> data->dsrc = *array;
> array++;
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 043593d..454d7f1 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -118,7 +118,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
> hists__new_col_len(hists, HISTC_SYMBOL_TO, symlen);
> hists__set_unres_dso_col_len(hists, HISTC_DSO_TO);
> }
> - } else if (h->mem_info) {
> + }
> +
> + if (h->mem_info) {
> /*
> * +4 accounts for '[x] ' priv level info
> * +2 account of 0x prefix on raw addresses
> @@ -141,11 +143,15 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
> symlen = unresolved_col_width + 4 + 2;
> hists__set_unres_dso_col_len(hists, HISTC_MEM_DADDR_DSO);
> }
> - hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
> - hists__new_col_len(hists, HISTC_MEM_TLB, 22);
> - hists__new_col_len(hists, HISTC_MEM_SNOOP, 12);
> - hists__new_col_len(hists, HISTC_MEM_LVL, 21+3);
> + } else {
> + symlen = unresolved_col_width + 4 + 2;
> + hists__new_col_len(hists, HISTC_MEM_DADDR_SYMBOL, symlen);
> + hists__set_unres_dso_col_len(hists, HISTC_MEM_DADDR_DSO);
> }
> + hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
> + hists__new_col_len(hists, HISTC_MEM_TLB, 22);
> + hists__new_col_len(hists, HISTC_MEM_SNOOP, 12);
> + hists__new_col_len(hists, HISTC_MEM_LVL, 21+3);
>
> }
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 1dc3860..0fdaac7 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -198,7 +198,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
> }
>
> ret += repsep_snprintf(bf + ret, size - ret, "[%c] ", level);
> - if (sym) {
> + if (sym && map) {
> if (map->type == MAP__VARIABLE) {
> ret += repsep_snprintf(bf + ret, size - ret, "%s", sym->name);
> ret += repsep_snprintf(bf + ret, size - ret, "+0x%llx",
> @@ -469,39 +469,72 @@ static int hist_entry__mispredict_snprintf(struct hist_entry *self, char *bf,
> static int64_t
> sort__daddr_cmp(struct hist_entry *left, struct hist_entry *right)
> {
> - struct addr_map_symbol *l = &left->mem_info->daddr;
> - struct addr_map_symbol *r = &right->mem_info->daddr;
> + uint64_t l = 0, r = 0;
>
> - return (int64_t)(r->addr - l->addr);
> + if (left->mem_info)
> + l = left->mem_info->daddr.addr;
> + if (right->mem_info)
> + r = right->mem_info->daddr.addr;
> +
> + return (int64_t)(r - l);
> }
>
> static int hist_entry__daddr_snprintf(struct hist_entry *self, char *bf,
> size_t size, unsigned int width)
> {
> - return _hist_entry__sym_snprintf(self->mem_info->daddr.map,
> - self->mem_info->daddr.sym,
> - self->mem_info->daddr.addr,
> - self->level, bf, size, width);
> + uint64_t addr = 0;
> + struct map *map = NULL;
> + struct symbol *sym = NULL;
> +
> + if (self->mem_info) {
> + addr = self->mem_info->daddr.addr;
> + map = self->mem_info->daddr.map;
> + sym = self->mem_info->daddr.sym;
> + }
> + return _hist_entry__sym_snprintf(map, sym, addr, self->level, bf, size,
> + width);
> }
>
> static int64_t
> sort__dso_daddr_cmp(struct hist_entry *left, struct hist_entry *right)
> {
> - return _sort__dso_cmp(left->mem_info->daddr.map, right->mem_info->daddr.map);
> + struct map *map_l = NULL;
> + struct map *map_r = NULL;
> +
> + if (left->mem_info)
> + map_l = left->mem_info->daddr.map;
> + if (right->mem_info)
> + map_r = right->mem_info->daddr.map;
> +
> + return _sort__dso_cmp(map_l, map_r);
> }
>
> static int hist_entry__dso_daddr_snprintf(struct hist_entry *self, char *bf,
> size_t size, unsigned int width)
> {
> - return _hist_entry__dso_snprintf(self->mem_info->daddr.map, bf, size,
> - width);
> + struct map *map = NULL;
> +
> + if (self->mem_info)
> + map = self->mem_info->daddr.map;
> +
> + return _hist_entry__dso_snprintf(map, bf, size, width);
> }
>
> static int64_t
> sort__locked_cmp(struct hist_entry *left, struct hist_entry *right)
> {
> - union perf_mem_data_src dsrc_l = left->mem_info->dsrc;
> - union perf_mem_data_src dsrc_r = right->mem_info->dsrc;
> + union perf_mem_data_src dsrc_l;
> + union perf_mem_data_src dsrc_r;
> +
> + if (left->mem_info)
> + dsrc_l = left->mem_info->dsrc;
> + else
> + dsrc_l.mem_lock = PERF_MEM_LOCK_NA;
> +
> + if (right->mem_info)
> + dsrc_r = right->mem_info->dsrc;
> + else
> + dsrc_r.mem_lock = PERF_MEM_LOCK_NA;
>
> return (int64_t)(dsrc_r.mem_lock - dsrc_l.mem_lock);
> }
> @@ -509,8 +542,11 @@ sort__locked_cmp(struct hist_entry *left, struct hist_entry *right)
> static int hist_entry__locked_snprintf(struct hist_entry *self, char *bf,
> size_t size, unsigned int width)
> {
> - const char *out = "??";
> - u64 mask = self->mem_info->dsrc.mem_lock;
> + const char *out;
> + u64 mask = PERF_MEM_LOCK_NA;
> +
> + if (self->mem_info)
> + mask = self->mem_info->dsrc.mem_lock;
>
> if (mask & PERF_MEM_LOCK_NA)
> out = "N/A";
> @@ -525,8 +561,18 @@ static int hist_entry__locked_snprintf(struct hist_entry *self, char *bf,
> static int64_t
> sort__tlb_cmp(struct hist_entry *left, struct hist_entry *right)
> {
> - union perf_mem_data_src dsrc_l = left->mem_info->dsrc;
> - union perf_mem_data_src dsrc_r = right->mem_info->dsrc;
> + union perf_mem_data_src dsrc_l;
> + union perf_mem_data_src dsrc_r;
> +
> + if (left->mem_info)
> + dsrc_l = left->mem_info->dsrc;
> + else
> + dsrc_l.mem_dtlb = PERF_MEM_TLB_NA;
> +
> + if (right->mem_info)
> + dsrc_r = right->mem_info->dsrc;
> + else
> + dsrc_r.mem_dtlb = PERF_MEM_TLB_NA;
>
> return (int64_t)(dsrc_r.mem_dtlb - dsrc_l.mem_dtlb);
> }
> @@ -548,11 +594,14 @@ static int hist_entry__tlb_snprintf(struct hist_entry *self, char *bf,
> char out[64];
> size_t sz = sizeof(out) - 1; /* -1 for null termination */
> size_t l = 0, i;
> - u64 m = self->mem_info->dsrc.mem_dtlb;
> + u64 m = PERF_MEM_TLB_NA;
> u64 hit, miss;
>
> out[0] = '\0';
>
> + if (self->mem_info)
> + m = self->mem_info->dsrc.mem_dtlb;
> +
> hit = m & PERF_MEM_TLB_HIT;
> miss = m & PERF_MEM_TLB_MISS;
>
> @@ -569,6 +618,8 @@ static int hist_entry__tlb_snprintf(struct hist_entry *self, char *bf,
> strncat(out, tlb_access[i], sz - l);
> l += strlen(tlb_access[i]);
> }
> + if (*out == '\0')
> + strcpy(out, "N/A");
> if (hit)
> strncat(out, " hit", sz - l);
> if (miss)
> @@ -580,8 +631,18 @@ static int hist_entry__tlb_snprintf(struct hist_entry *self, char *bf,
> static int64_t
> sort__lvl_cmp(struct hist_entry *left, struct hist_entry *right)
> {
> - union perf_mem_data_src dsrc_l = left->mem_info->dsrc;
> - union perf_mem_data_src dsrc_r = right->mem_info->dsrc;
> + union perf_mem_data_src dsrc_l;
> + union perf_mem_data_src dsrc_r;
> +
> + if (left->mem_info)
> + dsrc_l = left->mem_info->dsrc;
> + else
> + dsrc_l.mem_lvl = PERF_MEM_LVL_NA;
> +
> + if (right->mem_info)
> + dsrc_r = right->mem_info->dsrc;
> + else
> + dsrc_r.mem_lvl = PERF_MEM_LVL_NA;
>
> return (int64_t)(dsrc_r.mem_lvl - dsrc_l.mem_lvl);
> }
> @@ -610,9 +671,12 @@ static int hist_entry__lvl_snprintf(struct hist_entry *self, char *bf,
> char out[64];
> size_t sz = sizeof(out) - 1; /* -1 for null termination */
> size_t i, l = 0;
> - u64 m = self->mem_info->dsrc.mem_lvl;
> + u64 m = PERF_MEM_LVL_NA;
> u64 hit, miss;
>
> + if (self->mem_info)
> + m = self->mem_info->dsrc.mem_lvl;
> +
> out[0] = '\0';
>
> hit = m & PERF_MEM_LVL_HIT;
> @@ -631,6 +695,8 @@ static int hist_entry__lvl_snprintf(struct hist_entry *self, char *bf,
> strncat(out, mem_lvl[i], sz - l);
> l += strlen(mem_lvl[i]);
> }
> + if (*out == '\0')
> + strcpy(out, "N/A");
> if (hit)
> strncat(out, " hit", sz - l);
> if (miss)
> @@ -642,8 +708,18 @@ static int hist_entry__lvl_snprintf(struct hist_entry *self, char *bf,
> static int64_t
> sort__snoop_cmp(struct hist_entry *left, struct hist_entry *right)
> {
> - union perf_mem_data_src dsrc_l = left->mem_info->dsrc;
> - union perf_mem_data_src dsrc_r = right->mem_info->dsrc;
> + union perf_mem_data_src dsrc_l;
> + union perf_mem_data_src dsrc_r;
> +
> + if (left->mem_info)
> + dsrc_l = left->mem_info->dsrc;
> + else
> + dsrc_l.mem_snoop = PERF_MEM_SNOOP_NA;
> +
> + if (right->mem_info)
> + dsrc_r = right->mem_info->dsrc;
> + else
> + dsrc_r.mem_snoop = PERF_MEM_SNOOP_NA;
>
> return (int64_t)(dsrc_r.mem_snoop - dsrc_l.mem_snoop);
> }
> @@ -663,10 +739,13 @@ static int hist_entry__snoop_snprintf(struct hist_entry *self, char *bf,
> char out[64];
> size_t sz = sizeof(out) - 1; /* -1 for null termination */
> size_t i, l = 0;
> - u64 m = self->mem_info->dsrc.mem_snoop;
> + u64 m = PERF_MEM_SNOOP_NA;
>
> out[0] = '\0';
>
> + if (self->mem_info)
> + m = self->mem_info->dsrc.mem_snoop;
> +
> for (i = 0; m && i < NUM_SNOOP_ACCESS; i++, m >>= 1) {
> if (!(m & 0x1))
> continue;
> @@ -678,6 +757,9 @@ static int hist_entry__snoop_snprintf(struct hist_entry *self, char *bf,
> l += strlen(snoop_access[i]);
> }
>
> + if (*out == '\0')
> + strcpy(out, "N/A");
> +
> return repsep_snprintf(bf, size, "%-*s", width, out);
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] perf report: fix handling of memory sampling sort orders
2013-02-22 18:43 ` Arnaldo Carvalho de Melo
@ 2013-02-26 5:46 ` Namhyung Kim
0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2013-02-26 5:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Stephane Eranian, linux-kernel, jolsa, peterz, mingo
Hi,
On Fri, 22 Feb 2013 15:43:22 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 22, 2013 at 03:28:38PM +0100, Stephane Eranian escreveu:
>>
>> When the memory sampling sort orders were used on perf.data
>> files without memory sampling data, it would crash perf. This
>> patch fixes this by handling the lack of memory information
>> gracefully, printing N/A and formatting columns correctly
>> whenever necessary.
It'd be great if we could detect the perf.data contains memory sampling
data and then warn user about it and exit like I did for branch
sampling. What do you think?
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-02-26 5:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-22 14:28 [PATCH] perf report: fix handling of memory sampling sort orders Stephane Eranian
2013-02-22 18:43 ` Arnaldo Carvalho de Melo
2013-02-26 5:46 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox