diff mbox series

[RFC,5/6] perf/tools: Enhance JSON/metric infrastructure to handle "?"

Message ID 20200117124620.26094-6-kjain@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (20862247a368dbb75d6e97d82345999adaacf3cc)
snowpatch_ozlabs/checkpatch warning total: 0 errors, 1 warnings, 0 checks, 247 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Kajol Jain Jan. 17, 2020, 12:46 p.m. UTC
Patch enhances current metric infrastructure to handle "?" in the metric
expression. The "?" can be use for parameters whose value not known while
creating metric events and which can be replace later at runtime to
the proper value. It also add flexibility to create multiple events out
of single metric event added in json file.

Patch adds function 'arch_get_runtimeparam' which is a arch specific
function, returns the count of metric events need to be created.
By default it return 1.

One loop is added in function 'metricgroup__add_metric', which create
multiple events at run time depend on return value of
'arch_get_runtimeparam' and merge that event in 'group_list'.

This infrastructure needed for hv_24x7 socket/chip level events.
"hv_24x7" chip level events needs specific chip-id to which the
data is requested. Function 'arch_get_runtimeparam' implemented
in header.c which extract number of sockets from sysfs file
"sockets" under "/sys/devices/hv_24x7/interface/".

Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/arch/powerpc/util/header.c |  40 +++++++++
 tools/perf/util/expr.h                |   1 +
 tools/perf/util/expr.y                |  17 +++-
 tools/perf/util/metricgroup.c         | 117 +++++++++++++++++---------
 tools/perf/util/metricgroup.h         |   1 +
 tools/perf/util/stat-shadow.c         |   5 ++
 6 files changed, 138 insertions(+), 43 deletions(-)

Comments

Andi Kleen Jan. 17, 2020, 4:28 p.m. UTC | #1
On Fri, Jan 17, 2020 at 06:16:19PM +0530, Kajol Jain wrote:
> Patch enhances current metric infrastructure to handle "?" in the metric
> expression. The "?" can be use for parameters whose value not known while
> creating metric events and which can be replace later at runtime to
> the proper value. It also add flexibility to create multiple events out
> of single metric event added in json file.

Please add a proper specification how this ? thing is supposed to work,
what the exact semantics are, how it is different from the existing
# mechanism etc.

The standard way to do similar things before was to define an explicit
# name and let the expr code take care of it. 

-Andi
Kajol Jain Jan. 21, 2020, 10:17 a.m. UTC | #2
On 1/17/20 9:58 PM, Andi Kleen wrote:
> On Fri, Jan 17, 2020 at 06:16:19PM +0530, Kajol Jain wrote:
>> Patch enhances current metric infrastructure to handle "?" in the metric
>> expression. The "?" can be use for parameters whose value not known while
>> creating metric events and which can be replace later at runtime to
>> the proper value. It also add flexibility to create multiple events out
>> of single metric event added in json file.
> Please add a proper specification how this ? thing is supposed to work,
> what the exact semantics are, how it is different from the existing
> # mechanism etc.
>
> The standard way to do similar things before was to define an explicit
> # name and let the expr code take care of it.

Hi Andi,

     Thanks for reviewing my patchset.

I did review the existing '#' mechanism. It is quiet useful for runtime
dependency (like ex. event selection based on a return value). But I want
to handle couple of more things. Meaning, filling in parameter of event code
based on runtime value and replicating events at runtime.

So the '?' can handle, 1) replacement of a value in the event code based 
on runtime

and 2) replicate the event (metric expr) at runtime (handled in new 
metricgroup__add_metric()).

This patchset add an arch specific function called 
'arch_get_runtimeparam'. Ideally,
arch_get_runtimeparm() returns the number of metric exps one want to 
create (it defaults to 1).
The loop added in 'metricgroup__add_metric' will handle the event addition.

Example ppc64 hv_24x7 pmu metric event using '?':

(hv_24x7@PM_MCS01_128B_RD_DISP_PORT01\\,chip\\=?@ + 
hv_24x7@PM_MCS01_128B_RD_DISP_PORT23\\,chip\\=?@)

Here, '?' will be replaced with a runtime value and metric expression 
will be replicated.

Thanks,

Kajol

> -Andi
Andi Kleen Jan. 21, 2020, 7:42 p.m. UTC | #3
> Here, '?' will be replaced with a runtime value and metric expression will
> be replicated.

Okay seems reasonable to me.

Thanks,

-Andi
diff mbox series

Patch

diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index 3b4cdfc5efd6..28425edb901c 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -7,6 +7,11 @@ 
 #include <string.h>
 #include <linux/stringify.h>
 #include "header.h"
+#include "metricgroup.h"
+#include "evlist.h"
+#include <dirent.h>
+#include "pmu.h"
+#include <api/fs/fs.h>
 
 #define mfspr(rn)       ({unsigned long rval; \
 			 asm volatile("mfspr %0," __stringify(rn) \
@@ -16,6 +21,8 @@ 
 #define PVR_VER(pvr)    (((pvr) >>  16) & 0xFFFF) /* Version field */
 #define PVR_REV(pvr)    (((pvr) >>   0) & 0xFFFF) /* Revison field */
 
+#define SOCKETS_INFO_FILE_PATH "/devices/hv_24x7/interface/"
+
 int
 get_cpuid(char *buffer, size_t sz)
 {
@@ -44,3 +51,36 @@  get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 
 	return bufp;
 }
+
+int arch_get_runtimeparam(void)
+{
+	int count = 0;
+	DIR *dir;
+	char path[PATH_MAX];
+	const char *sysfs = sysfs__mountpoint();
+	char filename[] = "sockets";
+	FILE *file;
+	char buf[16], *num;
+	int data;
+
+	if (!sysfs)
+		goto out;
+	snprintf(path, PATH_MAX,
+		 "%s" SOCKETS_INFO_FILE_PATH, sysfs);
+	dir = opendir(path);
+	if (!dir)
+		goto out;
+	strcat(path, filename);
+	file = fopen(path, "r");
+	if (!file)
+		goto out;
+
+	data = fread(buf, 1, sizeof(buf), file);
+	if (data == 0)
+		goto out;
+	count = strtol(buf, &num, 10);
+out:
+	if (!count)
+		count = 1;
+	return count;
+}
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 046160831f90..85ebea68b0c5 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -15,6 +15,7 @@  struct parse_ctx {
 	struct parse_id ids[MAX_PARSE_ID];
 };
 
+extern int expr__runtimeparam;
 void expr__ctx_init(struct parse_ctx *ctx);
 void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
 #ifndef IN_EXPR_Y
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index f9a20a39b64a..26c62b631525 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -36,6 +36,8 @@ 
 %type <num> expr if_expr
 
 %{
+int expr__runtimeparam;
+
 static int expr__lex(YYSTYPE *res, const char **pp);
 
 static void expr__error(double *final_val __maybe_unused,
@@ -101,7 +103,7 @@  static int expr__symbol(YYSTYPE *res, const char *p, const char **pp)
 	if (*p == '#')
 		*dst++ = *p++;
 
-	while (isalnum(*p) || *p == '_' || *p == '.' || *p == ':' || *p == '@' || *p == '\\') {
+	while (isalnum(*p) || *p == '_' || *p == '.' || *p == ':' || *p == '@' || *p == '\\' || *p == '?') {
 		if (p - s >= MAXIDLEN)
 			return -1;
 		/*
@@ -112,6 +114,19 @@  static int expr__symbol(YYSTYPE *res, const char *p, const char **pp)
 			*dst++ = '/';
 		else if (*p == '\\')
 			*dst++ = *++p;
+		else if (*p == '?') {
+			int size = snprintf(NULL, 0, "%d", expr__runtimeparam);
+			char * paramval = (char *)malloc(size);
+			int i = 0;
+			if(!paramval)
+				*dst++ = '0';
+			else {
+				sprintf(paramval, "%d", expr__runtimeparam);
+				while(i < size)
+					*dst++ = paramval[i++];
+				free(paramval);
+			}
+		}
 		else
 			*dst++ = *p;
 		p++;
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 02aee946b6c1..ec87a120cd7e 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -399,6 +399,11 @@  void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 	strlist__delete(metriclist);
 }
 
+int __weak arch_get_runtimeparam(void)
+{
+	return 1;
+}
+
 static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 				   struct list_head *group_list)
 {
@@ -419,52 +424,80 @@  static int metricgroup__add_metric(const char *metric, struct strbuf *events,
 			continue;
 		if (match_metric(pe->metric_group, metric) ||
 		    match_metric(pe->metric_name, metric)) {
-			const char **ids;
-			int idnum;
-			struct egroup *eg;
-			bool no_group = false;
-
-			pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
-
-			if (expr__find_other(pe->metric_expr,
-					     NULL, &ids, &idnum) < 0)
-				continue;
-			if (events->len > 0)
-				strbuf_addf(events, ",");
-			for (j = 0; j < idnum; j++) {
-				pr_debug("found event %s\n", ids[j]);
-				/*
-				 * Duration time maps to a software event and can make
-				 * groups not count. Always use it outside a
-				 * group.
-				 */
-				if (!strcmp(ids[j], "duration_time")) {
-					if (j > 0)
-						strbuf_addf(events, "}:W,");
-					strbuf_addf(events, "duration_time");
-					no_group = true;
+			int k, count;
+
+			count = arch_get_runtimeparam();
+
+			/* This loop is added to create multiple
+			 * events depend on count value and add
+			 * those events to group_list.
+			 */
+			for (k = 0; k < count; k++) {
+				const char **ids;
+				int idnum;
+				struct egroup *eg;
+				bool no_group = false;
+				char value[PATH_MAX];
+
+				pr_debug("metric expr %s for %s\n",
+					 pe->metric_expr, pe->metric_name);
+				expr__runtimeparam = k;
+				if (expr__find_other(pe->metric_expr, NULL,
+						     &ids, &idnum) < 0)
 					continue;
+				if (events->len > 0)
+					strbuf_addf(events, ",");
+				for (j = 0; j < idnum; j++) {
+					pr_debug("found event %s\n", ids[j]);
+					/*
+					 * Duration time maps to a software
+					 * event and can make groups not count.
+					 * Always use it outside a group.
+					 */
+					if (!strcmp(ids[j], "duration_time")) {
+						if (j > 0)
+							strbuf_addf(events,
+								    "}:W,");
+						strbuf_addf(events,
+							    "duration_time");
+						no_group = true;
+						continue;
+					}
+					strbuf_addf(events, "%s%s",
+						    j == 0 || no_group ? "{" :
+						    ",", ids[j]);
+					no_group = false;
 				}
-				strbuf_addf(events, "%s%s",
-					j == 0 || no_group ? "{" : ",",
-					ids[j]);
-				no_group = false;
-			}
-			if (!no_group)
-				strbuf_addf(events, "}:W");
+				if (!no_group)
+					strbuf_addf(events, "}:W");
 
-			eg = malloc(sizeof(struct egroup));
-			if (!eg) {
-				ret = -ENOMEM;
-				break;
+				eg = malloc(sizeof(struct egroup));
+				if (!eg) {
+					ret = -ENOMEM;
+					break;
+				}
+				eg->ids = ids;
+				eg->idnum = idnum;
+				strcpy(value, pe->metric_name);
+				if (strstr(pe->metric_name, "?")) {
+					int size = snprintf(NULL, 0, "%d", k);
+					char *param = (char *)malloc(size + 1);
+
+					if (!param) {
+						strcat(value, "_0");
+					} else {
+						sprintf(param, "%c%d", '_', k);
+						value[strlen(value) - 1] = '\0';
+						strcat(value, param);
+						free(param);
+					}
+				}
+				eg->metric_name = strdup(value);
+				eg->metric_expr = pe->metric_expr;
+				eg->metric_unit = pe->unit;
+				list_add_tail(&eg->nd, group_list);
+				ret = 0;
 			}
-			eg->ids = ids;
-			eg->idnum = idnum;
-			eg->metric_name = pe->metric_name;
-			eg->metric_expr = pe->metric_expr;
-			eg->metric_unit = pe->unit;
-			list_add_tail(&eg->nd, group_list);
-			ret = 0;
 		}
 	}
 	return ret;
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 475c7f912864..81224ba1270d 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -34,4 +34,5 @@  int metricgroup__parse_groups(const struct option *opt,
 void metricgroup__print(bool metrics, bool groups, char *filter,
 			bool raw, bool details);
 bool metricgroup__has_metric(const char *metric);
+int arch_get_runtimeparam(void);
 #endif
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 2c41d47f6f83..147a39b1ff5b 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -780,7 +780,12 @@  static void generic_metric(struct perf_stat_config *config,
 
 	if (!metric_events[i]) {
 		const char *p = metric_expr;
+		if (strstr(p, "?")) {
+			char *tmp = strrchr(metric_name, '_');
 
+			tmp++;
+			expr__runtimeparam = strtol(tmp, &tmp, 10);
+		}
 		if (expr__parse(&ratio, &pctx, &p) == 0) {
 			char *unit;
 			char metric_bf[64];