From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753119AbdLAOob (ORCPT ); Fri, 1 Dec 2017 09:44:31 -0500 Received: from mail.kernel.org ([198.145.29.99]:51272 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753063AbdLAOo2 (ORCPT ); Fri, 1 Dec 2017 09:44:28 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EA0442187C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=acme@kernel.org Date: Fri, 1 Dec 2017 11:44:25 -0300 From: Arnaldo Carvalho de Melo To: Jin Yao Cc: jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org, ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com Subject: Re: [PATCH v5 10/12] perf util: Reuse thread_map__new_by_uid to enumerate threads from /proc Message-ID: <20171201144425.GZ3298@kernel.org> References: <1512125856-22056-1-git-send-email-yao.jin@linux.intel.com> <1512125856-22056-11-git-send-email-yao.jin@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1512125856-22056-11-git-send-email-yao.jin@linux.intel.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Fri, Dec 01, 2017 at 06:57:34PM +0800, Jin Yao escreveu: > Perf already has a function thread_map__new_by_uid() which can > enumerate all threads from /proc by uid. > > This patch creates a static function enumerate_threads() which > reuses the common code in thread_map__new_by_uid() to enumerate > threads from /proc. > > The enumerate_threads() is shared by thread_map__new_by_uid() > and a new function thread_map__new_threads(). > > The new function thread_map__new_threads() is called to enumerate > all threads from /proc. > > Signed-off-by: Jin Yao > --- > tools/perf/tests/thread-map.c | 2 +- > tools/perf/util/evlist.c | 3 ++- > tools/perf/util/thread_map.c | 19 ++++++++++++++++--- > tools/perf/util/thread_map.h | 3 ++- > 4 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c > index dbcb6a1..4de1939 100644 > --- a/tools/perf/tests/thread-map.c > +++ b/tools/perf/tests/thread-map.c > @@ -105,7 +105,7 @@ int test__thread_map_remove(struct test *test __maybe_unused, int subtest __mayb > TEST_ASSERT_VAL("failed to allocate map string", > asprintf(&str, "%d,%d", getpid(), getppid()) >= 0); > > - threads = thread_map__new_str(str, NULL, 0); > + threads = thread_map__new_str(str, NULL, 0, false); > > TEST_ASSERT_VAL("failed to allocate thread_map", > threads); > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index 199bb82..05b8f2b 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -1102,7 +1102,8 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target) > struct cpu_map *cpus; > struct thread_map *threads; > > - threads = thread_map__new_str(target->pid, target->tid, target->uid); > + threads = thread_map__new_str(target->pid, target->tid, target->uid, > + target->per_thread); > > if (!threads) > return -1; > diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c > index be0d5a7..5672268 100644 > --- a/tools/perf/util/thread_map.c > +++ b/tools/perf/util/thread_map.c > @@ -92,7 +92,7 @@ struct thread_map *thread_map__new_by_tid(pid_t tid) > return threads; > } > > -struct thread_map *thread_map__new_by_uid(uid_t uid) > +static struct thread_map *enumerate_threads(uid_t uid) > { > DIR *proc; > int max_threads = 32, items, i; > @@ -124,7 +124,7 @@ struct thread_map *thread_map__new_by_uid(uid_t uid) > if (stat(path, &st) != 0) > continue; Look, for the case where you want all threads enumerated you will incur the above stat() cost for all of them and will not use it at all... And new_threads() seems vague, I'm using the ter used by 'perf record' for system wide sampling, see the patch below: diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c index be0d5a736dea..79d11bd4543a 100644 --- a/tools/perf/util/thread_map.c +++ b/tools/perf/util/thread_map.c @@ -92,7 +92,7 @@ struct thread_map *thread_map__new_by_tid(pid_t tid) return threads; } -struct thread_map *thread_map__new_by_uid(uid_t uid) +static struct thread_map *__thread_map__new_all_cpus(uid_t uid) { DIR *proc; int max_threads = 32, items, i; @@ -113,7 +113,6 @@ struct thread_map *thread_map__new_by_uid(uid_t uid) while ((dirent = readdir(proc)) != NULL) { char *end; bool grow = false; - struct stat st; pid_t pid = strtol(dirent->d_name, &end, 10); if (*end) /* only interested in proper numerical dirents */ @@ -121,11 +120,12 @@ struct thread_map *thread_map__new_by_uid(uid_t uid) snprintf(path, sizeof(path), "/proc/%s", dirent->d_name); - if (stat(path, &st) != 0) - continue; + if (uid != UID_MAX) { + struct stat st; - if (st.st_uid != uid) - continue; + if (stat(path, &st) != 0 || st.st_uid != uid) + continue; + } snprintf(path, sizeof(path), "/proc/%d/task", pid); items = scandir(path, &namelist, filter, NULL); @@ -178,6 +178,16 @@ struct thread_map *thread_map__new_by_uid(uid_t uid) goto out_closedir; } +struct thread_map *thread_map__new_all_cpus(void) +{ + return __thread__new_all_cpus(UID_MAX); +} + +struct thread_map *thread_map__new_by_uid(uid_t uid) +{ + return __thread__new_all_cpus(uid); +} + struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid) { if (pid != -1) diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h index f15803985435..07a765fb22bb 100644 --- a/tools/perf/util/thread_map.h +++ b/tools/perf/util/thread_map.h @@ -23,6 +23,7 @@ struct thread_map *thread_map__new_dummy(void); struct thread_map *thread_map__new_by_pid(pid_t pid); struct thread_map *thread_map__new_by_tid(pid_t tid); struct thread_map *thread_map__new_by_uid(uid_t uid); +struct thread_map *thread_map__new_all_cpus(void); struct thread_map *thread_map__new(pid_t pid, pid_t tid, uid_t uid); struct thread_map *thread_map__new_event(struct thread_map_event *event);