perf tools: Speed-up perf report by using multi thread (v5)

tools/perf/util/evsel.h +3 -0

+/**
+ * perf_evsel__is_dummy_tracking - Return whether given evsel is a dummy
+ * event for tracking meta events only

On Fri, Oct 02, 2015 at 02:18:42PM +0900, Namhyung Kim wrote:

SNIP

Quoted 1 line

tools/perf/util/evsel.h static inline bool perf_evsel__is_function_event(struct perf_evsel *evsel) · line 374

Quoted 13 lines

tools/perf/util/evsel.h +13 -0

+/**
+ * perf_evsel__is_dummy_tracking - Return whether given evsel is a dummy
+ * event for tracking meta events only
+ *
+ * @evsel - evsel selector to be tested
+ *
+ * Return %true if event is a dummy tracking event
+ */
+static inline bool perf_evsel__is_dummy_tracking(struct perf_evsel *evsel)
+{
+	return evsel->attr.type == PERF_TYPE_SOFTWARE &&
+		evsel->attr.config == PERF_COUNT_SW_DUMMY &&
+		evsel->attr.task == 1 && evsel->attr.mmap == 1;

should this use now check for evsel->tracking ?

It is more convenient saving mmap length rather than (bit) mask. With this patch, we can eliminate dependency to perf_evlist other than getting mmap_desc for dealing with mmaps. The mask and length can be converted using perf_evlist__mmap_mask/len().

Cc Jiri Olsa <[email protected]>

Cc Adrian Hunter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

tools/perf/util/evlist.c +0 -0

index c5180a29db1b..e46adcd5b408 100644

tools/perf/util/evlist.c +4 -2

+
+		munmap(evlist->mmap[idx].base, mmap_len);
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}

@@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 }
 
 struct mmap_params {
-	int prot;
-	int mask;
+	int	prot;
+	size_t	len;
 	struct auxtrace_mmap_params auxtrace_mp;
 };
 

tools/perf/util/evlist.c +17 -1

+	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
 				      MAP_SHARED, fd, 0);
 	if (evlist->mmap[idx].base == MAP_FAILED) {
 		pr_debug2("failed to mmap perf event ring buffer, error %d\n",

@@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
 	return (pages + 1) * page_size;
 }
 
+static size_t perf_evlist__mmap_mask(size_t len)
+{
+	BUG_ON(len <= page_size);
+	BUG_ON((len % page_size) != 0);
+
+	return len - page_size - 1;
+}
+
+static size_t perf_evlist__mmap_len(size_t mask)
+{
+	BUG_ON(((mask + 1) % page_size) != 0);
+
+	return mask + 1 + page_size;
+}
+
 static long parse_pages_arg(const char *str, unsigned long min,
 			    unsigned long max)
 {

@@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 	evlist->overwrite = overwrite;
 	evlist->mmap_len = perf_evlist__mmap_size(pages);
 	pr_debug("mmap size %zuB\n", evlist->mmap_len);
-	mp.mask = evlist->mmap_len - page_size - 1;
+	mp.len = evlist->mmap_len;
 
 	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
 				   auxtrace_pages, auxtrace_overwrite);

Signature · 1 line

2.6.0

Em Fri, Oct 02, 2015 at 02:18:43PM +0900, Namhyung Kim escreveu:

Quoted 18 lines

It is more convenient saving mmap length rather than (bit) mask. With this patch, we can eliminate dependency to perf_evlist other than getting mmap_desc for dealing with mmaps. The mask and length can be converted using perf_evlist__mmap_mask/len().

Cc Jiri Olsa <[email protected]>

Cc Adrian Hunter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

tools/perf/util/evlist.c +0 -0

index c5180a29db1b..e46adcd5b408 100644
@@ -29,6 +29,8 @@
 

tools/perf/util/evlist.c line 33

Quoted 4 lines

tools/perf/util/evlist.c +2 -0

 static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx);
 static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx);
+static size_t perf_evlist__mmap_mask(size_t len);
+static size_t perf_evlist__mmap_len(size_t mask);

Are these "perf_evlist" methods? I don't think so, those are related to "perf_mmap".

tools/perf/util/evlist.c void __weak auxtrace_mmap_params__set_idx( · line 876

Quoted 9 lines

tools/perf/util/evlist.c +1 -1

 #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
 #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)

@@ -871,7 +873,9 @@ void __weak auxtrace_mmap_params__set_idx(
 static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
 {
 	if (evlist->mmap[idx].base != NULL) {
-		munmap(evlist->mmap[idx].base, evlist->mmap_len);
+		size_t mmap_len = perf_evlist__mmap_len(evlist->mmap[idx].mask);

I.e. here you could have it as:

		size_t mmap_len = perf_mmap__len(evlist->mmap[idx]);

Quoted 18 lines

tools/perf/util/evlist.c +4 -2

+
+		munmap(evlist->mmap[idx].base, mmap_len);
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}

@@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 }
 
 struct mmap_params {
-	int prot;
-	int mask;
+	int	prot;
+	size_t	len;
 	struct auxtrace_mmap_params auxtrace_mp;
 };
 

tools/perf/util/evlist.c static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx, · line 931

Quoted 6 lines

tools/perf/util/evlist.c +1 -2

@@ -924,8 +928,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 	 */
 	atomic_set(&evlist->mmap[idx].refcnt, 2);
 	evlist->mmap[idx].prev = 0;
-	evlist->mmap[idx].mask = mp->mask;
-	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
+	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);

Here, since you're not using a perf_mmap instance, but the calculation is relative to a perf_mmap property, we would use:

	evlist->mmap[idx].mask = __perf_mmap__mask(mp->len);

Quoted 39 lines

tools/perf/util/evlist.c +17 -1

+	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
 				      MAP_SHARED, fd, 0);
 	if (evlist->mmap[idx].base == MAP_FAILED) {
 		pr_debug2("failed to mmap perf event ring buffer, error %d\n",

@@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
 	return (pages + 1) * page_size;
 }
 
+static size_t perf_evlist__mmap_mask(size_t len)
+{
+	BUG_ON(len <= page_size);
+	BUG_ON((len % page_size) != 0);
+
+	return len - page_size - 1;
+}
+
+static size_t perf_evlist__mmap_len(size_t mask)
+{
+	BUG_ON(((mask + 1) % page_size) != 0);
+
+	return mask + 1 + page_size;
+}
+
 static long parse_pages_arg(const char *str, unsigned long min,
 			    unsigned long max)
 {

@@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 	evlist->overwrite = overwrite;
 	evlist->mmap_len = perf_evlist__mmap_size(pages);
 	pr_debug("mmap size %zuB\n", evlist->mmap_len);
-	mp.mask = evlist->mmap_len - page_size - 1;
+	mp.len = evlist->mmap_len;
 
 	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
 				   auxtrace_pages, auxtrace_overwrite);

Signature · 1 line

2.6.0

It is more convenient saving mmap length rather than (bit) mask. With this patch, we can eliminate dependency to perf_evlist other than getting mmap_desc for dealing with mmaps. The mask and length can be converted using perf_evlist__mmap_mask/len().

Cc Jiri Olsa <[email protected]>

Cc Adrian Hunter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

tools/perf/util/evlist.c +25 -6

index c5180a29db1b..e46adcd5b408 100644
@@ -29,6 +29,8 @@
 
 static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx);
 static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx);
