Messages in this thread | | | From | Mathieu Poirier <> | Date | Mon, 22 Jan 2018 14:10:48 -0700 | Subject | Re: [PATCH] perf util: Use target->per_thread and target->system_wide flags |
| |
On 22 January 2018 at 15:15, Jin Yao <yao.jin@linux.intel.com> wrote: > Mathieu Poirier reports issue in commit ("73c0ca1eee3d perf thread_map: > Enumerate all threads from /proc") that it has negative impact on > 'perf record --per-thread'. It has the effect of creating a kernel event > for each thread in the system for 'perf record --per-thread'. > > Mathieu Poirier's patch ("perf util: Do not reuse target->per_thread flag") > can fix this issue by creating a new target->all_threads flag. > > This patch is based on Mathieu Poirier's patch but it doesn't use a new > target->all_threads flag. This patch just uses 'target->per_thread && > target->system_wide' as a condition to check for all threads case. > > Signed-off-by: Jin Yao <yao.jin@linux.intel.com> > --- > tools/perf/util/evlist.c | 2 +- > tools/perf/util/thread_map.c | 4 ++-- > tools/perf/util/thread_map.h | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index 120efd8..9dff74a 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -1106,7 +1106,7 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target) > struct thread_map *threads; > > threads = thread_map__new_str(target->pid, target->tid, target->uid, > - target->per_thread); > + target->per_thread && target->system_wide);
At first glance I thought your solution would do the trick but perf record does use target->system_wide when the '-a' switch is used. Moreover specifying the '-a' switch doesn't prevent the '--per-thread' option from being used as well, making both target->perf_thread and target_system_wide equal to true (and that is not good).
Although not a fan of adding more to struct target, the advantage of having target->all_threads is that we are guaranteed that it isn't used anywhere else.
Let me know what you think, Mathieu
> > if (!threads) > return -1; > diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c > index 3e1038f..729dad8 100644 > --- a/tools/perf/util/thread_map.c > +++ b/tools/perf/util/thread_map.c > @@ -323,7 +323,7 @@ struct thread_map *thread_map__new_by_tid_str(const char *tid_str) > } > > struct thread_map *thread_map__new_str(const char *pid, const char *tid, > - uid_t uid, bool per_thread) > + uid_t uid, bool all_threads) > { > if (pid) > return thread_map__new_by_pid_str(pid); > @@ -331,7 +331,7 @@ struct thread_map *thread_map__new_str(const char *pid, const char *tid, > if (!tid && uid != UINT_MAX) > return thread_map__new_by_uid(uid); > > - if (per_thread) > + if (all_threads) > return thread_map__new_all_cpus(); > > return thread_map__new_by_tid_str(tid); > diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h > index 0a806b9..5ec91cf 100644 > --- a/tools/perf/util/thread_map.h > +++ b/tools/perf/util/thread_map.h > @@ -31,7 +31,7 @@ struct thread_map *thread_map__get(struct thread_map *map); > void thread_map__put(struct thread_map *map); > > struct thread_map *thread_map__new_str(const char *pid, > - const char *tid, uid_t uid, bool per_thread); > + const char *tid, uid_t uid, bool all_threads); > > struct thread_map *thread_map__new_by_tid_str(const char *tid_str); > > -- > 2.7.4 >
| |