On Tue, Mar 03, 2015 at 12:07:24PM +0900, Namhyung Kim wrote: > When data file indexing is enabled, it processes all task, comm and mmap > events first and then goes to the sample events. So all it sees is the > last comm of a thread although it has information at the time of sample. > > Sort thread's comm by time so that it can find appropriate comm at the > sample time. The thread__comm_time() will mostly work even if > PERF_SAMPLE_TIME bit is off since in that case, sample->time will be > -1 so it'll take the last comm anyway. > > Cc: Frederic Weisbecker <fweis...@gmail.com> > Signed-off-by: Namhyung Kim <namhy...@kernel.org> > --- > tools/perf/util/thread.c | 33 ++++++++++++++++++++++++++++++++- > tools/perf/util/thread.h | 2 ++ > 2 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c > index 9ebc8b1f9be5..ad96725105c2 100644 > --- a/tools/perf/util/thread.c > +++ b/tools/perf/util/thread.c > @@ -103,6 +103,21 @@ struct comm *thread__exec_comm(const struct thread > *thread) > return last; > } > > +struct comm *thread__comm_time(const struct thread *thread, u64 timestamp)
Usually thread__comm_foo() would suggest that we return the "foo" from a thread comm. For example thread__comm_len() returns the len of the last thread comm. thread__comm_str() returns the string of the last thread comm. So thread__comm_time() suggests that we return the timestamp of the default thread comm. Ideally all thread__comm_foo() accessors should now take a timestamp as an argument. Now there are quite some callers of such APIs, I'm not sure they will all turn into passing a precise timestamp, but the current callers are interested in the last comm so perhaps those can be turned into thread__last_comm[_str](). The call would gain some clarity. > +{ > + struct comm *comm; > + > + list_for_each_entry(comm, &thread->comm_list, list) { > + if (timestamp >= comm->start) > + return comm; > + } > + > + if (list_empty(&thread->comm_list)) > + return NULL; > + > + return list_last_entry(&thread->comm_list, struct comm, list); > +} Yes, handling the thread's comm lifecycle correctly with fetching the right comm at a given time is the evolution I had in mind. I haven't looked at the rest of your patchset but this change alone seem to go to the right direction. Thanks! > + > int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp, > bool exec) > { > @@ -118,7 +133,13 @@ int __thread__set_comm(struct thread *thread, const char > *str, u64 timestamp, > new = comm__new(str, timestamp, exec); > if (!new) > return -ENOMEM; > - list_add(&new->list, &thread->comm_list); > + > + /* sort by time */ > + list_for_each_entry(curr, &thread->comm_list, list) { > + if (timestamp >= curr->start) > + break; > + } > + list_add_tail(&new->list, &curr->list); > > if (exec) > unwind__flush_access(thread); > @@ -139,6 +160,16 @@ const char *thread__comm_str(const struct thread *thread) > return comm__str(comm); > } > > +const char *thread__comm_str_time(const struct thread *thread, u64 timestamp) > +{ > + const struct comm *comm = thread__comm_time(thread, timestamp); > + > + if (!comm) > + return NULL; > + > + return comm__str(comm); > +} > + > /* CHECKME: it should probably better return the max comm len from its comm > list */ > int thread__comm_len(struct thread *thread) > { > diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h > index 160fd066a7d1..be67c3bad5e7 100644 > --- a/tools/perf/util/thread.h > +++ b/tools/perf/util/thread.h > @@ -53,7 +53,9 @@ static inline int thread__set_comm(struct thread *thread, > const char *comm, > int thread__comm_len(struct thread *thread); > struct comm *thread__comm(const struct thread *thread); > struct comm *thread__exec_comm(const struct thread *thread); > +struct comm *thread__comm_time(const struct thread *thread, u64 timestamp); > const char *thread__comm_str(const struct thread *thread); > +const char *thread__comm_str_time(const struct thread *thread, u64 > timestamp); > void thread__insert_map(struct thread *thread, struct map *map); > int thread__fork(struct thread *thread, struct thread *parent, u64 > timestamp); > size_t thread__fprintf(struct thread *thread, FILE *fp); > -- > 2.2.2 > -- 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/