+static size_t perf_evlist__mmap_mask(size_t len);
+static size_t perf_evlist__mmap_len(size_t mask);
 
 #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
 #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)

@@ -871,7 +873,9 @@ void __weak auxtrace_mmap_params__set_idx(
 static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
 {
 	if (evlist->mmap[idx].base != NULL) {
-		munmap(evlist->mmap[idx].base, evlist->mmap_len);
+		size_t mmap_len = perf_evlist__mmap_len(evlist->mmap[idx].mask);
+
+		munmap(evlist->mmap[idx].base, mmap_len);
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}

@@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 }
 
 struct mmap_params {
-	int prot;
-	int mask;
+	int	prot;
+	size_t	len;
 	struct auxtrace_mmap_params auxtrace_mp;
 };
 

@@ -924,8 +928,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 	 */
 	atomic_set(&evlist->mmap[idx].refcnt, 2);
 	evlist->mmap[idx].prev = 0;
-	evlist->mmap[idx].mask = mp->mask;
-	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
+	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);
+	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
 				      MAP_SHARED, fd, 0);
 	if (evlist->mmap[idx].base == MAP_FAILED) {
 		pr_debug2("failed to mmap perf event ring buffer, error %d\n",

@@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
 	return (pages + 1) * page_size;
 }
 
+static size_t perf_evlist__mmap_mask(size_t len)
+{
+	BUG_ON(len <= page_size);
+	BUG_ON((len % page_size) != 0);
+
+	return len - page_size - 1;
+}
+
+static size_t perf_evlist__mmap_len(size_t mask)
+{
+	BUG_ON(((mask + 1) % page_size) != 0);
+
+	return mask + 1 + page_size;
+}
+
 static long parse_pages_arg(const char *str, unsigned long min,
 			    unsigned long max)
 {

@@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 	evlist->overwrite = overwrite;
 	evlist->mmap_len = perf_evlist__mmap_size(pages);
 	pr_debug("mmap size %zuB\n", evlist->mmap_len);
-	mp.mask = evlist->mmap_len - page_size - 1;
+	mp.len = evlist->mmap_len;
 
 	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
 				   auxtrace_pages, auxtrace_overwrite);

Signature · 1 line

2.6.0

Quoted 8 lines · Namhyung Kim

It is more convenient saving mmap length rather than (bit) mask. With this patch, we can eliminate dependency to perf_evlist other than getting mmap_desc for dealing with mmaps. The mask and length can be converted using perf_evlist__mmap_mask/len().

Cc Jiri Olsa <[email protected]>

Cc Adrian Hunter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

after this patch I'm hitting:

[jolsa@krava perf]$ ./perf record kill
kill: not enough arguments
perf: util/evlist.c:1003: perf_evlist__mmap_len: Assertion `!((mask & page_size) != 0)' failed. Aborted (core dumped)
[jolsa@krava perf]$

jirka

Quoted 84 lines

---
 tools/perf/util/evlist.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

tools/perf/util/evlist.c +25 -6

index c5180a29db1b..e46adcd5b408 100644
@@ -29,6 +29,8 @@
 
 static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx);
 static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx);
+static size_t perf_evlist__mmap_mask(size_t len);
+static size_t perf_evlist__mmap_len(size_t mask);
 
 #define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
 #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)

@@ -871,7 +873,9 @@ void __weak auxtrace_mmap_params__set_idx(
 static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
 {
 	if (evlist->mmap[idx].base != NULL) {
-		munmap(evlist->mmap[idx].base, evlist->mmap_len);
+		size_t mmap_len = perf_evlist__mmap_len(evlist->mmap[idx].mask);
+
+		munmap(evlist->mmap[idx].base, mmap_len);
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}

@@ -901,8 +905,8 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 }
 
 struct mmap_params {
-	int prot;
-	int mask;
+	int	prot;
+	size_t	len;
 	struct auxtrace_mmap_params auxtrace_mp;
 };
 

@@ -924,8 +928,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 	 */
 	atomic_set(&evlist->mmap[idx].refcnt, 2);
 	evlist->mmap[idx].prev = 0;
-	evlist->mmap[idx].mask = mp->mask;
-	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, mp->prot,
+	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);
+	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
 				      MAP_SHARED, fd, 0);
 	if (evlist->mmap[idx].base == MAP_FAILED) {
 		pr_debug2("failed to mmap perf event ring buffer, error %d\n",

@@ -1071,6 +1075,21 @@ static size_t perf_evlist__mmap_size(unsigned long pages)
 	return (pages + 1) * page_size;
 }
 
+static size_t perf_evlist__mmap_mask(size_t len)
+{
+	BUG_ON(len <= page_size);
+	BUG_ON((len % page_size) != 0);
+
+	return len - page_size - 1;
+}
+
+static size_t perf_evlist__mmap_len(size_t mask)
+{
+	BUG_ON(((mask + 1) % page_size) != 0);
+
+	return mask + 1 + page_size;
+}
+
 static long parse_pages_arg(const char *str, unsigned long min,
 			    unsigned long max)
 {

@@ -1176,7 +1195,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 	evlist->overwrite = overwrite;
 	evlist->mmap_len = perf_evlist__mmap_size(pages);
 	pr_debug("mmap size %zuB\n", evlist->mmap_len);
-	mp.mask = evlist->mmap_len - page_size - 1;
+	mp.len = evlist->mmap_len;
 
 	auxtrace_mmap_params__init(&mp.auxtrace_mp, evlist->mmap_len,
 				   auxtrace_pages, auxtrace_overwrite);

Signature · 1 line

2.6.0

Hi Jiri,

Quoted 10 lines · Jiri Olsa

Quoted 8 lines · Namhyung Kim

It is more convenient saving mmap length rather than (bit) mask. With this patch, we can eliminate dependency to perf_evlist other than getting mmap_desc for dealing with mmaps. The mask and length can be converted using perf_evlist__mmap_mask/len().

Cc Jiri Olsa <[email protected]>

Cc Adrian Hunter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

Quoted 7 lines

after this patch I'm hitting:

[jolsa@krava perf]$ ./perf record kill
kill: not enough arguments
perf: util/evlist.c:1003: perf_evlist__mmap_len: Assertion `!((mask & page_size) != 0)' failed. Aborted (core dumped)
[jolsa@krava perf]$

This is strange.. I think I fixed it already. And the expression in the assertion is different than the code in the patch:

  static size_t perf_evlist__mmap_len(size_t mask)
  {
         BUG_ON(((mask + 1) % page_size) != 0);

         return mask + 1 + page_size;

}

Could you please double check?

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Cc Adrian Hunter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/builtin-record.c |  4 ++--
 tools/perf/util/evlist.c    | 30 +++++++++++++++++++++---------
 tools/perf/util/evlist.h    |  2 +-
 3 files changed, 24 insertions(+), 12 deletions(-)

tools/perf/builtin-record.c +2 -2

index 5e01c070dbf2..0accac6e0812 100644
@@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct record *rec)
 
 	for (i = 0; i < rec->evlist->nr_mmaps; i++) {
 		struct auxtrace_mmap *mm =
-				&rec->evlist->mmap[i].auxtrace_mmap;
+				&rec->evlist->auxtrace_mmap[i];
 
 		if (!mm->base)
 			continue;

@@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
 	int rc = 0;
 
 	for (i = 0; i < rec->evlist->nr_mmaps; i++) {
-		struct auxtrace_mmap *mm = &rec->evlist->mmap[i].auxtrace_mmap;
+		struct auxtrace_mmap *mm = &rec->evlist->auxtrace_mmap[i];
 
 		if (rec->evlist->mmap[i].base) {
 			if (record__mmap_read(rec, i) != 0) {

tools/perf/util/evlist.c +21 -9

index e46adcd5b408..042dffc67986 100644
@@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
 	return event;
 }
 
-static bool perf_mmap__empty(struct perf_mmap *md)
+static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
 {
-	return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
+	struct perf_mmap *md = &evlist->mmap[idx];
+
+	return perf_mmap__read_head(md) == md->prev &&
+		evlist->auxtrace_mmap[idx].base == NULL;
 }
 
 static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)

@@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
 		perf_mmap__write_tail(md, old);
 	}
 
-	if (atomic_read(&md->refcnt) == 1 && perf_mmap__empty(md))
+	if (atomic_read(&md->refcnt) == 1 && perf_evlist__mmap_empty(evlist, idx))
 		perf_evlist__mmap_put(evlist, idx);
 }
 

