linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 00/10] counter read during perf sampling
@ 2015-09-22 14:13 kan.liang
  2015-09-22 14:13 ` [PATCH RFC 01/10] perf,tools: Add 'C' event/group modifier kan.liang
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

The patch series intends to read counter statistics with fixed frequency
during sampling. The instant benefit is that we can read memory bandwidth
from uncore event during cpu PMU event is sampling.

Introduce 'C' event/group modifier. The event with this modifier
will do counting not sampling. If a group with this modifier, only
group leader do sampling. The counter statistics will be wrote in
new RECORD type PERF_RECORD_COUNTER_READ and stored in perf.data.
So perf report can present the counter statistics data accordingly.

There may be an alternative way to get counter statistics during
sampling by running perf record and perf stat together by script.
But the script way have various issue and complex to parses the
output.

Example:

 $perf record -e 'cycles,uncore_imc_1/cas_count_read/C'
  --counter-read-interval 10 -a ./tchain_edit
 [ perf record: Woken up 438 times to write data ]
 [ perf record: Captured and wrote 1.232 MB perf.data (17901 samples) ]


 $perf report -D

 0x3cae0 [0x28]: PERF_RECORD_COUNTER_READ: uncore_imc_1/cas_count_read/C
                CPU 0: val 1205 ena 2046148 run 2046148

 0x3cb08 [0x28]: PERF_RECORD_COUNTER_READ: uncore_imc_1/cas_count_read/C
                CPU 18: val 1315 ena 2001918 run 2001918

 0x3dba0 [0x28]: PERF_RECORD_COUNTER_READ: uncore_imc_1/cas_count_read/C
                CPU 0: val 1588 ena 12191520 run 12191520

 0x3dbc8 [0x28]: PERF_RECORD_COUNTER_READ: uncore_imc_1/cas_count_read/C
                CPU 18: val 1686 ena 12162202 run 12162202

 $perf report --stdio --socket-filter 0

 # To display the perf.data header info, please use --header/--header-only options.
 #
 #
 # Total Lost Samples: 0
 #
 # Samples: 17K of event 'cycles'
 # Event count (approx.): 10119095556
 # Processor Socket: 0
 #
 # Overhead  Command       Shared Object        Symbol
 # ........  ............  ...................  ..................................
 #
    97.68%  tchain_edit   tchain_edit          [.] f3
     0.07%  tchain_edit   tchain_edit          [.] f2
     0.04%  swapper       [kernel.vmlinux]     [k] run_timer_softirq
     0.04%  swapper       [kernel.vmlinux]     [k] find_busiest_group

 # Samples: 0  of event 'uncore_imc_1/cas_count_read/C'
 # Event count (approx.): 0
 # Processor Socket: 0
 # uncore_imc_1/cas_count_read/C: 35937


Kan Liang (10):
  perf,tools: Add 'C' event/group modifier
  perf,tools: Enable counter statistic read for perf record
  perf,tools: don't validate counter read event
  perf,tools: New RECORD type PERF_RECORD_COUNTER_READ
  perf,tools: record counter statistics during sampling
  perf,tools: option to set counter read interval
  perf,report: handle PERF_RECORD_COUNTER_READ
  perf,tools: store counter val in events_stats
  perf,tools: show counter read result in studio
  perf,tools: show counter read result in tui browser title

 tools/perf/Documentation/perf-list.txt   |   5 ++
 tools/perf/Documentation/perf-record.txt |   8 +++
 tools/perf/builtin-record.c              |  73 ++++++++++++++++++++++
 tools/perf/builtin-report.c              | 101 ++++++++++++++++++++++++++++++-
 tools/perf/ui/browsers/hists.c           |  37 +++++++++++
 tools/perf/util/event.c                  |   1 +
 tools/perf/util/event.h                  |  21 +++++++
 tools/perf/util/evlist.c                 |  20 ++++++
 tools/perf/util/evsel.c                  |  29 +++++++++
 tools/perf/util/evsel.h                  |   1 +
 tools/perf/util/parse-events.c           |   8 ++-
 tools/perf/util/parse-events.l           |   2 +-
 tools/perf/util/record.c                 |   6 +-
 tools/perf/util/session.c                |  16 +++++
 tools/perf/util/tool.h                   |   1 +
 15 files changed, 324 insertions(+), 5 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH RFC 01/10] perf,tools: Add 'C' event/group modifier
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-22 14:13 ` [PATCH RFC 02/10] perf,tools: Enable counter statistic read for perf record kan.liang
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

Add a new event/group modifier 'C' to mark the event which will be read
counter statistics during sampling.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/util/evsel.h        | 1 +
 tools/perf/util/parse-events.c | 8 +++++++-
 tools/perf/util/parse-events.l | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 7906666..d017795 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -115,6 +115,7 @@ struct perf_evsel {
 	int			exclude_GH;
 	int			nr_members;
 	int			sample_read;
+	int			counter_read;
 	unsigned long		*per_pkg_mask;
 	struct perf_evsel	*leader;
 	char			*group_name;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 61c2bc2..95b43a4 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -860,6 +860,7 @@ struct event_modifier {
 	int precise;
 	int exclude_GH;
 	int sample_read;
+	int counter_read;
 	int pinned;
 };
 
@@ -874,6 +875,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 	int eI = evsel ? evsel->attr.exclude_idle : 0;
 	int precise = evsel ? evsel->attr.precise_ip : 0;
 	int sample_read = 0;
+	int counter_read = 0;
 	int pinned = evsel ? evsel->attr.pinned : 0;
 
 	int exclude = eu | ek | eh;
@@ -911,6 +913,8 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 				eG = 1;
 		} else if (*str == 'S') {
 			sample_read = 1;
+		} else if (*str == 'C') {
+			counter_read = 1;
 		} else if (*str == 'D') {
 			pinned = 1;
 		} else
@@ -941,6 +945,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 	mod->precise = precise;
 	mod->exclude_GH = exclude_GH;
 	mod->sample_read = sample_read;
+	mod->counter_read = counter_read;
 	mod->pinned = pinned;
 
 	return 0;
@@ -955,7 +960,7 @@ static int check_modifier(char *str)
 	char *p = str;
 
 	/* The sizeof includes 0 byte as well. */
-	if (strlen(str) > (sizeof("ukhGHpppSDI") - 1))
+	if (strlen(str) > (sizeof("ukhGHpppSCDI") - 1))
 		return -1;
 
 	while (*p) {
@@ -994,6 +999,7 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add)
 		evsel->attr.exclude_idle   = mod.eI;
 		evsel->exclude_GH          = mod.exclude_GH;
 		evsel->sample_read         = mod.sample_read;
+		evsel->counter_read           = mod.counter_read;
 
 		if (perf_evsel__is_group_leader(evsel))
 			evsel->attr.pinned = mod.pinned;
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 936d566..e6c5790 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -122,7 +122,7 @@ num_raw_hex	[a-fA-F0-9]+
 name		[a-zA-Z_*?][a-zA-Z0-9_*?.]*
 name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.]*
 /* If you add a modifier you need to update check_modifier() */
-modifier_event	[ukhpGHSDI]+
+modifier_event	[ukhpGHSCDI]+
 modifier_bp	[rwx]{1,3}
 
 %%
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 02/10] perf,tools: Enable counter statistic read for perf record
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
  2015-09-22 14:13 ` [PATCH RFC 01/10] perf,tools: Add 'C' event/group modifier kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-22 14:13 ` [PATCH RFC 03/10] perf,tools: don't validate counter read event kan.liang
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

Using 'C' event/group modifier to specify the event which want to read
counter statistics during sampling. For this event, the sampling will
be disabled.
The 'C' modifier only be available on system-wide/CPU mode. If a
group is marked as 'C' modifier, only group members read counter
statistics. Group leader always do sampling.
The other limit is that the first event cannot be counter statistics
read event, since many tools special handle first event.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/Documentation/perf-list.txt |  5 +++++
 tools/perf/util/evlist.c               |  3 +++
 tools/perf/util/evsel.c                | 29 +++++++++++++++++++++++++++++
 tools/perf/util/record.c               |  6 ++++--
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index bada893..a409fc9 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -31,6 +31,11 @@ counted. The following modifiers exist:
  H - host counting (not in KVM guests)
  p - precise level
  S - read sample value (PERF_SAMPLE_READ)
