From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755071AbbCCDMZ (ORCPT ); Mon, 2 Mar 2015 22:12:25 -0500 Received: from lgeamrelo01.lge.com ([156.147.1.125]:44695 "EHLO lgeamrelo01.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754832AbbCCDMV (ORCPT ); Mon, 2 Mar 2015 22:12:21 -0500 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 , Frederic Weisbecker , Adrian Hunter , Stephane Eranian , Andi Kleen , David Ahern Subject: [PATCH 28/38] perf tools: Add dso__data_get/put_fd() Date: Tue, 3 Mar 2015 12:07:40 +0900 Message-Id: <1425352070-1115-29-git-send-email-namhyung@kernel.org> X-Mailer: git-send-email 2.2.2 In-Reply-To: <1425352070-1115-1-git-send-email-namhyung@kernel.org> References: <1425352070-1115-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 64aaa45dcdd7..50bb2f93b7e9 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -389,14 +389,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, @@ -405,11 +406,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; @@ -432,10 +433,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; @@ -1144,10 +1166,12 @@ 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; } diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index da188a73d034..9f1e67da9c01 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -197,7 +197,9 @@ bool dso__needs_decompress(struct dso *dso); /* * 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 @@ -214,8 +216,9 @@ bool dso__needs_decompress(struct dso *dso); * 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); @@ -235,6 +238,8 @@ bool dso__needs_decompress(struct dso *dso); * 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 9d7ecb26fde9..e807ba9d375a 100644 --- a/tools/perf/util/unwind-libunwind.c +++ b/tools/perf/util/unwind-libunwind.c @@ -271,13 +271,13 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine, u64 offset = dso->data.frame_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.frame_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.frame_offset = offset; + } + dso__data_put_fd(dso); } if (offset) @@ -296,13 +296,19 @@ static int read_unwind_spec_debug_frame(struct dso *dso, u64 ofs = dso->data.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.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.frame_offset = ofs; + } else + ret = -EINVAL; + + dso__data_put_fd(dso); + if (ret) + return ret; } *offset = ofs; @@ -355,10 +361,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.2.2