@@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}
-	auxtrace_mmap__munmap(&evlist->mmap[idx].auxtrace_mmap);
+	auxtrace_mmap__munmap(&evlist->auxtrace_mmap[idx]);
 }
 
 void perf_evlist__munmap(struct perf_evlist *evlist)

@@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 	if (cpu_map__empty(evlist->cpus))
 		evlist->nr_mmaps = thread_map__nr(evlist->threads);
 	evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
-	return evlist->mmap != NULL ? 0 : -ENOMEM;
+	if (evlist->mmap == NULL)
+		return -ENOMEM;
+	evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
+				       sizeof(struct auxtrace_mmap));
+	if (evlist->auxtrace_mmap == NULL) {
+		zfree(&evlist->mmap);
+		return -ENOMEM;
+	}
+	return 0;
 }
 
 struct mmap_params {

@@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 		return -1;
 	}
 
-	if (auxtrace_mmap__mmap(&evlist->mmap[idx].auxtrace_mmap,
-				&mp->auxtrace_mp, evlist->mmap[idx].base, fd))
-		return -1;
-
 	return 0;
 }
 

@@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 			*output = fd;
 			if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
 				return -1;
+
+			if (auxtrace_mmap__mmap(&evlist->auxtrace_mmap[idx],
+						&mp->auxtrace_mp,
+						evlist->mmap[idx].base, fd))
+				return -1;
 		} else {
 			if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
 				return -1;

tools/perf/util/evlist.h +1 -1

index 414e383885f5..51574ce8ac69 100644
@@ -30,7 +30,6 @@ struct perf_mmap {
 	int		 mask;
 	atomic_t	 refcnt;
 	u64		 prev;
-	struct auxtrace_mmap auxtrace_mmap;
 	char		 event_copy[PERF_SAMPLE_MAX_SIZE] __attribute__((aligned(8)));
 };
 

@@ -53,6 +52,7 @@ struct perf_evlist {
 	} workload;
 	struct fdarray	 pollfd;
 	struct perf_mmap *mmap;
+	struct auxtrace_mmap *auxtrace_mmap;
 	struct thread_map *threads;
 	struct cpu_map	  *cpus;
 	struct perf_evsel *selected;

Signature · 1 line

2.6.0

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?

- Arnaldo

Quoted 129 lines

Cc Adrian Hunter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/builtin-record.c |  4 ++--
 tools/perf/util/evlist.c    | 30 +++++++++++++++++++++---------
 tools/perf/util/evlist.h    |  2 +-
 3 files changed, 24 insertions(+), 12 deletions(-)

tools/perf/builtin-record.c +2 -2

index 5e01c070dbf2..0accac6e0812 100644
@@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct record *rec)
 
 	for (i = 0; i < rec->evlist->nr_mmaps; i++) {
 		struct auxtrace_mmap *mm =
-				&rec->evlist->mmap[i].auxtrace_mmap;
+				&rec->evlist->auxtrace_mmap[i];
 
 		if (!mm->base)
 			continue;

@@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
 	int rc = 0;
 
 	for (i = 0; i < rec->evlist->nr_mmaps; i++) {
-		struct auxtrace_mmap *mm = &rec->evlist->mmap[i].auxtrace_mmap;
+		struct auxtrace_mmap *mm = &rec->evlist->auxtrace_mmap[i];
 
 		if (rec->evlist->mmap[i].base) {
 			if (record__mmap_read(rec, i) != 0) {

tools/perf/util/evlist.c +21 -9

index e46adcd5b408..042dffc67986 100644
@@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
 	return event;
 }
 
-static bool perf_mmap__empty(struct perf_mmap *md)
+static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
 {
-	return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
+	struct perf_mmap *md = &evlist->mmap[idx];
+
+	return perf_mmap__read_head(md) == md->prev &&
+		evlist->auxtrace_mmap[idx].base == NULL;
 }
 
 static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)

@@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
 		perf_mmap__write_tail(md, old);
 	}
 
-	if (atomic_read(&md->refcnt) == 1 && perf_mmap__empty(md))
+	if (atomic_read(&md->refcnt) == 1 && perf_evlist__mmap_empty(evlist, idx))
 		perf_evlist__mmap_put(evlist, idx);
 }
 

@@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}
-	auxtrace_mmap__munmap(&evlist->mmap[idx].auxtrace_mmap);
+	auxtrace_mmap__munmap(&evlist->auxtrace_mmap[idx]);
 }
 
 void perf_evlist__munmap(struct perf_evlist *evlist)

@@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 	if (cpu_map__empty(evlist->cpus))
 		evlist->nr_mmaps = thread_map__nr(evlist->threads);
 	evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
-	return evlist->mmap != NULL ? 0 : -ENOMEM;
+	if (evlist->mmap == NULL)
+		return -ENOMEM;
+	evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
+				       sizeof(struct auxtrace_mmap));
+	if (evlist->auxtrace_mmap == NULL) {
+		zfree(&evlist->mmap);
+		return -ENOMEM;
+	}
+	return 0;
 }
 
 struct mmap_params {

@@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 		return -1;
 	}
 
-	if (auxtrace_mmap__mmap(&evlist->mmap[idx].auxtrace_mmap,
-				&mp->auxtrace_mp, evlist->mmap[idx].base, fd))
-		return -1;
-
 	return 0;
 }
 