+ C - read counter statistics during sampling (Can be used to read
+     counter statistics of PMU_A event when PMU_B events are sampling.
+     For example, getting memory bandwidth by uncore events during the
+     CPU PMU events run time)
+     (Only available for group members or non-first event in system-wide/CPU mode)
  D - pin the event to the PMU
 
 The 'p' modifier can be used for specifying how precise the instruction
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a864373..603ee3e 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -885,6 +885,9 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
 		if (evsel->system_wide && thread)
 			continue;
 
+		if (evsel->counter_read)
+			continue;
+
 		fd = FD(evsel, cpu, thread);
 
 		if (*output == -1) {
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 5889004..8c18422 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -724,6 +724,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	struct perf_event_attr *attr = &evsel->attr;
 	int track = evsel->tracking;
 	bool per_cpu = opts->target.default_per_cpu && !opts->target.per_thread;
+	struct perf_evsel *first = perf_evlist__first(evsel->evlist);
 
 	attr->sample_id_all = perf_missing_features.sample_id_all ? 0 : 1;
 	attr->inherit	    = !opts->no_inherit;
@@ -882,6 +883,34 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 		attr->clockid = opts->clockid;
 	}
 
+	if (evsel->counter_read) {
+		if (!target__has_cpu(&opts->target)) {
+			evsel->counter_read = 0;
+			ui__warning("Counter statistics read only available "
+				    "on system-wide/CPU mode.\n"
+				    "Remove :C modifier for event %s\n",
+				    evsel->name);
+		} else {
+			/* Don't do counter read for Group leader */
+			if ((evsel->leader == evsel) && (evsel->leader->nr_members > 1)) {
+				evsel->counter_read = 0;
+			} else {
+				if (first == evsel) {
+					evsel->counter_read = 0;
+					ui__warning("The first event cannot be counter read event"
+						    "Remove :C modifier for event %s\n",
+						    evsel->name);
+				} else {
+					attr->read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
+							    PERF_FORMAT_TOTAL_TIME_RUNNING;
+					attr->sample_freq = 0;
+					attr->sample_period = 0;
+					attr->sample_type = 0;
+					evsel->sample_size = 0;
+				}
+			}
+		}
+	}
 	/*
 	 * Apply event specific term settings,
 	 * it overloads any global configuration.
diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
index 0467367..9ede1f7 100644
--- a/tools/perf/util/record.c
+++ b/tools/perf/util/record.c
@@ -171,8 +171,10 @@ void perf_evlist__config(struct perf_evlist *evlist, struct record_opts *opts)
 			use_sample_identifier = perf_can_sample_identifier();
 			break;
 		}
-		evlist__for_each(evlist, evsel)
-			perf_evsel__set_sample_id(evsel, use_sample_identifier);
+		evlist__for_each(evlist, evsel) {
+			if (!evsel->counter_read)
+				perf_evsel__set_sample_id(evsel, use_sample_identifier);
+		}
 	}
 
 	perf_evlist__set_id_pos(evlist);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 03/10] perf,tools: don't validate counter read event
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
  2015-09-22 14:13 ` [PATCH RFC 01/10] perf,tools: Add 'C' event/group modifier kan.liang
  2015-09-22 14:13 ` [PATCH RFC 02/10] perf,tools: Enable counter statistic read for perf record kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-22 14:13 ` [PATCH RFC 04/10] perf,tools: New RECORD type PERF_RECORD_COUNTER_READ kan.liang
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

Counter read event (:C) doesn't do sampling. So perf tool should not
validate its sample_type, read_format and sample_id_all.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/util/evlist.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 603ee3e..af6215f 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1258,6 +1258,17 @@ int perf_evlist__set_filter_pid(struct perf_evlist *evlist, pid_t pid)
 	return perf_evlist__set_filter_pids(evlist, 1, &pid);
 }
 
+static inline bool is_counter_read_event(struct perf_event_attr *attr)
+{
+	if ((attr->read_format & PERF_FORMAT_TOTAL_TIME_ENABLED) &&
+	    (attr->read_format & PERF_FORMAT_TOTAL_TIME_RUNNING) &&
+	    !attr->sample_freq && !attr->sample_period &&
+	    !attr->sample_type)
+		return true;
+
+	return false;
+}
+
 bool perf_evlist__valid_sample_type(struct perf_evlist *evlist)
 {
 	struct perf_evsel *pos;
@@ -1269,6 +1280,8 @@ bool perf_evlist__valid_sample_type(struct perf_evlist *evlist)
 		return false;
 
 	evlist__for_each(evlist, pos) {
+		if (is_counter_read_event(&pos->attr))
+			continue;
 		if (pos->id_pos != evlist->id_pos ||
 		    pos->is_pos != evlist->is_pos)
 			return false;
@@ -1313,6 +1326,8 @@ bool perf_evlist__valid_read_format(struct perf_evlist *evlist)
 	u64 sample_type = first->attr.sample_type;
 
 	evlist__for_each(evlist, pos) {
+		if (is_counter_read_event(&pos->attr))
+			continue;
 		if (read_format != pos->attr.read_format)
 			return false;
 	}
@@ -1370,6 +1385,8 @@ bool perf_evlist__valid_sample_id_all(struct perf_evlist *evlist)
 	struct perf_evsel *first = perf_evlist__first(evlist), *pos = first;
 
 	evlist__for_each_continue(evlist, pos) {
+		if (is_counter_read_event(&pos->attr))
+			continue;
 		if (first->attr.sample_id_all != pos->attr.sample_id_all)
 			return false;
 	}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 04/10] perf,tools: New RECORD type PERF_RECORD_COUNTER_READ
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
                   ` (2 preceding siblings ...)
  2015-09-22 14:13 ` [PATCH RFC 03/10] perf,tools: don't validate counter read event kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-22 14:13 ` [PATCH RFC 05/10] perf,tools: record counter statistics during sampling kan.liang
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

Introduce a new user RECORD type PERF_RECORD_COUNTER_READ to store the
counter statistics result.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/util/event.c   |  1 +
 tools/perf/util/event.h   | 18 ++++++++++++++++++
 tools/perf/util/session.c | 16 ++++++++++++++++
 tools/perf/util/tool.h    |  1 +
 4 files changed, 36 insertions(+)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 497157a..608c83a 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -37,6 +37,7 @@ static const char *perf_event__names[] = {
 	[PERF_RECORD_AUXTRACE_INFO]		= "AUXTRACE_INFO",
 	[PERF_RECORD_AUXTRACE]			= "AUXTRACE",
 	[PERF_RECORD_AUXTRACE_ERROR]		= "AUXTRACE_ERROR",
+	[PERF_RECORD_COUNTER_READ]		= "COUNTER_READ",
 };
 
 const char *perf_event__name(unsigned int id)
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index f729df5..7f125ce 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -226,6 +226,7 @@ enum perf_user_event_type { /* above any possible kernel type */
 	PERF_RECORD_AUXTRACE_INFO		= 70,
 	PERF_RECORD_AUXTRACE			= 71,
 	PERF_RECORD_AUXTRACE_ERROR		= 72,
+	PERF_RECORD_COUNTER_READ		= 73,
 	PERF_RECORD_HEADER_MAX
 };
 
@@ -355,6 +356,22 @@ struct context_switch_event {
 	u32 next_prev_tid;
 };
 
+struct counter_read_event {
+	struct perf_event_header header;
+
+	int	idx;
+	u32	cpu;
+
+	union {
+		struct {
+			u64 val;
+			u64 ena;
+			u64 run;
+		};
+		u64 values[3];
+	};
+};
+
 union perf_event {
 	struct perf_event_header	header;
 	struct mmap_event		mmap;
@@ -377,6 +394,7 @@ union perf_event {
 	struct aux_event		aux;
 	struct itrace_start_event	itrace_start;
 	struct context_switch_event	context_switch;
+	struct counter_read_event	counter_read;
 };
 
 void perf_event__print_totals(void);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index d1a43a3..4d5c644 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -322,6 +322,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
 		tool->context_switch = perf_event__process_switch;
 	if (tool->read == NULL)
 		tool->read = process_event_sample_stub;
+	if (tool->counter_read == NULL)
+		tool->counter_read = process_build_id_stub;
 	if (tool->throttle == NULL)
 		tool->throttle = process_event_stub;
 	if (tool->unthrottle == NULL)
@@ -425,6 +427,17 @@ static void perf_event__task_swap(union perf_event *event, bool sample_id_all)
 		swap_sample_id_all(event, &event->fork + 1);
 }
 
