All of lore.kernel.org
 help / color / mirror / Atom feed
From: tip-bot for Namhyung Kim <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: adrian.hunter@intel.com, linux-kernel@vger.kernel.org,
	mingo@kernel.org, acme@redhat.com, tglx@linutronix.de,
	a.p.zijlstra@chello.nl, dsahern@gmail.com, namhyung@kernel.org,
	hpa@zytor.com, jolsa@redhat.com
Subject: [tip:perf/core] perf tools: Add dso__data_get/put_fd()
Date: Wed, 27 May 2015 09:49:01 -0700	[thread overview]
Message-ID: <tip-4bb11d012ab248d0e383008d725be0d26a74fac2@git.kernel.org> (raw)
In-Reply-To: <1432137821-10853-3-git-send-email-namhyung@kernel.org>

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

  parent reply	other threads:[~2015-05-27 16:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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-bot for Namhyung Kim [this message]
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

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=tip-4bb11d012ab248d0e383008d725be0d26a74fac2@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=dsahern@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=tglx@linutronix.de \
    /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.