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>

diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index 513e5febbe5a5016ed5d9a2564bea1e5bc92e4e8..3e41c61bd8610406fcc4dd2afa45198a6d32be70 100644 (file)
--- 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 e95e850dd832682409e62016797e8087d01ce0ae..7e11a700303f805fe1d1f0b9dcbb3d2c12dbb96d 100644 (file)
--- 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 b26ec3ab1336a38a0aa4de72d4a59bda2e345b73..bcec06ad73a2af5ae6f1d9ef4af909953e90403c 100644 (file)
--- 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 7b09a443a280429d3de68f03ced731f66b36d3fc..f079b63f0b7fdc0a7fa30499fbf8f165d838f9f7 100644 (file)
--- 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))