+static void perf_event__counter_read_swap(union perf_event *event,
+				       bool sample_id_all __maybe_unused)
+{
+	event->counter_read.idx = bswap_32(event->counter_read.idx);
+	event->counter_read.cpu = bswap_32(event->counter_read.cpu);
+
+	event->counter_read.val = bswap_64(event->counter_read.val);
+	event->counter_read.ena = bswap_64(event->counter_read.ena);
+	event->counter_read.run = bswap_64(event->counter_read.run);
+}
+
 static void perf_event__read_swap(union perf_event *event, bool sample_id_all)
 {
 	event->read.pid		 = bswap_32(event->read.pid);
@@ -627,6 +640,7 @@ static perf_event__swap_op perf_event__swap_ops[] = {
 	[PERF_RECORD_EXIT]		  = perf_event__task_swap,
 	[PERF_RECORD_LOST]		  = perf_event__all64_swap,
 	[PERF_RECORD_READ]		  = perf_event__read_swap,
+	[PERF_RECORD_COUNTER_READ]	  = perf_event__counter_read_swap,
 	[PERF_RECORD_THROTTLE]		  = perf_event__throttle_swap,
 	[PERF_RECORD_UNTHROTTLE]	  = perf_event__throttle_swap,
 	[PERF_RECORD_SAMPLE]		  = perf_event__all64_swap,
@@ -1176,6 +1190,8 @@ static s64 perf_session__process_user_event(struct perf_session *session,
 	case PERF_RECORD_AUXTRACE_ERROR:
 		perf_session__auxtrace_error_inc(session, event);
 		return tool->auxtrace_error(tool, event, session);
+	case PERF_RECORD_COUNTER_READ:
+		return tool->counter_read(tool, event, session);
 	default:
 		return -EINVAL;
 	}
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index cab8cc2..89477b2 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -57,6 +57,7 @@ struct perf_tool {
 			auxtrace_info,
 			auxtrace_error;
 	event_op3	auxtrace;
+	event_op2	counter_read;
 	bool		ordered_events;
 	bool		ordering_requires_timestamps;
 };
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 05/10] perf,tools: record counter statistics during sampling
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
                   ` (3 preceding siblings ...)
  2015-09-22 14:13 ` [PATCH RFC 04/10] perf,tools: New RECORD type PERF_RECORD_COUNTER_READ kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-22 14:13 ` [PATCH RFC 06/10] perf,tools: option to set counter read interval kan.liang
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

Reading at the start and the end of sampling, and save the counter
statistics value to PERF_RECORD_COUNTER_READ.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/builtin-record.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 142eeb3..cc8fd08 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -398,6 +398,40 @@ static struct perf_event_header finished_round_event = {
 	.type = PERF_RECORD_FINISHED_ROUND,
 };
 
+static void process_counter_read_event(struct record *rec, int id,
+				    struct perf_evsel *pos,
+				    struct perf_counts_values *count)
+{
+	union perf_event ev;
+
+	memset(&ev, 0, sizeof(ev));
+	ev.counter_read.header.type = PERF_RECORD_COUNTER_READ;
+	ev.counter_read.header.size = sizeof(ev.counter_read);
+	ev.counter_read.cpu = pos->cpus->map[id];
+	ev.counter_read.idx = pos->idx;
+	memcpy(ev.counter_read.values, count, 3 * sizeof(u64));
+
+	record__write(rec, &ev, sizeof(ev.counter_read));
+}
+
+static void perf_read_counter(struct record *rec)
+{
+	struct perf_evlist *evlist = rec->evlist;
+	struct perf_counts_values count;
+	struct perf_evsel *pos;
+	int cpu;
+
+	evlist__for_each(evlist, pos) {
+		if (!pos->counter_read)
+			continue;
+
+		for (cpu = 0; cpu < cpu_map__nr(pos->cpus); cpu++) {
+			if (!perf_evsel__read(pos, cpu, 0, &count))
+				process_counter_read_event(rec, cpu, pos, &count);
+		}
+	}
+}
+
 static int record__mmap_read_all(struct record *rec)
 {
 	u64 bytes_written = rec->bytes_written;
@@ -644,6 +678,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		perf_evlist__enable(rec->evlist);
 	}
 
+	perf_read_counter(rec);
 	auxtrace_snapshot_enabled = 1;
 	for (;;) {
 		int hits = rec->samples;
@@ -688,6 +723,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		 */
 		if (done && !disabled && !target__none(&opts->target)) {
 			auxtrace_snapshot_enabled = 0;
+			perf_read_counter(rec);
 			perf_evlist__disable(rec->evlist);
 			disabled = true;
 		}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 06/10] perf,tools: option to set counter read interval
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
                   ` (4 preceding siblings ...)
  2015-09-22 14:13 ` [PATCH RFC 05/10] perf,tools: record counter statistics during sampling kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-23 18:55   ` Sukadev Bhattiprolu
  2015-09-22 14:13 ` [PATCH RFC 07/10] perf,report: handle PERF_RECORD_COUNTER_READ kan.liang
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

Add a timer to read counter regularly. Option --counter-read-interval
can be used to set the interval.
Only read counter statistics at the beginning and the end is not enough.
Sometimes, we need fine granularity to do sophisticated analysis. For
example,10-20ms is required to do sophisticated bandwidth analysis.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/Documentation/perf-record.txt |  8 +++++++
 tools/perf/builtin-record.c              | 37 ++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 2e9ce77..c1d9024 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -308,6 +308,14 @@ 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.
 
+--counter-read-interval::
+Sets the interval to do counter read regularly. This option is only valid
+with counter read event (:C). This option is disabled by default. It means
+that the event counter can only be read at the beginning and the end.
+This option could be used when we need fine granularity to do sophisticated
+analysis. For example, 10-20ms is required to do sophisticated memory
+bandwidth analysis.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cc8fd08..cdc9d3b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -505,6 +505,14 @@ static void workload_exec_failed_signal(int signo __maybe_unused,
 
 static void snapshot_sig_handler(int sig);
 
+static unsigned int interval;
+struct record *g_rec;
+
+static void perf_read_alarm(int sig __maybe_unused)
+{
+	perf_read_counter(g_rec);
+}
+
 static int __cmd_record(struct record *rec, int argc, const char **argv)
 {
 	int err;
@@ -517,6 +525,9 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	struct perf_data_file *file = &rec->file;
 	struct perf_session *session;
 	bool disabled = false, draining = false;
+	static timer_t timerid;
+	struct itimerspec timeout;
+	struct sigevent sigevent;
 	int fd;
 
 	rec->progname = argv[0];
@@ -530,6 +541,25 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	else
 		signal(SIGUSR2, SIG_IGN);
 
+	if (interval) {
+		if (interval < 10) {
+			pr_err("Regular interval for counter read must >= 10 ms.\n");
+			return -1;
+		}
+		signal(SIGALRM, perf_read_alarm);
+		g_rec = rec;
+
+		/* Create a timer. */
+		sigevent.sigev_notify = SIGEV_SIGNAL;
+		sigevent.sigev_signo = SIGALRM;
+		timer_create(CLOCK_REALTIME, &sigevent, &timerid);
+		memset(&timeout, 0, sizeof(timeout));
+		timeout.it_value.tv_sec = interval/1000;
+		timeout.it_interval.tv_sec = interval/1000;
+		timeout.it_value.tv_nsec = (interval % 1000) * 1000000ULL;
+		timeout.it_interval.tv_nsec = (interval % 1000) * 1000000ULL;
+	}
+
 	session = perf_session__new(file, false, tool);
 	if (session == NULL) {
 		pr_err("Perf session creation failed.\n");
@@ -679,6 +709,9 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	}
 
 	perf_read_counter(rec);
+	if (interval)
+		timer_settime(timerid, 0, &timeout, NULL);
+
 	auxtrace_snapshot_enabled = 1;
 	for (;;) {
 		int hits = rec->samples;
@@ -723,6 +756,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		 */
 		if (done && !disabled && !target__none(&opts->target)) {
 			auxtrace_snapshot_enabled = 0;
+			if (interval)
+				timer_delete(timerid);
 			perf_read_counter(rec);
 			perf_evlist__disable(rec->evlist);
 			disabled = true;
@@ -1132,6 +1167,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_UINTEGER(0, "counter-read-interval", &interval,
+			"Read event counter statistics at regular interval in ms (>= 10)"),
 	OPT_END()
 };
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 07/10] perf,report: handle PERF_RECORD_COUNTER_READ
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
                   ` (5 preceding siblings ...)
  2015-09-22 14:13 ` [PATCH RFC 06/10] perf,tools: option to set counter read interval kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-22 14:13 ` [PATCH RFC 08/10] perf,tools: store counter val in events_stats kan.liang
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

Add function to process PERF_RECORD_COUNTER_READ RECORD type event.
Simply print event name, cpu id, val and time information in
perf report -D.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/builtin-report.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index e4e3f14..1d8dce2 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -207,6 +207,29 @@ static int process_read_event(struct perf_tool *tool,
 	return 0;
 }
 
+static int process_counter_read_event(struct perf_tool *tool __maybe_unused,
+				      union perf_event *event,
+				      struct perf_session *session)
+{
+	struct perf_evlist *evlist = session->evlist;
+	struct perf_evsel *evsel;
+
+	evlist__for_each(evlist, evsel) {
+		if (evsel->idx == event->counter_read.idx)
+			break;
+	}
+
+	hists__inc_nr_events(evsel__hists(evsel), event->header.type);
+
+	dump_printf(": %s CPU %d: val %" PRIu64 " ena %" PRIu64 " run %" PRIu64 " \n",
+		    evsel ? perf_evsel__name(evsel) : "FAIL",
+		    event->counter_read.cpu,
+		    event->counter_read.val,
+		    event->counter_read.ena,
+		    event->counter_read.run);
+	return 0;
+}
+
 /* For pipe mode, sample_type is not currently set */
 static int report__setup_sample_type(struct report *rep)
 {
@@ -632,6 +655,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 			.fork		 = perf_event__process_fork,
 			.lost		 = perf_event__process_lost,
 			.read		 = process_read_event,
+			.counter_read	 = process_counter_read_event,
 			.attr		 = perf_event__process_attr,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 08/10] perf,tools: store counter val in events_stats
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
                   ` (6 preceding siblings ...)
  2015-09-22 14:13 ` [PATCH RFC 07/10] perf,report: handle PERF_RECORD_COUNTER_READ kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-22 14:13 ` [PATCH RFC 09/10] perf,tools: show counter read result in studio kan.liang
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

Add a new item counter_val in events_stats to store the total counter
statistics value on each CPU.
The absolate value from counter will be stored in prev_raw_counts. Then
the detla value will be caculated and added into counter_val.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/builtin-report.c | 40 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/event.h     |  3 +++
 2 files changed, 43 insertions(+)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 1d8dce2..31818bc 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -38,6 +38,8 @@
 
 #include "util/auxtrace.h"
 
+#include "util/stat.h"
+
 #include <dlfcn.h>
 #include <linux/bitmap.h>
 
@@ -213,12 +215,33 @@ static int process_counter_read_event(struct perf_tool *tool __maybe_unused,
 {
 	struct perf_evlist *evlist = session->evlist;
 	struct perf_evsel *evsel;
+	int cpu_nr = session->header.env.nr_cpus_online;
+	u32 cpu = event->counter_read.cpu;
+	struct hists *evsel_hists;
 
 	evlist__for_each(evlist, evsel) {
 		if (evsel->idx == event->counter_read.idx)
 			break;
 	}
 
+	if (evsel->prev_raw_counts == NULL) {
+		if (perf_evsel__alloc_prev_raw_counts(evsel, cpu_nr, 1))
+			return -ENOMEM;
+	}
+
+	evsel_hists = evsel__hists(evsel);
+	if (evsel_hists->stats.counter_val == NULL) {
+		evsel_hists->stats.counter_val = zalloc(cpu_nr * sizeof(u64));
+		if (evsel_hists->stats.counter_val == NULL) {
+			perf_evsel__free_prev_raw_counts(evsel);
+			return -ENOMEM;
+		}
+	}
+	evsel_hists->stats.counter_val[cpu] +=
+		event->counter_read.val - perf_counts(evsel->prev_raw_counts, cpu, 0)->val;
+
+	perf_counts(evsel->prev_raw_counts, cpu, 0)->val = event->counter_read.val;
+
 	hists__inc_nr_events(evsel__hists(evsel), event->header.type);
 
 	dump_printf(": %s CPU %d: val %" PRIu64 " ena %" PRIu64 " run %" PRIu64 " \n",
@@ -632,6 +655,21 @@ parse_percent_limit(const struct option *opt, const char *str,
 	return 0;
 }
 
+static void
+free_counter_read(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel;
+	struct hists *evsel_hists;
+
+	evlist__for_each(evlist, evsel) {
+		evsel_hists = evsel__hists(evsel);
+		if (evsel_hists->stats.counter_val != NULL)
+			zfree(&evsel_hists->stats.counter_val);
+		if (evsel->prev_raw_counts != NULL)
+			perf_evsel__free_prev_raw_counts(evsel);
+	}
+}
+
 int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	struct perf_session *session;
@@ -946,12 +984,14 @@ repeat:
 
 	ret = __cmd_report(&report);
 	if (ret == K_SWITCH_INPUT_DATA) {
+		free_counter_read(session->evlist);
 		perf_session__delete(session);
 		goto repeat;
 	} else
 		ret = 0;
 
 error:
+	free_counter_read(session->evlist);
 	perf_session__delete(session);
 	return ret;
 }
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 7f125ce..c8a663a 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -252,6 +252,8 @@ enum auxtrace_error_type {
  * multipling nr_events[PERF_EVENT_SAMPLE] by a frequency isn't possible to get
  * the total number of low level events, it is necessary to to sum all struct
  * sample_event.period and stash the result in total_period.
+ *
+ * counter_val is used to store the counter statistics value on each CPU
  */
 struct events_stats {
 	u64 total_period;
@@ -268,6 +270,7 @@ struct events_stats {
 	u32 nr_unprocessable_samples;
 	u32 nr_auxtrace_errors[PERF_AUXTRACE_ERROR_MAX];
 	u32 nr_proc_map_timeout;
+	u64 *counter_val;
 };
 
 struct attr_event {
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 09/10] perf,tools: show counter read result in studio
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
                   ` (7 preceding siblings ...)
  2015-09-22 14:13 ` [PATCH RFC 08/10] perf,tools: store counter val in events_stats kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-22 14:13 ` [PATCH RFC 10/10] perf,tools: show counter read result in tui browser title kan.liang
  2015-09-24  8:19 ` [PATCH RFC 00/10] counter read during perf sampling Jiri Olsa
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

There is no sampling for counter read event, so only show its value.

Here is an example,

 $perf record -e '{cycles,instructions}:C' --counter-read-interval 10 -a
 sleep 5
 [ perf record: Woken up 500 times to write data ]
 [ perf record: Captured and wrote 1.760 MB perf.data (5474 samples) ]

 $perf report --group --stdio

 # To display the perf.data header info, please use
 --header/--header-only options.
 #
 #
 # Total Lost Samples: 0
 #
 # Samples: 5K of event 'anon group { cycles, instructions }'
 # Event count (approx.): 2632480450
 # instructions: 823008971
 #
 #         Overhead  Command       Shared Object       Symbol
 # ................  ............  ..................
 ...........................................
 #
     69.02%   0.00%  perf          [kernel.vmlinux]    [k]
 smp_call_function_single
     11.66%   0.00%  swapper       [kernel.vmlinux]    [k]
 acpi_idle_do_entry

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/builtin-report.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 31818bc..33fda4b 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -334,12 +334,22 @@ static size_t hists__fprintf_nr_sample_events(struct hists *hists, struct report
 	char buf[512];
 	size_t size = sizeof(buf);
 	int socked_id = hists->socket_filter;
+	int i, idx = 1;
+	struct perf_env *env = evsel->evlist->env;
+	u64 *counter_val = zalloc(evsel->nr_members * sizeof(u64));
 
 	if (symbol_conf.filter_relative) {
 		nr_samples = hists->stats.nr_non_filtered_samples;
 		nr_events = hists->stats.total_non_filtered_period;
 	}
 
+	if (hists->stats.counter_val != NULL) {
+		for (i = 0; i < env->nr_cpus_online; i++) {
+			if ((socked_id < 0) || (env->cpu[i].socket_id == socked_id))
+				counter_val[0] += hists->stats.counter_val[i];
+		}
+	}
+
 	if (perf_evsel__is_group_event(evsel)) {
 		struct perf_evsel *pos;
 
@@ -356,6 +366,14 @@ static size_t hists__fprintf_nr_sample_events(struct hists *hists, struct report
 				nr_samples += pos_hists->stats.nr_events[PERF_RECORD_SAMPLE];
 				nr_events += pos_hists->stats.total_period;
 			}
+
+			if (pos_hists->stats.counter_val != NULL) {
+				for (i = 0; i < env->nr_cpus_online; i++) {
+					if ((socked_id < 0) || (env->cpu[i].socket_id == socked_id))
+						counter_val[idx] += pos_hists->stats.counter_val[i];
+				}
+			}
+			idx++;
 		}
 	}
 
@@ -378,6 +396,22 @@ static size_t hists__fprintf_nr_sample_events(struct hists *hists, struct report
 	if (socked_id > -1)
 		ret += fprintf(fp, "\n# Processor Socket: %d", socked_id);
 
+	if (hists->stats.counter_val != NULL)
+		ret += fprintf(fp, "\n# %s: %" PRIu64, evsel->name, counter_val[0]);
+
+	if (perf_evsel__is_group_event(evsel)) {
+		struct perf_evsel *pos;
+
+		idx = 1;
+		for_each_group_member(pos, evsel) {
+			const struct hists *pos_hists = evsel__hists(pos);
+
+			if (pos_hists->stats.counter_val != NULL)
+				ret += fprintf(fp, "\n# %s: %" PRIu64, pos->name, counter_val[idx]);
+			idx++;
+		}
+	}
+	free(counter_val);
 	return ret + fprintf(fp, "\n#\n");
 }
 
@@ -397,7 +431,8 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 			continue;
 
 		hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
-		hists__fprintf(hists, true, 0, 0, rep->min_percent, stdout);
+		if (pos->prev_raw_counts == NULL)
+			hists__fprintf(hists, true, 0, 0, rep->min_percent, stdout);
 		fprintf(stdout, "\n\n");
 	}
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH RFC 10/10] perf,tools: show counter read result in tui browser title
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
                   ` (8 preceding siblings ...)
  2015-09-22 14:13 ` [PATCH RFC 09/10] perf,tools: show counter read result in studio kan.liang
@ 2015-09-22 14:13 ` kan.liang
  2015-09-24  8:19 ` [PATCH RFC 00/10] counter read during perf sampling Jiri Olsa
  10 siblings, 0 replies; 19+ messages in thread
From: kan.liang @ 2015-09-22 14:13 UTC (permalink / raw)
  To: acme; +Cc: jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel, Kan Liang

From: Kan Liang <kan.liang@intel.com>

There is no sampling for counter read event, so only show its value on
the browser title in tui mode.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/ui/browsers/hists.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index e1f28f4..1900363 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1270,12 +1270,22 @@ static int hists__browser_title(struct hists *hists,
 	size_t buflen = sizeof(buf);
 	char ref[30] = " show reference callgraph, ";
 	bool enable_ref = false;
+	int i, idx = 1;
+	struct perf_env *env = evsel->evlist->env;
+	u64 *counter_val = zalloc(evsel->nr_members * sizeof(u64));
 
 	if (symbol_conf.filter_relative) {
 		nr_samples = hists->stats.nr_non_filtered_samples;
 		nr_events = hists->stats.total_non_filtered_period;
 	}
 
+	if (hists->stats.counter_val != NULL) {
+		for (i = 0; i < env->nr_cpus_online; i++) {
+			if ((socket_id < 0) || (env->cpu[i].socket_id == socket_id))
+				counter_val[0] += hists->stats.counter_val[i];
+		}
+	}
+
 	if (perf_evsel__is_group_event(evsel)) {
 		struct perf_evsel *pos;
 
@@ -1292,6 +1302,13 @@ static int hists__browser_title(struct hists *hists,
 				nr_samples += pos_hists->stats.nr_events[PERF_RECORD_SAMPLE];
 				nr_events += pos_hists->stats.total_period;
 			}
+			if (pos_hists->stats.counter_val != NULL) {
+				for (i = 0; i < env->nr_cpus_online; i++) {
+					if ((socket_id < 0) || (env->cpu[i].socket_id == socket_id))
+						counter_val[idx] += pos_hists->stats.counter_val[i];
+				}
+			}
+			idx++;
 		}
 	}
 
@@ -1318,6 +1335,26 @@ static int hists__browser_title(struct hists *hists,
 	if (socket_id > -1)
 		printed += scnprintf(bf + printed, size - printed,
 				    ", Processor Socket: %d", socket_id);
+
+	if (hists->stats.counter_val != NULL)
+		printed += scnprintf(bf + printed, size - printed,
+				    ", %s: %" PRIu64, evsel->name, counter_val[0]);
+
+	if (perf_evsel__is_group_event(evsel)) {
+		struct perf_evsel *pos;
+
+		idx = 1;
+		for_each_group_member(pos, evsel) {
+			const struct hists *pos_hists = evsel__hists(pos);
+
+			if (pos_hists->stats.counter_val != NULL)
+				printed += scnprintf(bf + printed, size - printed,
+						     ", %s: %" PRIu64, pos->name, counter_val[idx]);
+			idx++;
+		}
+	}
+	free(counter_val);
+
 	if (!is_report_browser(hbt)) {
 		struct perf_top *top = hbt->arg;
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 06/10] perf,tools: option to set counter read interval
  2015-09-22 14:13 ` [PATCH RFC 06/10] perf,tools: option to set counter read interval kan.liang
@ 2015-09-23 18:55   ` Sukadev Bhattiprolu
  2015-09-23 19:07     ` Liang, Kan
  0 siblings, 1 reply; 19+ messages in thread
From: Sukadev Bhattiprolu @ 2015-09-23 18:55 UTC (permalink / raw)
  To: kan.liang; +Cc: acme, jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel

kan.liang@intel.com [kan.liang@intel.com] wrote:
| From: Kan Liang <kan.liang@intel.com>
| 
| Add a timer to read counter regularly. Option --counter-read-interval
| can be used to set the interval.
| Only read counter statistics at the beginning and the end is not enough.
| Sometimes, we need fine granularity to do sophisticated analysis. For
| example,10-20ms is required to do sophisticated bandwidth analysis.
| 
| Signed-off-by: Kan Liang <kan.liang@intel.com>
| ---
|  tools/perf/Documentation/perf-record.txt |  8 +++++++
|  tools/perf/builtin-record.c              | 37 ++++++++++++++++++++++++++++++++
|  2 files changed, 45 insertions(+)
| 
| diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
| index 2e9ce77..c1d9024 100644
| --- a/tools/perf/Documentation/perf-record.txt
| +++ b/tools/perf/Documentation/perf-record.txt
| @@ -308,6 +308,14 @@ 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.
| 
| +--counter-read-interval::
| +Sets the interval to do counter read regularly. This option is only valid

How about "Sets the interval at which the counter should be read"?

| +with counter read event (:C). This option is disabled by default. It means
| +that the event counter can only be read at the beginning and the end.

Should this last sentence be: "When this option is disabled, the event
counter can only be read at the beginning and the end?"

| +This option could be used when we need fine granularity to do sophisticated
| +analysis. For example, 10-20ms is required to do sophisticated memory
| +bandwidth analysis.

I guess this is independent of the sampling frequency (perf-record -F)? 

IOW, is the need for --counter-read-interval so we can sample at one
frequency, but read the counter values/statistics at a different frequency?

Sukadev


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH RFC 06/10] perf,tools: option to set counter read interval
  2015-09-23 18:55   ` Sukadev Bhattiprolu
@ 2015-09-23 19:07     ` Liang, Kan
  0 siblings, 0 replies; 19+ messages in thread
