All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jin, Yao" <yao.jin@linux.intel.com>
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	jolsa@kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Linux-kernel@vger.kernel.org, Andi Kleen <ak@linux.intel.com>,
	kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH] perf util: Use target->per_thread and target->system_wide flags
Date: Tue, 23 Jan 2018 09:08:35 +0800	[thread overview]
Message-ID: <248c18bd-b2f2-694a-e08f-989e829041ca@linux.intel.com> (raw)
In-Reply-To: <CANLsYkw4ccyxiUKTbdeSSw4N1QxZd9mQJCh4=7cmEN9M1SVvYw@mail.gmail.com>



On 1/23/2018 7:56 AM, Mathieu Poirier wrote:
> 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);
>>
>>          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);
> 
> Reviewed-and-tested-by: Mathieu Poirier "mathieu.poirier@linaro.org"
> 

Thanks a lot for reviewing the patch.

Thanks
Jin Yao

      reply	other threads:[~2018-01-23  1:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-22 22:15 [PATCH] perf util: Use target->per_thread and target->system_wide flags Jin Yao
2018-01-22 21:10 ` Mathieu Poirier
2018-01-22 23:02   ` Jin, Yao
2018-01-22 23:55     ` Mathieu Poirier
2018-01-23 14:40     ` Jiri Olsa
2018-01-24  0:12       ` Jin, Yao
2018-01-22 23:56 ` Mathieu Poirier
2018-01-23  1:08   ` Jin, Yao [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=248c18bd-b2f2-694a-e08f-989e829041ca@linux.intel.com \
    --to=yao.jin@linux.intel.com \
    --cc=Linux-kernel@vger.kernel.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@intel.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=yao.jin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.