@@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 			*output = fd;
 			if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
 				return -1;
+
+			if (auxtrace_mmap__mmap(&evlist->auxtrace_mmap[idx],
+						&mp->auxtrace_mp,
+						evlist->mmap[idx].base, fd))
+				return -1;
 		} else {
 			if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
 				return -1;

tools/perf/util/evlist.h +1 -1

index 414e383885f5..51574ce8ac69 100644
@@ -30,7 +30,6 @@ struct perf_mmap {
 	int		 mask;
 	atomic_t	 refcnt;
 	u64		 prev;
-	struct auxtrace_mmap auxtrace_mmap;
 	char		 event_copy[PERF_SAMPLE_MAX_SIZE] __attribute__((aligned(8)));
 };
 

@@ -53,6 +52,7 @@ struct perf_evlist {
 	} workload;
 	struct fdarray	 pollfd;
 	struct perf_mmap *mmap;
+	struct auxtrace_mmap *auxtrace_mmap;
 	struct thread_map *threads;
 	struct cpu_map	  *cpus;
 	struct perf_evsel *selected;

Signature · 1 line

2.6.0

Quoted 4 lines · Arnaldo Carvalho de Melo

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?

Also, what if you wanted to capture AUX data and tracking together.

In addition, currently Intel PT can have either 1 dummy event for tracking plus sched_switch
or 2 dummy events to allow for system-wide tracking of context switches. i.e. there
are multiple tracking events.

Quoted 133 lines

- Arnaldo

Quoted 129 lines

Cc Adrian Hunter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/builtin-record.c |  4 ++--
 tools/perf/util/evlist.c    | 30 +++++++++++++++++++++---------
 tools/perf/util/evlist.h    |  2 +-
 3 files changed, 24 insertions(+), 12 deletions(-)

tools/perf/builtin-record.c +2 -2

index 5e01c070dbf2..0accac6e0812 100644
@@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct record *rec)
 
     for (i = 0; i < rec->evlist->nr_mmaps; i++) {
             struct auxtrace_mmap *mm =
-                            &rec->evlist->mmap[i].auxtrace_mmap;
+                            &rec->evlist->auxtrace_mmap[i];
 
             if (!mm->base)
                     continue;

@@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
     int rc = 0;
 
     for (i = 0; i < rec->evlist->nr_mmaps; i++) {
-            struct auxtrace_mmap *mm = &rec->evlist->mmap[i].auxtrace_mmap;
+            struct auxtrace_mmap *mm = &rec->evlist->auxtrace_mmap[i];
 
             if (rec->evlist->mmap[i].base) {
                     if (record__mmap_read(rec, i) != 0) {

tools/perf/util/evlist.c +21 -9

index e46adcd5b408..042dffc67986 100644
@@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
     return event;
 }
 
-static bool perf_mmap__empty(struct perf_mmap *md)
+static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
 {
-    return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
+    struct perf_mmap *md = &evlist->mmap[idx];
+
+    return perf_mmap__read_head(md) == md->prev &&
+            evlist->auxtrace_mmap[idx].base == NULL;
 }
 
 static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)

@@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
             perf_mmap__write_tail(md, old);
     }
 
-    if (atomic_read(&md->refcnt) == 1 && perf_mmap__empty(md))
+    if (atomic_read(&md->refcnt) == 1 && perf_evlist__mmap_empty(evlist, idx))
             perf_evlist__mmap_put(evlist, idx);
 }
 

@@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
             evlist->mmap[idx].base = NULL;
             atomic_set(&evlist->mmap[idx].refcnt, 0);
     }
-    auxtrace_mmap__munmap(&evlist->mmap[idx].auxtrace_mmap);
+    auxtrace_mmap__munmap(&evlist->auxtrace_mmap[idx]);
 }
 
 void perf_evlist__munmap(struct perf_evlist *evlist)

@@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
     if (cpu_map__empty(evlist->cpus))
             evlist->nr_mmaps = thread_map__nr(evlist->threads);
     evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
-    return evlist->mmap != NULL ? 0 : -ENOMEM;
+    if (evlist->mmap == NULL)
+            return -ENOMEM;
+    evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
+                                   sizeof(struct auxtrace_mmap));
+    if (evlist->auxtrace_mmap == NULL) {
+            zfree(&evlist->mmap);
+            return -ENOMEM;
+    }
+    return 0;
 }
 
 struct mmap_params {

@@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
             return -1;
     }
 
-    if (auxtrace_mmap__mmap(&evlist->mmap[idx].auxtrace_mmap,
-                            &mp->auxtrace_mp, evlist->mmap[idx].base, fd))
-            return -1;
-
     return 0;
 }
 

@@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
                     *output = fd;
                     if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
                             return -1;