From: Liang, Kan @ 2015-09-23 19:07 UTC (permalink / raw)
  To: Sukadev Bhattiprolu
  Cc: acme, jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel

> 
> kan.liang@intel.com [kan.liang@intel.com] wrote:
> | From: Kan Liang <kan.liang@intel.com>
> |
> | Add a timer to read counter regularly. Option --counter-read-interval
> | can be used to set the interval.
> | Only read counter statistics at the beginning and the end is not enough.
> | Sometimes, we need fine granularity to do sophisticated analysis. For
> | example,10-20ms is required to do sophisticated bandwidth analysis.
> |
> | Signed-off-by: Kan Liang <kan.liang@intel.com>
> | ---
> |  tools/perf/Documentation/perf-record.txt |  8 +++++++
> |  tools/perf/builtin-record.c              | 37
> ++++++++++++++++++++++++++++++++
> |  2 files changed, 45 insertions(+)
> |
> | diff --git a/tools/perf/Documentation/perf-record.txt
> | b/tools/perf/Documentation/perf-record.txt
> | index 2e9ce77..c1d9024 100644
> | --- a/tools/perf/Documentation/perf-record.txt
> | +++ b/tools/perf/Documentation/perf-record.txt
> | @@ -308,6 +308,14 @@ 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.
> |
> | +--counter-read-interval::
> | +Sets the interval to do counter read regularly. This option is only
> | +valid
> 
> How about "Sets the interval at which the counter should be read"?
> 
> | +with counter read event (:C). This option is disabled by default. It
> | +means that the event counter can only be read at the beginning and the
> end.
> 
> Should this last sentence be: "When this option is disabled, the event
> counter can only be read at the beginning and the end?"
> 
> | +This option could be used when we need fine granularity to do
> | +sophisticated analysis. For example, 10-20ms is required to do
> | +sophisticated memory bandwidth analysis.
> 
> I guess this is independent of the sampling frequency (perf-record -F)?
> 
> IOW, is the need for --counter-read-interval so we can sample at one
> frequency, but read the counter values/statistics at a different frequency?

