[GIT PULL 00/14] perf/core improvements and fixes

[GIT PULL 00/14] perf/core improvements and fixes

[Jiri Olsa]

hi Ingo,
please consider pulling

thanks,
jirka

The following changes since commit 7184062b94b4bfac08715fb786fd2df399c5d6ee:

  Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2014-06-12 13:54:42 +0200)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-core-for-mingo

for you to fetch changes up to 45dc1bb5c1d47f9519e2101f6b073bb4bb1d1f99:

  perf tests: Add test for closing dso objects on EMFILE error (2014-06-12 16:53:23 +0200)

----------------------------------------------------------------
perf/core improvements and fixes:

. Honor user freq/interval properly in record command (Namhyung Kim)

. Speedup DWARF unwind (Jiri Olsa)

Signed-off-by: Jiri Olsa <jolsa@kernel.org>

----------------------------------------------------------------
Jiri Olsa (13):
      perf tools: Cache register accesses for unwind processing
      perf tools: Separate dso data related variables
      perf tools: Add data_fd into dso object
      perf tools: Add global list of opened dso objects
      perf tools: Add global count of opened dso objects
      perf tools: Cache dso data file descriptor
      perf tools: Add file size check and factor dso__data_read_offset
      perf tools: Allow to close dso fd in case of open failure
      perf tools: Add dso__data_* interface descriptons
      perf tests: Spawn child for each test
      perf tests: Allow reuse of test_file function
      perf tests: Add test for caching dso file descriptors
      perf tests: Add test for closing dso objects on EMFILE error

Namhyung Kim (1):
      perf record: Fix to honor user freq/interval properly

 tools/perf/tests/builtin-test.c    |  42 +++++-
 tools/perf/tests/dso-data.c        | 214 +++++++++++++++++++++++++++-
 tools/perf/tests/tests.h           |   2 +
 tools/perf/util/dso.c              | 279 +++++++++++++++++++++++++++++++++----
 tools/perf/util/dso.h              |  50 ++++++-
 tools/perf/util/event.h            |   5 +
 tools/perf/util/evsel.c            |   4 +-
 tools/perf/util/perf_regs.c        |  10 +-
 tools/perf/util/perf_regs.h        |   4 +-
 tools/perf/util/unwind-libunwind.c |   2 -
 10 files changed, 574 insertions(+), 38 deletions(-)

[PATCH 01/14] perf record: Fix to honor user freq/interval properly

[Jiri Olsa]

From: Namhyung Kim <namhyung@kernel.org>

When configuring event perf checked a wrong condition that user
specified both of freq (-F) and period (-c) or the event has no
default value.  This worked because most of events don't have default
value and only tracepoint events have default of 1 (and it's not
desirable to change it for those events).

However, Andi's downloadable event patch changes the situation so it
cannot change the value for those events.  Fix it by allowing override
the default value if user gives one of the options.

  $ perf record -a -e uops_retired.all -F 4000 sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.325 MB perf.data (~14185 samples) ]

  $ perf evlist -F
  cpu/uops_retired.all/: sample_freq=4000

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Link: http://lkml.kernel.org/r/1402292617-26278-1-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/evsel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 21154da..8606175 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -589,10 +589,10 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	}
 
 	/*
-	 * We default some events to a 1 default interval. But keep
+	 * We default some events to have a default interval. But keep
 	 * it a weak assumption overridable by the user.
 	 */
-	if (!attr->sample_period || (opts->user_freq != UINT_MAX &&
+	if (!attr->sample_period || (opts->user_freq != UINT_MAX ||
 				     opts->user_interval != ULLONG_MAX)) {
 		if (opts->freq) {
 			perf_evsel__set_sample_bit(evsel, PERIOD);
-- 
1.8.3.1

[PATCH 02/14] perf tools: Cache register accesses for unwind processing

[Jiri Olsa]

Caching registers value into an array. Got about 4% speed up
of perf_reg_value function for report command processing
dwarf unwind stacks.

Output from report over 1.5 GB data with DWARF unwind stacks:
(TODO fix perf diff)

  current code:
   5.84%     perf  perf                       [.] perf_reg_value
  change:
   1.94%     perf  perf                       [.] perf_reg_value

And little bit of overall speed up:
(perf stat -r 5 -e '{cycles,instructions}:u' ...)

  current code:
   310,298,611,754      cycles                     ( +-  0.33% )
   439,669,689,341      instructions               ( +-  0.03% )

     188.656753166 seconds time elapsed            ( +-  0.82% )

  change:
   291,315,329,878      cycles                     ( +-  0.22% )
   391,763,485,304      instructions               ( +-  0.03%  )

     180.742249687 seconds time elapsed            ( +-  0.64% )

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-2-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/event.h     |  5 +++++
 tools/perf/util/perf_regs.c | 10 +++++++++-
 tools/perf/util/perf_regs.h |  4 +++-
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 9ba2eb3..e5dd40a 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -7,6 +7,7 @@
 #include "../perf.h"
 #include "map.h"
 #include "build-id.h"
+#include "perf_regs.h"
 
 struct mmap_event {
 	struct perf_event_header header;
@@ -89,6 +90,10 @@ struct regs_dump {
 	u64 abi;
 	u64 mask;
 	u64 *regs;
+
+	/* Cached values/mask filled by first register access. */
+	u64 cache_regs[PERF_REGS_MAX];
+	u64 cache_mask;
 };
 
 struct stack_dump {
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index a3539ef..43168fb 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -1,11 +1,15 @@
 #include <errno.h>
 #include "perf_regs.h"
+#include "event.h"
 
 int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
 {
 	int i, idx = 0;
 	u64 mask = regs->mask;
 
+	if (regs->cache_mask & (1 << id))
+		goto out;
+
 	if (!(mask & (1 << id)))
 		return -EINVAL;
 
@@ -14,6 +18,10 @@ int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
 			idx++;
 	}
 
-	*valp = regs->regs[idx];
+	regs->cache_mask |= (1 << id);
+	regs->cache_regs[id] = regs->regs[idx];
+
+out:
+	*valp = regs->cache_regs[id];
 	return 0;
 }
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 79c78f7..980dbf7 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -2,7 +2,8 @@
 #define __PERF_REGS_H
 
 #include <linux/types.h>
-#include "event.h"
+
+struct regs_dump;
 
 #ifdef HAVE_PERF_REGS_SUPPORT
 #include <perf_regs.h>
@@ -11,6 +12,7 @@ int perf_reg_value(u64 *valp, struct regs_dump *regs, int id);
 
 #else
 #define PERF_REGS_MASK	0
+#define PERF_REGS_MAX	0
 
 static inline const char *perf_reg_name(int id __maybe_unused)
 {
-- 
1.8.3.1

[PATCH 03/14] perf tools: Separate dso data related variables

[Jiri Olsa]

Add separated structure/namespace for data related
variables. We are going to add mode of them, so this
way they will be clearly separated.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-3-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/dso.c | 8 ++++----
 tools/perf/util/dso.h | 7 ++++++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 64453d6..1c3cdaf 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -292,7 +292,7 @@ dso_cache__read(struct dso *dso, struct machine *machine,
 
 		cache->offset = cache_offset;
 		cache->size   = ret;
-		dso_cache__insert(&dso->cache, cache);
+		dso_cache__insert(&dso->data.cache, cache);
 
 		ret = dso_cache__memcpy(cache, offset, data, size);
 
@@ -310,7 +310,7 @@ static ssize_t dso_cache_read(struct dso *dso, struct machine *machine,
 {
 	struct dso_cache *cache;
 
-	cache = dso_cache__find(&dso->cache, offset);
+	cache = dso_cache__find(&dso->data.cache, offset);
 	if (cache)
 		return dso_cache__memcpy(cache, offset, data, size);
 	else
@@ -473,7 +473,7 @@ struct dso *dso__new(const char *name)
 		dso__set_short_name(dso, dso->name, false);
 		for (i = 0; i < MAP__NR_TYPES; ++i)
 			dso->symbols[i] = dso->symbol_names[i] = RB_ROOT;
-		dso->cache = RB_ROOT;
+		dso->data.cache = RB_ROOT;
 		dso->symtab_type = DSO_BINARY_TYPE__NOT_FOUND;
 		dso->binary_type = DSO_BINARY_TYPE__NOT_FOUND;
 		dso->loaded = 0;
@@ -506,7 +506,7 @@ void dso__delete(struct dso *dso)
 		dso->long_name_allocated = false;
 	}
 
-	dso_cache__free(&dso->cache);
+	dso_cache__free(&dso->data.cache);
 	dso__free_a2l(dso);
 	zfree(&dso->symsrc_filename);
 	free(dso);
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 38efe95..7637fdd 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -76,7 +76,6 @@ struct dso {
 	struct list_head node;
 	struct rb_root	 symbols[MAP__NR_TYPES];
 	struct rb_root	 symbol_names[MAP__NR_TYPES];
-	struct rb_root	 cache;
 	void		 *a2l;
 	char		 *symsrc_filename;
 	unsigned int	 a2l_fails;
@@ -99,6 +98,12 @@ struct dso {
 	const char	 *long_name;
 	u16		 long_name_len;
 	u16		 short_name_len;
+
+	/* dso data file */
+	struct {
+		struct rb_root	 cache;
+	} data;
+
 	char		 name[0];
 };
 
-- 
1.8.3.1

[PATCH 04/14] perf tools: Add data_fd into dso object

[Jiri Olsa]

Adding data_fd into dso object so we could handle caching
of opened dso file data descriptors coming int next patches.

Adding dso__data_close interface to keep the data_fd updated
when the descriptor is closed.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-4-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/dso.c              | 23 +++++++++++++++++++----
 tools/perf/util/dso.h              |  3 +++
 tools/perf/util/unwind-libunwind.c |  4 ++--
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 1c3cdaf..5acb4b8 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -159,6 +159,14 @@ static int open_dso(struct dso *dso, struct machine *machine)
 	return fd;
 }
 
+void dso__data_close(struct dso *dso)
+{
+	if (dso->data.fd >= 0) {
+		close(dso->data.fd);
+		dso->data.fd = -1;
+	}
+}
+
 int dso__data_fd(struct dso *dso, struct machine *machine)
 {
 	enum dso_binary_type binary_type_data[] = {
@@ -168,8 +176,13 @@ int dso__data_fd(struct dso *dso, struct machine *machine)
 	};
 	int i = 0;
 
-	if (dso->binary_type != DSO_BINARY_TYPE__NOT_FOUND)
-		return open_dso(dso, machine);
+	if (dso->data.fd >= 0)
+		return dso->data.fd;
+
+	if (dso->binary_type != DSO_BINARY_TYPE__NOT_FOUND) {
+		dso->data.fd = open_dso(dso, machine);
+		return dso->data.fd;
+	}
 
 	do {
 		int fd;
@@ -178,7 +191,7 @@ int dso__data_fd(struct dso *dso, struct machine *machine)
 
 		fd = open_dso(dso, machine);
 		if (fd >= 0)
-			return fd;
+			return dso->data.fd = fd;
 
 	} while (dso->binary_type != DSO_BINARY_TYPE__NOT_FOUND);
 
@@ -301,7 +314,7 @@ dso_cache__read(struct dso *dso, struct machine *machine,
 	if (ret <= 0)
 		free(cache);
 
-	close(fd);
+	dso__data_close(dso);
 	return ret;
 }
 
@@ -474,6 +487,7 @@ struct dso *dso__new(const char *name)
 		for (i = 0; i < MAP__NR_TYPES; ++i)
 			dso->symbols[i] = dso->symbol_names[i] = RB_ROOT;
 		dso->data.cache = RB_ROOT;
+		dso->data.fd = -1;
 		dso->symtab_type = DSO_BINARY_TYPE__NOT_FOUND;
 		dso->binary_type = DSO_BINARY_TYPE__NOT_FOUND;
 		dso->loaded = 0;
@@ -506,6 +520,7 @@ void dso__delete(struct dso *dso)
 		dso->long_name_allocated = false;
 	}
 
+	dso__data_close(dso);
 	dso_cache__free(&dso->data.cache);
 	dso__free_a2l(dso);
 	zfree(&dso->symsrc_filename);
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 7637fdd..e48dcf5 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -102,6 +102,7 @@ struct dso {
 	/* dso data file */
 	struct {
 		struct rb_root	 cache;
+		int		 fd;
 	} data;
 
 	char		 name[0];
@@ -147,6 +148,8 @@ int dso__read_binary_type_filename(const struct dso *dso, enum dso_binary_type t
 				   char *root_dir, char *filename, size_t size);
 
 int dso__data_fd(struct dso *dso, struct machine *machine);
+void dso__data_close(struct dso *dso);
+
 ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
 			      u64 offset, u8 *data, ssize_t size);
 ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index bd5768d..4f8dd9e 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -250,7 +250,7 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine,
 
 	/* Check the .eh_frame section for unwinding info */
 	offset = elf_section_offset(fd, ".eh_frame_hdr");
-	close(fd);
+	dso__data_close(dso);
 
 	if (offset)
 		ret = unwind_spec_ehframe(dso, machine, offset,
@@ -271,7 +271,7 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 
 	/* Check the .debug_frame section for unwinding info */
 	*offset = elf_section_offset(fd, ".debug_frame");
-	close(fd);
+	dso__data_close(dso);
 
 	if (*offset)
 		return 0;
-- 
1.8.3.1

[PATCH 05/14] perf tools: Add global list of opened dso objects

[Jiri Olsa]

Adding global list of opened dso objects, so we can
track them and use the list for caching dso data file
descriptors.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-5-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/dso.c | 41 +++++++++++++++++++++++++++++++++++++++--
 tools/perf/util/dso.h |  1 +
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 5acb4b8..5d7c7bc 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -136,7 +136,22 @@ int dso__read_binary_type_filename(const struct dso *dso,
 	return ret;
 }
 
-static int open_dso(struct dso *dso, struct machine *machine)
+/*
+ * Global list of open DSOs.
+ */
+static LIST_HEAD(dso__data_open);
+
+static void dso__list_add(struct dso *dso)
+{
+	list_add_tail(&dso->data.open_entry, &dso__data_open);
+}
+
+static void dso__list_del(struct dso *dso)
+{
+	list_del(&dso->data.open_entry);
+}
+
+static int __open_dso(struct dso *dso, struct machine *machine)
 {
 	int fd;
 	char *root_dir = (char *)"";
@@ -159,14 +174,35 @@ static int open_dso(struct dso *dso, struct machine *machine)
 	return fd;
 }
 
-void dso__data_close(struct dso *dso)
+static int open_dso(struct dso *dso, struct machine *machine)
+{
+	int fd = __open_dso(dso, machine);
+
+	if (fd > 0)
+		dso__list_add(dso);
+
+	return fd;
+}
+
+static void close_data_fd(struct dso *dso)
 {
 	if (dso->data.fd >= 0) {
 		close(dso->data.fd);
 		dso->data.fd = -1;
+		dso__list_del(dso);
 	}
 }
 
+static void close_dso(struct dso *dso)
+{
+	close_data_fd(dso);
+}
+
+void dso__data_close(struct dso *dso)
+{
+	close_dso(dso);
+}
+
 int dso__data_fd(struct dso *dso, struct machine *machine)
 {
 	enum dso_binary_type binary_type_data[] = {
@@ -499,6 +535,7 @@ struct dso *dso__new(const char *name)
 		dso->kernel = DSO_TYPE_USER;
 		dso->needs_swap = DSO_SWAP__UNSET;
 		INIT_LIST_HEAD(&dso->node);
+		INIT_LIST_HEAD(&dso->data.open_entry);
 	}
 
 	return dso;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index e48dcf5..90988bf 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -103,6 +103,7 @@ struct dso {
 	struct {
 		struct rb_root	 cache;
 		int		 fd;
+		struct list_head open_entry;
 	} data;
 
 	char		 name[0];
-- 
1.8.3.1

[PATCH 06/14] perf tools: Add global count of opened dso objects

[Jiri Olsa]

Adding global count of opened dso objects so we could
properly limit the number of opened dso data file
descriptors.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-6-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/dso.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 5d7c7bc..76e5c13 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1,3 +1,4 @@
+#include <asm/bug.h>
 #include "symbol.h"
 #include "dso.h"
 #include "machine.h"
@@ -137,18 +138,23 @@ int dso__read_binary_type_filename(const struct dso *dso,
 }
 
 /*
- * Global list of open DSOs.
+ * Global list of open DSOs and the counter.
  */
 static LIST_HEAD(dso__data_open);
+static long dso__data_open_cnt;
 
 static void dso__list_add(struct dso *dso)
 {
 	list_add_tail(&dso->data.open_entry, &dso__data_open);
+	dso__data_open_cnt++;
 }
 
 static void dso__list_del(struct dso *dso)
 {
 	list_del(&dso->data.open_entry);
+	WARN_ONCE(dso__data_open_cnt <= 0,
+		  "DSO data fd counter out of bounds.");
+	dso__data_open_cnt--;
 }
 
 static int __open_dso(struct dso *dso, struct machine *machine)
-- 
1.8.3.1

[PATCH 07/14] perf tools: Cache dso data file descriptor

[Jiri Olsa]

Caching dso data file descriptors to avoid expensive re-opens
especially during DWARF unwind.

We keep dsos data file descriptors open until their count reaches
the half of the current fd open limit (RLIMIT_NOFILE). In this case
we close file descriptor of the first opened dso object.

We've got overall speedup (~27% for my workload) of report:
 'perf report --stdio -i perf-test.data' (3 runs)
  (perf-test.data size was around 12GB)

  current code:
   545,640,944,228      cycles                     ( +-  0.53% )
   785,255,798,320      instructions               ( +-  0.03% )

     366.340910010 seconds time elapsed            ( +-  3.65% )

  after change:
   435,895,036,114      cycles                     ( +-  0.26% )
   636,790,271,176      instructions               ( +-  0.04% )

     266.481463387 seconds time elapsed            ( +-  0.13% )

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-7-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/dso.c              | 61 ++++++++++++++++++++++++++++++++++++--
 tools/perf/util/unwind-libunwind.c |  2 --
 2 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 76e5c13..fbf6cc9 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1,4 +1,6 @@
 #include <asm/bug.h>
+#include <sys/time.h>
+#include <sys/resource.h>
 #include "symbol.h"
 #include "dso.h"
 #include "machine.h"
@@ -180,12 +182,20 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 	return fd;
 }
 
+static void check_data_close(void);
+
 static int open_dso(struct dso *dso, struct machine *machine)
 {
 	int fd = __open_dso(dso, machine);
 
-	if (fd > 0)
+	if (fd > 0) {
 		dso__list_add(dso);
+		/*
+		 * Check if we crossed the allowed number
+		 * of opened DSOs and close one if needed.
+		 */
+		check_data_close();
+	}
 
 	return fd;
 }
@@ -204,6 +214,54 @@ static void close_dso(struct dso *dso)
 	close_data_fd(dso);
 }
 
+static void close_first_dso(void)
+{
+	struct dso *dso;
+
+	dso = list_first_entry(&dso__data_open, struct dso, data.open_entry);
+	close_dso(dso);
+}
+
+static rlim_t get_fd_limit(void)
+{
+	struct rlimit l;
+	rlim_t limit = 0;
+
+	/* Allow half of the current open fd limit. */
+	if (getrlimit(RLIMIT_NOFILE, &l) == 0) {
+		if (l.rlim_cur == RLIM_INFINITY)
+			limit = l.rlim_cur;
+		else
+			limit = l.rlim_cur / 2;
+	} else {
+		pr_err("failed to get fd limit\n");
+		limit = 1;
+	}
+
+	return limit;
+}
+
+static bool may_cache_fd(void)
+{
+	static rlim_t limit;
+
+	if (!limit)
+		limit = get_fd_limit();
+
+	if (limit == RLIM_INFINITY)
+		return true;
+
+	return limit > (rlim_t) dso__data_open_cnt;
+}
+
+static void check_data_close(void)
+{
+	bool cache_fd = may_cache_fd();
+
+	if (!cache_fd)
+		close_first_dso();
+}
+
 void dso__data_close(struct dso *dso)
 {
 	close_dso(dso);
@@ -356,7 +414,6 @@ dso_cache__read(struct dso *dso, struct machine *machine,
 	if (ret <= 0)
 		free(cache);
 
-	dso__data_close(dso);
 	return ret;
 }
 
diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c
index 4f8dd9e..25578b9 100644
--- a/tools/perf/util/unwind-libunwind.c
+++ b/tools/perf/util/unwind-libunwind.c
@@ -250,7 +250,6 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine,
 
 	/* Check the .eh_frame section for unwinding info */
 	offset = elf_section_offset(fd, ".eh_frame_hdr");
-	dso__data_close(dso);
 
 	if (offset)
 		ret = unwind_spec_ehframe(dso, machine, offset,
@@ -271,7 +270,6 @@ static int read_unwind_spec_debug_frame(struct dso *dso,
 
 	/* Check the .debug_frame section for unwinding info */
 	*offset = elf_section_offset(fd, ".debug_frame");
-	dso__data_close(dso);
 
 	if (*offset)
 		return 0;
-- 
1.8.3.1

[PATCH 08/14] perf tools: Add file size check and factor dso__data_read_offset

[Jiri Olsa]

Adding file size check, because the lseek will succeed for
any offset behind file size and thus succeed when it was
expected to fail.

Factoring the code to check the offset against file size
earlier in the flow.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-8-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/dso.c | 64 +++++++++++++++++++++++++++++++++++++++------------
 tools/perf/util/dso.h |  1 +
 2 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index fbf6cc9..db63438 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -205,6 +205,7 @@ static void close_data_fd(struct dso *dso)
 	if (dso->data.fd >= 0) {
 		close(dso->data.fd);
 		dso->data.fd = -1;
+		dso->data.file_size = 0;
 		dso__list_del(dso);
 	}
 }
@@ -373,16 +374,10 @@ dso_cache__memcpy(struct dso_cache *cache, u64 offset,
 }
 
 static ssize_t
-dso_cache__read(struct dso *dso, struct machine *machine,
-		 u64 offset, u8 *data, ssize_t size)
+dso_cache__read(struct dso *dso, u64 offset, u8 *data, ssize_t size)
 {
 	struct dso_cache *cache;
 	ssize_t ret;
-	int fd;
-
-	fd = dso__data_fd(dso, machine);
-	if (fd < 0)
-		return -1;
 
 	do {
 		u64 cache_offset;
@@ -396,10 +391,10 @@ dso_cache__read(struct dso *dso, struct machine *machine,
 		cache_offset = offset & DSO__DATA_CACHE_MASK;
 		ret = -EINVAL;
 
-		if (-1 == lseek(fd, cache_offset, SEEK_SET))
+		if (-1 == lseek(dso->data.fd, cache_offset, SEEK_SET))
 			break;
 
-		ret = read(fd, cache->data, DSO__DATA_CACHE_SIZE);
+		ret = read(dso->data.fd, cache->data, DSO__DATA_CACHE_SIZE);
 		if (ret <= 0)
 			break;
 
@@ -417,8 +412,8 @@ dso_cache__read(struct dso *dso, struct machine *machine,
 	return ret;
 }
 
-static ssize_t dso_cache_read(struct dso *dso, struct machine *machine,
-			      u64 offset, u8 *data, ssize_t size)
+static ssize_t dso_cache_read(struct dso *dso, u64 offset,
+			      u8 *data, ssize_t size)
 {
 	struct dso_cache *cache;
 
@@ -426,11 +421,10 @@ static ssize_t dso_cache_read(struct dso *dso, struct machine *machine,
 	if (cache)
 		return dso_cache__memcpy(cache, offset, data, size);
 	else
-		return dso_cache__read(dso, machine, offset, data, size);
+		return dso_cache__read(dso, offset, data, size);
 }
 
-ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
-			      u64 offset, u8 *data, ssize_t size)
+static ssize_t cached_read(struct dso *dso, u64 offset, u8 *data, ssize_t size)
 {
 	ssize_t r = 0;
 	u8 *p = data;
@@ -438,7 +432,7 @@ ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
 	do {
 		ssize_t ret;
 
-		ret = dso_cache_read(dso, machine, offset, p, size);
+		ret = dso_cache_read(dso, offset, p, size);
 		if (ret < 0)
 			return ret;
 
@@ -458,6 +452,46 @@ ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
 	return r;
 }
 
+static int data_file_size(struct dso *dso)
+{
+	struct stat st;
+
+	if (!dso->data.file_size) {
+		if (fstat(dso->data.fd, &st)) {
+			pr_err("dso mmap failed, fstat: %s\n", strerror(errno));
+			return -1;
+		}
+		dso->data.file_size = st.st_size;
+	}
+
+	return 0;
+}
+
+static ssize_t data_read_offset(struct dso *dso, u64 offset,
+				u8 *data, ssize_t size)
+{
+	if (data_file_size(dso))
+		return -1;
+
+	/* Check the offset sanity. */
+	if (offset > dso->data.file_size)
+		return -1;
+
+	if (offset + size < offset)
+		return -1;
+
+	return cached_read(dso, offset, data, size);
+}
+
+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)
+		return -1;
+
+	return data_read_offset(dso, offset, data, size);
+}
+
 ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 			    struct machine *machine, u64 addr,
 			    u8 *data, ssize_t size)
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 90988bf..da47b13 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -103,6 +103,7 @@ struct dso {
 	struct {
 		struct rb_root	 cache;
 		int		 fd;
+		size_t		 file_size;
 		struct list_head open_entry;
 	} data;
 
-- 
1.8.3.1

[PATCH 09/14] perf tools: Allow to close dso fd in case of open failure

[Jiri Olsa]

Adding do_open function that tries to close opened
dso objects in case we fail to open the dso due to
to crossing the allowed RLIMIT_NOFILE limit.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-9-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/dso.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index db63438..c30752c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -159,6 +159,27 @@ static void dso__list_del(struct dso *dso)
 	dso__data_open_cnt--;
 }
 
+static void close_first_dso(void);
+
+static int do_open(char *name)
+{
+	int fd;
+
+	do {
+		fd = open(name, O_RDONLY);
+		if (fd >= 0)
+			return fd;
+
+		pr_debug("dso open failed, mmap: %s\n", strerror(errno));
+		if (!dso__data_open_cnt || errno != EMFILE)
+			break;
+
+		close_first_dso();
+	} while (1);
+
+	return -1;
+}
+
 static int __open_dso(struct dso *dso, struct machine *machine)
 {
 	int fd;
@@ -177,7 +198,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 		return -EINVAL;
 	}
 
-	fd = open(name, O_RDONLY);
+	fd = do_open(name);
 	free(name);
 	return fd;
 }
-- 
1.8.3.1

[PATCH 10/14] perf tools: Add dso__data_* interface descriptons

[Jiri Olsa]

Adding descriptions/explanations for dso__data_* interface
functions.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-10-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/dso.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/dso.h | 38 +++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index c30752c..819f104 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -205,6 +205,13 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 
 static void check_data_close(void);
 
+/**
+ * dso_close - Open DSO data file
+ * @dso: dso object
+ *
+ * Open @dso's data file descriptor and updates
+ * list/count of open DSO objects.
+ */
 static int open_dso(struct dso *dso, struct machine *machine)
 {
 	int fd = __open_dso(dso, machine);
@@ -231,6 +238,13 @@ static void close_data_fd(struct dso *dso)
 	}
 }
 
+/**
+ * dso_close - Close DSO data file
+ * @dso: dso object
+ *
+ * Close @dso's data file descriptor and updates
+ * list/count of open DSO objects.
+ */
 static void close_dso(struct dso *dso)
 {
 	close_data_fd(dso);
@@ -276,6 +290,11 @@ static bool may_cache_fd(void)
 	return limit > (rlim_t) dso__data_open_cnt;
 }
 
+/*
+ * Check and close LRU dso if we crossed allowed limit
+ * for opened dso file descriptors. The limit is half
+ * of the RLIMIT_NOFILE files opened.
+*/
 static void check_data_close(void)
 {
 	bool cache_fd = may_cache_fd();
@@ -284,11 +303,25 @@ static void check_data_close(void)
 		close_first_dso();
 }
 
+/**
+ * dso__data_close - Close DSO data file
+ * @dso: dso object
+ *
+ * External interface to close @dso's data file descriptor.
+ */
 void dso__data_close(struct dso *dso)
 {
 	close_dso(dso);
 }
 
+/**
+ * dso__data_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.
+ */
 int dso__data_fd(struct dso *dso, struct machine *machine)
 {
 	enum dso_binary_type binary_type_data[] = {
@@ -445,6 +478,11 @@ static ssize_t dso_cache_read(struct dso *dso, u64 offset,
 		return dso_cache__read(dso, offset, data, size);
 }
 
+/*
+ * Reads and caches dso data DSO__DATA_CACHE_SIZE size chunks
+ * 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)
 {
 	ssize_t r = 0;
@@ -504,6 +542,17 @@ static ssize_t data_read_offset(struct dso *dso, u64 offset,
 	return cached_read(dso, offset, data, size);
 }
 
+/**
+ * dso__data_read_offset - Read data from dso file offset
+ * @dso: dso object
+ * @machine: machine object
+ * @offset: file offset
+ * @data: buffer to store data
+ * @size: size of the @data buffer
+ *
+ * External interface to read data from dso file offset. Open
+ * dso data file and use cached_read to get the data.
+ */
 ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
 			      u64 offset, u8 *data, ssize_t size)
 {
@@ -513,6 +562,16 @@ ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
 	return data_read_offset(dso, offset, data, size);
 }
 
+/**
+ * dso__data_read_addr - Read data from dso address
+ * @dso: dso object
+ * @machine: machine object
+ * @add: virtual memory address
+ * @data: buffer to store data
+ * @size: size of the @data buffer
+ *
+ * External interface to read data from dso address.
+ */
 ssize_t dso__data_read_addr(struct dso *dso, struct map *map,
 			    struct machine *machine, u64 addr,
 			    u8 *data, ssize_t size)
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index da47b13..ad553ba 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -149,6 +149,44 @@ char dso__symtab_origin(const struct dso *dso);
 int dso__read_binary_type_filename(const struct dso *dso, enum dso_binary_type type,
 				   char *root_dir, char *filename, size_t size);
 
+/*
+ * The dso__data_* external interface provides following functions:
+ *   dso__data_fd
+ *   dso__data_close
+ *   dso__data_read_offset
+ *   dso__data_read_addr
+ *
+ * Please refer to the dso.c object code for each function and
+ * arguments documentation. Following text tries to explain the
+ * dso file descriptor caching.
+ *
+ * The dso__data* interface allows caching of opened file descriptors
+ * to speed up the dso data accesses. The idea is to leave the file
+ * descriptor opened ideally for the whole life of the dso object.
+ *
+ * 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
+ *
+ * Read DSO's data:
+ *   n = dso__data_read_offset(dso_0, &machine, 0, buf, BUFSIZE);
+ *   n = dso__data_read_addr(dso_0, &machine, 0, buf, BUFSIZE);
+ *
+ * Eventually close DSO's fd:
+ *   dso__data_close(dso);
+ *
+ * It is not necessary to close the DSO object data file. Each time new
+ * DSO data file is opened, the limit (RLIMIT_NOFILE/2) is checked. Once
+ * it is crossed, the oldest opened DSO object is closed.
+ *
+ * The dso__delete function calls close_dso function to ensure the
+ * data file descriptor gets closed/unmapped before the dso object
+ * is freed.
+ *
+ * TODO
+*/
 int dso__data_fd(struct dso *dso, struct machine *machine);
 void dso__data_close(struct dso *dso);
 
-- 
1.8.3.1

[PATCH 11/14] perf tests: Spawn child for each test

[Jiri Olsa]

In upcoming tests we will setup process limits, which
might affect other tests. Spawning child for each test
to prevent this.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Reviewed-by: David Ahern <dsahern@gmail.com>
Link: http://lkml.kernel.org/r/1401892622-30848-11-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/builtin-test.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 802e3cd..9677a5c 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -3,6 +3,8 @@
  *
  * Builtin regression testing command: ever growing number of sanity tests
  */
+#include <unistd.h>
+#include <string.h>
 #include "builtin.h"
 #include "intlist.h"
 #include "tests.h"
@@ -172,6 +174,34 @@ static bool perf_test__matches(int curr, int argc, const char *argv[])
 	return false;
 }
 
+static int run_test(struct test *test)
+{
+	int status, err = -1, child = fork();
+
+	if (child < 0) {
+		pr_err("failed to fork test: %s\n", strerror(errno));
+		return -1;
+	}
+
+	if (!child) {
+		pr_debug("test child forked, pid %d\n", getpid());
+		err = test->func();
+		exit(err);
+	}
+
+	wait(&status);
+
+	if (WIFEXITED(status)) {
+		err = WEXITSTATUS(status);
+		pr_debug("test child finished with %d\n", err);
+	} else if (WIFSIGNALED(status)) {
+		err = -1;
+		pr_debug("test child interrupted\n");
+	}
+
+	return err;
+}
+
 static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 {
 	int i = 0;
@@ -200,7 +230,7 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 		}
 
 		pr_debug("\n--- start ---\n");
-		err = tests[curr].func();
+		err = run_test(&tests[curr]);
 		pr_debug("---- end ----\n%s:", tests[curr].desc);
 
 		switch (err) {
-- 
1.8.3.1

[PATCH 12/14] perf tests: Allow reuse of test_file function

[Jiri Olsa]

Making the test_file function to be reusable for
new tests coming in following patches.

Also changing the template name of temp files to
"/tmp/perf-test-XXXXXX" to easily identify & blame.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-12-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/dso-data.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index 3e6cb17..7384381 100644
--- a/tools/perf/tests/dso-data.c
+++ b/tools/perf/tests/dso-data.c
@@ -12,11 +12,15 @@
 
 static char *test_file(int size)
 {
-	static char buf_templ[] = "/tmp/test-XXXXXX";
+#define TEMPL "/tmp/perf-test-XXXXXX"
+	static char buf_templ[sizeof(TEMPL)];
 	char *templ = buf_templ;
 	int fd, i;
 	unsigned char *buf;
 
+	strcpy(buf_templ, TEMPL);
+#undef TEMPL
+
 	fd = mkstemp(templ);
 	if (fd < 0) {
 		perror("mkstemp failed");
-- 
1.8.3.1

[PATCH 13/14] perf tests: Add test for caching dso file descriptors

[Jiri Olsa]

Adding test that setup test_dso_data__fd_limit and test
dso data file descriptors are cached appropriately.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1401892622-30848-13-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/builtin-test.c |   6 +-
 tools/perf/tests/dso-data.c     | 135 +++++++++++++++++++++++++++++++++++++++-
 tools/perf/tests/tests.h        |   1 +
 3 files changed, 138 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 9677a5c..b8a6358 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -52,10 +52,14 @@ static struct test {
 		.func = test__pmu,
 	},
 	{
-		.desc = "Test dso data interface",
+		.desc = "Test dso data read",
 		.func = test__dso_data,
 	},
 	{
+		.desc = "Test dso data cache",
+		.func = test__dso_data_cache,
+	},
+	{
 		.desc = "roundtrip evsel->name check",
 		.func = test__perf_evsel__roundtrip_name_test,
 	},
diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index 7384381..2d30014 100644
--- a/tools/perf/tests/dso-data.c
+++ b/tools/perf/tests/dso-data.c
@@ -1,11 +1,12 @@
-#include "util.h"
-
 #include <stdlib.h>
 #include <linux/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <string.h>
-
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <api/fs/fs.h>
+#include "util.h"
 #include "machine.h"
 #include "symbol.h"
 #include "tests.h"
@@ -154,3 +155,131 @@ int test__dso_data(void)
 	unlink(file);
 	return 0;
 }
+
+static long open_files_cnt(void)
+{
+	char path[PATH_MAX];
+	struct dirent *dent;
+	DIR *dir;
+	long nr = 0;
+
+	scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
+	pr_debug("fd path: %s\n", path);
+
+	dir = opendir(path);
+	TEST_ASSERT_VAL("failed to open fd directory", dir);
+
+	while ((dent = readdir(dir)) != NULL) {
+		if (!strcmp(dent->d_name, ".") ||
+		    !strcmp(dent->d_name, ".."))
+			continue;
+
+		nr++;
+	}
+
+	closedir(dir);
+	return nr - 1;
+}
+
+static struct dso **dsos;
+
+static int dsos__create(int cnt, int size)
+{
+	int i;
+
+	dsos = malloc(sizeof(dsos) * cnt);
+	TEST_ASSERT_VAL("failed to alloc dsos array", dsos);
+
+	for (i = 0; i < cnt; i++) {
+		char *file;
+
+		file = test_file(size);
+		TEST_ASSERT_VAL("failed to get dso file", file);
+
+		dsos[i] = dso__new(file);
+		TEST_ASSERT_VAL("failed to get dso", dsos[i]);
+	}
+
+	return 0;
+}
+
+static void dsos__delete(int cnt)
+{
+	int i;
+
+	for (i = 0; i < cnt; i++) {
+		struct dso *dso = dsos[i];
+
+		unlink(dso->name);
+		dso__delete(dso);
+	}
+
+	free(dsos);
+}
+
+static int set_fd_limit(int n)
+{
+	struct rlimit rlim;
+
+	if (getrlimit(RLIMIT_NOFILE, &rlim))
+		return -1;
+
+	pr_debug("file limit %ld, new %d\n", (long) rlim.rlim_cur, n);
+
+	rlim.rlim_cur = n;
+	return setrlimit(RLIMIT_NOFILE, &rlim);
+}
+
+int test__dso_data_cache(void)
+{
+	struct machine machine;
+	long nr_end, nr = open_files_cnt();
+	int dso_cnt, limit, i, fd;
+
+	memset(&machine, 0, sizeof(machine));
+
+	/* set as system limit */
+	limit = nr * 4;
+	TEST_ASSERT_VAL("failed to set file limit", !set_fd_limit(limit));
+
+	/* and this is now our dso open FDs limit + 1 extra */
+	dso_cnt = limit / 2 + 1;
+	TEST_ASSERT_VAL("failed to create dsos\n",
+		!dsos__create(dso_cnt, TEST_FILE_SIZE));
+
+	for (i = 0; i < (dso_cnt - 1); i++) {
+		struct dso *dso = dsos[i];
+
+		/*
+		 * Open dsos via dso__data_fd or dso__data_read_offset.
+		 * Both opens the data file and keep it open.
+		 */
+		if (i % 2) {
+			fd = dso__data_fd(dso, &machine);
+			TEST_ASSERT_VAL("failed to get fd", fd > 0);
+		} else {
+			#define BUFSIZE 10
+			u8 buf[BUFSIZE];
+			ssize_t n;
+
+			n = dso__data_read_offset(dso, &machine, 0, buf, BUFSIZE);
+			TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
+		}
+	}
+
+	/* open +1 dso over the allowed limit */
+	fd = dso__data_fd(dsos[i], &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/* should force the first one to be closed */
+	TEST_ASSERT_VAL("failed to close dsos[0]", dsos[0]->data.fd == -1);
+
+	/* cleanup everything */
+	dsos__delete(dso_cnt);
+
+	/* Make sure we did not leak any file descriptor. */
+	nr_end = open_files_cnt();
+	pr_debug("nr start %ld, nr stop %ld\n", nr, nr_end);
+	TEST_ASSERT_VAL("failed leadking files", nr == nr_end);
+	return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 022bb68..ccc4deb 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -28,6 +28,7 @@ int test__syscall_open_tp_fields(void);
 int test__pmu(void);
 int test__attr(void);
 int test__dso_data(void);
+int test__dso_data_cache(void);
 int test__parse_events(void);
 int test__hists_link(void);
 int test__python_use(void);
-- 
1.8.3.1

[PATCH 14/14] perf tests: Add test for closing dso objects on EMFILE error

[Jiri Olsa]

Testing that perf properly closes opened dso objects
and tries to reopen in case we run out of allowed file
descriptors for dso data.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Reviewed by: David Ahern <dsahern@gmail.com>
Link: http://lkml.kernel.org/r/1401892622-30848-14-git-send-email-jolsa@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/builtin-test.c |  4 +++
 tools/perf/tests/dso-data.c     | 73 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |  1 +
 3 files changed, 78 insertions(+)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index b8a6358..6f8b01b 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -60,6 +60,10 @@ static struct test {
 		.func = test__dso_data_cache,
 	},
 	{
+		.desc = "Test dso data reopen",
+		.func = test__dso_data_reopen,
+	},
+	{
 		.desc = "roundtrip evsel->name check",
 		.func = test__perf_evsel__roundtrip_name_test,
 	},
diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index 2d30014..630808c 100644
--- a/tools/perf/tests/dso-data.c
+++ b/tools/perf/tests/dso-data.c
@@ -283,3 +283,76 @@ int test__dso_data_cache(void)
 	TEST_ASSERT_VAL("failed leadking files", nr == nr_end);
 	return 0;
 }
+
+int test__dso_data_reopen(void)
+{
+	struct machine machine;
+	long nr_end, nr = open_files_cnt();
+	int fd, fd_extra;
+
+#define dso_0 (dsos[0])
+#define dso_1 (dsos[1])
+#define dso_2 (dsos[2])
+
+	memset(&machine, 0, sizeof(machine));
+
+	/*
+	 * Test scenario:
+	 * - create 3 dso objects
+	 * - set process file descriptor limit to current
+	 *   files count + 3
+	 * - test that the first dso gets closed when we
+	 *   reach the files count limit
+	 */
+
+	/* Make sure we are able to open 3 fds anyway */
+	TEST_ASSERT_VAL("failed to set file limit",
+			!set_fd_limit((nr + 3)));
+
+	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(3, TEST_FILE_SIZE));
+
+	/* open dso_0 */
+	fd = dso__data_fd(dso_0, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/* open dso_1 */
+	fd = dso__data_fd(dso_1, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/*
+	 * open extra file descriptor and we just
+	 * reached the files count limit
+	 */
+	fd_extra = open("/dev/null", O_RDONLY);
+	TEST_ASSERT_VAL("failed to open extra fd", fd_extra > 0);
+
+	/* open dso_2 */
+	fd = dso__data_fd(dso_2, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/*
+	 * dso_0 should get closed, because we reached
+	 * the file descriptor limit
+	 */
+	TEST_ASSERT_VAL("failed to close dso_0", dso_0->data.fd == -1);
+
+	/* open dso_0 */
+	fd = dso__data_fd(dso_0, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/*
+	 * dso_1 should get closed, because we reached
+	 * the file descriptor limit
+	 */
+	TEST_ASSERT_VAL("failed to close dso_1", dso_1->data.fd == -1);
+
+	/* cleanup everything */
+	close(fd_extra);
+	dsos__delete(3);
+
+	/* Make sure we did not leak any file descriptor. */
+	nr_end = open_files_cnt();
+	pr_debug("nr start %ld, nr stop %ld\n", nr, nr_end);
+	TEST_ASSERT_VAL("failed leadking files", nr == nr_end);
+	return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index ccc4deb..ed64790 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -29,6 +29,7 @@ int test__pmu(void);
 int test__attr(void);
 int test__dso_data(void);
 int test__dso_data_cache(void);
+int test__dso_data_reopen(void);
 int test__parse_events(void);
 int test__hists_link(void);
 int test__python_use(void);
-- 
1.8.3.1

Re: [GIT PULL 00/14] perf/core improvements and fixes

[Jean Pihet]

Hi Jiri,


On 12 June 2014 17:30, Jiri Olsa <jolsa@kernel.org> wrote:
> hi Ingo,
> please consider pulling
>
> thanks,
> jirka
>
> The following changes since commit 7184062b94b4bfac08715fb786fd2df399c5d6ee:
>
>   Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2014-06-12 13:54:42 +0200)
>
> are available in the git repository at:
>
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-core-for-mingo
>
> for you to fetch changes up to 45dc1bb5c1d47f9519e2101f6b073bb4bb1d1f99:
>
>   perf tests: Add test for closing dso objects on EMFILE error (2014-06-12 16:53:23 +0200)
>
> ----------------------------------------------------------------
> perf/core improvements and fixes:
>
> . Honor user freq/interval properly in record command (Namhyung Kim)
>
> . Speedup DWARF unwind (Jiri Olsa)
Here are the results of the performance assessment on ARMv7, FWIW:

The results for unwind_speedup (v4) on ARMv7 are:
- libunwind: between -17% in execution time for light load (i.e. using
not-so-deep backtraces from the stress app.) and -25% for deep
backtrace (the stress_bt app.),
- libdw: no significant improvement (0-3% improvement).

The results for unwind_speedup (v3) on ARMv7 are:
- libunwind: between -29% in execution time for light load (i.e. using
not-so-deep backtraces from the stress app.) and -49% for deep
backtrace (the stress_bt app.),
- libdw: no significant improvement (0-2% improvement).

Note: v3 is faster than v4 by 13-25%, with and without the speed-up
patches. The real cause has been investigated, yet.

Cf. https://wiki.linaro.org/LEG/Engineering/TOOLS/perf-callstack-unwinding#Speed_improvement
for the details.

Regards,
Jean

>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>
> ----------------------------------------------------------------
> Jiri Olsa (13):
>       perf tools: Cache register accesses for unwind processing
>       perf tools: Separate dso data related variables
>       perf tools: Add data_fd into dso object
>       perf tools: Add global list of opened dso objects
>       perf tools: Add global count of opened dso objects
>       perf tools: Cache dso data file descriptor
>       perf tools: Add file size check and factor dso__data_read_offset
>       perf tools: Allow to close dso fd in case of open failure
>       perf tools: Add dso__data_* interface descriptons
>       perf tests: Spawn child for each test
>       perf tests: Allow reuse of test_file function
>       perf tests: Add test for caching dso file descriptors
>       perf tests: Add test for closing dso objects on EMFILE error
>
> Namhyung Kim (1):
>       perf record: Fix to honor user freq/interval properly
>
>  tools/perf/tests/builtin-test.c    |  42 +++++-
>  tools/perf/tests/dso-data.c        | 214 +++++++++++++++++++++++++++-
>  tools/perf/tests/tests.h           |   2 +
>  tools/perf/util/dso.c              | 279 +++++++++++++++++++++++++++++++++----
>  tools/perf/util/dso.h              |  50 ++++++-
>  tools/perf/util/event.h            |   5 +
>  tools/perf/util/evsel.c            |   4 +-
>  tools/perf/util/perf_regs.c        |  10 +-
>  tools/perf/util/perf_regs.h        |   4 +-
>  tools/perf/util/unwind-libunwind.c |   2 -
>  10 files changed, 574 insertions(+), 38 deletions(-)

Re: [GIT PULL 00/14] perf/core improvements and fixes

[Ingo Molnar]

* Jiri Olsa <jolsa@kernel.org> wrote:

> hi Ingo,
> please consider pulling
> 
> thanks,
> jirka
> 
> The following changes since commit 7184062b94b4bfac08715fb786fd2df399c5d6ee:
> 
>   Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2014-06-12 13:54:42 +0200)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-core-for-mingo
> 
> for you to fetch changes up to 45dc1bb5c1d47f9519e2101f6b073bb4bb1d1f99:
> 
>   perf tests: Add test for closing dso objects on EMFILE error (2014-06-12 16:53:23 +0200)
> 
> ----------------------------------------------------------------
> perf/core improvements and fixes:
> 
> . Honor user freq/interval properly in record command (Namhyung Kim)
> 
> . Speedup DWARF unwind (Jiri Olsa)
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> ----------------------------------------------------------------
> Jiri Olsa (13):
>       perf tools: Cache register accesses for unwind processing
>       perf tools: Separate dso data related variables
>       perf tools: Add data_fd into dso object
>       perf tools: Add global list of opened dso objects
>       perf tools: Add global count of opened dso objects
>       perf tools: Cache dso data file descriptor
>       perf tools: Add file size check and factor dso__data_read_offset
>       perf tools: Allow to close dso fd in case of open failure
>       perf tools: Add dso__data_* interface descriptons
>       perf tests: Spawn child for each test
>       perf tests: Allow reuse of test_file function
>       perf tests: Add test for caching dso file descriptors
>       perf tests: Add test for closing dso objects on EMFILE error
> 
> Namhyung Kim (1):
>       perf record: Fix to honor user freq/interval properly
> 
>  tools/perf/tests/builtin-test.c    |  42 +++++-
>  tools/perf/tests/dso-data.c        | 214 +++++++++++++++++++++++++++-
>  tools/perf/tests/tests.h           |   2 +
>  tools/perf/util/dso.c              | 279 +++++++++++++++++++++++++++++++++----
>  tools/perf/util/dso.h              |  50 ++++++-
>  tools/perf/util/event.h            |   5 +
>  tools/perf/util/evsel.c            |   4 +-
>  tools/perf/util/perf_regs.c        |  10 +-
>  tools/perf/util/perf_regs.h        |   4 +-
>  tools/perf/util/unwind-libunwind.c |   2 -
>  10 files changed, 574 insertions(+), 38 deletions(-)

Pulled, thanks a lot Jiri!

	Ingo

Re: [GIT PULL 00/14] perf/core improvements and fixes

[Jiri Olsa]

On Thu, Jun 12, 2014 at 10:55:55PM +0200, Jean Pihet wrote:
> Hi Jiri,
> 
> 
> On 12 June 2014 17:30, Jiri Olsa <jolsa@kernel.org> wrote:
> > hi Ingo,
> > please consider pulling
> >
> > thanks,
> > jirka
> >
> > The following changes since commit 7184062b94b4bfac08715fb786fd2df399c5d6ee:
> >
> >   Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2014-06-12 13:54:42 +0200)
> >
> > are available in the git repository at:
> >
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git tags/perf-core-for-mingo
> >
> > for you to fetch changes up to 45dc1bb5c1d47f9519e2101f6b073bb4bb1d1f99:
> >
> >   perf tests: Add test for closing dso objects on EMFILE error (2014-06-12 16:53:23 +0200)
> >
> > ----------------------------------------------------------------
> > perf/core improvements and fixes:
> >
> > . Honor user freq/interval properly in record command (Namhyung Kim)
> >
> > . Speedup DWARF unwind (Jiri Olsa)
> Here are the results of the performance assessment on ARMv7, FWIW:
> 
> The results for unwind_speedup (v4) on ARMv7 are:
> - libunwind: between -17% in execution time for light load (i.e. using
> not-so-deep backtraces from the stress app.) and -25% for deep
> backtrace (the stress_bt app.),
> - libdw: no significant improvement (0-3% improvement).
> 
> The results for unwind_speedup (v3) on ARMv7 are:
> - libunwind: between -29% in execution time for light load (i.e. using
> not-so-deep backtraces from the stress app.) and -49% for deep
> backtrace (the stress_bt app.),
> - libdw: no significant improvement (0-2% improvement).
> 
> Note: v3 is faster than v4 by 13-25%, with and without the speed-up
> patches. The real cause has been investigated, yet.

hi,
yep, I also found the v4 to be slower, because of the
caching code in each open.. I believe the first version
you meassured was the original, that kept all dso objects
open.. now the caching code spends more cycles

also we are not mmapping dso so far.. there's the cached
read in place

please let me know if you think it's something else

thanks for testing,
jirka

Re: [PATCH 13/14] perf tests: Add test for caching dso file descriptors

[Jiri Olsa]

On Fri, May 30, 2014 at 11:11:20AM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Thu, 29 May 2014 10:37:32 +0200, Jiri Olsa wrote:
> > On Thu, May 29, 2014 at 09:06:05AM +0900, Namhyung Kim wrote:
> >> On Tue, 27 May 2014 09:54:36 +0200, Jiri Olsa wrote:
> >> > On Tue, May 27, 2014 at 10:36:44AM +0900, Namhyung Kim wrote:
> >> >> On Thu, 15 May 2014 19:23:34 +0200, Jiri Olsa wrote:
> >> >> > Adding test that setup test_dso_data__fd_limit and test
> >> >> > dso data file descriptors are cached appropriately.
> >> >> 
> >> >> [SNIP]
> >> >> > +static long open_files_cnt(void)
> >> >> > +{
> >> >> > +	char path[PATH_MAX];
> >> >> > +	struct dirent *dent;
> >> >> > +	DIR *dir;
> >> >> > +	long nr = 0;
> >> >> > +	int n;
> >> >> > +
> >> >> > +	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
> >> >> > +	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);
> >> >> 
> >> >> Looks like an unnecessary check since the scnprintf() cannot return more
> >> >> than (or equal to) PATH_MAX.
> >> >
> >> > once it's equal it's bad.. as the man says:
> >> > "return value of size or more means that the output was  truncated"
> >> 
> >> Did you see "sn"printf?
> >
> > yes, I just double checked.. jirka
> 
> Please see "scn"printf then. :)  It does something like below..
> 
>   i = snprintf(buf, size, ...);
>   return (i >= size) ? : size - 1 : i;

aaargh... ok ;-)

thanks,
jirka

Re: [PATCH 13/14] perf tests: Add test for caching dso file descriptors

[Namhyung Kim]

Hi Jiri,

On Thu, 29 May 2014 10:37:32 +0200, Jiri Olsa wrote:
> On Thu, May 29, 2014 at 09:06:05AM +0900, Namhyung Kim wrote:
>> On Tue, 27 May 2014 09:54:36 +0200, Jiri Olsa wrote:
>> > On Tue, May 27, 2014 at 10:36:44AM +0900, Namhyung Kim wrote:
>> >> On Thu, 15 May 2014 19:23:34 +0200, Jiri Olsa wrote:
>> >> > Adding test that setup test_dso_data__fd_limit and test
>> >> > dso data file descriptors are cached appropriately.
>> >> 
>> >> [SNIP]
>> >> > +static long open_files_cnt(void)
>> >> > +{
>> >> > +	char path[PATH_MAX];
>> >> > +	struct dirent *dent;
>> >> > +	DIR *dir;
>> >> > +	long nr = 0;
>> >> > +	int n;
>> >> > +
>> >> > +	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
>> >> > +	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);
>> >> 
>> >> Looks like an unnecessary check since the scnprintf() cannot return more
>> >> than (or equal to) PATH_MAX.
>> >
>> > once it's equal it's bad.. as the man says:
>> > "return value of size or more means that the output was  truncated"
>> 
>> Did you see "sn"printf?
>
> yes, I just double checked.. jirka

Please see "scn"printf then. :)  It does something like below..

  i = snprintf(buf, size, ...);
  return (i >= size) ? : size - 1 : i;


Thanks,
Namhyung

Re: [PATCH 13/14] perf tests: Add test for caching dso file descriptors

[Jiri Olsa]

On Thu, May 29, 2014 at 09:06:05AM +0900, Namhyung Kim wrote:
> On Tue, 27 May 2014 09:54:36 +0200, Jiri Olsa wrote:
> > On Tue, May 27, 2014 at 10:36:44AM +0900, Namhyung Kim wrote:
> >> On Thu, 15 May 2014 19:23:34 +0200, Jiri Olsa wrote:
> >> > Adding test that setup test_dso_data__fd_limit and test
> >> > dso data file descriptors are cached appropriately.
> >> 
> >> [SNIP]
> >> > +static long open_files_cnt(void)
> >> > +{
> >> > +	char path[PATH_MAX];
> >> > +	struct dirent *dent;
> >> > +	DIR *dir;
> >> > +	long nr = 0;
> >> > +	int n;
> >> > +
> >> > +	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
> >> > +	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);
> >> 
> >> Looks like an unnecessary check since the scnprintf() cannot return more
> >> than (or equal to) PATH_MAX.
> >
> > once it's equal it's bad.. as the man says:
> > "return value of size or more means that the output was  truncated"
> 
> Did you see "sn"printf?

yes, I just double checked.. jirka

Re: [PATCH 13/14] perf tests: Add test for caching dso file descriptors

[Namhyung Kim]

On Tue, 27 May 2014 09:54:36 +0200, Jiri Olsa wrote:
> On Tue, May 27, 2014 at 10:36:44AM +0900, Namhyung Kim wrote:
>> On Thu, 15 May 2014 19:23:34 +0200, Jiri Olsa wrote:
>> > Adding test that setup test_dso_data__fd_limit and test
>> > dso data file descriptors are cached appropriately.
>> 
>> [SNIP]
>> > +static long open_files_cnt(void)
>> > +{
>> > +	char path[PATH_MAX];
>> > +	struct dirent *dent;
>> > +	DIR *dir;
>> > +	long nr = 0;
>> > +	int n;
>> > +
>> > +	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
>> > +	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);
>> 
>> Looks like an unnecessary check since the scnprintf() cannot return more
>> than (or equal to) PATH_MAX.
>
> once it's equal it's bad.. as the man says:
> "return value of size or more means that the output was  truncated"

Did you see "sn"printf?

Thanks,
Namhyung

Re: [PATCH 13/14] perf tests: Add test for caching dso file descriptors

[Jiri Olsa]

On Tue, May 27, 2014 at 10:36:44AM +0900, Namhyung Kim wrote:
> On Thu, 15 May 2014 19:23:34 +0200, Jiri Olsa wrote:
> > Adding test that setup test_dso_data__fd_limit and test
> > dso data file descriptors are cached appropriately.
> 
> [SNIP]
> > +static long open_files_cnt(void)
> > +{
> > +	char path[PATH_MAX];
> > +	struct dirent *dent;
> > +	DIR *dir;
> > +	long nr = 0;
> > +	int n;
> > +
> > +	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
> > +	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);
> 
> Looks like an unnecessary check since the scnprintf() cannot return more
> than (or equal to) PATH_MAX.

once it's equal it's bad.. as the man says:
"return value of size or more means that the output was  truncated"

I'll fix the rest you found

thanks,
jirka

Re: [PATCH 13/14] perf tests: Add test for caching dso file descriptors

[Namhyung Kim]

On Thu, 15 May 2014 19:23:34 +0200, Jiri Olsa wrote:
> Adding test that setup test_dso_data__fd_limit and test
> dso data file descriptors are cached appropriately.

[SNIP]
> +static long open_files_cnt(void)
> +{
> +	char path[PATH_MAX];
> +	struct dirent *dent;
> +	DIR *dir;
> +	long nr = 0;
> +	int n;
> +
> +	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
> +	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);

Looks like an unnecessary check since the scnprintf() cannot return more
than (or equal to) PATH_MAX.

> +
> +	pr_debug("fd path: %s\n", path);
> +
> +	dir = opendir(path);
> +	TEST_ASSERT_VAL("failed to open fd directory", dir);
> +
> +	while ((dent = readdir(dir)) != NULL) {
> +		if (!strcmp(dent->d_name, ".") ||
> +		    !strcmp(dent->d_name, ".."))
> +			continue;
> +
> +		nr++;
> +	}
> +
> +	closedir(dir);
> +	return nr - 1;
> +}

[SNIP]
> +static int set_fd_limit(int n)
> +{
> +	struct rlimit rlim;
> +
> +	if (getrlimit(RLIMIT_NOFILE, &rlim))
> +		return -1;
> +
> +	pr_debug("file limit %ld, new %d\n", (long) rlim.rlim_cur, n);
> +
> +	rlim.rlim_cur = n;
> +	return setrlimit(RLIMIT_NOFILE, &rlim);
> +}
> +
> +int test__dso_data_cache(void)
> +{
> +	struct machine machine;
> +	long nr = open_files_cnt();
> +#define BUFSIZE 10
> +	u8 buf[BUFSIZE];
> +	ssize_t n;
> +	int fd;
> +
> +	memset(&machine, 0, sizeof(machine));
> +
> +	/* Make sure we are able to open 3 fds anyway */
> +	TEST_ASSERT_VAL("failed to set file limit",
> +			!set_fd_limit((nr + 6)));

3 or 6?

> +
> +	/*
> +	 * Test scenario:
> +	 * - create 3 dso objects
> +	 * - set the limit of opened data file descriptors to 2
> +	 * - open/close dsos data fds and check for proper
> +	 *   handling of the dso data cache
> +	 */
> +
> +	test_dso_data__fd_limit = 3;

2 or 3?

> +
> +	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(TEST_FILE_SIZE));
> +
> +	/* open dso_0 */
> +	fd = dso__data_fd(dso_0, &machine);
> +	TEST_ASSERT_VAL("failed to get fd", fd > 0);
> +
> +	n = dso__data_read_offset(dso_0, &machine, 0, buf, BUFSIZE);
> +	TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
> +
> +	/*
> +	 * Close dso_0 data with cache = true,

What does it mean by 'cache = true'?


> +	 * dso_0 should remain open.
> +	 */
> +	dso__data_close(dso_0);
> +	TEST_ASSERT_VAL("failed to not close dso", dso_0->data.fd != -1);
> +
> +	/* open dso_1 */
> +	n = dso__data_read_offset(dso_1, &machine, 0, buf, BUFSIZE);
> +	TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
> +
> +	/*
> +	 * Close dso_1 data with cache = true,
> +	 * dso_0 and dso_1 should remain open.
> +	 */
> +	dso__data_close(dso_1);
> +	TEST_ASSERT_VAL("failed to not close dso", dso_0->data.fd != -1);
> +	TEST_ASSERT_VAL("failed to not close dso", dso_1->data.fd != -1);
> +
> +	/* open dso_2 */
> +	fd = dso__data_fd(dso_2, &machine);
> +	TEST_ASSERT_VAL("failed to get fd", fd > 0);
> +
> +	/*
> +	 * Close dso_1 data with cache = true,

You meant dso_2 right? :)

Thanks,
Namhyung


> +	 * dso_0 should get closed now
> +	 */
> +	dso__data_close(dso_2);
> +	TEST_ASSERT_VAL("failed to close dso_0", dso_0->data.fd == -1);
> +
> +	/* reopen dso_0 */
> +	fd = dso__data_fd(dso_0, &machine);
> +	TEST_ASSERT_VAL("failed to get fd", fd > 0);
> +
> +	/*
> +	 * Close dso_0 data with cache = true,
> +	 * dso_1 should get closed now.
> +	 */
> +	dso__data_close(dso_0);
> +	TEST_ASSERT_VAL("failed to close dso_1", dso_1->data.fd == -1);
> +
> +	/* reopen dso_1 */
> +	n = dso__data_read_offset(dso_1, &machine, 0, buf, BUFSIZE);
> +	TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
> +
> +	/*
> +	 * Close dso_1 data with cache = true,
> +	 * dso_2 should get closed now.
> +	 */
> +	dso__data_close(dso_1);
> +	TEST_ASSERT_VAL("failed to close dso_2", dso_2->data.fd == -1);
> +
> +	/* dso_0 remains open */
> +	TEST_ASSERT_VAL("failed to keep open dso_0", dso_0->data.fd >= 0);
> +
> +	/* cleanup everything */
> +	dsos__delete();
> +
> +	pr_debug("nr start %ld, nr stop %ld\n", nr, open_files_cnt());
> +
> +	/* Make sure we did not leak any file descriptor. */
> +	TEST_ASSERT_VAL("failed leadking files", nr == open_files_cnt());
> +	return 0;
> +}
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index a9d7cb0..61e12b6 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -28,6 +28,7 @@ int test__syscall_open_tp_fields(void);
>  int test__pmu(void);
>  int test__attr(void);
>  int test__dso_data(void);
> +int test__dso_data_cache(void);
>  int test__parse_events(void);
>  int test__hists_link(void);
>  int test__python_use(void);

[PATCH 13/14] perf tests: Add test for caching dso file descriptors

[Jiri Olsa]

Adding test that setup test_dso_data__fd_limit and test
dso data file descriptors are cached appropriately.

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean Pihet <jean.pihet@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/builtin-test.c |   6 +-
 tools/perf/tests/dso-data.c     | 180 +++++++++++++++++++++++++++++++++++++++-
 tools/perf/tests/tests.h        |   1 +
 3 files changed, 183 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index fe46f3e..c4d581a 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -52,10 +52,14 @@ static struct test {
 		.func = test__pmu,
 	},
 	{
-		.desc = "Test dso data interface",
+		.desc = "Test dso data read",
 		.func = test__dso_data,
 	},
 	{
+		.desc = "Test dso data cache",
+		.func = test__dso_data_cache,
+	},
+	{
 		.desc = "roundtrip evsel->name check",
 		.func = test__perf_evsel__roundtrip_name_test,
 	},
diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index f5c8743..84ab939 100644
--- a/tools/perf/tests/dso-data.c
+++ b/tools/perf/tests/dso-data.c
@@ -1,11 +1,12 @@
-#include "util.h"
-
 #include <stdlib.h>
 #include <linux/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <string.h>
-
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <api/fs/fs.h>
+#include "util.h"
 #include "machine.h"
 #include "symbol.h"
 #include "tests.h"
@@ -154,3 +155,176 @@ int test__dso_data(void)
 	unlink(file);
 	return 0;
 }
+
+static long open_files_cnt(void)
+{
+	char path[PATH_MAX];
+	struct dirent *dent;
+	DIR *dir;
+	long nr = 0;
+	int n;
+
+	n = scnprintf(path, PATH_MAX, "%s/self/fd", procfs__mountpoint());
+	TEST_ASSERT_VAL("couldn't get fd path", n < PATH_MAX);
+
+	pr_debug("fd path: %s\n", path);
+
+	dir = opendir(path);
+	TEST_ASSERT_VAL("failed to open fd directory", dir);
+
+	while ((dent = readdir(dir)) != NULL) {
+		if (!strcmp(dent->d_name, ".") ||
+		    !strcmp(dent->d_name, ".."))
+			continue;
+
+		nr++;
+	}
+
+	closedir(dir);
+	return nr - 1;
+}
+
+static struct dso *dsos[3];
+#define dso_0 (dsos[0])
+#define dso_1 (dsos[1])
+#define dso_2 (dsos[2])
+
+static int dsos__create(int size)
+{
+	int i;
+
+	for (i = 0; i < 3; i++) {
+		char *file;
+
+		file = test_file(size);
+		TEST_ASSERT_VAL("failed to get dso file", file);
+
+		dsos[i] = dso__new((const char *)file);
+		TEST_ASSERT_VAL("failed to get dso", dsos[i]);
+	}
+
+	return 0;
+}
+
+static void dsos__delete(void)
+{
+	int i;
+
+	for (i = 0; i < 3; i++) {
+		struct dso *dso = dsos[i];
+		unlink(dso->name);
+		dso__delete(dso);
+	}
+}
+
+static int set_fd_limit(int n)
+{
+	struct rlimit rlim;
+
+	if (getrlimit(RLIMIT_NOFILE, &rlim))
+		return -1;
+
+	pr_debug("file limit %ld, new %d\n", (long) rlim.rlim_cur, n);
+
+	rlim.rlim_cur = n;
+	return setrlimit(RLIMIT_NOFILE, &rlim);
+}
+
+int test__dso_data_cache(void)
+{
+	struct machine machine;
+	long nr = open_files_cnt();
+#define BUFSIZE 10
+	u8 buf[BUFSIZE];
+	ssize_t n;
+	int fd;
+
+	memset(&machine, 0, sizeof(machine));
+
+	/* Make sure we are able to open 3 fds anyway */
+	TEST_ASSERT_VAL("failed to set file limit",
+			!set_fd_limit((nr + 6)));
+
+	/*
+	 * Test scenario:
+	 * - create 3 dso objects
+	 * - set the limit of opened data file descriptors to 2
+	 * - open/close dsos data fds and check for proper
+	 *   handling of the dso data cache
+	 */
+
+	test_dso_data__fd_limit = 3;
+
+	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(TEST_FILE_SIZE));
+
+	/* open dso_0 */
+	fd = dso__data_fd(dso_0, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	n = dso__data_read_offset(dso_0, &machine, 0, buf, BUFSIZE);
+	TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
+
+	/*
+	 * Close dso_0 data with cache = true,
+	 * dso_0 should remain open.
+	 */
+	dso__data_close(dso_0);
+	TEST_ASSERT_VAL("failed to not close dso", dso_0->data.fd != -1);
+
+	/* open dso_1 */
+	n = dso__data_read_offset(dso_1, &machine, 0, buf, BUFSIZE);
+	TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
+
+	/*
+	 * Close dso_1 data with cache = true,
+	 * dso_0 and dso_1 should remain open.
+	 */
+	dso__data_close(dso_1);
+	TEST_ASSERT_VAL("failed to not close dso", dso_0->data.fd != -1);
+	TEST_ASSERT_VAL("failed to not close dso", dso_1->data.fd != -1);
+
+	/* open dso_2 */
+	fd = dso__data_fd(dso_2, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/*
+	 * Close dso_1 data with cache = true,
+	 * dso_0 should get closed now
+	 */
+	dso__data_close(dso_2);
+	TEST_ASSERT_VAL("failed to close dso_0", dso_0->data.fd == -1);
+
+	/* reopen dso_0 */
+	fd = dso__data_fd(dso_0, &machine);
+	TEST_ASSERT_VAL("failed to get fd", fd > 0);
+
+	/*
+	 * Close dso_0 data with cache = true,
+	 * dso_1 should get closed now.
+	 */
+	dso__data_close(dso_0);
+	TEST_ASSERT_VAL("failed to close dso_1", dso_1->data.fd == -1);
+
+	/* reopen dso_1 */
+	n = dso__data_read_offset(dso_1, &machine, 0, buf, BUFSIZE);
+	TEST_ASSERT_VAL("failed to read dso", n == BUFSIZE);
+
+	/*
+	 * Close dso_1 data with cache = true,
+	 * dso_2 should get closed now.
+	 */
+	dso__data_close(dso_1);
+	TEST_ASSERT_VAL("failed to close dso_2", dso_2->data.fd == -1);
+
+	/* dso_0 remains open */
+	TEST_ASSERT_VAL("failed to keep open dso_0", dso_0->data.fd >= 0);
+
+	/* cleanup everything */
+	dsos__delete();
+
+	pr_debug("nr start %ld, nr stop %ld\n", nr, open_files_cnt());
+
+	/* Make sure we did not leak any file descriptor. */
+	TEST_ASSERT_VAL("failed leadking files", nr == open_files_cnt());
+	return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index a9d7cb0..61e12b6 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -28,6 +28,7 @@ int test__syscall_open_tp_fields(void);
 int test__pmu(void);
 int test__attr(void);
 int test__dso_data(void);
+int test__dso_data_cache(void);
 int test__parse_events(void);
 int test__hists_link(void);
 int test__python_use(void);
-- 
1.8.3.1