+
+                    if (auxtrace_mmap__mmap(&evlist->auxtrace_mmap[idx],
+                                            &mp->auxtrace_mp,
+                                            evlist->mmap[idx].base, fd))
+                            return -1;
             } else {
                     if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
                             return -1;

tools/perf/util/evlist.h +1 -2

index 414e383885f5..51574ce8ac69 100644
@@ -30,7 +30,6 @@ struct perf_mmap {
     int              mask;
     atomic_t         refcnt;
     u64              prev;
-    struct auxtrace_mmap auxtrace_mmap;
     char             event_copy[PERF_SAMPLE_MAX_SIZE] __attribute__((aligned(8)));
 };
 

@@ -53,6 +52,7 @@ struct perf_evlist {
     } workload;
     struct fdarray   pollfd;
     struct perf_mmap *mmap;
+    struct auxtrace_mmap *auxtrace_mmap;
     struct thread_map *threads;
     struct cpu_map    *cpus;
     struct perf_evsel *selected;
--

2.6.0

Hi Adrian,

Quoted 4 lines · Adrian Hunter

Quoted 3 lines · Arnaldo Carvalho de Melo

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Quoted 3 lines

Quoted 1 line

Is this moving around _strictly_ needed?

Also, what if you wanted to capture AUX data and tracking together.

Hmm.. I don't know what's the problem. It should be orthogonal and support doing that together IMHO. Maybe I'm missing something about the aux data processing and Intel PT. I'll take a look at it..

Quoted 11 lines · Namhyung Kim

Hi Adrian,

Quoted 7 lines · Adrian Hunter

Quoted 4 lines · Arnaldo Carvalho de Melo

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?

Also, what if you wanted to capture AUX data and tracking together.

Quoted 4 lines

Hmm.. I don't know what's the problem. It should be orthogonal and support doing that together IMHO. Maybe I'm missing something about the aux data processing and Intel PT. I'll take a look at it..

It is only orthogonal if you assume we will never want to support parallel processing with Intel PT.

The only change that needs to be made is not to assume there is only 1 tracking event.

IMHO there could be separate mmap_params also, which would allow for different mmap sizes for the tracking and main mmaps.

Hi Adrian,

Quoted 17 lines · Adrian Hunter

Quoted 15 lines · Namhyung Kim

Hi Adrian,

Quoted 7 lines · Adrian Hunter

Quoted 4 lines · Arnaldo Carvalho de Melo

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?

Also, what if you wanted to capture AUX data and tracking together.

Hmm.. I don't know what's the problem. It should be orthogonal and support doing that together IMHO. Maybe I'm missing something about the aux data processing and Intel PT. I'll take a look at it..

Quoted 2 lines

It is only orthogonal if you assume we will never want to support parallel processing with Intel PT.

We'll definitely want it. :)

Quoted 3 lines

The only change that needs to be made is not to assume there is only 1 tracking event.

IIUC Intel PT (and BTS?) needs maximum 2 dummy events - one is to track task/mmap and another is to track context switches. The latter is basically a light-weight version of the sched_switch event, right?

For parallel processing, each cpu needs to keep current thread to synthesize events from auxtrace data. So if it processed the switch events before processing samples, it'd need to build long lists of current thread per cpu. IMHO it'd be better to process the switch events with samples using multi-thread rather than processing them prior to samples.

So how about this? It'd use *always* 2 dummy (or 1 dummy + 1 sched_switch) events. The tracking dummy events would be recorded on the tracking mmaps and switch (dummy) event would be recorded on the main mmaps. This way we can parallelize the auxtrace processing without the list of current thread IMHO.

Do I miss something?

Quoted 4 lines

IMHO there could be separate mmap_params also, which would allow for different mmap sizes for the tracking and main mmaps.

Currently, the tracking mmap size is fixed at an arbitrary size (128KiB) regardless of the main mmaps. I can add an option to change the tracking mmap size too.

Quoted 23 lines · Namhyung Kim

Hi Adrian,

Quoted 19 lines · Adrian Hunter

Quoted 7 lines · Namhyung Kim

Hi Adrian,

Quoted 4 lines · Adrian Hunter

Quoted 3 lines · Arnaldo Carvalho de Melo

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Is this moving around _strictly_ needed?

Also, what if you wanted to capture AUX data and tracking together.

Hmm.. I don't know what's the problem. It should be orthogonal and support doing that together IMHO. Maybe I'm missing something about the aux data processing and Intel PT. I'll take a look at it..

It is only orthogonal if you assume we will never want to support parallel processing with Intel PT.

Quoted 5 lines

We'll definitely want it. :)

Quoted 3 lines

The only change that needs to be made is not to assume there is only 1 tracking event.

Sorry for the slow reply.

Quoted 5 lines

IIUC Intel PT (and BTS?) needs maximum 2 dummy events - one is to track task/mmap and another is to track context switches. The latter is basically a light-weight version of the sched_switch event, right?

Yes

Quoted 7 lines

For parallel processing, each cpu needs to keep current thread to synthesize events from auxtrace data. So if it processed the switch events before processing samples, it'd need to build long lists of current thread per cpu. IMHO it'd be better to process the switch events with samples using multi-thread rather than processing them prior to samples.

That is a good point.

But that would be limited to dividing the data by cpu. It would be more useful to divide it any which way. Does 'perf report' care if the data is not in order?

Quoted 7 lines

So how about this? It'd use *always* 2 dummy (or 1 dummy + 1 sched_switch) events. The tracking dummy events would be recorded on the tracking mmaps and switch (dummy) event would be recorded on the main mmaps. This way we can parallelize the auxtrace processing without the list of current thread IMHO.

Quoted 1 line

Do I miss something?

Thinking about it now, it would probably make sense to put the AUX event with the tracking events as well, so the data can be queued up ready for processing, then the AUX index would not be needed. But of course, if there were no other events, then there would be no main mmap at all.

From that point of view, I guess I don't need to worry about splitting up the mmaps at all, just process them more than once if need be.

Quoted 6 lines

Quoted 5 lines

IMHO there could be separate mmap_params also, which would allow for different mmap sizes for the tracking and main mmaps.

Quoted 3 lines

Currently, the tracking mmap size is fixed at an arbitrary size (128KiB) regardless of the main mmaps. I can add an option to change the tracking mmap size too.

I meant more from the program point of view, to allow different parameters. Such as allowing one mmap to be PROT_READ and the other PROT_READ|PROT_WRITE i.e. collect all the tracking events but let the other events overwrite
- perhaps as some kind of snapshot mode like we do with Intel PT.

It seemed to me that it would be more flexible to put evsels into mmap groups. Then those groups could have any events or be used in various ways. I also thought it might make the mmap code more readable, instead of having lots of "if tracking event do something different".

On the other hand, it is just a thought. As I mentioned above, I realized I could probably manage without splitting the mmaps.

Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Is this moving around _strictly_ needed?

tools/perf/util/evlist.c +2 -2

@@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
 		perf_mmap__write_tail(md, old);
 	}
 
-	if (atomic_read(&md->refcnt) == 1 && perf_mmap__empty(md))
+	if (atomic_read(&md->refcnt) == 1 && perf_evlist__mmap_empty(evlist, idx))
 		perf_evlist__mmap_put(evlist, idx);
 }
 

@@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}
-	auxtrace_mmap__munmap(&evlist->mmap[idx].auxtrace_mmap);
+	auxtrace_mmap__munmap(&evlist->auxtrace_mmap[idx]);
 }
 
 void perf_evlist__munmap(struct perf_evlist *evlist)

@@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 	if (cpu_map__empty(evlist->cpus))
 		evlist->nr_mmaps = thread_map__nr(evlist->threads);

On Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim wrote:

SNIP

Quoted 17 lines

tools/perf/util/evlist.c +2 -2

@@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
 		perf_mmap__write_tail(md, old);
 	}
 
-	if (atomic_read(&md->refcnt) == 1 && perf_mmap__empty(md))
+	if (atomic_read(&md->refcnt) == 1 && perf_evlist__mmap_empty(evlist, idx))
 		perf_evlist__mmap_put(evlist, idx);
 }
 

@@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
 		evlist->mmap[idx].base = NULL;
 		atomic_set(&evlist->mmap[idx].refcnt, 0);
 	}
-	auxtrace_mmap__munmap(&evlist->mmap[idx].auxtrace_mmap);
+	auxtrace_mmap__munmap(&evlist->auxtrace_mmap[idx]);
 }
 

Quoted 4 lines

tools/perf/util/evlist.c +0 -0

 void perf_evlist__munmap(struct perf_evlist *evlist)

@@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
 	if (cpu_map__empty(evlist->cpus))
 		evlist->nr_mmaps = thread_map__nr(evlist->threads);

tools/perf/util/evlist.c static int perf_evlist__alloc_mmap(struct perf_evlist *evlist) · line 914

Quoted 10 lines

tools/perf/util/evlist.c +8 -1

 	evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
-	return evlist->mmap != NULL ? 0 : -ENOMEM;
+	if (evlist->mmap == NULL)
+		return -ENOMEM;
+	evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
+				       sizeof(struct auxtrace_mmap));
+	if (evlist->auxtrace_mmap == NULL) {
+		zfree(&evlist->mmap);
+		return -ENOMEM;
+	}

can't see evlist->auxtrace_mmap being freed

