Re: [PATCH RFC V2 03/10] petf tools: using comm_str to replace comm in hist_entry
On Wed, Sep 13, 2017 at 12:21:56PM -0300, Arnaldo Carvalho de Melo wrote: > Em Sun, Sep 10, 2017 at 07:23:16PM -0700, kan.li...@intel.com escreveu: > > From: Kan Liang> > > > For hist_entry, it only needs comm_str for cmp. > > And thinking a bit more, isn't this even a bug fix, as at the time of > that sample that was the comm string associated with that thread? > > Frédéric, am I nutz? I think it's ok, because struct comm holds just one comm string which is 'correct/accurate' at the time the hist entry is created this patch just seems to switch comm pointer with comm string pointer, I'm guessing it's useful in follow up patches jirka > > - Arnaldo > > > Signed-off-by: Kan Liang > > --- > > tools/perf/util/hist.c | 4 ++-- > > tools/perf/util/sort.c | 8 > > tools/perf/util/sort.h | 2 +- > > 3 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > > index e60d8d8..0f00dd9 100644 > > --- a/tools/perf/util/hist.c > > +++ b/tools/perf/util/hist.c > > @@ -587,7 +587,7 @@ __hists__add_entry(struct hists *hists, > > struct namespaces *ns = thread__namespaces(al->thread); > > struct hist_entry entry = { > > .thread = al->thread, > > - .comm = thread__comm(al->thread), > > + .comm_str = thread__comm_str(al->thread), > > .cgroup_id = { > > .dev = ns ? ns->link_info[CGROUP_NS_INDEX].dev : 0, > > .ino = ns ? ns->link_info[CGROUP_NS_INDEX].ino : 0, > > @@ -944,7 +944,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter > > *iter, > > .hists = evsel__hists(evsel), > > .cpu = al->cpu, > > .thread = al->thread, > > - .comm = thread__comm(al->thread), > > + .comm_str = thread__comm_str(al->thread), > > .ip = al->addr, > > .ms = { > > .map = al->map, > > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > > index eb3ab90..99ab411 100644 > > --- a/tools/perf/util/sort.c > > +++ b/tools/perf/util/sort.c > > @@ -114,26 +114,26 @@ static int64_t > > sort__comm_cmp(struct hist_entry *left, struct hist_entry *right) > > { > > /* Compare the addr that should be unique among comm */ > > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > > + return strcmp(right->comm_str, left->comm_str); > > } > > > > static int64_t > > sort__comm_collapse(struct hist_entry *left, struct hist_entry *right) > > { > > /* Compare the addr that should be unique among comm */ > > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > > + return strcmp(right->comm_str, left->comm_str); > > } > > > > static int64_t > > sort__comm_sort(struct hist_entry *left, struct hist_entry *right) > > { > > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > > + return strcmp(right->comm_str, left->comm_str); > > } > > > > static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf, > > size_t size, unsigned int width) > > { > > - return repsep_snprintf(bf, size, "%-*.*s", width, width, > > comm__str(he->comm)); > > + return repsep_snprintf(bf, size, "%-*.*s", width, width, he->comm_str); > > } > > > > struct sort_entry sort_comm = { > > diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h > > index f36dc49..861cbe7 100644 > > --- a/tools/perf/util/sort.h > > +++ b/tools/perf/util/sort.h > > @@ -96,7 +96,7 @@ struct hist_entry { > > struct he_stat *stat_acc; > > struct map_symbol ms; > > struct thread *thread; > > - struct comm *comm; > > + const char *comm_str; > > struct namespace_id cgroup_id; > > u64 ip; > > u64 transaction; > > -- > > 2.5.5
Re: [PATCH RFC V2 03/10] petf tools: using comm_str to replace comm in hist_entry
On Wed, Sep 13, 2017 at 12:21:56PM -0300, Arnaldo Carvalho de Melo wrote: > Em Sun, Sep 10, 2017 at 07:23:16PM -0700, kan.li...@intel.com escreveu: > > From: Kan Liang > > > > For hist_entry, it only needs comm_str for cmp. > > And thinking a bit more, isn't this even a bug fix, as at the time of > that sample that was the comm string associated with that thread? > > Frédéric, am I nutz? I think it's ok, because struct comm holds just one comm string which is 'correct/accurate' at the time the hist entry is created this patch just seems to switch comm pointer with comm string pointer, I'm guessing it's useful in follow up patches jirka > > - Arnaldo > > > Signed-off-by: Kan Liang > > --- > > tools/perf/util/hist.c | 4 ++-- > > tools/perf/util/sort.c | 8 > > tools/perf/util/sort.h | 2 +- > > 3 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > > index e60d8d8..0f00dd9 100644 > > --- a/tools/perf/util/hist.c > > +++ b/tools/perf/util/hist.c > > @@ -587,7 +587,7 @@ __hists__add_entry(struct hists *hists, > > struct namespaces *ns = thread__namespaces(al->thread); > > struct hist_entry entry = { > > .thread = al->thread, > > - .comm = thread__comm(al->thread), > > + .comm_str = thread__comm_str(al->thread), > > .cgroup_id = { > > .dev = ns ? ns->link_info[CGROUP_NS_INDEX].dev : 0, > > .ino = ns ? ns->link_info[CGROUP_NS_INDEX].ino : 0, > > @@ -944,7 +944,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter > > *iter, > > .hists = evsel__hists(evsel), > > .cpu = al->cpu, > > .thread = al->thread, > > - .comm = thread__comm(al->thread), > > + .comm_str = thread__comm_str(al->thread), > > .ip = al->addr, > > .ms = { > > .map = al->map, > > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > > index eb3ab90..99ab411 100644 > > --- a/tools/perf/util/sort.c > > +++ b/tools/perf/util/sort.c > > @@ -114,26 +114,26 @@ static int64_t > > sort__comm_cmp(struct hist_entry *left, struct hist_entry *right) > > { > > /* Compare the addr that should be unique among comm */ > > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > > + return strcmp(right->comm_str, left->comm_str); > > } > > > > static int64_t > > sort__comm_collapse(struct hist_entry *left, struct hist_entry *right) > > { > > /* Compare the addr that should be unique among comm */ > > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > > + return strcmp(right->comm_str, left->comm_str); > > } > > > > static int64_t > > sort__comm_sort(struct hist_entry *left, struct hist_entry *right) > > { > > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > > + return strcmp(right->comm_str, left->comm_str); > > } > > > > static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf, > > size_t size, unsigned int width) > > { > > - return repsep_snprintf(bf, size, "%-*.*s", width, width, > > comm__str(he->comm)); > > + return repsep_snprintf(bf, size, "%-*.*s", width, width, he->comm_str); > > } > > > > struct sort_entry sort_comm = { > > diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h > > index f36dc49..861cbe7 100644 > > --- a/tools/perf/util/sort.h > > +++ b/tools/perf/util/sort.h > > @@ -96,7 +96,7 @@ struct hist_entry { > > struct he_stat *stat_acc; > > struct map_symbol ms; > > struct thread *thread; > > - struct comm *comm; > > + const char *comm_str; > > struct namespace_id cgroup_id; > > u64 ip; > > u64 transaction; > > -- > > 2.5.5
Re: [PATCH RFC V2 03/10] petf tools: using comm_str to replace comm in hist_entry
Em Sun, Sep 10, 2017 at 07:23:16PM -0700, kan.li...@intel.com escreveu: > From: Kan Liang> > For hist_entry, it only needs comm_str for cmp. And thinking a bit more, isn't this even a bug fix, as at the time of that sample that was the comm string associated with that thread? Frédéric, am I nutz? - Arnaldo > Signed-off-by: Kan Liang > --- > tools/perf/util/hist.c | 4 ++-- > tools/perf/util/sort.c | 8 > tools/perf/util/sort.h | 2 +- > 3 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index e60d8d8..0f00dd9 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -587,7 +587,7 @@ __hists__add_entry(struct hists *hists, > struct namespaces *ns = thread__namespaces(al->thread); > struct hist_entry entry = { > .thread = al->thread, > - .comm = thread__comm(al->thread), > + .comm_str = thread__comm_str(al->thread), > .cgroup_id = { > .dev = ns ? ns->link_info[CGROUP_NS_INDEX].dev : 0, > .ino = ns ? ns->link_info[CGROUP_NS_INDEX].ino : 0, > @@ -944,7 +944,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter > *iter, > .hists = evsel__hists(evsel), > .cpu = al->cpu, > .thread = al->thread, > - .comm = thread__comm(al->thread), > + .comm_str = thread__comm_str(al->thread), > .ip = al->addr, > .ms = { > .map = al->map, > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > index eb3ab90..99ab411 100644 > --- a/tools/perf/util/sort.c > +++ b/tools/perf/util/sort.c > @@ -114,26 +114,26 @@ static int64_t > sort__comm_cmp(struct hist_entry *left, struct hist_entry *right) > { > /* Compare the addr that should be unique among comm */ > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > + return strcmp(right->comm_str, left->comm_str); > } > > static int64_t > sort__comm_collapse(struct hist_entry *left, struct hist_entry *right) > { > /* Compare the addr that should be unique among comm */ > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > + return strcmp(right->comm_str, left->comm_str); > } > > static int64_t > sort__comm_sort(struct hist_entry *left, struct hist_entry *right) > { > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > + return strcmp(right->comm_str, left->comm_str); > } > > static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf, >size_t size, unsigned int width) > { > - return repsep_snprintf(bf, size, "%-*.*s", width, width, > comm__str(he->comm)); > + return repsep_snprintf(bf, size, "%-*.*s", width, width, he->comm_str); > } > > struct sort_entry sort_comm = { > diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h > index f36dc49..861cbe7 100644 > --- a/tools/perf/util/sort.h > +++ b/tools/perf/util/sort.h > @@ -96,7 +96,7 @@ struct hist_entry { > struct he_stat *stat_acc; > struct map_symbol ms; > struct thread *thread; > - struct comm *comm; > + const char *comm_str; > struct namespace_id cgroup_id; > u64 ip; > u64 transaction; > -- > 2.5.5
Re: [PATCH RFC V2 03/10] petf tools: using comm_str to replace comm in hist_entry
Em Sun, Sep 10, 2017 at 07:23:16PM -0700, kan.li...@intel.com escreveu: > From: Kan Liang > > For hist_entry, it only needs comm_str for cmp. And thinking a bit more, isn't this even a bug fix, as at the time of that sample that was the comm string associated with that thread? Frédéric, am I nutz? - Arnaldo > Signed-off-by: Kan Liang > --- > tools/perf/util/hist.c | 4 ++-- > tools/perf/util/sort.c | 8 > tools/perf/util/sort.h | 2 +- > 3 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index e60d8d8..0f00dd9 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -587,7 +587,7 @@ __hists__add_entry(struct hists *hists, > struct namespaces *ns = thread__namespaces(al->thread); > struct hist_entry entry = { > .thread = al->thread, > - .comm = thread__comm(al->thread), > + .comm_str = thread__comm_str(al->thread), > .cgroup_id = { > .dev = ns ? ns->link_info[CGROUP_NS_INDEX].dev : 0, > .ino = ns ? ns->link_info[CGROUP_NS_INDEX].ino : 0, > @@ -944,7 +944,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter > *iter, > .hists = evsel__hists(evsel), > .cpu = al->cpu, > .thread = al->thread, > - .comm = thread__comm(al->thread), > + .comm_str = thread__comm_str(al->thread), > .ip = al->addr, > .ms = { > .map = al->map, > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > index eb3ab90..99ab411 100644 > --- a/tools/perf/util/sort.c > +++ b/tools/perf/util/sort.c > @@ -114,26 +114,26 @@ static int64_t > sort__comm_cmp(struct hist_entry *left, struct hist_entry *right) > { > /* Compare the addr that should be unique among comm */ > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > + return strcmp(right->comm_str, left->comm_str); > } > > static int64_t > sort__comm_collapse(struct hist_entry *left, struct hist_entry *right) > { > /* Compare the addr that should be unique among comm */ > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > + return strcmp(right->comm_str, left->comm_str); > } > > static int64_t > sort__comm_sort(struct hist_entry *left, struct hist_entry *right) > { > - return strcmp(comm__str(right->comm), comm__str(left->comm)); > + return strcmp(right->comm_str, left->comm_str); > } > > static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf, >size_t size, unsigned int width) > { > - return repsep_snprintf(bf, size, "%-*.*s", width, width, > comm__str(he->comm)); > + return repsep_snprintf(bf, size, "%-*.*s", width, width, he->comm_str); > } > > struct sort_entry sort_comm = { > diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h > index f36dc49..861cbe7 100644 > --- a/tools/perf/util/sort.h > +++ b/tools/perf/util/sort.h > @@ -96,7 +96,7 @@ struct hist_entry { > struct he_stat *stat_acc; > struct map_symbol ms; > struct thread *thread; > - struct comm *comm; > + const char *comm_str; > struct namespace_id cgroup_id; > u64 ip; > u64 transaction; > -- > 2.5.5
[PATCH RFC V2 03/10] petf tools: using comm_str to replace comm in hist_entry
From: Kan LiangFor hist_entry, it only needs comm_str for cmp. Signed-off-by: Kan Liang --- tools/perf/util/hist.c | 4 ++-- tools/perf/util/sort.c | 8 tools/perf/util/sort.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index e60d8d8..0f00dd9 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -587,7 +587,7 @@ __hists__add_entry(struct hists *hists, struct namespaces *ns = thread__namespaces(al->thread); struct hist_entry entry = { .thread = al->thread, - .comm = thread__comm(al->thread), + .comm_str = thread__comm_str(al->thread), .cgroup_id = { .dev = ns ? ns->link_info[CGROUP_NS_INDEX].dev : 0, .ino = ns ? ns->link_info[CGROUP_NS_INDEX].ino : 0, @@ -944,7 +944,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter, .hists = evsel__hists(evsel), .cpu = al->cpu, .thread = al->thread, - .comm = thread__comm(al->thread), + .comm_str = thread__comm_str(al->thread), .ip = al->addr, .ms = { .map = al->map, diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index eb3ab90..99ab411 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -114,26 +114,26 @@ static int64_t sort__comm_cmp(struct hist_entry *left, struct hist_entry *right) { /* Compare the addr that should be unique among comm */ - return strcmp(comm__str(right->comm), comm__str(left->comm)); + return strcmp(right->comm_str, left->comm_str); } static int64_t sort__comm_collapse(struct hist_entry *left, struct hist_entry *right) { /* Compare the addr that should be unique among comm */ - return strcmp(comm__str(right->comm), comm__str(left->comm)); + return strcmp(right->comm_str, left->comm_str); } static int64_t sort__comm_sort(struct hist_entry *left, struct hist_entry *right) { - return strcmp(comm__str(right->comm), comm__str(left->comm)); + return strcmp(right->comm_str, left->comm_str); } static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf, size_t size, unsigned int width) { - return repsep_snprintf(bf, size, "%-*.*s", width, width, comm__str(he->comm)); + return repsep_snprintf(bf, size, "%-*.*s", width, width, he->comm_str); } struct sort_entry sort_comm = { diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h index f36dc49..861cbe7 100644 --- a/tools/perf/util/sort.h +++ b/tools/perf/util/sort.h @@ -96,7 +96,7 @@ struct hist_entry { struct he_stat *stat_acc; struct map_symbol ms; struct thread *thread; - struct comm *comm; + const char *comm_str; struct namespace_id cgroup_id; u64 ip; u64 transaction; -- 2.5.5
[PATCH RFC V2 03/10] petf tools: using comm_str to replace comm in hist_entry
From: Kan Liang For hist_entry, it only needs comm_str for cmp. Signed-off-by: Kan Liang --- tools/perf/util/hist.c | 4 ++-- tools/perf/util/sort.c | 8 tools/perf/util/sort.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index e60d8d8..0f00dd9 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -587,7 +587,7 @@ __hists__add_entry(struct hists *hists, struct namespaces *ns = thread__namespaces(al->thread); struct hist_entry entry = { .thread = al->thread, - .comm = thread__comm(al->thread), + .comm_str = thread__comm_str(al->thread), .cgroup_id = { .dev = ns ? ns->link_info[CGROUP_NS_INDEX].dev : 0, .ino = ns ? ns->link_info[CGROUP_NS_INDEX].ino : 0, @@ -944,7 +944,7 @@ iter_add_next_cumulative_entry(struct hist_entry_iter *iter, .hists = evsel__hists(evsel), .cpu = al->cpu, .thread = al->thread, - .comm = thread__comm(al->thread), + .comm_str = thread__comm_str(al->thread), .ip = al->addr, .ms = { .map = al->map, diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index eb3ab90..99ab411 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -114,26 +114,26 @@ static int64_t sort__comm_cmp(struct hist_entry *left, struct hist_entry *right) { /* Compare the addr that should be unique among comm */ - return strcmp(comm__str(right->comm), comm__str(left->comm)); + return strcmp(right->comm_str, left->comm_str); } static int64_t sort__comm_collapse(struct hist_entry *left, struct hist_entry *right) { /* Compare the addr that should be unique among comm */ - return strcmp(comm__str(right->comm), comm__str(left->comm)); + return strcmp(right->comm_str, left->comm_str); } static int64_t sort__comm_sort(struct hist_entry *left, struct hist_entry *right) { - return strcmp(comm__str(right->comm), comm__str(left->comm)); + return strcmp(right->comm_str, left->comm_str); } static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf, size_t size, unsigned int width) { - return repsep_snprintf(bf, size, "%-*.*s", width, width, comm__str(he->comm)); + return repsep_snprintf(bf, size, "%-*.*s", width, width, he->comm_str); } struct sort_entry sort_comm = { diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h index f36dc49..861cbe7 100644 --- a/tools/perf/util/sort.h +++ b/tools/perf/util/sort.h @@ -96,7 +96,7 @@ struct hist_entry { struct he_stat *stat_acc; struct map_symbol ms; struct thread *thread; - struct comm *comm; + const char *comm_str; struct namespace_id cgroup_id; u64 ip; u64 transaction; -- 2.5.5