Right, --counter-read-interval is independent of sampling freq.
If we set --counter-read-interval to 10ms, it will read counter every 10ms
no matter what sampling frequency is set.

Thanks,
Kan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 00/10] counter read during perf sampling
  2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
                   ` (9 preceding siblings ...)
  2015-09-22 14:13 ` [PATCH RFC 10/10] perf,tools: show counter read result in tui browser title kan.liang
@ 2015-09-24  8:19 ` Jiri Olsa
  2015-09-24 19:47   ` Liang, Kan
  10 siblings, 1 reply; 19+ messages in thread
From: Jiri Olsa @ 2015-09-24  8:19 UTC (permalink / raw)
  To: kan.liang; +Cc: acme, jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel

On Tue, Sep 22, 2015 at 10:13:33AM -0400, kan.liang@intel.com wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> The patch series intends to read counter statistics with fixed frequency
> during sampling. The instant benefit is that we can read memory bandwidth
> from uncore event during cpu PMU event is sampling.
> 
> Introduce 'C' event/group modifier. The event with this modifier
> will do counting not sampling. If a group with this modifier, only
> group leader do sampling. The counter statistics will be wrote in
> new RECORD type PERF_RECORD_COUNTER_READ and stored in perf.data.
> So perf report can present the counter statistics data accordingly.
> 
> There may be an alternative way to get counter statistics during
> sampling by running perf record and perf stat together by script.
> But the script way have various issue and complex to parses the
> output.

