Skip to contentSkip to conversation

perf tools: Speedup DWARF unwind

PATCH Superseded Jiri Olsa 3 participants 26 messages Jun 2, 2014 – Jun 3, 2014 perf events

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

v3 cover perf tools: Speedup DWARF unwind

hi,
trying to speedup DWARF unwind report code by factoring related code:
- caching sample's registers access
- keep dso data file descriptor open for the life of the dso object
- replace dso cache code by mapping dso data file directly for the life of the dso object

The speedup is mainly for libunwind unwind. The libdw will benefit mainly from cached registers access, because it handles dso data accesses by itself.. and anyway it's still faster ;-).

v3 changes:
- moved the cache logic into dso__data_fd (Namhyung Kim)
- omited test_dso_data__fd_limit variable,
dso data cache test changed accordingly (Namhyung Kim)
- several small fixies from review (Namhyung Kim)

v2 changes:
- adding limit for open dso objects with sort of LRU mechanism to pick up and close dso objects if we are over the limit
- file mmaping changes are omitted, because I couldn't prove the improvement exactly, will resubmit later
- added dso close logic in case of no memory
- added tests

On 10GB perf data file with dwarf unwind data I've got around 30% speed up.

Also reachable in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_unwind_speedup

thanks,
jirka

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>

---
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

 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/perf_regs.c        |  10 +++-
 tools/perf/util/perf_regs.h        |   4 +-
 tools/perf/util/unwind-libunwind.c |   2 -
 9 files changed, 572 insertions(+), 36 deletions(-)
author reviewer
#1 lore raw

PATCH v3 1/13 perf tools: Cache register accesses for unwind processing

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% )
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 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(-)
tools/perf/util/event.h +5 -0
index d970232..d369ad9 100644
@@ -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;
@@ -87,6 +88,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 {
tools/perf/util/perf_regs.c +9 -1
index a3539ef..43168fb 100644
@@ -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;
 }
tools/perf/util/perf_regs.h +3 -1
index 79c78f7..980dbf7 100644
@@ -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)
 {
Signature · 1 line
1.8.3.1
author reviewer
#2 lore raw

PATCH v3 2/13 perf tools: Separate dso data related variables

In reply to 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.

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/dso.c | 8 ++++----
 tools/perf/util/dso.h | 7 ++++++-
 2 files changed, 10 insertions(+), 5 deletions(-)
tools/perf/util/dso.c +4 -4
index 64453d6..1c3cdaf 100644
@@ -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);
tools/perf/util/dso.h +6 -1
index 38efe95..7637fdd 100644
@@ -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];
 };
 
Signature · 1 line
1.8.3.1
author reviewer
#3 lore raw

PATCH v3 3/13 perf tools: Add data_fd into dso object

In reply to 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.

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 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(-)
tools/perf/util/dso.c +19 -4
index 1c3cdaf..5acb4b8 100644
@@ -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);
tools/perf/util/dso.h +3 -0
index 7637fdd..e48dcf5 100644
@@ -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,
tools/perf/util/unwind-libunwind.c +2 -2
index bd5768d..4f8dd9e 100644
@@ -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;
Signature · 1 line
1.8.3.1
author reviewer
#4 lore raw

PATCH v3 4/13 perf tools: Add global list of opened dso objects

In reply to Jiri Olsa

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

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/dso.c | 41 +++++++++++++++++++++++++++++++++++++++--
 tools/perf/util/dso.h |  1 +
 2 files changed, 40 insertions(+), 2 deletions(-)
tools/perf/util/dso.c +39 -2
index 5acb4b8..5d7c7bc 100644
@@ -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;
tools/perf/util/dso.h +1 -0
index e48dcf5..90988bf 100644
@@ -103,6 +103,7 @@ struct dso {
 	struct {
 		struct rb_root	 cache;
 		int		 fd;
+		struct list_head open_entry;
 	} data;
 
 	char		 name[0];
Signature · 1 line
1.8.3.1
author reviewer
#5 lore raw

PATCH v3 5/13 perf tools: Add global count of opened dso objects

In reply to Jiri Olsa

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

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/dso.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
tools/perf/util/dso.c +7 -1
index 5d7c7bc..76e5c13 100644
@@ -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)
Signature · 1 line
1.8.3.1
author reviewer
#6 lore raw

PATCH v3 6/13 perf tools: Cache dso data file descriptor

In reply to 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 (24% for my workload) of report:
'perf report --stdio -i perf-test.data' (3 runs)
(perf-test.data size was around 12GB)

current code:

   446,826,474,217      cycles                     ( +-  0.23% )
   669,435,100,846      instructions               ( +-  0.03% )
     266.380674551 seconds time elapsed            ( +-  0.37% )

change:

   558,345,509,819      cycles                     ( +-  0.55% )
   817,283,647,637      instructions               ( +-  0.03% )
     351.564209617 seconds time elapsed            ( +-  0.89% )
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/dso.c              | 61 ++++++++++++++++++++++++++++++++++++--
 tools/perf/util/unwind-libunwind.c |  2 --
 2 files changed, 59 insertions(+), 4 deletions(-)
tools/perf/util/dso.c +59 -2
index 76e5c13..fbf6cc9 100644
@@ -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;
 }
 
tools/perf/util/unwind-libunwind.c +0 -2
index 4f8dd9e..25578b9 100644
@@ -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;
Signature · 1 line
1.8.3.1
#7 lore raw

Re: [PATCH 06/13] perf tools: Cache dso data file descriptor

Quoted 8 lines · Jiri Olsa

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 (24% for my workload) of report:
'perf report --stdio -i perf-test.data' (3 runs)
(perf-test.data size was around 12GB)

Quoted 11 lines
   current code:
    446,826,474,217      cycles                     ( +-  0.23% )
    669,435,100,846      instructions               ( +-  0.03% )

      266.380674551 seconds time elapsed            ( +-  0.37% )

   change:
    558,345,509,819      cycles                     ( +-  0.55% )
    817,283,647,637      instructions               ( +-  0.03% )

      351.564209617 seconds time elapsed            ( +-  0.89% )

message is confusing. Looks like time went up.

David

#8 lore raw

Re: [PATCH 06/13] perf tools: Cache dso data file descriptor

Quoted 15 lines · David Ahern
Quoted 14 lines · Jiri Olsa

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 (24% for my workload) of report:
'perf report --stdio -i perf-test.data' (3 runs)
(perf-test.data size was around 12GB)

current code:

   446,826,474,217      cycles                     ( +-  0.23% )
   669,435,100,846      instructions               ( +-  0.03% )
     266.380674551 seconds time elapsed            ( +-  0.37% )
Quoted 7 lines
Quoted 5 lines

change:

   558,345,509,819      cycles                     ( +-  0.55% )
   817,283,647,637      instructions               ( +-  0.03% )
     351.564209617 seconds time elapsed            ( +-  0.89% )

message is confusing. Looks like time went up.

aaargh.. right ;-) it should bethe other way around

thanks,
jirka

#9 lore raw

Re: [PATCH 06/13] perf tools: Cache dso data file descriptor

In reply to Jiri Olsa

tools/perf/util/dso.c static void close_dso(struct dso *dso) · line 233

Quoted 9 lines · Jiri Olsa
tools/perf/util/dso.c +9 -0
+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;

Why not limit this to something reasonable -- 512 or 1024 -- rather "infinity" (~0UL).

David

#10 lore raw

Re: [PATCH 06/13] perf tools: Cache dso data file descriptor

Quoted 14 lines · David Ahern

tools/perf/util/dso.c static void close_dso(struct dso *dso) · line 233

Quoted 9 lines · Jiri Olsa
tools/perf/util/dso.c +9 -0
+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;

Why not limit this to something reasonable -- 512 or 1024 -- rather "infinity" (~0UL).

hum, if there's no limit for the user at the first place why limit him in the perf..?

jirka

author reviewer
#11 lore raw

PATCH v3 7/13 perf tools: Add file size check and factor dso__data_read_offset

In reply to 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.

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/dso.c | 64 +++++++++++++++++++++++++++++++++++++++------------
 tools/perf/util/dso.h |  1 +
 2 files changed, 50 insertions(+), 15 deletions(-)
tools/perf/util/dso.c +49 -15
index fbf6cc9..db63438 100644
@@ -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)
tools/perf/util/dso.h +1 -0
index 90988bf..da47b13 100644
@@ -103,6 +103,7 @@ struct dso {
 	struct {
 		struct rb_root	 cache;
 		int		 fd;
+		size_t		 file_size;
 		struct list_head open_entry;
 	} data;
 
Signature · 1 line
1.8.3.1
author reviewer
#12 lore raw

PATCH v3 8/13 perf tools: Allow to close dso fd in case of open failure

In reply to 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.

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/dso.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)
tools/perf/util/dso.c +22 -1
index db63438..c30752c 100644
@@ -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;
 }
Signature · 1 line
1.8.3.1
author reviewer
#13 lore raw

PATCH v3 9/13 perf tools: Add dso__data_* interface descriptons

In reply to Jiri Olsa

Adding descriptions/explanations for dso__data_* interface functions.

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/dso.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/dso.h | 38 +++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)
tools/perf/util/dso.c +59 -0
index c30752c..819f104 100644
@@ -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)
tools/perf/util/dso.h +38 -0
index da47b13..ad553ba 100644
@@ -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);
 
Signature · 1 line
1.8.3.1
#14 lore raw

Re: [PATCH 09/13] perf tools: Add dso__data_* interface descriptons

Quoted 2 lines · Jiri Olsa

Adding descriptions/explanations for dso__data_* interface functions.

Shouldn't the descriptions be added when the functions are?

David

#15 lore raw

Re: [PATCH 09/13] perf tools: Add dso__data_* interface descriptons

Quoted 5 lines · David Ahern
Quoted 2 lines · Jiri Olsa

Adding descriptions/explanations for dso__data_* interface functions.

Shouldn't the descriptions be added when the functions are?

well, some of the described functions were already there, so i thought it'd be better to put it there alltogether with new ones..

jirka

author reviewer
#16 lore raw

PATCH v3 10/13 perf tests: Spawn child for each test

