From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752936AbbERAqe (ORCPT ); Sun, 17 May 2015 20:46:34 -0400 Received: from LGEMRELSE6Q.lge.com ([156.147.1.121]:55862 "EHLO lgemrelse6q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752407AbbERAmL (ORCPT ); Sun, 17 May 2015 20:42:11 -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 27/40] perf tools: Protect dso cache fd with a mutex Date: Mon, 18 May 2015 09:30:42 +0900 Message-Id: <1431909055-21442-28-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 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 425989e85302..8857287afc14 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -265,6 +265,7 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, */ 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) { @@ -434,7 +435,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); } /** @@ -457,6 +460,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; @@ -479,6 +484,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; } @@ -583,7 +589,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; @@ -592,11 +599,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; @@ -606,6 +626,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 */ @@ -614,8 +639,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); @@ -623,8 +647,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; @@ -632,7 +656,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); } /* @@ -640,7 +664,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; @@ -648,7 +673,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; @@ -668,21 +693,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; } /** @@ -700,17 +746,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. */ @@ -720,7 +766,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); } /** @@ -737,10 +783,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.4.0