From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755897AbbCCDQ4 (ORCPT ); Mon, 2 Mar 2015 22:16:56 -0500 Received: from lgeamrelo01.lge.com ([156.147.1.125]:44669 "EHLO lgeamrelo01.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754814AbbCCDMU (ORCPT ); Mon, 2 Mar 2015 22:12:20 -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 26/38] perf tools: Protect dso cache fd with a mutex Date: Tue, 3 Mar 2015 12:07:38 +0900 Message-Id: <1425352070-1115-27-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 When dso cache is accessed in multi-thread environment, it's possible to close other dso->data.fd during operation due to open file limit. Protect the file descriptors using a separate mutex. Signed-off-by: Namhyung Kim --- tools/perf/util/dso.c | 98 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 72 insertions(+), 26 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 3bfbe0e76e96..64aaa45dcdd7 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -213,6 +213,7 @@ bool dso__needs_decompress(struct dso *dso) */ static LIST_HEAD(dso__data_open); static long dso__data_open_cnt; +static pthread_mutex_t dso__data_open_lock = PTHREAD_MUTEX_INITIALIZER; static void dso__list_add(struct dso *dso) { @@ -382,7 +383,9 @@ static void check_data_close(void) */ void dso__data_close(struct dso *dso) { + pthread_mutex_lock(&dso__data_open_lock); close_dso(dso); + pthread_mutex_unlock(&dso__data_open_lock); } /** @@ -405,6 +408,8 @@ 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); + if (dso->data.fd >= 0) goto out; @@ -427,6 +432,7 @@ 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; } @@ -531,7 +537,8 @@ dso_cache__memcpy(struct dso_cache *cache, u64 offset, } static ssize_t -dso_cache__read(struct dso *dso, u64 offset, u8 *data, ssize_t size) +dso_cache__read(struct dso *dso, struct machine *machine, + u64 offset, u8 *data, ssize_t size) { struct dso_cache *cache; struct dso_cache *old; @@ -540,11 +547,24 @@ dso_cache__read(struct dso *dso, u64 offset, u8 *data, ssize_t size) do { u64 cache_offset; - ret = -ENOMEM; - cache = zalloc(sizeof(*cache) + DSO__DATA_CACHE_SIZE); if (!cache) - break; + return -ENOMEM; + + 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). + */ + 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; + } + } cache_offset = offset & DSO__DATA_CACHE_MASK; @@ -554,6 +574,11 @@ dso_cache__read(struct dso *dso, u64 offset, u8 *data, ssize_t size) cache->offset = cache_offset; cache->size = ret; + } while (0); + + pthread_mutex_unlock(&dso__data_open_lock); + + if (ret > 0) { old = dso_cache__insert(dso, cache); if (old) { /* we lose the race */ @@ -562,8 +587,7 @@ dso_cache__read(struct dso *dso, u64 offset, u8 *data, ssize_t size) } ret = dso_cache__memcpy(cache, offset, data, size); - - } while (0); + } if (ret <= 0) free(cache); @@ -571,8 +595,8 @@ dso_cache__read(struct dso *dso, u64 offset, u8 *data, ssize_t size) return ret; } -static ssize_t dso_cache_read(struct dso *dso, u64 offset, - u8 *data, ssize_t size) +static ssize_t dso_cache_read(struct dso *dso, struct machine *machine, + u64 offset, u8 *data, ssize_t size) { struct dso_cache *cache; @@ -580,7 +604,7 @@ static ssize_t dso_cache_read(struct dso *dso, u64 offset, if (cache) return dso_cache__memcpy(cache, offset, data, size); else - return dso_cache__read(dso, offset, data, size); + return dso_cache__read(dso, machine, offset, data, size); } /* @@ -588,7 +612,8 @@ static ssize_t dso_cache_read(struct dso *dso, u64 offset, * in the rb_tree. Any read to already cached data is served * by cached data. */ -static ssize_t cached_read(struct dso *dso, u64 offset, u8 *data, ssize_t size) +static ssize_t cached_read(struct dso *dso, struct machine *machine, + u64 offset, u8 *data, ssize_t size) { ssize_t r = 0; u8 *p = data; @@ -596,7 +621,7 @@ static ssize_t cached_read(struct dso *dso, u64 offset, u8 *data, ssize_t size) do { ssize_t ret; - ret = dso_cache_read(dso, offset, p, size); + ret = dso_cache_read(dso, machine, offset, p, size); if (ret < 0) return ret; @@ -616,21 +641,42 @@ static ssize_t cached_read(struct dso *dso, u64 offset, u8 *data, ssize_t size) return r; } -static int data_file_size(struct dso *dso) +static int data_file_size(struct dso *dso, struct machine *machine) { + int ret = 0; struct stat st; char sbuf[STRERR_BUFSIZE]; - if (!dso->data.file_size) { - if (fstat(dso->data.fd, &st)) { - pr_err("dso mmap failed, fstat: %s\n", - strerror_r(errno, sbuf, sizeof(sbuf))); - return -1; + if (dso->data.file_size) + return 0; + + 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). + */ + 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; } - dso->data.file_size = st.st_size; } - return 0; + if (fstat(dso->data.fd, &st) < 0) { + ret = -errno; + pr_err("dso cache fstat failed: %s\n", + strerror_r(errno, sbuf, sizeof(sbuf))); + dso->data.status = DSO_DATA_STATUS_ERROR; + goto out; + } + dso->data.file_size = st.st_size; + +out: + pthread_mutex_unlock(&dso__data_open_lock); + return ret; } /** @@ -648,17 +694,17 @@ off_t dso__data_size(struct dso *dso, struct machine *machine) if (fd < 0) return fd; - if (data_file_size(dso)) + if (data_file_size(dso, machine)) return -1; /* For now just estimate dso data size is close to file size */ return dso->data.file_size; } -static ssize_t data_read_offset(struct dso *dso, u64 offset, - u8 *data, ssize_t size) +static ssize_t data_read_offset(struct dso *dso, struct machine *machine, + u64 offset, u8 *data, ssize_t size) { - if (data_file_size(dso)) + if (data_file_size(dso, machine)) return -1; /* Check the offset sanity. */ @@ -668,7 +714,7 @@ static ssize_t data_read_offset(struct dso *dso, u64 offset, if (offset + size < offset) return -1; - return cached_read(dso, offset, data, size); + return cached_read(dso, machine, offset, data, size); } /** @@ -685,10 +731,10 @@ static ssize_t data_read_offset(struct dso *dso, u64 offset, ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine, u64 offset, u8 *data, ssize_t size) { - if (dso__data_fd(dso, machine) < 0) + if (dso->data.status == DSO_DATA_STATUS_ERROR) return -1; - return data_read_offset(dso, offset, data, size); + return data_read_offset(dso, machine, offset, data, size); } /** -- 2.2.2