In reply to Jiri Olsa

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

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/tests/builtin-test.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)
tools/perf/tests/builtin-test.c +31 -1
index 802e3cd..9677a5c 100644
@@ -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) {
Signature · 1 line
1.8.3.1
Reviewed-by on v3 10/13
#17 lore raw

Re: [PATCH 10/13] perf tests: Spawn child for each test

Quoted 3 lines · Jiri Olsa

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

author reviewer
#18 lore raw

PATCH v3 11/13 perf tests: Allow reuse of test_file function

In reply to Jiri Olsa

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

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/tests/dso-data.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tools/perf/tests/dso-data.c +5 -1
index 3e6cb17..f5c8743 100644
@@ -12,11 +12,15 @@
 
 static char *test_file(int size)
 {
-	static char buf_templ[] = "/tmp/test-XXXXXX";
+#define TEMPL "/tmp/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");
Signature · 1 line
1.8.3.1
#19 lore raw

Re: [PATCH 11/13] perf tests: Allow reuse of test_file function

Quoted 22 lines · Jiri Olsa

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

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
  tools/perf/tests/dso-data.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
tools/perf/tests/dso-data.c +0 -0
index 3e6cb17..f5c8743 100644

tools/perf/tests/dso-data.c line 15

Quoted 5 lines
tools/perf/tests/dso-data.c +1 -1
@@ -12,11 +12,15 @@
 
  static char *test_file(int size)
  {
-	static char buf_templ[] = "/tmp/test-XXXXXX";
+#define TEMPL "/tmp/test-XXXXXX"

use /tmp/perf-test-XXXXXX or /tmp/perf-XXXXXX to we know where all these tmp files came from.

tools/perf/tests/dso-data.c line 16

Quoted 1 line
tools/perf/tests/dso-data.c +1 -0
+	static char buf_templ[sizeof(TEMPL)];

why static? and ...

tools/perf/tests/dso-data.c line 22

Quoted 6 lines
tools/perf/tests/dso-data.c +2 -0
  	char *templ = buf_templ;
  	int fd, i;
  	unsigned char *buf;
 
+	strcpy(buf_templ, TEMPL);
+#undef TEMPL

why the need to change from char buf_templ[] to strcpy?

David

#20 lore raw

Re: [PATCH 11/13] perf tests: Allow reuse of test_file function

Quoted 24 lines · David Ahern
Quoted 23 lines · Jiri Olsa

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

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/tests/dso-data.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tools/perf/tests/dso-data.c +0 -0
index 3e6cb17..f5c8743 100644
@@ -12,11 +12,15 @@
 
Quoted 7 lines

tools/perf/tests/dso-data.c line 15

Quoted 4 lines
tools/perf/tests/dso-data.c +1 -1
 static char *test_file(int size)
 {
-	static char buf_templ[] = "/tmp/test-XXXXXX";
+#define TEMPL "/tmp/test-XXXXXX"

use /tmp/perf-test-XXXXXX or /tmp/perf-XXXXXX to we know where all these tmp files came from.

ok

Quoted 4 lines

tools/perf/tests/dso-data.c line 16

Quoted 2 lines
tools/perf/tests/dso-data.c +1 -0
+	static char buf_templ[sizeof(TEMPL)];

why static? and ...

I need to use it outside the test_file function

Quoted 9 lines

tools/perf/tests/dso-data.c line 22

Quoted 7 lines
tools/perf/tests/dso-data.c +2 -0
 	char *templ = buf_templ;
 	int fd, i;
 	unsigned char *buf;
 
+	strcpy(buf_templ, TEMPL);
+#undef TEMPL

why the need to change from char buf_templ[] to strcpy?

because it gets overwritten via mkstemp function

thanks,
jirka

#21 lore raw

Re: [PATCH 11/13] perf tests: Allow reuse of test_file function

Quoted 5 lines · Jiri Olsa
Quoted 3 lines

tools/perf/tests/dso-data.c line 16

Quoted 1 line
tools/perf/tests/dso-data.c +1 -0
+	static char buf_templ[sizeof(TEMPL)];

why static? and ...

I need to use it outside the test_file function

Oh, it's a return variable.

David

author reviewer
#22 lore raw

PATCH v3 12/13 perf tests: Add test for caching dso file descriptors

In reply to 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 <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 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(-)
tools/perf/tests/builtin-test.c +5 -1
index 9677a5c..b8a6358 100644
@@ -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,
 	},
tools/perf/tests/dso-data.c +132 -3

146 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index 022bb68..ccc4deb 100644
@@ -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);
Signature · 1 line
1.8.3.1
#23 lore raw

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

Quoted 5 lines · Jiri Olsa
tools/perf/tests/dso-data.c +5 -0
+static int dsos__create(int cnt, int size)
+{
+	int i;
+
+	dsos = malloc(sizeof(dsos) * cnt);

tools/perf/tests/dso-data.c int test__dso_data(void) · line 200

Quoted 10 lines
tools/perf/tests/dso-data.c +10 -0
+	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((const char *)file);
+		TEST_ASSERT_VAL("failed to get dso", dsos[i]);

typecast is not needed.

David

author reviewer
#24 lore raw

PATCH v3 13/13 perf tests: Add test for closing dso objects on EMFILE error

In reply to 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.

Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 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(+)
tools/perf/tests/builtin-test.c +4 -0
index b8a6358..6f8b01b 100644
@@ -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,
 	},
tools/perf/tests/dso-data.c +73 -0

76 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index ccc4deb..ed64790 100644
@@ -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);
Signature · 1 line
1.8.3.1
#25 lore raw

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

Quoted 3 lines · 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.

Reviewed by: David Ahern <[email protected]>