From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752839AbbERAqM (ORCPT ); Sun, 17 May 2015 20:46:12 -0400 Received: from LGEMRELSE6Q.lge.com ([156.147.1.121]:55956 "EHLO lgemrelse6q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752426AbbERAmR (ORCPT ); Sun, 17 May 2015 20:42:17 -0400 X-Original-SENDERIP: 10.177.220.203 X-Original-MAILFROM: namhyung@kernel.org From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , David Ahern , Adrian Hunter , Andi Kleen , Frederic Weisbecker , Stephane Eranian Subject: [PATCH 29/40] perf tools: Add dso__data_get/put_fd() Date: Mon, 18 May 2015 09:30:44 +0900 Message-Id: <1431909055-21442-30-git-send-email-namhyung@kernel.org> X-Mailer: git-send-email 2.4.0 In-Reply-To: <1431909055-21442-1-git-send-email-namhyung@kernel.org> References: <1431909055-21442-1-git-send-email-namhyung@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- tools/perf/util/dso.c | 44 +++++++++++++++++++++++++++++--------- tools/perf/util/dso.h | 9 ++++++-- tools/perf/util/unwind-libunwind.c | 38 +++++++++++++++++++------------- 3 files changed, 64 insertions(+), 27 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 8857287afc14..272b207f4bef 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -441,14 +441,15 @@ void dso__data_close(struct dso *dso) } /** - * 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. Should be paired with + * dso__data_put_fd(). */ -int dso__data_fd(struct dso *dso, struct machine *machine) +int dso__data_get_fd(struct dso *dso, struct machine *machine) { enum dso_binary_type binary_type_data[] = { DSO_BINARY_TYPE__BUILD_ID_CACHE, @@ -457,11 +458,11 @@ int dso__data_fd(struct dso *dso, struct machine *machine) }; int i = 0; + pthread_mutex_lock(&dso__data_open_lock); + if (dso->data.status == DSO_DATA_STATUS_ERROR) return -1; - pthread_mutex_lock(&dso__data_open_lock); - if (dso->data.fd >= 0) goto out; @@ -484,10 +485,31 @@ int dso__data_fd(struct dso *dso, struct machine *machine) else dso->data.status = DSO_DATA_STATUS_ERROR; - 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); +} + +/** + * dso__data_get_fd - Get dso's data file descriptor + * @dso: dso object + * @machine: machine object + * + * Obsolete interface to find dso's file, open it and + * returns file descriptor. It's not thread-safe in that + * the returned fd may be reused for other file. + */ +int dso__data_fd(struct dso *dso, struct machine *machine) +{ + int fd = dso__data_get_fd(dso, machine); + + dso__data_put_fd(dso); + return fd; +} + bool dso__data_status_seen(struct dso *dso, enum dso_data_status_seen by) { u32 flag = 1 << by; @@ -1200,12 +1222,14 @@ 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..de9d98c44ae2 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -240,7 +240,9 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, /* * The dso__data_* external interface provides following functions: - * dso__data_fd + * dso__data_fd (obsolete) + * dso__data_get_fd + * dso__data_put_fd * dso__data_close * dso__data_size * dso__data_read_offset @@ -257,8 +259,9 @@ 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); + * int fd = dso__data_get_fd(dso, machine); * USE 'fd' SOMEHOW + * dso__data_put_fd(dso) * * Read DSO's data: * n = dso__data_read_offset(dso_0, &machine, 0, buf, BUFSIZE); @@ -278,6 +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); 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 b3214f76fd43..58a9238b9b3e 100644 --- a/tools/perf/util/unwind-libunwind.c +++ b/tools/perf/util/unwind-libunwind.c @@ -270,13 +270,13 @@ 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); - 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; + fd = dso__data_get_fd(dso, machine); + if (fd >= 0) { + /* 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) @@ -295,13 +295,19 @@ 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); - 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; + int ret = 0; + + fd = dso__data_get_fd(dso, machine); + if (fd >= 0) { + /* Check the .debug_frame section for unwinding info */ + ofs = elf_section_offset(fd, ".debug_frame"); + dso->data.debug_frame_offset = ofs; + } else + ret = -EINVAL; + + dso__data_put_fd(dso); + if (ret) + return ret; } *offset = ofs; @@ -356,10 +362,12 @@ 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; + dso__data_put_fd(map->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