Since it's gonna share struct mmap with dummy tracking evsel to track meta events only, let's move auxtrace out of struct perf_mmap.

Pass struct perf_mmap to mmap handling functions directly. This will be used by both of normal mmap and track mmap later.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/util/evlist.c | 24 +++++++++++++++---------
 tools/perf/util/evlist.h |  1 +
 2 files changed, 16 insertions(+), 9 deletions(-)

tools/perf/util/evlist.c +8 -8

index 042dffc67986..8d31883cbeb8 100644
@@ -921,8 +921,8 @@ struct mmap_params {
 	struct auxtrace_mmap_params auxtrace_mp;
 };
 
-static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
-			       struct mmap_params *mp, int fd)
+static int perf_mmap__mmap(struct perf_mmap *desc,
+			   struct mmap_params *mp, int fd)
 {
 	/*
 	 * The last one will be done at perf_evlist__mmap_consume(), so that we

@@ -937,21 +937,26 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 	 * evlist layer can't just drop it when filtering events in
 	 * perf_evlist__filter_pollfd().
 	 */
-	atomic_set(&evlist->mmap[idx].refcnt, 2);
-	evlist->mmap[idx].prev = 0;
-	evlist->mmap[idx].mask = perf_evlist__mmap_mask(mp->len);
-	evlist->mmap[idx].base = mmap(NULL, mp->len, mp->prot,
+	atomic_set(&desc->refcnt, 2);
+	desc->prev = 0;
+	desc->mask = perf_evlist__mmap_mask(mp->len);
+	desc->base = mmap(NULL, mp->len, mp->prot,
 				      MAP_SHARED, fd, 0);
-	if (evlist->mmap[idx].base == MAP_FAILED) {
+	if (desc->base == MAP_FAILED) {
 		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
 			  errno);
-		evlist->mmap[idx].base = NULL;
+		desc->base = NULL;
 		return -1;

tools/perf/util/evlist.c +2 -1

 static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 				       struct mmap_params *mp, int cpu,
 				       int thread, int *output)

@@ -960,6 +965,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 
 	evlist__for_each(evlist, evsel) {
 		int fd;
+		struct perf_mmap *desc = perf_evlist__mmap_desc(evlist, idx);
 
 		if (evsel->system_wide && thread)
 			continue;

@@ -968,7 +974,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 
 		if (*output == -1) {
 			*output = fd;
-			if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
+			if (perf_mmap__mmap(desc, mp, *output) < 0)
 				return -1;
 
 			if (auxtrace_mmap__mmap(&evlist->auxtrace_mmap[idx],

tools/perf/util/evlist.h +1 -0

index 51574ce8ac69..79f8245300ad 100644
@@ -145,6 +145,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
 int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 		      bool overwrite);
 void perf_evlist__munmap(struct perf_evlist *evlist);
+struct perf_mmap *perf_evlist__mmap_desc(struct perf_evlist *evlist, int idx);
 
 void perf_evlist__disable(struct perf_evlist *evlist);
 void perf_evlist__enable(struct perf_evlist *evlist);

Signature · 1 line

2.6.0

The new --index option will create indexed data file which can be processed by multiple threads parallelly. It saves meta event and sample data in separate files and merges them with an index table.

If there's an index table in the data file, the HEADER_DATA_INDEX feature bit is set and session->header.index[0] will point to the meta event area, and rest are sample data. It'd look like below:

        +---------------------+
        |     file header     |
        |---------------------|
        |                     |
        |    meta events[0] <-+--+
        |                     |  |
        |---------------------|  |
        |                     |  |
        |    sample data[1] <-+--+
        |                     |  |
        |---------------------|  |
        |                     |  |
        |    sample data[2] <-|--+
        |                     |  |
        |---------------------|  |
        |         ...         | ...
        |---------------------|  |
        |     feature data    |  |
        |   (contains index) -+--+
        +---------------------+

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/Documentation/perf-record.txt |   4 +
 tools/perf/builtin-record.c              | 178 ++++++++++++++++++++++++++++---
 tools/perf/perf.h                        |   1 +
 tools/perf/util/header.c                 |   2 +
 tools/perf/util/session.c                |   1 +
 5 files changed, 173 insertions(+), 13 deletions(-)

tools/perf/Documentation/perf-record.txt +4 -0

index 2e9ce77b5e14..71a9520b10b0 100644
@@ -308,6 +308,10 @@ This option sets the time out limit. The default value is 500 ms.
 Record context switch events i.e. events of type PERF_RECORD_SWITCH or
 PERF_RECORD_SWITCH_CPU_WIDE.
 
+--index::
+Build an index table for sample data.  This will speed up perf report by
+parallel processing.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]

tools/perf/builtin-record.c +165 -13

index 623984c81478..096634c4c5ea 100644
@@ -43,6 +43,7 @@ struct record {
 	u64			bytes_written;
 	struct perf_data_file	file;
 	struct auxtrace_record	*itr;
+	int			*fds;
 	struct perf_evlist	*evlist;
 	struct perf_session	*session;
 	const char		*progname;

@@ -52,9 +53,16 @@ struct record {
 	long			samples;
 };
 
-static int record__write(struct record *rec, void *bf, size_t size)
+static int record__write(struct record *rec, void *bf, size_t size, int idx)
 {
-	if (perf_data_file__write(rec->session->file, bf, size) < 0) {
+	int fd;
+
+	if (rec->fds && idx >= 0)
+		fd = rec->fds[idx];
+	else
+		fd = perf_data_file__fd(rec->session->file);
+
+	if (writen(fd, bf, size) < 0) {
 		pr_err("failed to write perf data, error: %m\n");
 		return -1;
 	}

@@ -69,7 +77,7 @@ static int process_synthesized_event(struct perf_tool *tool,
 				     struct machine *machine __maybe_unused)
 {
 	struct record *rec = container_of(tool, struct record, tool);
-	return record__write(rec, event, event->header.size);
+	return record__write(rec, event, event->header.size, -1);
 }
 
 static int record__mmap_read(struct record *rec, int idx)

@@ -94,7 +102,7 @@ static int record__mmap_read(struct record *rec, int idx)
 		size = md->mask + 1 - (old & md->mask);
 		old += size;
 
-		if (record__write(rec, buf, size) < 0) {
+		if (record__write(rec, buf, size, idx) < 0) {
 			rc = -1;
 			goto out;
 		}

@@ -104,7 +112,7 @@ static int record__mmap_read(struct record *rec, int idx)
 	size = head - old;
 	old += size;
 
-	if (record__write(rec, buf, size) < 0) {
+	if (record__write(rec, buf, size, idx) < 0) {
 		rc = -1;
 		goto out;
 	}

@@ -151,6 +159,7 @@ static int record__process_auxtrace(struct perf_tool *tool,
 	struct perf_data_file *file = &rec->file;
 	size_t padding;
 	u8 pad[8] = {0};
+	int idx = event->auxtrace.idx;
 
 	if (!perf_data_file__is_pipe(file)) {
 		off_t file_offset;

@@ -171,11 +180,11 @@ static int record__process_auxtrace(struct perf_tool *tool,
 	if (padding)
 		padding = 8 - padding;
 
-	record__write(rec, event, event->header.size);
-	record__write(rec, data1, len1);
+	record__write(rec, event, event->header.size, idx);
+	record__write(rec, data1, len1, idx);
 	if (len2)
-		record__write(rec, data2, len2);
-	record__write(rec, &pad, padding);
+		record__write(rec, data2, len2, idx);
+	record__write(rec, &pad, padding, idx);
 
 	return 0;
 }

@@ -268,6 +277,110 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused)
 
 #endif
 
+#define INDEX_FILE_FMT  "%s.dir/perf.data.%d"
+
+static int record__create_index_files(struct record *rec, int nr_index)
+{
+	int i = 0;
+	int ret = -1;
+	char path[PATH_MAX];
+	struct perf_data_file *file = &rec->file;
+
+	rec->fds = malloc(nr_index * sizeof(int));
+	if (rec->fds == NULL)
+		return -ENOMEM;
+
+	scnprintf(path, sizeof(path), "%s.dir", file->path);
+	if (rm_rf(path) < 0 || mkdir(path, S_IRWXU) < 0)
+		goto out_err;
+
+	for (i = 0; i < nr_index; i++) {
+		scnprintf(path, sizeof(path), INDEX_FILE_FMT, file->path, i);
+		ret = open(path, O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR);
+		if (ret < 0)
+			goto out_err;
+
+		rec->fds[i] = ret;
+	}
+	return 0;
+
+out_err:
+	while (--i >= 1)
+		close(rec->fds[i]);
+	zfree(&rec->fds);
+
+	scnprintf(path, sizeof(path), "%s.dir", file->path);
+	rm_rf(path);
+
+	return ret;
+}
+
+static int record__merge_index_files(struct record *rec, int nr_index)
+{
+	int i;
+	int ret = -ENOMEM;
+	u64 offset;
+	char path[PATH_MAX];
+	struct perf_file_section *idx;
+	struct perf_data_file *file = &rec->file;
+	struct perf_session *session = rec->session;
+	int output_fd = perf_data_file__fd(file);
+
+	/* +1 for header file itself */
+	nr_index++;
+
+	idx = calloc(nr_index, sizeof(*idx));
+	if (idx == NULL)
+		goto out_close;
+
+	offset = lseek(output_fd, 0, SEEK_END);
+
+	idx[0].offset = session->header.data_offset;
+	idx[0].size   = offset - idx[0].offset;
+
+	for (i = 1; i < nr_index; i++) {
+		struct stat stbuf;
+		int fd = rec->fds[i - 1];
+
+		ret = fstat(fd, &stbuf);
+		if (ret < 0)
+			goto out_close;
+
+		idx[i].offset = offset;
+		idx[i].size   = stbuf.st_size;
+
+		offset += stbuf.st_size;
+
+		if (idx[i].size == 0)
+			continue;
+
+		ret = copyfile_offset(fd, 0, output_fd, idx[i].offset,
+				      idx[i].size);
+		if (ret < 0)
+			goto out_close;
+	}
+
+	session->header.index = idx;
+	session->header.nr_index = nr_index;
+
+	perf_has_index = true;
+
+	ret = 0;
+
+out_close:
+	if (ret < 0)
+		pr_err("failed to merge index files: %d\n", ret);
+
+	for (i = 0; i < nr_index - 1; i++)
+		close(rec->fds[i]);
+
+	scnprintf(path, sizeof(path), "%s.dir", file->path);
+	rm_rf(path);
+
+	zfree(&rec->fds);
+	return ret;
+}
+
 static int record__open(struct record *rec)
 {
 	char msg[512];

@@ -306,7 +419,8 @@ try_again:
 
 	if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false,
 				 opts->auxtrace_mmap_pages,
-				 opts->auxtrace_snapshot_mode, false) < 0) {
+				 opts->auxtrace_snapshot_mode,
+				 opts->index) < 0) {
 		if (errno == EPERM) {
 			pr_err("Permission error mapping pages.\n"
 			       "Consider increasing "

@@ -323,6 +437,14 @@ try_again:
 		goto out;
 	}
 
+	if (opts->index) {
+		rc = record__create_index_files(rec, evlist->nr_mmaps);
+		if (rc < 0) {
+			pr_err("failed to create index file: %d\n", rc);
+			goto out;
+		}
+	}
+
 	session->evlist = evlist;
 	perf_session__set_id_hdr_size(session);
 out:

@@ -347,7 +469,9 @@ static int process_buildids(struct record *rec)
 	struct perf_data_file *file  = &rec->file;
 	struct perf_session *session = rec->session;
 
-	if (file->size == 0)
+	/* update file size after merging sample files with index */
+	u64 size = lseek(perf_data_file__fd(file), 0, SEEK_END);
+	if (size == 0)
 		return 0;
 
 	/*

@@ -414,6 +538,13 @@ static int record__mmap_read_all(struct record *rec)
 			}
 		}
 
+		if (rec->evlist->track_mmap && rec->evlist->track_mmap[i].base) {
+			if (record__mmap_read(rec, track_mmap_idx(i)) != 0) {
+				rc = -1;
+				goto out;
+			}
+		}
+
 		if (mm->base && !rec->opts.auxtrace_snapshot_mode &&
 		    record__auxtrace_mmap_read(rec, mm) != 0) {
 			rc = -1;

@@ -426,7 +557,8 @@ static int record__mmap_read_all(struct record *rec)
 	 * at least one event.
 	 */
 	if (bytes_written != rec->bytes_written)
-		rc = record__write(rec, &finished_round_event, sizeof(finished_round_event));
+		rc = record__write(rec, &finished_round_event,
+				   sizeof(finished_round_event), -1);
 
 out:
 	return rc;

@@ -452,7 +584,8 @@ static void record__init_features(struct record *rec)
 	if (!rec->opts.full_auxtrace)
 		perf_header__clear_feat(&session->header, HEADER_AUXTRACE);
 
-	perf_header__clear_feat(&session->header, HEADER_DATA_INDEX);
+	if (!rec->opts.index)
+		perf_header__clear_feat(&session->header, HEADER_DATA_INDEX);
 }
 
 static volatile int workload_exec_errno;

@@ -520,6 +653,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		}
 	}
 
+	if (file->is_pipe && opts->index) {
+		pr_warning("Indexing is disabled for pipe output\n");
+		opts->index = false;
+	}
+
 	if (record__open(rec) != 0) {
 		err = -1;
 		goto out_child;

@@ -753,6 +891,9 @@ out_child:
 		rec->session->header.data_size += rec->bytes_written;
 		file->size = lseek(perf_data_file__fd(file), 0, SEEK_CUR);
 
+		if (rec->opts.index)
+			record__merge_index_files(rec, rec->evlist->nr_mmaps);
+
 		if (!rec->no_buildid) {
 			process_buildids(rec);
 			/*

@@ -1119,6 +1260,8 @@ struct option __record_options[] = {
 			"per thread proc mmap processing timeout in ms"),
 	OPT_BOOLEAN(0, "switch-events", &record.opts.record_switch_events,
 		    "Record context switch events"),
+	OPT_BOOLEAN(0, "index", &record.opts.index,
+		    "make index for sample data to speed-up processing"),
 	OPT_END()
 };
 

@@ -1186,6 +1329,15 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 		goto out_symbol_exit;
 	}
 
+	if (rec->opts.index) {
+		if (!rec->opts.sample_time) {
+			pr_err("Sample timestamp is required for indexing\n");
+			goto out_symbol_exit;
+		}
+
+		perf_evlist__add_dummy_tracking(rec->evlist);
+	}
+
 	if (rec->opts.target.tid && !rec->opts.no_inherit_set)
 		rec->opts.no_inherit = true;
 

tools/perf/perf.h +1 -0

index f4b4d7d8752c..df7c208abb74 100644
@@ -60,6 +60,7 @@ struct record_opts {
 	bool	     full_auxtrace;
 	bool	     auxtrace_snapshot_mode;
 	bool	     record_switch_events;
+	bool	     index;
 	unsigned int freq;
 	unsigned int mmap_pages;
 	unsigned int auxtrace_mmap_pages;

tools/perf/util/header.c +2 -0

index c357f7f47d32..13ba1402ec1b 100644
@@ -2706,6 +2706,8 @@ int perf_session__read_header(struct perf_session *session)
 						   session->tevent.pevent))
 		goto out_delete_evlist;
 
+	perf_has_index = perf_header__has_feat(&session->header, HEADER_DATA_INDEX);
+
 	return 0;
 out_errno:
 	return -errno;

tools/perf/util/session.c +1 -0

index 91fa9647f565..7546c4d147b9 100644
@@ -182,6 +182,7 @@ void perf_session__delete(struct perf_session *session)
 	machines__exit(&session->machines);
 	if (session->file)
 		perf_data_file__close(session->file);
+	free(session->header.index);
 	free(session);
 }
 

Signature · 1 line

2.6.0

tools/perf/builtin-record.c +39 -0

+static int record__merge_index_files(struct record *rec, int nr_index)
+{
+	int i;
+	int ret = -ENOMEM;
+	u64 offset;
+	char path[PATH_MAX];
+	struct perf_file_section *idx;
+	struct perf_data_file *file = &rec->file;
+	struct perf_session *session = rec->session;
+	int output_fd = perf_data_file__fd(file);
+
+	/* +1 for header file itself */
+	nr_index++;
+
+	idx = calloc(nr_index, sizeof(*idx));
+	if (idx == NULL)
+		goto out_close;
+
+	offset = lseek(output_fd, 0, SEEK_END);
+
+	idx[0].offset = session->header.data_offset;
+	idx[0].size   = offset - idx[0].offset;
+
+	for (i = 1; i < nr_index; i++) {
+		struct stat stbuf;
+		int fd = rec->fds[i - 1];
+
+		ret = fstat(fd, &stbuf);
+		if (ret < 0)
+			goto out_close;
+
+		idx[i].offset = offset;
+		idx[i].size   = stbuf.st_size;
+
+		offset += stbuf.st_size;
+
+		if (idx[i].size == 0)
+			continue;
+

On Fri, Oct 02, 2015 at 02:18:50PM +0900, Namhyung Kim wrote:

SNIP

Quoted 44 lines

tools/perf/builtin-record.c +44 -0

+static int record__merge_index_files(struct record *rec, int nr_index)
+{
+     int i;
+     int ret = -ENOMEM;
+     u64 offset;
+     char path[PATH_MAX];
+     struct perf_file_section *idx;
+     struct perf_data_file *file = &rec->file;
+     struct perf_session *session = rec->session;
+     int output_fd = perf_data_file__fd(file);
+
+     /* +1 for header file itself */
+     nr_index++;
+
+     idx = calloc(nr_index, sizeof(*idx));
+     if (idx == NULL)
+             goto out_close;
+
+     offset = lseek(output_fd, 0, SEEK_END);
+
+     idx[0].offset = session->header.data_offset;
+     idx[0].size   = offset - idx[0].offset;
+
+     for (i = 1; i < nr_index; i++) {
+             struct stat stbuf;
+             int fd = rec->fds[i - 1];
+
+             ret = fstat(fd, &stbuf);
+             if (ret < 0)
+                     goto out_close;
+
+             idx[i].offset = offset;
+             idx[i].size   = stbuf.st_size;
+
+             offset += stbuf.st_size;
+
+             if (idx[i].size == 0)
+                     continue;
+
+             ret = copyfile_offset(fd, 0, output_fd, idx[i].offset,
+                                   idx[i].size);
+             if (ret < 0)
+                     goto out_close;
+     }

tools/perf/builtin-record.c int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused) · line 366

Quoted 5 lines

tools/perf/builtin-record.c +5 -0

+
+     session->header.index = idx;
+     session->header.nr_index = nr_index;
+
+     perf_has_index = true;

I might have asked earlier, but why is this global? seems like perf_session member to me..

Hi Jiri,

Quoted 48 lines · Jiri Olsa

On Fri, Oct 02, 2015 at 02:18:50PM +0900, Namhyung Kim wrote:

SNIP

Quoted 44 lines

tools/perf/builtin-record.c +44 -0

+static int record__merge_index_files(struct record *rec, int nr_index)
+{
+     int i;
+     int ret = -ENOMEM;
+     u64 offset;
+     char path[PATH_MAX];
+     struct perf_file_section *idx;
+     struct perf_data_file *file = &rec->file;
+     struct perf_session *session = rec->session;
+     int output_fd = perf_data_file__fd(file);
+
+     /* +1 for header file itself */
+     nr_index++;
+
+     idx = calloc(nr_index, sizeof(*idx));
+     if (idx == NULL)
+             goto out_close;
+
+     offset = lseek(output_fd, 0, SEEK_END);
+
+     idx[0].offset = session->header.data_offset;
+     idx[0].size   = offset - idx[0].offset;
+
+     for (i = 1; i < nr_index; i++) {
+             struct stat stbuf;
+             int fd = rec->fds[i - 1];
+
+             ret = fstat(fd, &stbuf);
+             if (ret < 0)
+                     goto out_close;
+
+             idx[i].offset = offset;
+             idx[i].size   = stbuf.st_size;
+
+             offset += stbuf.st_size;
+
+             if (idx[i].size == 0)
+                     continue;
+
+             ret = copyfile_offset(fd, 0, output_fd, idx[i].offset,
+                                   idx[i].size);
+             if (ret < 0)
+                     goto out_close;
+     }

Quoted 8 lines

tools/perf/builtin-record.c int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused) · line 366

Quoted 5 lines

tools/perf/builtin-record.c +5 -0

+
+     session->header.index = idx;
+     session->header.nr_index = nr_index;
+
+     perf_has_index = true;

I might have asked earlier, but why is this global? seems like perf_session member to me..

Yes you did. :-)

https://lkml.org/lkml/2015/5/19/110