just a thought, but isn't the way then llow to store the data from perf stat? ;-)
and be able to merge perf.data-s from perf record and stat afterwards

> 
> Example:
> 
>  $perf record -e 'cycles,uncore_imc_1/cas_count_read/C'
>   --counter-read-interval 10 -a ./tchain_edit
>  [ perf record: Woken up 438 times to write data ]
>  [ perf record: Captured and wrote 1.232 MB perf.data (17901 samples) ]

but if we go this way I think we should keep/allow all the options perf stat

something like:
  $ perf record -e cycles stat -e 'uncore_imc_1/cas_count_read/' -I 10000 -a ./tchain_edit

with all the stat option we allow -c -F ... 
and reusing existing stat code

>  $perf report -D
> 
>  0x3cae0 [0x28]: PERF_RECORD_COUNTER_READ: uncore_imc_1/cas_count_read/C
>                 CPU 0: val 1205 ena 2046148 run 2046148
> 
>  0x3cb08 [0x28]: PERF_RECORD_COUNTER_READ: uncore_imc_1/cas_count_read/C
>                 CPU 18: val 1315 ena 2001918 run 2001918
> 
>  0x3dba0 [0x28]: PERF_RECORD_COUNTER_READ: uncore_imc_1/cas_count_read/C
>                 CPU 0: val 1588 ena 12191520 run 12191520
> 
>  0x3dbc8 [0x28]: PERF_RECORD_COUNTER_READ: uncore_imc_1/cas_count_read/C
>                 CPU 18: val 1686 ena 12162202 run 12162202
> 
>  $perf report --stdio --socket-filter 0
> 
>  # To display the perf.data header info, please use --header/--header-only options.
>  #
>  #
>  # Total Lost Samples: 0
>  #
>  # Samples: 17K of event 'cycles'
>  # Event count (approx.): 10119095556
>  # Processor Socket: 0
>  #
>  # Overhead  Command       Shared Object        Symbol
>  # ........  ............  ...................  ..................................
>  #
>     97.68%  tchain_edit   tchain_edit          [.] f3
>      0.07%  tchain_edit   tchain_edit          [.] f2
>      0.04%  swapper       [kernel.vmlinux]     [k] run_timer_softirq
>      0.04%  swapper       [kernel.vmlinux]     [k] find_busiest_group
> 
>  # Samples: 0  of event 'uncore_imc_1/cas_count_read/C'
>  # Event count (approx.): 0
>  # Processor Socket: 0
>  # uncore_imc_1/cas_count_read/C: 35937

I think we'll need special output/display for non sampling events,
something like extra window in TUI and distinguished output in stdio,
the above is hacked sampling output ;-)

thoughts?
jirka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH RFC 00/10] counter read during perf sampling
  2015-09-24  8:19 ` [PATCH RFC 00/10] counter read during perf sampling Jiri Olsa
@ 2015-09-24 19:47   ` Liang, Kan
  2015-09-24 22:28     ` Jiri Olsa
  0 siblings, 1 reply; 19+ messages in thread
From: Liang, Kan @ 2015-09-24 19:47 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: acme, jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel



> On Tue, Sep 22, 2015 at 10:13:33AM -0400, kan.liang@intel.com wrote:
> > From: Kan Liang <kan.liang@intel.com>
> >
> > The patch series intends to read counter statistics with fixed
> > frequency during sampling. The instant benefit is that we can read
> > memory bandwidth from uncore event during cpu PMU event is
> sampling.
> >
> > Introduce 'C' event/group modifier. The event with this modifier will
> > do counting not sampling. If a group with this modifier, only group
> > leader do sampling. The counter statistics will be wrote in new RECORD
> > type PERF_RECORD_COUNTER_READ and stored in perf.data.
> > So perf report can present the counter statistics data accordingly.
> >
> > There may be an alternative way to get counter statistics during
> > sampling by running perf record and perf stat together by script.
> > But the script way have various issue and complex to parses the
> > output.
> 
> just a thought, but isn't the way then llow to store the data from perf
> stat? ;-) and be able to merge perf.data-s from perf record and stat
> afterwards

Yes, the way to store the data from perf stat is better than pure script
way. I guess your patch "perf stat record" can do that, right?

If so, how should we run perf record and stat in parallel? By scripts or
modify perf record/stat? 

Also, we need an option in perf report to merge the perf.data-s. Right?

> 
> >
> > Example:
> >
> >  $perf record -e 'cycles,uncore_imc_1/cas_count_read/C'
> >   --counter-read-interval 10 -a ./tchain_edit  [ perf record: Woken up
> > 438 times to write data ]  [ perf record: Captured and wrote 1.232 MB
> > perf.data (17901 samples) ]
> 
> but if we go this way I think we should keep/allow all the options perf stat

Do you mean something like "perf record stat"?
That's not the way I designed. I don't want to run perf record and perf stat
together in one command.

I just want to do similar thing like what sample read did. Sample read can read
counters on each sample. While the counter read can read counters in a fix
frequency (set by --counter-read-interval). So it's an extension of perf record.
It applies all possible options of perf record, like -C -a -g... 
I introduce a new option --counter-read-interval is because that there is no
 interval options in perf record.

> 
> something like:
>   $ perf record -e cycles stat -e 'uncore_imc_1/cas_count_read/' -I 10000 -
> a ./tchain_edit
> 
> with all the stat option we allow -c -F ...
> and reusing existing stat code
> 
> >  $perf report -D
> >
> >  0x3cae0 [0x28]: PERF_RECORD_COUNTER_READ:
> uncore_imc_1/cas_count_read/C
> >                 CPU 0: val 1205 ena 2046148 run 2046148
> >
> >  0x3cb08 [0x28]: PERF_RECORD_COUNTER_READ:
> uncore_imc_1/cas_count_read/C
> >                 CPU 18: val 1315 ena 2001918 run 2001918
> >
> >  0x3dba0 [0x28]: PERF_RECORD_COUNTER_READ:
> uncore_imc_1/cas_count_read/C
> >                 CPU 0: val 1588 ena 12191520 run 12191520
> >
> >  0x3dbc8 [0x28]: PERF_RECORD_COUNTER_READ:
> uncore_imc_1/cas_count_read/C
> >                 CPU 18: val 1686 ena 12162202 run 12162202
> >
> >  $perf report --stdio --socket-filter 0
> >
> >  # To display the perf.data header info, please use --header/--header-
> only options.
> >  #
> >  #
> >  # Total Lost Samples: 0
> >  #
> >  # Samples: 17K of event 'cycles'
> >  # Event count (approx.): 10119095556
> >  # Processor Socket: 0
> >  #
> >  # Overhead  Command       Shared Object        Symbol
> >  # ........  ............  ...................  ..................................
> >  #
> >     97.68%  tchain_edit   tchain_edit          [.] f3
> >      0.07%  tchain_edit   tchain_edit          [.] f2
> >      0.04%  swapper       [kernel.vmlinux]     [k] run_timer_softirq
> >      0.04%  swapper       [kernel.vmlinux]     [k] find_busiest_group
> >
> >  # Samples: 0  of event 'uncore_imc_1/cas_count_read/C'
> >  # Event count (approx.): 0
> >  # Processor Socket: 0
> >  # uncore_imc_1/cas_count_read/C: 35937
> 
> I think we'll need special output/display for non sampling events,
> something like extra window in TUI and distinguished output in stdio, the
> above is hacked sampling output ;-)

I think it depends on what way we finally use.

If we use the way which merging perf.data from perf stat and record, I think
we need special output for the data from perf stat in TUI/stdio.

But if we use the way counter read (:C), I think the best place to show the
counter read results is the header/title (just like the patch did). Because the
results are the aggregate counts during the whole sampling process.
Something like,
# Event count: 35937 of event 'uncore_imc_1/cas_count_read/C'

Thanks,
Kan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 00/10] counter read during perf sampling
  2015-09-24 19:47   ` Liang, Kan
