* [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening
@ 2015-05-20 16:03 Namhyung Kim
2015-05-20 16:03 ` [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size() Namhyung Kim
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Namhyung Kim @ 2015-05-20 16:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Adrian Hunter,
David Ahern
When dso__data_read_offset/addr() is called without prior
dso__data_fd() (or other functions which call it internally), it
failed to open dso in data_file_size() since its binary type was not
identified.
However calling dso__data_fd() in dso__data_read_offset() will hurt
performance as it grabs a global lock everytime. So factor out the
loop on the binary type in dso__data_fd(), and call it from both.
Reported-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/dso.c | 59 ++++++++++++++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 1b96c8d18435..516e0c25ea16 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -440,15 +440,7 @@ void dso__data_close(struct dso *dso)
pthread_mutex_unlock(&dso__data_open_lock);
}
-/**
- * dso__data_fd - Get dso's data file descriptor
- * @dso: dso object
- * @machine: machine object
- *
- * External interface to find dso's file, open it and
- * returns file descriptor.
- */
-int dso__data_fd(struct dso *dso, struct machine *machine)
+static void try_to_open_dso(struct dso *dso, struct machine *machine)
{
enum dso_binary_type binary_type_data[] = {
DSO_BINARY_TYPE__BUILD_ID_CACHE,
@@ -457,13 +449,8 @@ int dso__data_fd(struct dso *dso, struct machine *machine)
};
int i = 0;
- if (dso->data.status == DSO_DATA_STATUS_ERROR)
- return -1;
-
- pthread_mutex_lock(&dso__data_open_lock);
-
if (dso->data.fd >= 0)
- goto out;
+ return;
if (dso->binary_type != DSO_BINARY_TYPE__NOT_FOUND) {
dso->data.fd = open_dso(dso, machine);
@@ -483,8 +470,25 @@ out:
dso->data.status = DSO_DATA_STATUS_OK;
else
dso->data.status = DSO_DATA_STATUS_ERROR;
+}
+
+/**
+ * dso__data_fd - Get dso's data file descriptor
+ * @dso: dso object
+ * @machine: machine object
+ *
+ * External interface to find dso's file, open it and
+ * returns file descriptor.
+ */
+int dso__data_fd(struct dso *dso, struct machine *machine)
+{
+ if (dso->data.status == DSO_DATA_STATUS_ERROR)
+ return -1;
+ pthread_mutex_lock(&dso__data_open_lock);
+ try_to_open_dso(dso, machine);
pthread_mutex_unlock(&dso__data_open_lock);
+
return dso->data.fd;
}
@@ -609,13 +613,12 @@ dso_cache__read(struct dso *dso, struct machine *machine,
* dso->data.fd might be closed if other thread opened another
* file (dso) due to open file limit (RLIMIT_NOFILE).
*/
+ try_to_open_dso(dso, machine);
+
if (dso->data.fd < 0) {
- dso->data.fd = open_dso(dso, machine);
- if (dso->data.fd < 0) {
- ret = -errno;
- dso->data.status = DSO_DATA_STATUS_ERROR;
- break;
- }
+ ret = -errno;
+ dso->data.status = DSO_DATA_STATUS_ERROR;
+ break;
}
cache_offset = offset & DSO__DATA_CACHE_MASK;
@@ -702,19 +705,21 @@ static int data_file_size(struct dso *dso, struct machine *machine)
if (dso->data.file_size)
return 0;
+ if (dso->data.status == DSO_DATA_STATUS_ERROR)
+ return -1;
+
pthread_mutex_lock(&dso__data_open_lock);
/*
* dso->data.fd might be closed if other thread opened another
* file (dso) due to open file limit (RLIMIT_NOFILE).
*/
+ try_to_open_dso(dso, machine);
+
if (dso->data.fd < 0) {
- dso->data.fd = open_dso(dso, machine);
- if (dso->data.fd < 0) {
- ret = -errno;
- dso->data.status = DSO_DATA_STATUS_ERROR;
- goto out;
- }
+ ret = -errno;
+ dso->data.status = DSO_DATA_STATUS_ERROR;
+ goto out;
}
if (fstat(dso->data.fd, &st) < 0) {
--
2.4.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size() 2015-05-20 16:03 [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Namhyung Kim @ 2015-05-20 16:03 ` Namhyung Kim 2015-05-21 7:03 ` Adrian Hunter 2015-05-27 16:48 ` [tip:perf/core] " tip-bot for Namhyung Kim 2015-05-20 16:03 ` [PATCH v2 3/3] perf tools: Add dso__data_get/put_fd() Namhyung Kim ` (2 subsequent siblings) 3 siblings, 2 replies; 10+ messages in thread From: Namhyung Kim @ 2015-05-20 16:03 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Adrian Hunter, David Ahern It seems that the dso__data_fd() was needed to find a binary type since open in data_file_size() alone used to fail. But as it can open the dso fine now, the dso__data_fd() can go away. Cc: Adrian Hunter <adrian.hunter@intel.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/dso.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 516e0c25ea16..e95e850dd832 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -745,12 +745,6 @@ out: */ off_t dso__data_size(struct dso *dso, struct machine *machine) { - int fd; - - fd = dso__data_fd(dso, machine); - if (fd < 0) - return fd; - if (data_file_size(dso, machine)) return -1; -- 2.4.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size() 2015-05-20 16:03 ` [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size() Namhyung Kim @ 2015-05-21 7:03 ` Adrian Hunter 2015-05-27 16:48 ` [tip:perf/core] " tip-bot for Namhyung Kim 1 sibling, 0 replies; 10+ messages in thread From: Adrian Hunter @ 2015-05-21 7:03 UTC (permalink / raw) To: Namhyung Kim, Arnaldo Carvalho de Melo Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern On 20/05/15 19:03, Namhyung Kim wrote: > It seems that the dso__data_fd() was needed to find a binary type > since open in data_file_size() alone used to fail. But as it can open > the dso fine now, the dso__data_fd() can go away. > > Cc: Adrian Hunter <adrian.hunter@intel.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Adrian Hunter <adrian.hunter@intel.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tip:perf/core] perf tools: Get rid of dso__data_fd() from dso__data_size() 2015-05-20 16:03 ` [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size() Namhyung Kim 2015-05-21 7:03 ` Adrian Hunter @ 2015-05-27 16:48 ` tip-bot for Namhyung Kim 1 sibling, 0 replies; 10+ messages in thread From: tip-bot for Namhyung Kim @ 2015-05-27 16:48 UTC (permalink / raw) To: linux-tip-commits Cc: dsahern, hpa, a.p.zijlstra, jolsa, acme, mingo, adrian.hunter, tglx, linux-kernel, namhyung Commit-ID: e840238d7c6afcde0f6402aac3a74723ee9c448f Gitweb: http://git.kernel.org/tip/e840238d7c6afcde0f6402aac3a74723ee9c448f Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Thu, 21 May 2015 01:03:40 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Wed, 27 May 2015 12:21:44 -0300 perf tools: Get rid of dso__data_fd() from dso__data_size() It seems that the dso__data_fd() was needed to find a binary type since open in data_file_size() alone used to fail. But as it can open the dso fine now, the dso__data_fd() can go away. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Adrian Hunter <adrian.hunter@intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Link: http://lkml.kernel.org/r/1432137821-10853-2-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/dso.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 516e0c2..e95e850 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -745,12 +745,6 @@ out: */ off_t dso__data_size(struct dso *dso, struct machine *machine) { - int fd; - - fd = dso__data_fd(dso, machine); - if (fd < 0) - return fd; - if (data_file_size(dso, machine)) return -1; ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] perf tools: Add dso__data_get/put_fd() 2015-05-20 16:03 [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Namhyung Kim 2015-05-20 16:03 ` [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size() Namhyung Kim @ 2015-05-20 16:03 ` Namhyung Kim 2015-05-21 7:06 ` Adrian Hunter 2015-05-27 16:49 ` [tip:perf/core] " tip-bot for Namhyung Kim 2015-05-21 7:03 ` [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Adrian Hunter 2015-05-27 16:48 ` [tip:perf/core] " tip-bot for Namhyung Kim 3 siblings, 2 replies; 10+ messages in thread From: Namhyung Kim @ 2015-05-20 16:03 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Adrian Hunter, David Ahern Using dso__data_fd() in multi-thread environment is not safe since returned fd can be closed and/or reused anytime. So convert it to the dso__data_get/put_fd() pair to protect the access with lock. The original dso__data_fd() is deprecated and kept only for testing. Cc: Adrian Hunter <adrian.hunter@intel.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/tests/dso-data.c | 11 +++++++++++ tools/perf/util/dso.c | 31 ++++++++++++++++++++++--------- tools/perf/util/dso.h | 13 +++++++++---- tools/perf/util/unwind-libunwind.c | 11 ++++++++--- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c index 513e5febbe5a..3e41c61bd861 100644 --- a/tools/perf/tests/dso-data.c +++ b/tools/perf/tests/dso-data.c @@ -99,6 +99,17 @@ struct test_data_offset offsets[] = { }, }; +/* move it from util/dso.c for compatibility */ +static int dso__data_fd(struct dso *dso, struct machine *machine) +{ + int fd = dso__data_get_fd(dso, machine); + + if (fd >= 0) + dso__data_put_fd(dso); + + return fd; +} + int test__dso_data(void) { struct machine machine; diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index e95e850dd832..7e11a700303f 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -473,25 +473,35 @@ out: } /** - * dso__data_fd - Get dso's data file descriptor + * dso__data_get_fd - Get dso's data file descriptor * @dso: dso object * @machine: machine object * * External interface to find dso's file, open it and - * returns file descriptor. + * returns file descriptor. It should be paired with + * dso__data_put_fd() if it returns non-negative value. */ -int dso__data_fd(struct dso *dso, struct machine *machine) +int dso__data_get_fd(struct dso *dso, struct machine *machine) { if (dso->data.status == DSO_DATA_STATUS_ERROR) return -1; - pthread_mutex_lock(&dso__data_open_lock); + if (pthread_mutex_lock(&dso__data_open_lock) < 0) + return -1; + try_to_open_dso(dso, machine); - pthread_mutex_unlock(&dso__data_open_lock); + + if (dso->data.fd < 0) + pthread_mutex_unlock(&dso__data_open_lock); return dso->data.fd; } +void dso__data_put_fd(struct dso *dso __maybe_unused) +{ + pthread_mutex_unlock(&dso__data_open_lock); +} + bool dso__data_status_seen(struct dso *dso, enum dso_data_status_seen by) { u32 flag = 1 << by; @@ -1199,12 +1209,15 @@ size_t dso__fprintf(struct dso *dso, enum map_type type, FILE *fp) enum dso_type dso__type(struct dso *dso, struct machine *machine) { int fd; + enum dso_type type = DSO__TYPE_UNKNOWN; - fd = dso__data_fd(dso, machine); - if (fd < 0) - return DSO__TYPE_UNKNOWN; + fd = dso__data_get_fd(dso, machine); + if (fd >= 0) { + type = dso__type_fd(fd); + dso__data_put_fd(dso); + } - return dso__type_fd(fd); + return type; } int dso__strerror_load(struct dso *dso, char *buf, size_t buflen) diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index b26ec3ab1336..bcec06ad73a2 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -240,7 +240,8 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, /* * The dso__data_* external interface provides following functions: - * dso__data_fd + * dso__data_get_fd + * dso__data_put_fd * dso__data_close * dso__data_size * dso__data_read_offset @@ -257,8 +258,11 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, * The current usage of the dso__data_* interface is as follows: * * Get DSO's fd: - * int fd = dso__data_fd(dso, machine); - * USE 'fd' SOMEHOW + * int fd = dso__data_get_fd(dso, machine); + * if (fd >= 0) { + * USE 'fd' SOMEHOW + * dso__data_put_fd(dso); + * } * * Read DSO's data: * n = dso__data_read_offset(dso_0, &machine, 0, buf, BUFSIZE); @@ -277,7 +281,8 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, * * TODO */ -int dso__data_fd(struct dso *dso, struct machine *machine); +int dso__data_get_fd(struct dso *dso, struct machine *machine); +void dso__data_put_fd(struct dso *dso __maybe_unused); void dso__data_close(struct dso *dso); off_t dso__data_size(struct dso *dso, struct machine *machine); diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c index 7b09a443a280..f079b63f0b7f 100644 --- a/tools/perf/util/unwind-libunwind.c +++ b/tools/perf/util/unwind-libunwind.c @@ -269,13 +269,14 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine, u64 offset = dso->data.eh_frame_hdr_offset; if (offset == 0) { - fd = dso__data_fd(dso, machine); + fd = dso__data_get_fd(dso, machine); if (fd < 0) return -EINVAL; /* Check the .eh_frame section for unwinding info */ offset = elf_section_offset(fd, ".eh_frame_hdr"); dso->data.eh_frame_hdr_offset = offset; + dso__data_put_fd(dso); } if (offset) @@ -294,13 +295,14 @@ static int read_unwind_spec_debug_frame(struct dso *dso, u64 ofs = dso->data.debug_frame_offset; if (ofs == 0) { - fd = dso__data_fd(dso, machine); + fd = dso__data_get_fd(dso, machine); if (fd < 0) return -EINVAL; /* Check the .debug_frame section for unwinding info */ ofs = elf_section_offset(fd, ".debug_frame"); dso->data.debug_frame_offset = ofs; + dso__data_put_fd(dso); } *offset = ofs; @@ -353,10 +355,13 @@ find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pi, #ifndef NO_LIBUNWIND_DEBUG_FRAME /* Check the .debug_frame section for unwinding info */ if (!read_unwind_spec_debug_frame(map->dso, ui->machine, &segbase)) { - int fd = dso__data_fd(map->dso, ui->machine); + int fd = dso__data_get_fd(map->dso, ui->machine); int is_exec = elf_is_exec(fd, map->dso->name); unw_word_t base = is_exec ? 0 : map->start; + if (fd >= 0) + dso__data_put_fd(dso); + memset(&di, 0, sizeof(di)); if (dwarf_find_debug_frame(0, &di, ip, base, map->dso->name, map->start, map->end)) -- 2.4.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] perf tools: Add dso__data_get/put_fd() 2015-05-20 16:03 ` [PATCH v2 3/3] perf tools: Add dso__data_get/put_fd() Namhyung Kim @ 2015-05-21 7:06 ` Adrian Hunter 2015-05-27 16:49 ` [tip:perf/core] " tip-bot for Namhyung Kim 1 sibling, 0 replies; 10+ messages in thread From: Adrian Hunter @ 2015-05-21 7:06 UTC (permalink / raw) To: Namhyung Kim, Arnaldo Carvalho de Melo Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern On 20/05/15 19:03, Namhyung Kim wrote: > Using dso__data_fd() in multi-thread environment is not safe since > returned fd can be closed and/or reused anytime. So convert it to the > dso__data_get/put_fd() pair to protect the access with lock. > > The original dso__data_fd() is deprecated and kept only for testing. > > Cc: Adrian Hunter <adrian.hunter@intel.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Could convert dso_cache__read() and data_file_size() over to dso__data_get/put_fd() too, but that can come later, so: Acked-by: Adrian Hunter <adrian.hunter@intel.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tip:perf/core] perf tools: Add dso__data_get/put_fd() 2015-05-20 16:03 ` [PATCH v2 3/3] perf tools: Add dso__data_get/put_fd() Namhyung Kim 2015-05-21 7:06 ` Adrian Hunter @ 2015-05-27 16:49 ` tip-bot for Namhyung Kim 1 sibling, 0 replies; 10+ messages in thread From: tip-bot for Namhyung Kim @ 2015-05-27 16:49 UTC (permalink / raw) To: linux-tip-commits Cc: adrian.hunter, linux-kernel, mingo, acme, tglx, a.p.zijlstra, dsahern, namhyung, hpa, jolsa Commit-ID: 4bb11d012ab248d0e383008d725be0d26a74fac2 Gitweb: http://git.kernel.org/tip/4bb11d012ab248d0e383008d725be0d26a74fac2 Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Thu, 21 May 2015 01:03:41 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Wed, 27 May 2015 12:21:44 -0300 perf tools: Add dso__data_get/put_fd() Using dso__data_fd() in multi-thread environment is not safe since returned fd can be closed and/or reused anytime. So convert it to the dso__data_get/put_fd() pair to protect the access with lock. The original dso__data_fd() is deprecated and kept only for testing. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Adrian Hunter <adrian.hunter@intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Link: http://lkml.kernel.org/r/1432137821-10853-3-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/tests/dso-data.c | 11 +++++++++++ tools/perf/util/dso.c | 31 ++++++++++++++++++++++--------- tools/perf/util/dso.h | 13 +++++++++---- tools/perf/util/unwind-libunwind.c | 11 ++++++++--- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c index 513e5fe..3e41c61 100644 --- a/tools/perf/tests/dso-data.c +++ b/tools/perf/tests/dso-data.c @@ -99,6 +99,17 @@ struct test_data_offset offsets[] = { }, }; +/* move it from util/dso.c for compatibility */ +static int dso__data_fd(struct dso *dso, struct machine *machine) +{ + int fd = dso__data_get_fd(dso, machine); + + if (fd >= 0) + dso__data_put_fd(dso); + + return fd; +} + int test__dso_data(void) { struct machine machine; diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index e95e850..7e11a70 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -473,25 +473,35 @@ out: } /** - * dso__data_fd - Get dso's data file descriptor + * dso__data_get_fd - Get dso's data file descriptor * @dso: dso object * @machine: machine object * * External interface to find dso's file, open it and - * returns file descriptor. + * returns file descriptor. It should be paired with + * dso__data_put_fd() if it returns non-negative value. */ -int dso__data_fd(struct dso *dso, struct machine *machine) +int dso__data_get_fd(struct dso *dso, struct machine *machine) { if (dso->data.status == DSO_DATA_STATUS_ERROR) return -1; - pthread_mutex_lock(&dso__data_open_lock); + if (pthread_mutex_lock(&dso__data_open_lock) < 0) + return -1; + try_to_open_dso(dso, machine); - pthread_mutex_unlock(&dso__data_open_lock); + + if (dso->data.fd < 0) + pthread_mutex_unlock(&dso__data_open_lock); return dso->data.fd; } +void dso__data_put_fd(struct dso *dso __maybe_unused) +{ + pthread_mutex_unlock(&dso__data_open_lock); +} + bool dso__data_status_seen(struct dso *dso, enum dso_data_status_seen by) { u32 flag = 1 << by; @@ -1199,12 +1209,15 @@ size_t dso__fprintf(struct dso *dso, enum map_type type, FILE *fp) enum dso_type dso__type(struct dso *dso, struct machine *machine) { int fd; + enum dso_type type = DSO__TYPE_UNKNOWN; - fd = dso__data_fd(dso, machine); - if (fd < 0) - return DSO__TYPE_UNKNOWN; + fd = dso__data_get_fd(dso, machine); + if (fd >= 0) { + type = dso__type_fd(fd); + dso__data_put_fd(dso); + } - return dso__type_fd(fd); + return type; } int dso__strerror_load(struct dso *dso, char *buf, size_t buflen) diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index b26ec3a..bcec06a 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -240,7 +240,8 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, /* * The dso__data_* external interface provides following functions: - * dso__data_fd + * dso__data_get_fd + * dso__data_put_fd * dso__data_close * dso__data_size * dso__data_read_offset @@ -257,8 +258,11 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, * The current usage of the dso__data_* interface is as follows: * * Get DSO's fd: - * int fd = dso__data_fd(dso, machine); - * USE 'fd' SOMEHOW + * int fd = dso__data_get_fd(dso, machine); + * if (fd >= 0) { + * USE 'fd' SOMEHOW + * dso__data_put_fd(dso); + * } * * Read DSO's data: * n = dso__data_read_offset(dso_0, &machine, 0, buf, BUFSIZE); @@ -277,7 +281,8 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, * * TODO */ -int dso__data_fd(struct dso *dso, struct machine *machine); +int dso__data_get_fd(struct dso *dso, struct machine *machine); +void dso__data_put_fd(struct dso *dso __maybe_unused); void dso__data_close(struct dso *dso); off_t dso__data_size(struct dso *dso, struct machine *machine); diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c index 7b09a44..f079b63 100644 --- a/tools/perf/util/unwind-libunwind.c +++ b/tools/perf/util/unwind-libunwind.c @@ -269,13 +269,14 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine, u64 offset = dso->data.eh_frame_hdr_offset; if (offset == 0) { - fd = dso__data_fd(dso, machine); + fd = dso__data_get_fd(dso, machine); if (fd < 0) return -EINVAL; /* Check the .eh_frame section for unwinding info */ offset = elf_section_offset(fd, ".eh_frame_hdr"); dso->data.eh_frame_hdr_offset = offset; + dso__data_put_fd(dso); } if (offset) @@ -294,13 +295,14 @@ static int read_unwind_spec_debug_frame(struct dso *dso, u64 ofs = dso->data.debug_frame_offset; if (ofs == 0) { - fd = dso__data_fd(dso, machine); + fd = dso__data_get_fd(dso, machine); if (fd < 0) return -EINVAL; /* Check the .debug_frame section for unwinding info */ ofs = elf_section_offset(fd, ".debug_frame"); dso->data.debug_frame_offset = ofs; + dso__data_put_fd(dso); } *offset = ofs; @@ -353,10 +355,13 @@ find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pi, #ifndef NO_LIBUNWIND_DEBUG_FRAME /* Check the .debug_frame section for unwinding info */ if (!read_unwind_spec_debug_frame(map->dso, ui->machine, &segbase)) { - int fd = dso__data_fd(map->dso, ui->machine); + int fd = dso__data_get_fd(map->dso, ui->machine); int is_exec = elf_is_exec(fd, map->dso->name); unw_word_t base = is_exec ? 0 : map->start; + if (fd >= 0) + dso__data_put_fd(dso); + memset(&di, 0, sizeof(di)); if (dwarf_find_debug_frame(0, &di, ip, base, map->dso->name, map->start, map->end)) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening 2015-05-20 16:03 [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Namhyung Kim 2015-05-20 16:03 ` [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size() Namhyung Kim 2015-05-20 16:03 ` [PATCH v2 3/3] perf tools: Add dso__data_get/put_fd() Namhyung Kim @ 2015-05-21 7:03 ` Adrian Hunter 2015-05-21 14:49 ` Arnaldo Carvalho de Melo 2015-05-27 16:48 ` [tip:perf/core] " tip-bot for Namhyung Kim 3 siblings, 1 reply; 10+ messages in thread From: Adrian Hunter @ 2015-05-21 7:03 UTC (permalink / raw) To: Namhyung Kim, Arnaldo Carvalho de Melo Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern On 20/05/15 19:03, Namhyung Kim wrote: > When dso__data_read_offset/addr() is called without prior > dso__data_fd() (or other functions which call it internally), it > failed to open dso in data_file_size() since its binary type was not > identified. > > However calling dso__data_fd() in dso__data_read_offset() will hurt > performance as it grabs a global lock everytime. So factor out the > loop on the binary type in dso__data_fd(), and call it from both. > > Reported-by: Adrian Hunter <adrian.hunter@intel.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Adrian Hunter <adrian.hunter@intel.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening 2015-05-21 7:03 ` [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Adrian Hunter @ 2015-05-21 14:49 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2015-05-21 14:49 UTC (permalink / raw) To: Adrian Hunter Cc: Namhyung Kim, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, David Ahern Em Thu, May 21, 2015 at 10:03:27AM +0300, Adrian Hunter escreveu: > On 20/05/15 19:03, Namhyung Kim wrote: > > When dso__data_read_offset/addr() is called without prior > > dso__data_fd() (or other functions which call it internally), it > > failed to open dso in data_file_size() since its binary type was not > > identified. > > > > However calling dso__data_fd() in dso__data_read_offset() will hurt > > performance as it grabs a global lock everytime. So factor out the > > loop on the binary type in dso__data_fd(), and call it from both. > > > > Reported-by: Adrian Hunter <adrian.hunter@intel.com> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > Acked-by: Adrian Hunter <adrian.hunter@intel.com> Thanks, all applied. - Arnaldo ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tip:perf/core] perf tools: Fix dso__data_read_offset() file opening 2015-05-20 16:03 [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Namhyung Kim ` (2 preceding siblings ...) 2015-05-21 7:03 ` [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Adrian Hunter @ 2015-05-27 16:48 ` tip-bot for Namhyung Kim 3 siblings, 0 replies; 10+ messages in thread From: tip-bot for Namhyung Kim @ 2015-05-27 16:48 UTC (permalink / raw) To: linux-tip-commits Cc: namhyung, tglx, acme, adrian.hunter, jolsa, dsahern, mingo, linux-kernel, hpa, a.p.zijlstra Commit-ID: 71ff824a60a7b0d9d0746e6e237fe4735077e5b4 Gitweb: http://git.kernel.org/tip/71ff824a60a7b0d9d0746e6e237fe4735077e5b4 Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Thu, 21 May 2015 01:03:39 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Wed, 27 May 2015 12:21:44 -0300 perf tools: Fix dso__data_read_offset() file opening When dso__data_read_offset/addr() is called without prior dso__data_fd() (or other functions which call it internally), it failed to open dso in data_file_size() since its binary type was not identified. However calling dso__data_fd() in dso__data_read_offset() will hurt performance as it grabs a global lock everytime. So factor out the loop on the binary type in dso__data_fd(), and call it from both. Reported-by: Adrian Hunter <adrian.hunter@intel.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Adrian Hunter <adrian.hunter@intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Link: http://lkml.kernel.org/r/1432137821-10853-1-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/dso.c | 59 ++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 1b96c8d..516e0c2 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -440,15 +440,7 @@ void dso__data_close(struct dso *dso) pthread_mutex_unlock(&dso__data_open_lock); } -/** - * dso__data_fd - Get dso's data file descriptor - * @dso: dso object - * @machine: machine object - * - * External interface to find dso's file, open it and - * returns file descriptor. - */ -int dso__data_fd(struct dso *dso, struct machine *machine) +static void try_to_open_dso(struct dso *dso, struct machine *machine) { enum dso_binary_type binary_type_data[] = { DSO_BINARY_TYPE__BUILD_ID_CACHE, @@ -457,13 +449,8 @@ int dso__data_fd(struct dso *dso, struct machine *machine) }; int i = 0; - if (dso->data.status == DSO_DATA_STATUS_ERROR) - return -1; - - pthread_mutex_lock(&dso__data_open_lock); - if (dso->data.fd >= 0) - goto out; + return; if (dso->binary_type != DSO_BINARY_TYPE__NOT_FOUND) { dso->data.fd = open_dso(dso, machine); @@ -483,8 +470,25 @@ out: dso->data.status = DSO_DATA_STATUS_OK; else dso->data.status = DSO_DATA_STATUS_ERROR; +} + +/** + * dso__data_fd - Get dso's data file descriptor + * @dso: dso object + * @machine: machine object + * + * External interface to find dso's file, open it and + * returns file descriptor. + */ +int dso__data_fd(struct dso *dso, struct machine *machine) +{ + if (dso->data.status == DSO_DATA_STATUS_ERROR) + return -1; + pthread_mutex_lock(&dso__data_open_lock); + try_to_open_dso(dso, machine); pthread_mutex_unlock(&dso__data_open_lock); + return dso->data.fd; } @@ -609,13 +613,12 @@ dso_cache__read(struct dso *dso, struct machine *machine, * dso->data.fd might be closed if other thread opened another * file (dso) due to open file limit (RLIMIT_NOFILE). */ + try_to_open_dso(dso, machine); + if (dso->data.fd < 0) { - dso->data.fd = open_dso(dso, machine); - if (dso->data.fd < 0) { - ret = -errno; - dso->data.status = DSO_DATA_STATUS_ERROR; - break; - } + ret = -errno; + dso->data.status = DSO_DATA_STATUS_ERROR; + break; } cache_offset = offset & DSO__DATA_CACHE_MASK; @@ -702,19 +705,21 @@ static int data_file_size(struct dso *dso, struct machine *machine) if (dso->data.file_size) return 0; + if (dso->data.status == DSO_DATA_STATUS_ERROR) + return -1; + pthread_mutex_lock(&dso__data_open_lock); /* * dso->data.fd might be closed if other thread opened another * file (dso) due to open file limit (RLIMIT_NOFILE). */ + try_to_open_dso(dso, machine); + if (dso->data.fd < 0) { - dso->data.fd = open_dso(dso, machine); - if (dso->data.fd < 0) { - ret = -errno; - dso->data.status = DSO_DATA_STATUS_ERROR; - goto out; - } + ret = -errno; + dso->data.status = DSO_DATA_STATUS_ERROR; + goto out; } if (fstat(dso->data.fd, &st) < 0) { ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-05-27 16:49 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-20 16:03 [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Namhyung Kim 2015-05-20 16:03 ` [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size() Namhyung Kim 2015-05-21 7:03 ` Adrian Hunter 2015-05-27 16:48 ` [tip:perf/core] " tip-bot for Namhyung Kim 2015-05-20 16:03 ` [PATCH v2 3/3] perf tools: Add dso__data_get/put_fd() Namhyung Kim 2015-05-21 7:06 ` Adrian Hunter 2015-05-27 16:49 ` [tip:perf/core] " tip-bot for Namhyung Kim 2015-05-21 7:03 ` [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening Adrian Hunter 2015-05-21 14:49 ` Arnaldo Carvalho de Melo 2015-05-27 16:48 ` [tip:perf/core] " tip-bot for Namhyung Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox