[PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening

[PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening

[Namhyung Kim]

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

[PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size()

[Namhyung Kim]

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

[PATCH v2 3/3] perf tools: Add dso__data_get/put_fd()

[Namhyung Kim]

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

Re: [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening

[Adrian Hunter]

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>

Re: [PATCH v2 2/3] perf tools: Get rid of dso__data_fd() from dso__data_size()

[Adrian Hunter]

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>

Re: [PATCH v2 3/3] perf tools: Add dso__data_get/put_fd()

[Adrian Hunter]

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>

Re: [PATCH v2 1/3] perf tools: Fix dso__data_read_offset() file opening

[Arnaldo Carvalho de Melo]

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

[tip:perf/core] perf tools: Fix dso__data_read_offset() file opening

[tip-bot for Namhyung Kim]

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) {

[tip:perf/core] perf tools: Get rid of dso__data_fd() from dso__data_size()

[tip-bot for Namhyung Kim]

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;
 

[tip:perf/core] perf tools: Add dso__data_get/put_fd()

[tip-bot for Namhyung Kim]

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))