@ 2015-09-24 22:28     ` Jiri Olsa
  2015-09-25 14:57       ` Liang, Kan
  0 siblings, 1 reply; 19+ messages in thread
From: Jiri Olsa @ 2015-09-24 22:28 UTC (permalink / raw)
  To: Liang, Kan; +Cc: acme, jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel

On Thu, Sep 24, 2015 at 07:47:40PM +0000, Liang, Kan wrote:
> 
> 
> > On Tue, Sep 22, 2015 at 10:13:33AM -0400, kan.liang@intel.com wrote:
> > > From: Kan Liang <kan.liang@intel.com>
> > >
> > > The patch series intends to read counter statistics with fixed
> > > frequency during sampling. The instant benefit is that we can read
> > > memory bandwidth from uncore event during cpu PMU event is
> > sampling.
> > >
> > > Introduce 'C' event/group modifier. The event with this modifier will
> > > do counting not sampling. If a group with this modifier, only group
> > > leader do sampling. The counter statistics will be wrote in new RECORD
> > > type PERF_RECORD_COUNTER_READ and stored in perf.data.
> > > So perf report can present the counter statistics data accordingly.
> > >
> > > There may be an alternative way to get counter statistics during
> > > sampling by running perf record and perf stat together by script.
> > > But the script way have various issue and complex to parses the
> > > output.
> > 
> > just a thought, but isn't the way then llow to store the data from perf
> > stat? ;-) and be able to merge perf.data-s from perf record and stat
> > afterwards
> 
> Yes, the way to store the data from perf stat is better than pure script
> way. I guess your patch "perf stat record" can do that, right?
> 
> If so, how should we run perf record and stat in parallel? By scripts or
> modify perf record/stat? 
> 
> Also, we need an option in perf report to merge the perf.data-s. Right?

either that or extra step with 'perf data merge' or somthing like that

SNIP

> > but if we go this way I think we should keep/allow all the options perf stat
> 
> Do you mean something like "perf record stat"?
> That's not the way I designed. I don't want to run perf record and perf stat
> together in one command.
> 
> I just want to do similar thing like what sample read did. Sample read can read
> counters on each sample. While the counter read can read counters in a fix

yea, but sample read stores data into ring buffer in the kernel
while you read the data like in perf stat

> frequency (set by --counter-read-interval). So it's an extension of perf record.
> It applies all possible options of perf record, like -C -a -g... 
> I introduce a new option --counter-read-interval is because that there is no
>  interval options in perf record.

the way I see it you implemented 'perf stat' logic within record command
you create counter (non sampling) and read it via read syscall

I think it's good idea, but I think we should follow the way we do
in perf stat command and reuse the interface (and code)

like having the 'stat' keyword separating the non-sampling config:

  $ perf record -e cycles stat -e 'uncore_imc_1/cas_count_read/' -I 10000 - ./tchain_edit

just an idea.. but I dont think the :C modifier is a good way

SNIP

> > I think we'll need special output/display for non sampling events,
> > something like extra window in TUI and distinguished output in stdio, the
> > above is hacked sampling output ;-)
> 
> I think it depends on what way we finally use.
> 
> If we use the way which merging perf.data from perf stat and record, I think
> we need special output for the data from perf stat in TUI/stdio.
> 
> But if we use the way counter read (:C), I think the best place to show the
> counter read results is the header/title (just like the patch did). Because the
> results are the aggregate counts during the whole sampling process.
> Something like,
> # Event count: 35937 of event 'uncore_imc_1/cas_count_read/C'

hum, how the --counter-read-interval data displayed then? it's not single number right?


jirka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH RFC 00/10] counter read during perf sampling
  2015-09-24 22:28     ` Jiri Olsa
@ 2015-09-25 14:57       ` Liang, Kan
  2015-09-27 19:57         ` Jiri Olsa
  0 siblings, 1 reply; 19+ messages in thread
From: Liang, Kan @ 2015-09-25 14:57 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: acme, jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel


> On Thu, Sep 24, 2015 at 07:47:40PM +0000, Liang, Kan wrote:
> >
> >
> > > On Tue, Sep 22, 2015 at 10:13:33AM -0400, kan.liang@intel.com wrote:
> > > > From: Kan Liang <kan.liang@intel.com>
> > > >
> > > > The patch series intends to read counter statistics with fixed
> > > > frequency during sampling. The instant benefit is that we can read
> > > > memory bandwidth from uncore event during cpu PMU event is
> > > sampling.
> > > >
> > > > Introduce 'C' event/group modifier. The event with this modifier
> > > > will do counting not sampling. If a group with this modifier, only
> > > > group leader do sampling. The counter statistics will be wrote in
> > > > new RECORD type PERF_RECORD_COUNTER_READ and stored in
> perf.data.
> > > > So perf report can present the counter statistics data accordingly.
> > > >
> > > > There may be an alternative way to get counter statistics during
> > > > sampling by running perf record and perf stat together by script.
> > > > But the script way have various issue and complex to parses the
> > > > output.
> > >
> > > just a thought, but isn't the way then llow to store the data from
> > > perf stat? ;-) and be able to merge perf.data-s from perf record and
> > > stat afterwards
> >
> > Yes, the way to store the data from perf stat is better than pure
> > script way. I guess your patch "perf stat record" can do that, right?
> >
> > If so, how should we run perf record and stat in parallel? By scripts
> > or modify perf record/stat?
> >
> > Also, we need an option in perf report to merge the perf.data-s. Right?
> 
> either that or extra step with 'perf data merge' or somthing like that
>

Any update about "perf stat record" patch set? That will help a lot, if
we finally choose the 'perf data merge' way. Right?  
 
> SNIP
> 
> > > but if we go this way I think we should keep/allow all the options
> > > perf stat
> >
> > Do you mean something like "perf record stat"?
> > That's not the way I designed. I don't want to run perf record and
> > perf stat together in one command.
> >
> > I just want to do similar thing like what sample read did. Sample read
> > can read counters on each sample. While the counter read can read
> > counters in a fix
> 
> yea, but sample read stores data into ring buffer in the kernel while you
> read the data like in perf stat
> 
> > frequency (set by --counter-read-interval). So it's an extension of perf
> record.
> > It applies all possible options of perf record, like -C -a -g...
> > I introduce a new option --counter-read-interval is because that there
> > is no  interval options in perf record.
> 
> the way I see it you implemented 'perf stat' logic within record command
> you create counter (non sampling) and read it via read syscall
> 
> I think it's good idea, but I think we should follow the way we do in perf
> stat command and reuse the interface (and code)
> 
> like having the 'stat' keyword separating the non-sampling config:
> 
>   $ perf record -e cycles stat -e 'uncore_imc_1/cas_count_read/' -I 10000 -
>  ./tchain_edit
>

Another thing is that there is limitation for --interval-print in perf stat.
The interval must >= 100ms. However, we need the interval >=10ms.

Any idea about where 100ms is from? Print limit?  
 
If we choose this way, I think we need to introduce a new option for perf
stat to break up the limitation. 

> just an idea.. but I dont think the :C modifier is a good way
>
> SNIP
> 
> > > I think we'll need special output/display for non sampling events,
> > > something like extra window in TUI and distinguished output in
> > > stdio, the above is hacked sampling output ;-)
> >
> > I think it depends on what way we finally use.
> >
> > If we use the way which merging perf.data from perf stat and record, I
> > think we need special output for the data from perf stat in TUI/stdio.
> >
> > But if we use the way counter read (:C), I think the best place to
> > show the counter read results is the header/title (just like the patch
> > did). Because the results are the aggregate counts during the whole
> sampling process.
> > Something like,
> > # Event count: 35937 of event 'uncore_imc_1/cas_count_read/C'
> 
> hum, how the --counter-read-interval data displayed then? it's not single
> number right?
>
No matter which way we choose, I think the output should be similar.

As my original design, perf only output every --counter-read-interval data
in perf report -D.
For tui and stdio, it only output the aggregate number. So, yes, single number.

I think it should be enough. In tui/stdio, perf gives user a roughly image by
the total number during the whole sampling process. If they want details,
they can check by report -D.
Considering the interval is only 10ms, if perf output everything in tui/stdio,
the output is too huge.

Thanks,
Kan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC 00/10] counter read during perf sampling
  2015-09-25 14:57       ` Liang, Kan
@ 2015-09-27 19:57         ` Jiri Olsa
  2015-09-28 15:11           ` Liang, Kan
  0 siblings, 1 reply; 19+ messages in thread
From: Jiri Olsa @ 2015-09-27 19:57 UTC (permalink / raw)
  To: Liang, Kan; +Cc: acme, jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel

On Fri, Sep 25, 2015 at 02:57:14PM +0000, Liang, Kan wrote:

SNIP

> > >
> > > Yes, the way to store the data from perf stat is better than pure
> > > script way. I guess your patch "perf stat record" can do that, right?
> > >
> > > If so, how should we run perf record and stat in parallel? By scripts
> > > or modify perf record/stat?
> > >
> > > Also, we need an option in perf report to merge the perf.data-s. Right?
> > 
> > either that or extra step with 'perf data merge' or somthing like that
> >
> 
> Any update about "perf stat record" patch set? That will help a lot, if

I'll try to post new version this week

> we finally choose the 'perf data merge' way. Right?  

I think we could do both ways.. let user choose whatever is more convenient

SNIP

> > 
> > the way I see it you implemented 'perf stat' logic within record command
> > you create counter (non sampling) and read it via read syscall
> > 
> > I think it's good idea, but I think we should follow the way we do in perf
> > stat command and reuse the interface (and code)
> > 
> > like having the 'stat' keyword separating the non-sampling config:
> > 
> >   $ perf record -e cycles stat -e 'uncore_imc_1/cas_count_read/' -I 10000 -
> >  ./tchain_edit
> >
> 
> Another thing is that there is limitation for --interval-print in perf stat.
> The interval must >= 100ms. However, we need the interval >=10ms.
> 
> Any idea about where 100ms is from? Print limit?  

I don't recall any reason for this limitation, IMO it was just convenient
to have higher unit because lower wasn't needed.. so I think we can change
it do 10ms

SNIP

> > 
> > hum, how the --counter-read-interval data displayed then? it's not single
> > number right?
> >
> No matter which way we choose, I think the output should be similar.
> 
> As my original design, perf only output every --counter-read-interval data
> in perf report -D.
> For tui and stdio, it only output the aggregate number. So, yes, single number.
> 
> I think it should be enough. In tui/stdio, perf gives user a roughly image by
> the total number during the whole sampling process. If they want details,
> they can check by report -D.
> Considering the interval is only 10ms, if perf output everything in tui/stdio,
> the output is too huge.

what is the reason to read the counter multiple times if you display only
single number at the end? overflow issues?

thanks,
jirka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH RFC 00/10] counter read during perf sampling
  2015-09-27 19:57         ` Jiri Olsa
@ 2015-09-28 15:11           ` Liang, Kan
  0 siblings, 0 replies; 19+ messages in thread
From: Liang, Kan @ 2015-09-28 15:11 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: acme, jolsa, a.p.zijlstra, mingo, namhyung, ak, linux-kernel



> > >
> > > hum, how the --counter-read-interval data displayed then? it's not
> > > single number right?
> > >
> > No matter which way we choose, I think the output should be similar.
> >
> > As my original design, perf only output every --counter-read-interval
> > data in perf report -D.
> > For tui and stdio, it only output the aggregate number. So, yes, single
> number.
> >
> > I think it should be enough. In tui/stdio, perf gives user a roughly
> > image by the total number during the whole sampling process. If they
> > want details, they can check by report -D.
> > Considering the interval is only 10ms, if perf output everything in
> > tui/stdio, the output is too huge.
> 
> what is the reason to read the counter multiple times

For doing sophisticated memory bandwidth analysis, it requires 10-20ms
interval to read uncore counter.

> if you display only
> single number at the end? overflow issues?
>

Display issue. If we set 10ms interval and do perf record stat for 10s, there
will be 1000 records per cpu. For a system with 64 cpu, there will be 64,000
number. We cannot show all of them in tui/stdio mode.
I think it's better to only show the total number in tui/stdio mode.
If the user want to do sophisticated analysis, they can use -D to dump all
records.

Thanks,
Kan 
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-09-28 15:11 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-22 14:13 [PATCH RFC 00/10] counter read during perf sampling kan.liang
2015-09-22 14:13 ` [PATCH RFC 01/10] perf,tools: Add 'C' event/group modifier kan.liang
2015-09-22 14:13 ` [PATCH RFC 02/10] perf,tools: Enable counter statistic read for perf record kan.liang
2015-09-22 14:13 ` [PATCH RFC 03/10] perf,tools: don't validate counter read event kan.liang
2015-09-22 14:13 ` [PATCH RFC 04/10] perf,tools: New RECORD type PERF_RECORD_COUNTER_READ kan.liang
2015-09-22 14:13 ` [PATCH RFC 05/10] perf,tools: record counter statistics during sampling kan.liang
2015-09-22 14:13 ` [PATCH RFC 06/10] perf,tools: option to set counter read interval kan.liang
2015-09-23 18:55   ` Sukadev Bhattiprolu
2015-09-23 19:07     ` Liang, Kan
2015-09-22 14:13 ` [PATCH RFC 07/10] perf,report: handle PERF_RECORD_COUNTER_READ kan.liang
2015-09-22 14:13 ` [PATCH RFC 08/10] perf,tools: store counter val in events_stats kan.liang
2015-09-22 14:13 ` [PATCH RFC 09/10] perf,tools: show counter read result in studio kan.liang
2015-09-22 14:13 ` [PATCH RFC 10/10] perf,tools: show counter read result in tui browser title kan.liang
2015-09-24  8:19 ` [PATCH RFC 00/10] counter read during perf sampling Jiri Olsa
2015-09-24 19:47   ` Liang, Kan
2015-09-24 22:28     ` Jiri Olsa
2015-09-25 14:57       ` Liang, Kan
2015-09-27 19:57         ` Jiri Olsa
2015-09-28 15:11           ` Liang, Kan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).